fix: git slot suppresses SHA-only partial probe results (plan #660, source #659) - #661
Conversation
dvui.button/labelNoFmt pins one face. Noto has no Arrows block so U+2191 tofus; fontSymbols() on the whole chip tofus Latin. Same split rich text already uses. Measure the arrow on the symbols face.
…wrap-to-clip in narrow canvas (review #654 L1+L9)
…f ↑ on narrow canvas (review #654 Minor L1)
Move paint helpers, state, metrics, and chrome out of the 1628-line ui.zig into 11 sibling files under ui/: state, metrics, scroll, kinds, chrome, toolrun, thinking, chip, status, skill, composer. ui.zig is now the public facade: BUILD_ID, onInit, onDeinit, frame. Import graph is acyclic (no state->paint cycles). No behavior, palette, protocol, or cap changes. Living docs path pointers updated. Plan: #656 Source: #652
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #661
Verdict: APPROVE
Repo: btipling/invincible
Scope: main ← plan/fix-git-slot-sha-only @ 73fb452 · 3 files · +72/−1 · git-slot SHA-only guard
Lenses run: L1 L3 L4 L5 L6 L8 L9 (skip: L2 — no API/secrets/runner; L7 — no host/config bind)
AGENTS.md read: yes · docs/feature-divide.md yes · SECURITY.md N/A
The mid-turn @sha clobber is real (formatGitStatusSlot with stripped/'' branch), the guard sits in the single funnel (refreshGitStatusSlot), empty-clear / 429 / network keep-last still work, and the 7 new tests hit the plan matrix. Layer is correct (host display policy, probe stays pure).
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L1 | startsWith('@') rests on a false invariant. Plan #660 / plan-review: “git branch names cannot start with @.” git check-ref-format --branch '@foo' exits 0; git checkout -b '@hotfix' succeeds; --abbrev-ref HEAD returns @hotfix; formatGitStatusSlot then emits @hotfix@abc1234. |
Operator (or agent) is on @hotfix / @1. Every cadence + on-demand refresh is dropped. Slot stays empty or frozen on the previous branch forever. The wire already distinguishes this: { git: { branch: "@hotfix", sha } } vs { git: { sha } } (no branch). Host types the JSON as { value?: string } and ignores git.branch. |
Defender: “nobody names branches @….” Legal and I just created one. Fix: suppress when git.sha is set and git.branch is absent — not when the formatted string happens to start with @. |
high |
| Minor | L9 | Lasting detached HEAD is indistinguishable from a transient lock, so the bar keeps painting the previous branch name. Source #659 allowed showing @sha for a real detach; Goal 3 in the plan still says the cadence should. Decision A froze the last branch@sha and called it “honest.” After checkout --detach / rebase / bisect it is a lie. |
Detach, commit, keep working. Slot still says main@old. Operator thinks they are on main. |
Defender: “plan-review accepted the tradeoff.” Accepted ≠ honest. Mid-turn-only suppression (plan option B) or “two consecutive cadence SHA-only ticks → accept @sha” would keep the flicker fix without a stale branch. Not a merge block; do not document the leftover as “honest.” |
high |
| Nit | L6 | New 429 / network cases duplicate the existing keep-last test in the same describe. No case for dirty SHA-only (@abc*, which formatGitStatusSlot can emit) or for @hotfix@sha pass-through. |
A later edit to the 429 short-circuit is still green because the @ guard would also keep-last. |
Rename / drop the dupes; add the two missing shapes if the discriminator stays string-based. | high |
Residual risk
No unit-test workflow ran on this SHA (repo has no vitest CI; only Vercel ignore). I did not re-run vitest in this workspace. Detached-at-boot stays empty (plan-accepted, same as non-git). refreshGitStatusSlot’s fail-soft comment was not updated to mention SHA-only.
Merge guidance
APPROVE — the reported mid-turn @sha flash is fixed in the right layer. Ship if the Minors stay residual. Prefer keying the guard on structured git.branch / git.sha from the same 200 body before a follow-up if @-prefixed branch names or lasting detach matter.
What was not attacked
Live /harness mid-exec lock, real checkout --detach on a bound sandbox, prod Gateway, DO runner.
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #661 (verdict correction)
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/fix-git-slot-sha-only @ 73fb452 · 3 files · +72/−1 · git-slot SHA-only guard
Lenses run: L1 L3 L4 L5 L6 L8 L9 (skip: L2 — no API/secrets/runner; L7 — no host/config bind)
AGENTS.md read: yes · docs/feature-divide.md yes · SECURITY.md N/A
Supersedes the earlier comment on this PR that said APPROVE. That is not a skill verdict. Official scale is BLOCK / CONCERNS / PASS WITH NOTES / INCOMPLETE. Findings unchanged; two Minors + one Nit → PASS WITH NOTES. merge-pr can treat this head as review-satisfied.
The mid-turn @sha clobber is real (formatGitStatusSlot with stripped/'' branch), the guard sits in the single funnel (refreshGitStatusSlot), empty-clear / 429 / network keep-last still work, and the 7 new tests hit the plan matrix. Layer is correct (host display policy, probe stays pure).
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L1 | startsWith('@') rests on a false invariant. Plan #660 / plan-review: “git branch names cannot start with @.” git check-ref-format --branch '@foo' exits 0; git checkout -b '@hotfix' succeeds; --abbrev-ref HEAD returns @hotfix; formatGitStatusSlot then emits @hotfix@abc1234. |
Operator (or agent) is on @hotfix / @1. Every cadence + on-demand refresh is dropped. Slot stays empty or frozen on the previous branch forever. The wire already distinguishes this: { git: { branch: "@hotfix", sha } } vs { git: { sha } } (no branch). Host types the JSON as { value?: string } and ignores git.branch. |
Defender: “nobody names branches @….” Legal and I just created one. Fix: suppress when git.sha is set and git.branch is absent — not when the formatted string happens to start with @. |
high |
| Minor | L9 | Lasting detached HEAD is indistinguishable from a transient lock, so the bar keeps painting the previous branch name. Source #659 allowed showing @sha for a real detach; Goal 3 in the plan still says the cadence should. Decision A froze the last branch@sha and called it “honest.” After checkout --detach / rebase / bisect it is a lie. |
Detach, commit, keep working. Slot still says main@old. Operator thinks they are on main. |
Defender: “plan-review accepted the tradeoff.” Accepted ≠ honest. Mid-turn-only suppression (plan option B) or “two consecutive cadence SHA-only ticks → accept @sha” would keep the flicker fix without a stale branch. Not a merge block; do not document the leftover as “honest.” |
high |
| Nit | L6 | New 429 / network cases duplicate the existing keep-last test in the same describe. No case for dirty SHA-only (@abc*, which formatGitStatusSlot can emit) or for @hotfix@sha pass-through. |
A later edit to the 429 short-circuit is still green because the @ guard would also keep-last. |
Rename / drop the dupes; add the two missing shapes if the discriminator stays string-based. | high |
Residual risk
No unit-test workflow ran on this SHA (repo has no vitest CI; only Vercel ignore). I did not re-run vitest in this workspace. Detached-at-boot stays empty (plan-accepted, same as non-git). refreshGitStatusSlot’s fail-soft comment was not updated to mention SHA-only.
Merge guidance
PASS WITH NOTES — the reported mid-turn @sha flash is fixed in the right layer. Safe to merge from this attack. Prefer keying the guard on structured git.branch / git.sha from the same 200 body in a follow-up if @-prefixed branch names or lasting detach matter.
What was not attacked
Live /harness mid-exec lock, real checkout --detach on a bound sandbox, prod Gateway, DO runner.
…prefix (review L1+L6+L9)
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #661 (round 2)
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/fix-git-slot-sha-only @ 9c1ad2f · 3 files · SHA-only git-slot guard
Lenses run: L1 L3 L4 L5 L6 L8 L9 (skip: L2 — no API/secrets/runner; L7 — no host/config bind)
AGENTS.md read: yes · docs/feature-divide.md yes · SECURITY.md N/A
Supersedes reviews on 73fb452 (unofficial APPROVE, then PASS WITH NOTES). New head keys the guard on structured git.sha && !git.branch instead of value.startsWith('@').
The mid-turn @sha clobber is still fixed in the single funnel. Layer is still correct (host display policy; probe stays pure). @hotfix@sha now passes through. Dirty SHA-only (git.sha + dirty, no branch) is covered.
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L9 | Lasting detached HEAD is still the same wire shape as a transient lock (sha set, branch absent), so the bar keeps the previous branch name. Docs and the new comment still call that the “last honest branch@sha.” After checkout --detach / rebase / bisect it is a stale name. |
Detach, commit, keep working. Slot still says main@old. Operator thinks they are on main. |
Defender: “plan Decision A + we named detached HEAD in the docs.” Named ≠ honest. Mid-turn-only suppression or “two cadence SHA-only ticks → accept @sha” would keep the flicker fix. Not a merge block. |
high |
| Nit | L8 | refreshGitStatusSlot’s fail-soft bullet list (network / empty / ellipsize) was not updated for the new SHA-only return. |
Next editor treats “any non-empty value” as the whole policy and deletes the structured guard. |
Add one bullet: sha-present + branch-absent → keep last. | high |
Prior findings (round 1 @ 73fb452) — disposition
| Prior | Status |
|---|---|
Minor L1 startsWith('@') false invariant / @hotfix |
Fixed. Guard is data.git?.sha && !data.git?.branch. New test paints @hotfix@abc1234. |
| Minor L9 stale branch after detach | Still open (same policy). |
Nit L6 duplicate 429/network + missing @sha* / @hotfix |
Mostly fixed. Dirty SHA-only and @hotfix tests added. 429/network cases still overlap the older keep-last test; not re-raised. |
Residual risk
If a 200 body has value: '@abc' and omits git, the guard does not fire (git?.sha is missing) and @abc still paints. The live route always includes git; this is fail-open only on a hand-rolled / future-thinner payload. No vitest CI on this SHA (repo has none; only Vercel ignore). I did not re-run vitest here. Detached-at-boot stays empty (plan-accepted).
Merge guidance
PASS WITH NOTES — review-satisfied for merge-pr. The reported mid-turn @sha flash is fixed; the structured discriminator closes the @hotfix hole. Lasting-detach staleness stays residual.
What was not attacked
Live /harness mid-exec lock, real checkout --detach on a bound sandbox, prod Gateway, DO runner.
What
One-line guard in
refreshGitStatusSlot: when the server probe returns a SHA-only value (starts with@, e.g.@abc1234), skip the update and keep the last honestbranch@sha.Source issue: #659
Plan: #660
Root cause
Two paths produce SHA-only partial results (branch missing, SHA present):
HEADrejection —rev-parse --abbrev-ref HEADreturnsHEAD→ probe strips it →@shaexecholds lock →runGitreturns''→@shaBoth converge on
refreshGitStatusSlotwhich unconditionally trusted any non-emptydata.value.Fix
One new line between the non-empty check and
setStatusSlot.Files
lib/harnessChat.tsstartsWith('@')guardlib/harnessChat.test.tsdocs/harness-limits.mdTest results
npx vitest run lib/harnessChat.test.tsnpx tsc --noEmit