[brettheap]

Two commits, the second the fix round of this branch's independent review. The two halves of the first are ONE commit because neither is safe alone.

THE HOLE, and what it cost. Every value-taking arm tested [ $# -ge 2 ] && [ -n "$2" ], which asks whether a second argument EXISTS and whether it is non-empty and never what it HOLDS — so park Atlas --lane --dry-run ran a REAL park with the lane set to --dry-run: the flag a person typed was swallowed as a value, --dry-run itself never took effect, and the only sign was a WIP commit subject labelled with it. PR #17 named the hole and left it open; this is the ruling made against something concrete, and if it goes the other way the whole of it is one revert. Proved again on this base, in a temp estate with a probe Makefile, against origin/main's OWN three commands: park Atlas --lane --dry-run printed make park ARGS='--lane --dry-run' and RAN it, exit 0; resume Atlas --org --dry-run went past the rehearsal it was asked for and on to the workspace record, which on a machine that has one is a clone; status --name --fetch fetched nothing and refused about an estate named --fetch. After: each is REFUSED: <flag> needs a value: …, exit 2, from the argument loop with an EMPTY stdout, and the refusal's own sentence names the spelling that does take such a value. THE TWO HALVES ARE ONE COMMIT because refusing -* alone takes a value away with nowhere to put it, and the refusal's words name the = spelling as the exit, so the exit has to exist the moment the refusal does. All nine value-taking arms — park's --lane, --repo, --name | --project; resume's --repo, --workspace, --org, --name | --project; status's --repo, --name | --project, none of them in the shared estate resolver — read their value through flag_value, a new function held BYTE-IDENTICAL in the three files the way expand_home already is, and OUTSIDE that block for the reason its own header gives: the resolver answers "which estate is this", and this answers "did this flag get a value". It asks the arity question #14 asked, the emptiness question #17 added, and one more: a next argument beginning with - is a flag, and nothing here swallows the next word whatever it is. Each arm keeps its own words and its own die at exit 2; the --led refusal adds one sentence naming the spelling that does take such a value, asserted per flag so a hint cannot name the wrong one. --flag=value takes whatever follows the FIRST = and asks nothing about it, so --lane=--weird and --repo=https://… both arrive whole; --flag= is an empty value and is refused as #17 refuses "". park Atlas --lane=--dry-run --dry-run gives ARGS='--dry-run --lane --dry-run' — the rehearsal flag and the --led lane both through — and status --name=--fetch is read as a NAME, refused as "no estate named '--fetch'" and not as a flag. THE STANDARD'S OWN PYTHON HAS ALWAYS DONE THIS: openRepoShape's scripts/repo_shape.py (checked_value, "ARGUMENT INJECTION IS THE THREAT … A value that begins with - is therefore refused outright") and setup-project.py refuse a --led value, and their argparse entry points take --flag=value for free; the standard's bash front door, openRepoShape --org, does not yet — ORG="${2:?--org needs a value}", no = arm, still so at the pinned ba14e1d — and that is a pull request THERE, then a pin bump here. Each --help gains one line for the = spelling and nothing else. Tests: eighteen parametrized command cases across park and resume (nine flags, a --led value refused with nothing run, the = form byte-for-byte the same run as the spaced one, --flag= refused), plus status's own three flags in both shapes, park's lane case end to end, resume --workspace= / --org= doing the work those flags name rather than being echoed, and two hygiene tests. Suite at the branch tip, which adds no test after this commit: 571 passed, 0 skipped.

The fix round of the independent REPORT-ONLY review of this branch (2026-09-11; 168 runs across the twelve flag spellings and fourteen forms, main against the branch: every accepted form byte-identical to the spaced one, every --led value refused with an empty stdout, the -- passthrough and the no-value flags unchanged; verdict MERGE AFTER FIXES), every finding taken. THE ONE THAT MATTERED: test_every_value_taking_arm_reads_its_value_through_flag_value counted only arms that read $FLAG_VALUE, so a TENTH arm written the old way — X="$2", no emptiness test, no - test, no = spelling — would have left the count at nine and PASSED the suite, where the test it replaced had caught exactly that. It now names any ="$2" outside flag_value itself, and the count is still nine and still hand-kept, so a tenth arm is read rather than silently passed; re-proved on THIS base by injecting such an arm into a scratch copy of park — AssertionError: park: TENTH="$2"; shift 2 ;; takes $2 straight from the command line. THE SECOND: for --lane the = spelling buys park's own parsing and NOT the extension's — the recipe word-splits $(ARGS), so --lane=--dry-run hands the extension the same two words the accident did — which the lane test's docstring now says instead of claiming the value "survived"; park --help's example moves to --repo=<owner/name>, a value that never leaves the command, and status --help's to --project=<Name>, the one value-taking flag its OPTIONS block lists. THE THIRD: FLAG_VALUE, FLAG_SHIFT and FLAG_HINT are declared beside the other globals in all three files, so a die that reached $FLAG_HINT before any arm had asked prints an empty hint under set -u rather than "unbound variable" at exit 1 — the one exit these arms must not produce; unreachable today, since every arm asks first, and cheap to keep that way. Suite: 571 passed, 0 skipped.

BASED ON THE MAIN OF THE MOMENT. The branch was built on a40cbb7 and rebased onto 8a36eb3 for this first push — 35 commits, in four steps as main moved under it, and NOT ONE CONFLICT at any of them: park and resume had not moved on main at all (both byte-identical to a40cbb7's), and status's growth from 2,619 lines to 5,767 lands BETWEEN the regions these commits edit — the globals at line 102 and the OPTIONS block at 222 above it, flag_value and the argument loop at 5,631 and 5,704 below it. Every number here is the re-measurement on that base, not one carried over. THE BITE: all 24 of these tests FAIL against origin/main's own park, resume and status — the 23 the first commit adds and status's own test_refusals_read_nothing, which took the flag-shaped readings — measured in a scratch tree with main's three commands dropped in beside this branch's tests: 24 failed, 436 passed. Suite on the branch: 571 passed, 0 skipped, against main's 549 collected; 23 test ids added, one renamed away (test_every_value_taking_arm_refuses_an_empty_value, which this replaces). INVARIANTS, each asserted by the suite and checked again by hand: the shared estate resolver byte-identical across the three AND byte-identical to main's, because this branch does not touch it; expand_home byte-identical in status and resume; flag_value byte-identical in all three, which is the new test; bash -n clean on all three; no ${x,,}, mapfile, declare -A or local -n, so it parses under macOS bash 3.2; every "${arr[@]}" guard is main's own, since this branch adds no array. NO CAP MOVES: README.md and AGENTS.md are untouched and still sit exactly at their caps, 409 and 224.

Ruling: "ruling (c): take dash-values as built, push and PR" — Brett Heap, 2026-09-14.

Lane: openrepotools-3 (openRepoTools-3)

🤖 Generated with Claude Code

Summary by Sourcery

Harden value-taking option parsing across the estate commands while adding an explicit equals-sign form for values that begin with dashes.

New Features:

• Support --flag=value syntax for all value-taking options in park, resume, and status, including values beginning with -.

Bug Fixes:

• Reject missing, empty, and flag-shaped values instead of consuming the next option as a value or falling through to estate resolution.
• Prevent commands from performing work or producing normal output before invalid option values are refused.

Enhancements:

• Centralize value validation through a byte-identical flag_value helper shared across the three estate commands.
• Strengthen hygiene checks to ensure every value-taking option uses the shared parser and supports both option spellings.

Documentation:

• Update command help text with the supported --flag=<value> spelling.

Tests:

• Add coverage for invalid flag-shaped and empty values, equivalent spaced and equals forms, dash-prefixed values, and end-to-end park, resume, and status behavior.

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 6 hours and 21 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

[sourcery-ai]

Reviewer's Guide

The PR fixes option-value injection across park, resume, and status by introducing a byte-identical parser that rejects missing, empty, and dash-prefixed spaced values, supports --flag=value, and prevents execution or resolution before refusal. Tests cover all value-taking flags, command-specific behavior, side effects, parser consistency, and future-arm hygiene.

File-Level Changes

Change	Details	Files
Centralize and harden value parsing for every value-taking option across the three estate commands.	Add identical flag_value helpers that distinguish missing, empty, dash-prefixed, spaced, and equals-form values. Route all nine value-taking arms through the helper and add both spaced and --flag=* case patterns. Declare shared parser state defensively and preserve command-specific refusal messages, hints, and exit status 2.	park resume status
Prevent dash-prefixed flags from being consumed as option values while preserving an explicit spelling for legitimate dash-prefixed values.	Refuse forms such as --lane --dry-run before estate resolution or command execution, with empty stdout and a per-flag --flag=value hint. Accept --flag=value using the bytes after the first equals sign, including values beginning with '-', while refusing --flag= as empty. Keep no-value flags, -- passthrough behavior, and existing estate resolution semantics unchanged.	park resume status
Expand behavioral coverage for value parsing, side effects, and command-specific integrations.	Add parametrized park/resume coverage for dash-prefixed, equals-form, and empty values. Verify park lane handling and resume workspace/org values perform the intended work rather than being echoed or swallowed. Add status coverage for all three value-taking options, including name resolution for --name=--fetch and refusal side effects.	tests/test_park_resume_commands.py tests/test_status_command.py
Strengthen repository hygiene checks so future value-taking arms cannot bypass the shared parser or drift between commands.	Assert flag_value is byte-identical across park, resume, and status and contains all required validation checks. Detect direct "$2" assignments outside the helper, require each value-taking arm to call flag_value, and verify matching spaced and equals spellings. Maintain the hand-counted nine-arm invariant.	tests/test_repo_hygiene.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[Copilot]

🔵 Needs a closer look

The hygiene detector must scope its FLAG_VALUE="$2" exemption to the helper body and detect general assignments outside it.

Pull request overview

This PR hardens value-taking option parsing across park, resume, and status, adding safe --flag=value support.

Changes:

• Reject missing, empty, and flag-shaped values.
• Add consistent flag_value parsing and help documentation.
• Expand command and hygiene test coverage.

File summaries

File	Summary
tests/test_status_command.py	Tests status parsing, equals-form values, and refusals.
tests/test_repo_hygiene.py	Verifies parser consistency; moderate issue remains in the detector’s exemption logic (1 vote).
tests/test_park_resume_commands.py	Tests refusal and equals-form behavior for park and resume.
status	Adds validated option parsing and equals-form values.
resume	Adds validated value parsing for all value-taking options.
park	Adds validated option parsing and equals-form values.

Review details

Suppressed comments (1)

tests/test_repo_hygiene.py:416

• This detector exempts any line exactly matching FLAG_VALUE="$2" without proving it is inside flag_value(). A future value-taking arm could bypass the helper with that same assignment and continue, leaving found == 9 so the regression passes; the uppercase-only regex also misses value="$2". Scope the exemption to the helper body and match a general shell variable assignment outside it.

            if re.match(r'\s*[A-Z_]+="\$2"', line):
                assert line.strip() == 'FLAG_VALUE="$2"', (
                    f"{name}: {line.strip()} takes $2 straight from the "
                    f"command line; every value-taking arm reads its value "
                    f"through flag_value")

• Files reviewed: 6/6 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[brettheap]

Copilot's first round on this branch (2026-09-14): one suppressed comment, no inline comments. TAKEN, in 594a591 — verified empirically before anything moved, and real in both of its halves.

The comment, verbatim:

This detector exempts any line exactly matching FLAG_VALUE="$2" without proving it is inside flag_value(). A future value-taking arm could bypass the helper with that same assignment and continue, leaving found == 9 so the regression passes; the uppercase-only regex also misses value="$2". Scope the exemption to the helper body and match a general shell variable assignment outside it.

Both halves reproduce. Against the pushed detector (f07c23f), on a scratch copy of park with a tenth arm injected above --lane:

injected tenth arm	f07c23f	594a591
TENTH="$2"; shift 2 ;;	caught	caught
FLAG_VALUE="$2" then TENTH="${FLAG_VALUE}"	MISSED — 1 passed	caught
value2="$2"; shift 2 ;;	MISSED — 1 passed	caught
TENTH="${2}"; shift 2 ;;	MISSED — 1 passed	caught

One of four, and the second row is the comment's own scenario: the arm writes the exempted line itself, then reads the value back in the braced spelling, which the counter's ="$FLAG_VALUE" did not match either — so it left found at nine and PASSED. That is the same silent pass the independent review of 2026-09-11 caught in the version before this one, reached through a different door, which is exactly what this test exists to prevent.

The exemption is a LINE RANGE now, not a string. flag_value() { to the } in column 0 that closes it is located once per file — asserted to be exactly one definition, and to close — and "the legitimate read of $2" means a line number inside that range. The assignment the detector reads is any shell variable in either spelling of $2, [A-Za-z_][A-Za-z0-9_]*="\$\{?2\}?", so value2="$2" and X="${2}" are the same arm in a different case and in braces; and the counter reads ${FLAG_VALUE} as well as $FLAG_VALUE, so bracing hides nothing at the other end either. All four rows above are caught by the range assertion itself, with the arm's own text in the message, and the clean tree still passes.

One part is not taken as written, and the difference is not convenience. "Match a general shell variable assignment outside it" as an unanchored search makes a finding of local file="$1" key="$2" line — a function reading its own parameters, which is not an arm and never was. Measured on the comment-stripped text this test reads, outside flag_value's body: 36 such lines (2 in park, 6 in resume, 28 in status), none of them anchored. An unanchored search would report 36 failures on a tree with no fault in it at all. The match therefore stays anchored at the start of the statement — the arms live at the top level of the argument loop, where local is not even legal, so anchoring costs the check nothing it was buying.

The count is still nine and still hand-kept. No document line moves, so no cap moves. Suite: 571 passed, 0 skipped.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🟢 Approval recommended

No unresolved blocking issues remain, and the reported test suite passes.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Lite