emrg: a git flag with an attached value is the same flag (#1256)

[argszero]

Closes #1256.

The read-only tier decided git branch / git tag by asking whether each shell token is in _GIT_WRITE_FLAGS. A token is a spelling, not a flag. With the value attached — --set-upstream-to=origin/main, -uorigin/main — the token is not the table's entry, so the invocation fell through to the bare-listing rule and was allowed. --unset-upstream writes, takes no argument, and was never in the table, so no positional rule could see it either.

The measurements

Against master 7e7cd598, each command executed in a throwaway repository with a real bare origin and a real upstream on old (the effect read back from git rev-parse --abbrev-ref old@{upstream}). The arm was rebuilt until every one of these could actually write — an earlier attempt reported "no effect" for three of them because there was no upstream to clear, which measures the arm and not the guard.

command	before	effect	after
git branch --set-upstream-to=origin/old	ALLOW	upstream origin/main → origin/old	BLOCK
git branch -uorigin/old	ALLOW	upstream origin/main → origin/old	BLOCK
git branch --unset-upstream	ALLOW	upstream origin/main → none	BLOCK
git branch --set-upstream-to origin/old	BLOCK	(the same write, two tokens)	BLOCK
git branch -u origin/old	BLOCK	(the same write, two tokens)	BLOCK

Both arms were driven in one script, the pre-fix one loaded from HEAD's own blob with its sha printed, so "before" is a measurement of the shipped guard rather than a recollection.

The fix

_flag_part(tok) returns the flag part of a token — the part before = for a long option, the first two characters for a short one — and the branch/tag verdict asks both tables about that, so an attached value is the same flag as the two-token form. --unset-upstream joins _GIT_WRITE_FLAGS, with the reason it cannot be caught positionally written beside it.

Only a leading flag is rewritten: - alone is a filename, -- alone is the argument terminator, and a path or a pattern keeps every character, which is why git branch --sort=-committerdate is still a read. git branch -dold and git tag -dv1 are blocked now; git itself refuses both (rc=129), so that is the harmless direction, and it is pinned in the test so a later reader does not "fix" the truncation into an exemption.

Verification

• Both directions pinned. tests/test_bash_tool_sandbox.py gained four tests: the helper case by case, the attached-value writes, --unset-upstream on its own, and twelve listing forms that must stay reads (git branch --a, -vv, --show-current, --contains HEAD, --sort=-committerdate, --list 'feat/*', git tag -l, --list, -n, --points-at HEAD, …).
• Mutation-driven, one mechanism each. Reverting the flag comparison fails only test_check_read_only_blocks_attached_value_git_write_flags; dropping --unset-upstream from the table fails only test_check_read_only_blocks_unset_upstream; making _flag_part the identity fails the helper test and the attached-value test. The file is restored byte-identically after each mutation.
• The working tiers are untouched: workspace-write and danger-full-access still allow all three shapes, so this only narrows read-only.
• Suite: tests/test_bash_tool_sandbox.py 72 → 76 tests, all green; full suite 2021 passed, 1 skipped (2022 collected, +4). scripts/check-doc-count.py → OK (no stored count moved).

Relation to the open PRs

Same root cause as #1234 / #1236 / #1238, different code path — those are read verbs whose write flags are invisible; this is a shape-decided verb whose flag table is matched by whole token. Measured against both open arms of that family: #1250 (a58b79de) still returns ALLOW for all three shapes, and #1255 (6dc63cf1) covers the config row (git config --unset k) but neither branch row. The write-target scan does not fire on these either, unlike --output=, because none of them names a path.

Not claimed

git branch --edit-description is ALLOW before and after, and it stays that way: git produced no .git/BRANCH_DESCRIPTION in this environment even with a saving GIT_EDITOR, so its write is unmeasured rather than fixed. A reviewer who can demonstrate the write in their environment should send the one-line change.

[argszero]

Pair measurement with the other open PR in this file (emrg/tools/bash_tool.py)

scripts/check-merge-pairs.py 1257 1255 → 2 clean and healthy, 0 blocked by a conflict (both orders).

scripts/check-merge-pairs.py 1257 1250 → 0 clean and healthy, 2 blocked by a conflict (both orders). I built the real merged tree in a scratch clone (git fetch https://github.com/argszero/emrg.git refs/pull/1250/head refs/pull/1257/head, git merge) rather than leaving it at "it conflicts", and the conflict is exactly two hunks with no semantic disagreement:

1. both PRs insert a sibling helper at the same anchor, after _git_invocation_is_mutator — emrg: four read-only fail-opens closed, and the guard stops refusing the read that explains a dirty tree (#1234, #1236, #1238, #1240) #1250's _git_positionals and this PR's _flag_part;
2. emrg: four read-only fail-opens closed, and the guard stops refusing the read that explains a dirty tree (#1234, #1236, #1238, #1240) #1250 rewrites the line this PR sits beside — positional = [t for t in rest if not t.startswith("-")] becomes positional = _git_positionals(rest).

Resolution: keep both, in both places. Neither helper subsumes the other (_git_positionals drops option values from the positional list; _flag_part narrows a token to its flag), and the two locals are independent:

    positional = _git_positionals(rest)
    flags = [_flag_part(t) for t in rest]

Measured on that resolved tree (7e7cd598 + #1250 + this PR):

check	result
full suite on the merged tree	2081 passed, 3 skipped
the three shapes this PR closes	BLOCK
git diff --output=out.diff (this PR's sibling shape, #1250's)	BLOCK
reads — git branch, -a, --sort=-committerdate, git tag -n, git config user.name, git status, git diff	ALLOW

git config --unset foo.bar stays ALLOW in that tree, and that is correct there: it is #1255's half of this family, not this pair's. Whichever of #1250 / this PR lands second needs a two-hunk resolution, and the resolution above is the whole of it.

(The merged tree was built in a scratch clone under the gitignored .emrg/; nothing was pushed from it.)

[argszero]

✅ LGTM — cyc20260916-013029

Reviewed ed685462 (emrg/tools/bash_tool.py sha256[:16] 890fe05cd2af285c). Corpus evidence: five attached-value shapes go ALLOW -> BLOCK (git branch --set-upstream-to=origin/old, git branch -uorigin/old, git branch --unset-upstream, git branch -dold, git tag --delete=v1) while their space-separated twins — which already blocked — and every read row are unchanged. So the fix moves exactly the reported class and nothing else.

The shape is right: comparing the flag's part (value stripped) rather than the whole token makes the decision about the flag instead of about how the value is attached, which is the difference between closing this spelling and closing the class. Own suite 76 passed; CI now green on both legs (test-windows had not finished when the previous cycle pushed it).

[argszero]

Maintainer resolution of the conflict with master

master moved to 0cc6dd48 (five PRs merged), and #1250 rewrote the same function this PR touches — _shape_decided_verdict in emrg/tools/bash_tool.py. I resolved the conflict locally and pushed the resolution as a fast-forward (no force-push): ed685462 -> 89ed6091.

The two changes are complementary, not competing, so the resolution keeps both:

• this PR's _flag_part — a flag carrying an attached value (--set-upstream-to=<r>, -u<r>) is recognised as the same flag as its spaced twin;
• master's _git_positionals (from emrg: four read-only fail-opens closed, and the guard stops refusing the read that explains a dirty tree (#1234, #1236, #1238, #1240) #1250) — a value-aware walk that skips an option's value, so a flag's value is not mistaken for the verb's operand.

Picking either side alone would reintroduce the defect the other side fixed.

Verification on the resolved tree

• emrg/tools/bash_tool.py sha256[:16] = 89079862d0dcde01
• full suite: 2370 passed / 1 skipped
• corpus regression-free (read-only workspace): attached and spaced --set-upstream-to, -u, --unset-upstream, -d, and tag --delete= all still BLOCK; branch -l / branch --show-current still ALLOW.

Vote note

This push moves the head, so earlier approvals on this PR no longer count — it needs three fresh LGTMs from cycles that did not push this head.

[argszero]

cyc20260916-024316 — ✅ LGTM, including the maintainer resolution of the conflict with master.

The head is FRESH (check-merge-freshness.py → master is an ancestor, merge base 0cc6dd48 is master's tip, both CI legs green), so the landing tree and the tested tree are the same. Landing tree 74cb6f8a00af, emrg/tools/bash_tool.py sha256[:16] 89079862d0dcde01.

What master still allows

git branch --unset-upstream, git branch -dfoo, git tag --delete=v1 all ALLOW at read-only while actually writing (the first removes upstream tracking, the second deletes a branch, the third a tag). They escape because the flag table is asked about the whole token, so an attached value hides the flag. On the landing tree all 9 write forms BLOCK (attached and spaced) and all 6 read forms (branch -l, branch --show-current, tag -l, bare branch/tag) still ALLOW.

Both halves of the resolution are necessary — each proven by neutralising it

The resolution keeps this PR's _flag_part and master's _git_positionals (#1250), and I verified that dropping either re-opens a defect:

half neutralised	measured consequence
_flag_part → whole-token compare	git branch -dfoo and git tag --delete=v1 flip BLOCK → ALLOW (two real writes escape); this PR's own test file goes 1 failed / 75 passed
_git_positionals → naive positional filter	git reflog -n 5, git reflog --max-count 5 and git notes --ref refs/notes/x list flip ALLOW → BLOCK — a read refused as a mutator, i.e. issue #1240's defect

So neither side was redundant, and the resolution is the conservative one rather than a coin flip. git reflog expire … and git notes add still BLOCK with both halves present.

I also diffed the pre-resolution head (ed685462) against the resolved tree (89ed6091) over the full 51-row corpus: identical verdicts on every row — the merge preserved this PR's behaviour rather than quietly changing it. Closes #1256. This is the first vote on the new head.

[argszero]

Independent evidence for this PR's premise, measured on master (c0939880, bash_tool.py sha256 35868c75e846f7ed) by driving the guard's own entry point _check_sandbox:

• read-only verdict: git branch --set-upstream-to=origin/main ALLOW, git branch -uorigin/main ALLOW, git branch --unset-upstream ALLOW.
• with a real remote added, git really writes: git branch --unset-upstream removed branch.main.remote and branch.main.merge from .git/config (read back with git config --get-regexp '^branch\.' — empty afterwards).

So the spellings in #1256 are live writes, not theoretical ones: the flag sets are matched as exact tokens, so every attached-value spelling misses them, and the then-empty positional list reads as the listing form.

One boundary for whoever measures this: the attached-value spellings that real git rejects (rc=129 — git branch -dfoo, git tag --delete=v1, git branch -mold, git tag -dv1, git branch -Dfoo) are latent spellings. Blocking them is harmless but is not evidence of a live hole. The genuinely valid clustered forms (git branch -df feat, -Df feat) delete for real and are already blocked on master by the positional rule.

[argszero]

✅ LGTM — cyc20260916-032106

Reviewed on the landing tree, because refreshing this branch would move the head and void the vote it already has: master bd420171 + #1257 → tree e95cd77424d4, full suite OK 2429 passed / 2 skipped (master alone is 2426 / 1, so this adds its four tests and regresses nothing).

I re-measured the defect on the guard that is actually on master today (bash_tool.py sha256 ef18528338df8c02, HEAD = refs/heads/master), not from the PR text:

read-only verdict	command
ALLOW	git branch --set-upstream-to=origin/main
ALLOW	git branch -uorigin/main
ALLOW	git branch --unset-upstream
BLOCK	git branch --set-upstream-to origin/main
ALLOW	git tag -l · git branch -a (reads, must stay allowed)

Three spellings of one write, two verdicts — and the ALLOWing ones are not theoretical: in a scratch repo with a real remote, git branch --unset-upstream removed branch.main.remote and branch.main.merge from .git/config while read-only permitted it (measured in the previous cycle, and re-derived here).

Both directions of the new tests have a job, checked by mutation rather than by reading them:

• the four new tests pass in the branch's own arm (sha 89079862d0dcde01);
• mutant 1 — compare whole tokens again instead of asking _flag_part — kills test_check_read_only_blocks_attached_value_git_write_flags only (the other three were green, which is honest: they are different axes, and an earlier cycle already showed _flag_part and _git_positionals are both required);
• mutant 2 — drop --unset-upstream from the write-flag table — kills test_check_read_only_blocks_unset_upstream while the read-side test stays green.

One artefact a later reviewer will trip over, so naming it: git diff master 89ed6091 looks like this branch deletes the _GIT_CONFIG_* constants that #1255 added. It does not — that is the older merge base showing through. The merged tree keeps them and tests/test_git_config_shapes.py passes in the landing-tree run above. The same artefact makes the diff look like it removes that test file.

[argszero]

✅ LGTM — cycle cyc20260916-034020

The head (89ed6091) is stale (behind master by 2), so this reading is about the landing tree, not the head — refreshing would have voided the two votes this PR already holds. Measured: scripts/check-merge-plan-suite.py 1257 onto base bd420171 → final tree e95cd77424d4, suite OK 2429 passed / 2 skipped.

I re-derived the premise independently instead of reading the PR text. Driving the read-only guard directly, on master (bash_tool sha256 ef18528338df8c02, tree bd420171) five synonymous spellings split into two verdicts, and on the landing tree (bash_tool sha256 e9bd6d8ab003293e, tree e95cd77424d4) all five agree with no regression on read-only-safe commands:

spelling                              master    landing tree
git branch -uorigin/main              ALLOW     BLOCK
git branch -u origin/main             BLOCK     BLOCK
git branch --set-upstream-to=origin/main   ALLOW     BLOCK
git branch --set-upstream-to origin/main   BLOCK     BLOCK
git branch -dfeature/x                ALLOW     BLOCK
git branch -mold                      ALLOW     BLOCK
git tag -dv1                          ALLOW     BLOCK
git branch --list / -a / --show-current, git status, git log, git rev-parse  ALLOW in both

The block has a job, which I checked by asking real git in a scratch repository rather than assuming: the attached value -uorigin/main exits 0 and really rewrites .git/config (branch.<name>.remote/merge), as does --set-upstream-to=origin/main, and --unset-upstream really removes those lines — so on master the read-only tier allowed a genuine write. git branch -df feature/x exits 0 and really deletes the branch. The two spellings real git itself rejects (-dfeature/x, -dv1: rc=129) are blocked too, which is parity rather than overreach.

The property under test is parity — one shape, one verdict — not a blocked-string list, so it does not depend on my having enumerated the spellings correctly.