diff --git a/park b/park index 0d6b31cf..132ce6db 100755 --- a/park +++ b/park @@ -50,6 +50,13 @@ REPO_ARG="" DRY_RUN=0 LANE="" PASSTHROUGH=() +#: `flag_value`'s three answers, declared with the other globals so that a +#: `die` reaching `$FLAG_HINT` before any arm has asked is an empty hint under +#: `set -u`, never "unbound variable" at exit 1 — the one exit these arms must +#: not produce (the independent review of this branch, 2026-09-11). +FLAG_VALUE="" +FLAG_SHIFT=2 +FLAG_HINT="" #: 1 when EVERY estate under the projects directory is to be parked: set by #: `--all` / `-a` on the command line, or by a YES to the question the bare #: form asks when the walk-up found no estate around the cwd. Every other @@ -100,6 +107,9 @@ command that clones). --project the positional under a flag (also --name) --dry-run rehearse: `make park ARGS=--dry-run`, which writes nothing --lane the lane recorded in the WIP commit subject + --flag= every flag that takes a value takes it after an `=` too + (`--repo=`): the one way to give a value + that begins with `-`, which the spaced spelling refuses -- ARGS... passed on to the extension through `make park ARGS='...'` $PROJECTS_DIR your projects directory (default: ~/projects, then @@ -881,6 +891,57 @@ $(estates_or_none)" } # --- arguments ------------------------------------------------------------- + +# THE VALUE OF A VALUE-TAKING FLAG, in either spelling, read in ONE place so +# that the ways of not giving one are ONE refusal. `flag_value "$@"` from an +# arm sets `FLAG_VALUE` and `FLAG_SHIFT` — the arguments that arm then +# consumes, 2 for `--flag value` and 1 for `--flag=value` — or it returns 1 +# with `FLAG_HINT` carrying whatever that refusal has to add. THE WORDS STAY +# AT THE ARM: each flag says what its own value is, and this says only whether +# one was given. +# A VALUE BEGINNING WITH `-` IS NOT ONE in the spaced spelling — the hole #17 +# named and left open. `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. Nothing here swallows the next word whatever it +# is, so that is a refusal. +# `--flag=value` IS THE OTHER HALF, and the escape that refusing `-*` would +# otherwise foreclose: a value that really does begin with `-` is spelled +# `--lane=--weird`, which takes whatever follows the FIRST `=` and asks +# nothing about it. An `=` with nothing after it is an empty value and is +# refused as #17 refuses one. +# BYTE-IDENTICAL IN `park`, `resume` AND `status`, held by +# `tests/test_repo_hygiene.py` the way the estate resolver and `expand_home` +# are: three files a person has on PATH, and one answer to what a flag's value +# is. +flag_value() { + FLAG_VALUE="" + FLAG_SHIFT=2 + FLAG_HINT="" + case "$1" in + *=*) + FLAG_SHIFT=1 + FLAG_VALUE="${1#*=}" + [ -n "$FLAG_VALUE" ] || return 1 + return 0 ;; + esac + [ $# -ge 2 ] && [ -n "$2" ] || return 1 + # THE NEXT ARGUMENT, ONLY IF IT CANNOT BE A FLAG. The hint names the `=` + # spelling because that is the whole exit: this refuses nothing a person + # cannot then say. + case "$2" in + -*) + FLAG_HINT=" + \`$2\` begins with a \`-\`, so it is a flag and not this one's value: + nothing here swallows the next word whatever it is. A value that really + does begin with \`-\` is given after an \`=\`: + + $1=$2" + return 1 ;; + esac + FLAG_VALUE="$2" +} + while [ $# -gt 0 ]; do case "$1" in -h | --help) usage; exit 0 ;; @@ -889,13 +950,14 @@ while [ $# -gt 0 ]; do # anywhere, with no question (the RULING of 2026-09-10 at the sweep's # section head: "park all should be park -a or park --all"). -a | --all) PARK_ALL=1; shift ;; - # A MISSING OR EMPTY VALUE IS THIS COMMAND'S REFUSAL, exit 2, in its own - # words: `${2:?…}` would be bash's message and bash's exit 1, which is not - # a status this command means anything by (`status` gave the reason first: - # its 1 is "findings"; here, a typo must not read like `make park`'s own - # exit). - # EMPTY IS REFUSED THE SAME WAY, which is why every arm below asks for a - # NON-EMPTY `$2` and not only for a second argument. `${2:?…}` rejected + # A MISSING, EMPTY OR FLAG-SHAPED VALUE IS THIS COMMAND'S REFUSAL, exit 2, + # in its own words: `${2:?…}` would be bash's message and bash's exit 1, + # which is not a status this command means anything by (`status` gave the + # reason first: its 1 is "findings"; here, a typo must not read like `make + # park`'s own exit). + # EMPTY IS REFUSED THE SAME WAY, and so is a flag where a value belongs: + # `flag_value` above asks all three questions once for every arm below, + # and each arm keeps its own words. `${2:?…}` rejected # unset AND null; `[ $# -ge 2 ]` alone rejects only the first, and what it # lets through is read later as "was this given at all?" — so # `park --lane ""` would carry on with `LANE` empty, the lane is then left @@ -906,23 +968,23 @@ while [ $# -gt 0 ]; do # origin was there to prevent. A value that came out empty is a flag that # was typed and lost, never one that was never typed, and the two must not # do different things (Copilot's review of #14). - --lane) - [ $# -ge 2 ] && [ -n "$2" ] || - die "--lane needs a value: the lane recorded in the WIP commit subject." - LANE="$2"; shift 2 ;; - --repo) - [ $# -ge 2 ] && [ -n "$2" ] || - die "--repo needs a value: /, or any url spelling of one." - REPO_SLUG="$2"; REPO_ARG="$REPO_SLUG"; shift 2 ;; + --lane | --lane=*) + flag_value "$@" || + die "--lane needs a value: the lane recorded in the WIP commit subject.$FLAG_HINT" + LANE="$FLAG_VALUE"; shift "$FLAG_SHIFT" ;; + --repo | --repo=*) + flag_value "$@" || + die "--repo needs a value: /, or any url spelling of one.$FLAG_HINT" + REPO_SLUG="$FLAG_VALUE"; REPO_ARG="$REPO_SLUG"; shift "$FLAG_SHIFT" ;; # `--project ` and `--name ` are the positional under a flag, # because the issue's own words are "park --project or park # --repo and it defaults to project" (Brett Heap, 2026-09-09). The # value is never mistaken for the positional this way, which is the same # reason `openRepoShape` names its value-taking flags. - --name | --project) - [ $# -ge 2 ] && [ -n "$2" ] || - die "$1 needs a value: one estate folder name under your projects directory." - NAME="$2"; shift 2 ;; + --name | --project | --name=* | --project=*) + flag_value "$@" || + die "${1%%=*} needs a value: one estate folder name under your projects directory.$FLAG_HINT" + NAME="$FLAG_VALUE"; shift "$FLAG_SHIFT" ;; # EVERYTHING AFTER `--` IS THE EXTENSION'S, verbatim. `park.sh` has flags # this command has no opinion about (--feature, --message, --no-push, # --retire-parked-wip) and a command that ate them would be a command diff --git a/resume b/resume index b37ce05d..92fd4303 100755 --- a/resume +++ b/resume @@ -47,6 +47,13 @@ WORKSPACE_SLUG="" ORG_WANTED="" DRY_RUN=0 PASSTHROUGH=() +#: `flag_value`'s three answers, declared with the other globals so that a +#: `die` reaching `$FLAG_HINT` before any arm has asked is an empty hint under +#: `set -u`, never "unbound variable" at exit 1 — the one exit these arms must +#: not produce (the independent review of this branch, 2026-09-11). +FLAG_VALUE="" +FLAG_SHIFT=2 +FLAG_HINT="" say() { printf '%s\n' "$*"; } die() { printf '\nREFUSED: %s\n' "$1" >&2; exit "${2:-2}"; } @@ -75,6 +82,9 @@ With no the estate around the current directory is refreshed. --project the positional under a flag (also --name) --org which org's manifest, when two of them record a --dry-run say what would happen; clone nothing, pull nothing + --flag= every flag that takes a value takes it after an `=` + too (`--org=`): the one way to give a value that + begins with `-`, which the spaced spelling refuses -- ARGS... passed on to the extension through `make resume ARGS=` $PROJECTS_DIR your projects directory (default: ~/projects, then @@ -754,16 +764,68 @@ run_make() { } # --- arguments ------------------------------------------------------------- + +# THE VALUE OF A VALUE-TAKING FLAG, in either spelling, read in ONE place so +# that the ways of not giving one are ONE refusal. `flag_value "$@"` from an +# arm sets `FLAG_VALUE` and `FLAG_SHIFT` — the arguments that arm then +# consumes, 2 for `--flag value` and 1 for `--flag=value` — or it returns 1 +# with `FLAG_HINT` carrying whatever that refusal has to add. THE WORDS STAY +# AT THE ARM: each flag says what its own value is, and this says only whether +# one was given. +# A VALUE BEGINNING WITH `-` IS NOT ONE in the spaced spelling — the hole #17 +# named and left open. `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. Nothing here swallows the next word whatever it +# is, so that is a refusal. +# `--flag=value` IS THE OTHER HALF, and the escape that refusing `-*` would +# otherwise foreclose: a value that really does begin with `-` is spelled +# `--lane=--weird`, which takes whatever follows the FIRST `=` and asks +# nothing about it. An `=` with nothing after it is an empty value and is +# refused as #17 refuses one. +# BYTE-IDENTICAL IN `park`, `resume` AND `status`, held by +# `tests/test_repo_hygiene.py` the way the estate resolver and `expand_home` +# are: three files a person has on PATH, and one answer to what a flag's value +# is. +flag_value() { + FLAG_VALUE="" + FLAG_SHIFT=2 + FLAG_HINT="" + case "$1" in + *=*) + FLAG_SHIFT=1 + FLAG_VALUE="${1#*=}" + [ -n "$FLAG_VALUE" ] || return 1 + return 0 ;; + esac + [ $# -ge 2 ] && [ -n "$2" ] || return 1 + # THE NEXT ARGUMENT, ONLY IF IT CANNOT BE A FLAG. The hint names the `=` + # spelling because that is the whole exit: this refuses nothing a person + # cannot then say. + case "$2" in + -*) + FLAG_HINT=" + \`$2\` begins with a \`-\`, so it is a flag and not this one's value: + nothing here swallows the next word whatever it is. A value that really + does begin with \`-\` is given after an \`=\`: + + $1=$2" + return 1 ;; + esac + FLAG_VALUE="$2" +} + while [ $# -gt 0 ]; do case "$1" in -h | --help) usage; exit 0 ;; --dry-run) DRY_RUN=1; shift ;; - # A MISSING OR EMPTY VALUE IS THIS COMMAND'S REFUSAL, exit 2, in its own - # words: `${2:?…}` would be bash's message and bash's exit 1 — and a - # `resume` that exits non-zero is read as "something was refused", so the - # refusal has to be one this command wrote and a person can act on. - # EMPTY IS REFUSED THE SAME WAY, which is why every arm below asks for a - # NON-EMPTY `$2` and not only for a second argument. `${2:?…}` rejected + # A MISSING, EMPTY OR FLAG-SHAPED VALUE IS THIS COMMAND'S REFUSAL, exit 2, + # in its own words: `${2:?…}` would be bash's message and bash's exit 1 — + # and a `resume` that exits non-zero is read as "something was refused", + # so the refusal has to be one this command wrote and a person can act on. + # EMPTY IS REFUSED THE SAME WAY, and so is a flag where a value belongs: + # `flag_value` above asks all three questions once for every arm below, + # and each arm keeps its own words. `${2:?…}` rejected # unset AND null; `[ $# -ge 2 ]` alone rejects only the first, and every # value here is read later as "was this given at all?" — so # `resume --repo ""` would fall past the `--repo` branch of the resolver @@ -782,22 +844,22 @@ while [ $# -gt 0 ]; do # A value that came out empty is a flag that was typed and lost, never one # that was never typed, and the two must not do different things # (Copilot's review of #14). - --repo) - [ $# -ge 2 ] && [ -n "$2" ] || - die "--repo needs a value: /, or any url spelling of one." - REPO_SLUG="$2"; REPO_ARG="$REPO_SLUG"; shift 2 ;; - --workspace) - [ $# -ge 2 ] && [ -n "$2" ] || - die "--workspace needs a value: / of your private workspace repository." - WORKSPACE_SLUG="$2"; shift 2 ;; - --org) - [ $# -ge 2 ] && [ -n "$2" ] || - die "--org needs a value: the organisation whose manifest to read." - ORG_WANTED="$2"; shift 2 ;; - --name | --project) - [ $# -ge 2 ] && [ -n "$2" ] || - die "$1 needs a value: one estate folder name under your projects directory." - NAME="$2"; shift 2 ;; + --repo | --repo=*) + flag_value "$@" || + die "--repo needs a value: /, or any url spelling of one.$FLAG_HINT" + REPO_SLUG="$FLAG_VALUE"; REPO_ARG="$REPO_SLUG"; shift "$FLAG_SHIFT" ;; + --workspace | --workspace=*) + flag_value "$@" || + die "--workspace needs a value: / of your private workspace repository.$FLAG_HINT" + WORKSPACE_SLUG="$FLAG_VALUE"; shift "$FLAG_SHIFT" ;; + --org | --org=*) + flag_value "$@" || + die "--org needs a value: the organisation whose manifest to read.$FLAG_HINT" + ORG_WANTED="$FLAG_VALUE"; shift "$FLAG_SHIFT" ;; + --name | --project | --name=* | --project=*) + flag_value "$@" || + die "${1%%=*} needs a value: one estate folder name under your projects directory.$FLAG_HINT" + NAME="$FLAG_VALUE"; shift "$FLAG_SHIFT" ;; # `park --all` exists; `resume --all` deliberately does not (see the # header). Said here in so many words, because the generic answer below # - "put it after `--`" - would hand the extension a flag it does not diff --git a/status b/status index 53972212..bb5c2559 100755 --- a/status +++ b/status @@ -102,6 +102,13 @@ SELF_NAME="status" NAME="" REPO_SLUG="" REPO_ARG="" +#: `flag_value`'s three answers, declared with the other globals so that a +#: `die` reaching `$FLAG_HINT` before any arm has asked is an empty hint under +#: `set -u`, never "unbound variable" at exit 1 — the one exit these arms must +#: not produce (the independent review of this branch, 2026-09-11). +FLAG_VALUE="" +FLAG_SHIFT=2 +FLAG_HINT="" #: 1 when EVERY estate under the projects directory is to be read: `--all` / #: `-a`, or the bare form standing outside every estate — which SWEEPS WITHOUT #: ASKING, unlike `park`'s, because nothing here writes and a question before @@ -215,6 +222,9 @@ a clone you ALREADY HAVE, in any spelling git accepts. fetch, not to the row you are looking at. --all, -a read EVERY estate under your projects directory --project the positional under a flag (also --name) + --flag= every flag that takes a value takes it after an `=` too + (`--project=`): the one way to give a value that + begins with `-`, which the spaced spelling refuses $PROJECTS_DIR your projects directory (default: ~/projects, then ~/Projects - people spell it both ways) @@ -5621,16 +5631,68 @@ status_every_estate() { } # --- arguments ------------------------------------------------------------- + +# THE VALUE OF A VALUE-TAKING FLAG, in either spelling, read in ONE place so +# that the ways of not giving one are ONE refusal. `flag_value "$@"` from an +# arm sets `FLAG_VALUE` and `FLAG_SHIFT` — the arguments that arm then +# consumes, 2 for `--flag value` and 1 for `--flag=value` — or it returns 1 +# with `FLAG_HINT` carrying whatever that refusal has to add. THE WORDS STAY +# AT THE ARM: each flag says what its own value is, and this says only whether +# one was given. +# A VALUE BEGINNING WITH `-` IS NOT ONE in the spaced spelling — the hole #17 +# named and left open. `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. Nothing here swallows the next word whatever it +# is, so that is a refusal. +# `--flag=value` IS THE OTHER HALF, and the escape that refusing `-*` would +# otherwise foreclose: a value that really does begin with `-` is spelled +# `--lane=--weird`, which takes whatever follows the FIRST `=` and asks +# nothing about it. An `=` with nothing after it is an empty value and is +# refused as #17 refuses one. +# BYTE-IDENTICAL IN `park`, `resume` AND `status`, held by +# `tests/test_repo_hygiene.py` the way the estate resolver and `expand_home` +# are: three files a person has on PATH, and one answer to what a flag's value +# is. +flag_value() { + FLAG_VALUE="" + FLAG_SHIFT=2 + FLAG_HINT="" + case "$1" in + *=*) + FLAG_SHIFT=1 + FLAG_VALUE="${1#*=}" + [ -n "$FLAG_VALUE" ] || return 1 + return 0 ;; + esac + [ $# -ge 2 ] && [ -n "$2" ] || return 1 + # THE NEXT ARGUMENT, ONLY IF IT CANNOT BE A FLAG. The hint names the `=` + # spelling because that is the whole exit: this refuses nothing a person + # cannot then say. + case "$2" in + -*) + FLAG_HINT=" + \`$2\` begins with a \`-\`, so it is a flag and not this one's value: + nothing here swallows the next word whatever it is. A value that really + does begin with \`-\` is given after an \`=\`: + + $1=$2" + return 1 ;; + esac + FLAG_VALUE="$2" +} + while [ $# -gt 0 ]; do case "$1" in -h | --help) usage; exit 0 ;; -a | --all) STATUS_ALL=1; shift ;; - # A MISSING OR EMPTY VALUE IS A REFUSAL, exit 2, in this command's own - # words: `${2:?…}` would be bash's message and bash's exit 1 — and 1 is - # what this command means by "findings were printed", so a typo must not - # be able to read as a report. - # EMPTY IS REFUSED THE SAME WAY, which is why both arms below ask for a - # NON-EMPTY `$2` and not only for a second argument. THESE TWO WERE NEVER + # A MISSING, EMPTY OR FLAG-SHAPED VALUE IS A REFUSAL, exit 2, in this + # command's own words: `${2:?…}` would be bash's message and bash's exit 1 + # — and 1 is what this command means by "findings were printed", so a typo + # must not be able to read as a report. + # EMPTY IS REFUSED THE SAME WAY, and so is a flag where a value belongs: + # `flag_value` above asks all three questions once for both arms below, + # and each arm keeps its own words. THESE TWO WERE NEVER # `${2:?…}`: they have been arity-only since this file's first commit, so # when Copilot's review of #14 said that `[ $# -ge 2 ]` checks only that # an argument slot exists — said of `park` and `resume`, which had just @@ -5642,14 +5704,14 @@ while [ $# -gt 0 ]; do # had never been typed. A wrong answer is worse here than in either verb, # not better: nothing was changed, so there is nothing to undo and no # reason for anybody to look twice. - --repo) - [ $# -ge 2 ] && [ -n "$2" ] || - die "--repo needs a value: /, or any url spelling of one." - REPO_SLUG="$2"; REPO_ARG="$REPO_SLUG"; shift 2 ;; - --name | --project) - [ $# -ge 2 ] && [ -n "$2" ] || - die "$1 needs a value: one estate folder name under your projects directory." - NAME="$2"; shift 2 ;; + --repo | --repo=*) + flag_value "$@" || + die "--repo needs a value: /, or any url spelling of one.$FLAG_HINT" + REPO_SLUG="$FLAG_VALUE"; REPO_ARG="$REPO_SLUG"; shift "$FLAG_SHIFT" ;; + --name | --project | --name=* | --project=*) + flag_value "$@" || + die "${1%%=*} needs a value: one estate folder name under your projects directory.$FLAG_HINT" + NAME="$FLAG_VALUE"; shift "$FLAG_SHIFT" ;; # THE SECOND LAYER (Brett Heap's RULING of 2026-09-11, "next layer: # --fetch"): ask origin in every repository first. See `fetch_remote`. --fetch) FETCH=1; CHANGED_CLAUSE="only remote-tracking refs and fetched objects were changed (--fetch)."; shift ;; diff --git a/tests/test_park_resume_commands.py b/tests/test_park_resume_commands.py index 9e00aa88..8cf874fe 100644 --- a/tests/test_park_resume_commands.py +++ b/tests/test_park_resume_commands.py @@ -1468,6 +1468,150 @@ def test_a_flag_given_an_empty_value_is_refused_the_same_way( assert not (home / ".agents").exists(), "nothing may be written before a refusal" +@pytest.mark.parametrize("command, flag", VALUE_TAKING_FLAGS, ids=flag_id) +def test_a_flag_given_a_flag_for_its_value_is_refused_and_nothing_runs( + home, command, flag): + """A FLAG IS NOT A VALUE, and it was read as one until now. + + `${2:?…}` and the arity check that replaced it both asked only whether + there was a next argument; `park Atlas --lane --dry-run` therefore ran a + REAL park with its lane set to `--dry-run` — the flag a person typed, + swallowed, with `--dry-run` itself never taking effect and nothing but + the WIP commit subject to say so. That is the same silence as `--lane ""`, + one argument further along, and this is the same refusal: exit 2, in the + arm's own words, in the argument loop before anything is resolved or run. + + THE REFUSAL NAMES THE WAY BACK, which is the whole reason `-*` can be + refused at all: `--lane=--dry-run` says the same thing and is not + ambiguous, so nothing a person could legitimately mean has been taken + away. Asserted here per flag, because a hint that named the wrong flag + would be worse than none. + + RUN FROM INSIDE AN ESTATE, which is what makes this bite: the walk-up + finds `Atlas` from there, so before the fix `park` went all the way into + `make park` and `resume` as far as the workspace record. An empty stdout + is the proof that neither the verb nor the fallback was reached. + """ + root = probe_project(home / "projects", "Atlas") + result = run(command, flag, "--dry-run", home=home, cwd=root) + assert result.returncode == 2, result.stdout + result.stderr + assert f"REFUSED: {flag} needs a value:" in result.stderr + assert "`--dry-run` begins with a `-`" in result.stderr + assert f"{flag}=--dry-run" in result.stderr, ( + "the refusal must name the spelling that does take this value") + assert result.stdout == "", ( + "nothing may be resolved, said or run before a flag given a flag is " + f"refused; stdout was {result.stdout!r}") + assert "PROBE" not in result.stdout + result.stderr, ( + "the estate's own make target ran: nothing was parked, and nothing " + "may have been") + assert not (home / ".agents").exists(), "nothing may be written before a refusal" + + +@pytest.mark.parametrize("command, flag", VALUE_TAKING_FLAGS, ids=flag_id) +def test_a_flag_takes_its_value_after_an_equals_sign_too(home, command, flag): + """`--flag=value` IS `--flag value`, and it is the only way to pass a + value that begins with `-`. + + Refusing a `-`-led value in the spaced spelling would otherwise foreclose + such a value entirely, because these arms had no `=` form at all: before + this, `--lane=x` fell through to the catch-all and came back as "park does + not know --lane=x". So the two halves are asserted together — the same + command, spelled both ways, has to produce the same bytes and the same + exit; and `--flag=--dry-run` has to be ACCEPTED, which is what makes the + refusal above a spelling rule rather than a value this toolset can no + longer express. + + AN `=` WITH NOTHING AFTER IT IS AN EMPTY VALUE, refused exactly as `""` + is: `${1#*=}` cannot tell the two apart and must not, or `--repo=` would + be the walk-up again under a new spelling. + """ + root = probe_project(home / "projects", "Atlas") + spaced = run(command, flag, "zz-value", home=home, cwd=root) + equals = run(command, f"{flag}=zz-value", home=home, cwd=root) + assert (equals.returncode, equals.stdout, equals.stderr) == \ + (spaced.returncode, spaced.stdout, spaced.stderr), ( + f"{flag}=zz-value and {flag} zz-value are not the same run:\n" + f" = : {equals.returncode} {equals.stdout!r} {equals.stderr!r}\n" + f" ' ': {spaced.returncode} {spaced.stdout!r} {spaced.stderr!r}") + assert "does not know" not in equals.stderr + + weird = run(command, f"{flag}=--dry-run", home=home, cwd=root) + assert f"{flag} needs a value" not in weird.stderr, ( + f"{flag}=--dry-run is the spelling the refusal names; it must take " + f"the value, not repeat the refusal") + assert "does not know" not in weird.stderr, ( + f"{flag}=--dry-run reached the catch-all: the `=` arm is missing") + + nothing = run(command, f"{flag}=", home=home, cwd=root) + assert nothing.returncode == 2, nothing.stdout + nothing.stderr + assert f"REFUSED: {flag} needs a value:" in nothing.stderr + assert "begins with a `-`" not in nothing.stderr, ( + "an empty value is empty, not flag-shaped") + assert nothing.stdout == "", ( + f"nothing may be run before `{flag}=` is refused; stdout was " + f"{nothing.stdout!r}") + + +def test_park_takes_a_lane_beginning_with_a_dash_through_the_equals_form(home): + """THE CASE #17 NAMED, both halves, end to end. + + `park Atlas --lane --dry-run` is the accident: two flags typed, one + swallowed as the other's value, a REAL park where a rehearsal was meant. + It is now a refusal that runs nothing — the probe Makefile would say so if + it had. + + And said with an `=` it is a lane again: `--lane=--dry-run --dry-run` puts + BOTH through, so `ARGS` carries the rehearsal flag once from `--dry-run` + and once as the lane's value. That second `--dry-run` is what proves the + value reached `LANE` and went out in `ARGS` — which is as far as this + command can carry it: the recipe word-splits `$(ARGS)`, so the extension + receives the same two words from `--lane=--dry-run` as it did from the + accident, and the `=` form buys `park`'s own parsing, not the extension's + (the independent review of this branch, 2026-09-11). For `--repo`, + `--name` and `--project`, which never leave the command, it buys the value. + """ + root = probe_project(home / "projects", "Atlas") + refused = run(PARK, "Atlas", "--lane", "--dry-run", home=home) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert "REFUSED: --lane needs a value:" in refused.stderr + assert "--lane=--dry-run" in refused.stderr + assert refused.stdout == "", f"a refusal parked: {refused.stdout!r}" + assert "PROBE park" not in refused.stdout + refused.stderr + + result = run(PARK, "Atlas", "--lane=--dry-run", "--dry-run", home=home) + assert result.returncode == 0, result.stderr + result.stdout + assert "make park ARGS='--dry-run --lane --dry-run'" in result.stdout + assert f"PROBE park in {root} ARGS=[--dry-run --lane --dry-run]" \ + in result.stdout + + +def test_resume_takes_the_workspace_and_the_org_after_an_equals_sign(remotes, + home): + """The `=` spelling is not a second parser: the value does the same work. + + Both of these flags CHANGE WHAT HAPPENS rather than merely being echoed — + `--workspace` names the private repository to clone, `--org` picks between + two manifests that record one name — so a spelling that arrived but was + read as something else would show here and nowhere in the parametrized + pair above. The two calls are the same two `test_resume_refuses_when_two_ + orgs_record_the_same_name` makes, spelled with an `=`. + """ + refused = run(RESUME, FAMILY, "--workspace=tester/twoorgs", home=home, + env=offline(remotes)) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert "workspace manifests record 'TestFam'" in refused.stderr + assert f"resume {FAMILY} --org " in refused.stderr + assert (home / ".agents" / "workspace.yaml").is_file(), ( + "the workspace slug arrived: the config it writes is the proof") + + answered = run(RESUME, FAMILY, "--org=orgb", home=home, + env=offline(remotes)) + assert answered.returncode == 0, answered.stderr + answered.stdout + assert "org orgb" in answered.stdout + assert (home / "projects" / FAMILY / FAMILY / "family.yaml").is_file() + + def test_a_leg_whose_submodule_name_carries_a_space_is_still_reported(home, tmp_path): """`.gitmodules` is read with `git config -z`: a submodule NAME with a diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index 7d8ed2a9..986f56ea 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -245,6 +245,44 @@ def function(path: Path) -> str: "status and resume have drifted apart in expand_home") +def test_the_estate_commands_carry_the_same_flag_value_byte_for_byte(): + """`flag_value` reads `--flag value` and `--flag=value` the same way in + all three, and the comment above it is held too. + + It sits OUTSIDE the shared estate resolver on purpose: that block answers + "which estate is this", and this answers "did this flag get a value" — + naming one the other would make the resolver's own header a lie. So it is + held here, the way `expand_home` is, and for the same reason: three files + a person has on PATH, and one answer. + + THE QUESTIONS IT ASKS ARE ASSERTED TOO, because byte-identity alone is + satisfied by three copies of a function that asks none of them. Each is + the whole of one refusal: the arity check #14 wrote (`park --lane` at the + end of the line), the emptiness check `${2:?…}` used to make and #17 put + back (`--lane ""`), and the `-*` check (`--lane --dry-run`, which parked + with the lane set to the flag a person typed). The `=` split is the + fourth, and it is what keeps a value that really does begin with `-` + sayable at all. + """ + def block(path: Path) -> str: + text = path.read_text(encoding="utf-8") + start = text.index("# THE VALUE OF A VALUE-TAKING FLAG") + return text[start:text.index("\n}\n", start)] + + blocks = {name: block(REPO / name) for name in ESTATE_COMMANDS} + reference = blocks["park"] + for name, text in blocks.items(): + assert text == reference, ( + f"park and {name} have drifted apart in flag_value") + for asked in ('[ $# -ge 2 ] && [ -n "$2" ] || return 1', + '[ -n "$FLAG_VALUE" ] || return 1', + 'FLAG_VALUE="${1#*=}"'): + assert asked in reference, f"flag_value no longer asks: {asked}" + dash = reference.split('case "$2" in', 1)[1] + assert dash.lstrip().startswith("-*)"), ( + "flag_value no longer refuses a value that begins with `-`") + + def code_lines(path: Path) -> str: """The file with its whole-line comments dropped. @@ -312,31 +350,45 @@ def test_the_estate_commands_name_no_runtime_dependency_on_the_standard(): #: THE VALUE-TAKING ARMS ACROSS THE THREE COMMANDS: `park`'s three (--lane, #: --repo, --name | --project), `resume`'s four (--repo, --workspace, --org, #: --name | --project) and `status`'s two (--repo, --name | --project). Kept -#: by hand so that a TENTH is read rather than merely guarded — see the test. +#: by hand so that a TENTH is read through `flag_value` rather than straight +#: from `$2` — the test names one that is not, and counts one that is. VALUE_TAKING_ARMS = 9 -def test_every_value_taking_arm_refuses_an_empty_value(): - """A FLAG GIVEN AN EMPTY VALUE IS REFUSED, in all three commands. - - `[ $# -ge 2 ]` checks only that an argument slot exists; the `${2:?…}` - it replaced rejected an unset OR NULL one. The difference is silent every - time, because every one of these values is read further down as "was this - given at all?": `park --lane ""` parked with the lane dropped out of the - WIP commit subject, and `--repo ""` in any of the three fell past the - `--repo` branch of the resolver into the walk-up and acted on the estate - around the current directory — the one thing naming a clone's origin was - there to prevent. - - THIS SHIPPED TWICE, AND AN OUTSIDE REVIEWER CAUGHT IT BOTH TIMES. Into - `park` and `resume` at 83a1601, which swapped `${2:?…}` for the arity - check; and into `status` at fbf6f11, its first commit, where the arm was - written that way from the start and so was never in front of the review - that caught the other two. Nothing in this suite could see either one, - because each command's own tests ask what a REFUSAL says, and an arm that - does not refuse says nothing for them to read. So it is asked here, of the - text: every line that takes `$2` into a variable carries the whole guard - within the three lines above it. +def test_every_value_taking_arm_reads_its_value_through_flag_value(): + """A FLAG GIVEN NO VALUE, AN EMPTY ONE, OR A FLAG is refused, in all three + commands, and every arm can also be given its value after an `=`. + + `[ $# -ge 2 ]` checks only that an argument slot exists; the `${2:?…}` it + replaced rejected an unset OR NULL one; and neither ever asked what the + slot HELD. The difference is silent every time, because every one of these + values is read further down as "was this given at all?": `park --lane ""` + parked with the lane dropped out of the WIP commit subject, `--repo ""` in + any of the three fell past the `--repo` branch of the resolver into the + walk-up and acted on the estate around the current directory, and `park + Atlas --lane --dry-run` ran a REAL park whose lane was `--dry-run` — the + flag a person typed, swallowed as a value, with nothing but the commit + subject to say so. + + THE EMPTY HALF SHIPPED TWICE, AND AN OUTSIDE REVIEWER CAUGHT IT BOTH + TIMES. Into `park` and `resume` at 83a1601, which swapped `${2:?…}` for + the arity check; and into `status` at fbf6f11, its first commit, where the + arm was written that way from the start and so was never in front of the + review that caught the other two. Nothing in this suite could see either + one, because each command's own tests ask what a REFUSAL says, and an arm + that does not refuse says nothing for them to read. So it is asked here, + of the text. + + ONE QUESTION PER ARM NOW, not three: `flag_value` (held byte-identical + above) asks all of them, and each arm supplies its own words and its own + `die`. So what this reads is that every line taking a value into a + variable goes through it, within the three lines above; that the arm's + refusal carries both its own sentence and `$FLAG_HINT`, which is where the + `-`-led value's own line comes from; and that the case label above it + carries BOTH spellings of every flag it names — `--lane` and `--lane=*` — + because the `=` form is the only way left to pass a value beginning with + `-`, and an arm that had only the spaced spelling would have taken one + away without giving it back. THE COUNT IS ASSERTED TOO, and it is hand-kept on purpose. That the guard is right on nine arms is what the loop proves; the number is what makes a @@ -344,23 +396,90 @@ def test_every_value_taking_arm_refuses_an_empty_value(): to be read rather than as a silent pass. Move it in the same commit that adds the arm. - Whole-line comments are dropped first, by `code_lines`, so the three - comment lines that QUOTE `[ $# -ge 2 ]` in order to explain why it was not - enough on its own cannot stand in for the guard they describe. + Whole-line comments are dropped first, by `code_lines`, so a comment that + QUOTES an arm in order to explain it cannot stand in for the arm. + + THE EXEMPTION IS A LINE RANGE, NOT A STRING (Copilot's review of #37, + 2026-09-14). The one legitimate read of `$2` in each file is the one + INSIDE `flag_value`, and exempting the text `FLAG_VALUE="$2"` wherever it + appeared was not the same claim: an arm that wrote that very line and then + read the value back as `${FLAG_VALUE}` was exempted here AND invisible to + the counter below, so it left nine and PASSED — the same silent pass the + 2026-09-11 review caught in the version before this one, reached by a + different door. Proved on a scratch copy of `park` before it was changed. + So the helper's own body is located by line number and the exemption is + "inside it", and the assignment this reads is any shell variable in either + spelling of `$2` — `value2="$2"` and `X="${2}"` are the same arm written + in a different case and in braces, and neither was matched before. The + counter reads `${FLAG_VALUE}` as well as `$FLAG_VALUE` for the same + reason. The match stays ANCHORED at the start of the statement, because a + `local file="$1" key="$2" line` is a function reading its own parameters + and not an arm at all — thirty-five of those lines across the three files + is what an unanchored search would have called a finding. """ + #: An assignment that takes `$2` straight, in either spelling and whatever + #: the variable is called, ANCHORED at the start of the statement so that + #: a function reading its own parameters (`local file="$1" key="$2" line`) + #: is not one. + takes_dollar_two = re.compile(r'\s*[A-Za-z_][A-Za-z0-9_]*="\$\{?2\}?"') + #: An arm reading the value the helper answered with, in either spelling. + reads_flag_value = re.compile( + r'\s*[A-Za-z_][A-Za-z0-9_]*="\$\{?FLAG_VALUE\}?"') + found = 0 for name in ESTATE_COMMANDS: lines = code_lines(REPO / name).splitlines() + # THE HELPER'S OWN BODY, by line number: `flag_value() {` to the `}` + # in column 0 that closes it. This is the whole of the exemption + # below, and it is a RANGE rather than a string for the reason the + # docstring gives. + opens = [i for i, l in enumerate(lines) + if l.startswith("flag_value() {")] + assert len(opens) == 1, ( + f"{name}: {len(opens)} definitions of flag_value, expected one") + start = opens[0] + closes = [i for i in range(start + 1, len(lines)) if lines[i] == "}"] + assert closes, f"{name}: flag_value is never closed in column 0" + end = closes[0] for i, line in enumerate(lines): - if re.match(r'\s*[A-Z_]+="\$2"', line): - found += 1 - guard = "\n".join(lines[max(0, i - 3):i]) - assert '[ $# -ge 2 ] && [ -n "$2" ]' in guard, ( - f"{name}: {line.strip()} takes $2 without refusing an " - f"empty one") + # A TENTH ARM WRITTEN THE OLD WAY — `X="$2"` — must be caught, not + # missed: the count below sees only arms that read `$FLAG_VALUE`, + # so an arm that bypasses `flag_value` would leave it at nine and + # pass (the independent review of this branch, 2026-09-11, and + # Copilot's of #37). The one legitimate read of `$2` in each file + # is the one inside `flag_value` itself, and INSIDE is asked of + # the line number, not of the text. + if takes_dollar_two.match(line): + assert start < i < end, ( + f"{name}: {line.strip()} takes $2 straight from the " + f"command line, outside flag_value; every value-taking " + f"arm reads its value through flag_value") + continue + if not reads_flag_value.match(line): + continue + found += 1 + where = f"{name}: {line.strip()}" + guard = "\n".join(lines[max(0, i - 3):i]) + assert 'flag_value "$@" ||' in guard, ( + f"{where} takes a value without asking flag_value for it") + assert "needs a value:" in guard and "$FLAG_HINT" in guard, ( + f"{where} refuses in words that are not this arm's, or drops " + f"the line that names the `=` spelling") + head = i - 1 + while head >= 0 and not re.match(r"\s*-.*\)$", lines[head]): + head -= 1 + assert head >= 0, f"{where} is not inside a case arm at all" + spellings = [s.strip() + for s in lines[head].strip()[:-1].split("|")] + spaced = [s for s in spellings if not s.endswith("=*")] + assert sorted(spellings) == sorted( + spaced + [s + "=*" for s in spaced]), ( + f"{where}: the arm matches {spellings}, which is not every " + f"flag it names in both spellings") assert found == VALUE_TAKING_ARMS, ( - f"{found} lines take $2, not {VALUE_TAKING_ARMS}: a value-taking flag " - f"was added or removed. Read the new arm, then move the number.") + f"{found} lines take a flag's value, not {VALUE_TAKING_ARMS}: a " + f"value-taking flag was added or removed. Read the new arm, then move " + f"the number.") #: `--flag ` and `--flag=` are two spellings of ONE flag. The diff --git a/tests/test_status_command.py b/tests/test_status_command.py index e4a2fc20..b5c1b5bc 100644 --- a/tests/test_status_command.py +++ b/tests/test_status_command.py @@ -647,10 +647,68 @@ def test_refusals_read_nothing(home, status_remotes): assert result.stdout == "", ( "nothing may be resolved, said or read before an empty value is " f"refused; stdout was {result.stdout!r}") - for result in (both, unknown, nope, valueless, *(r for _, r in empty)): + # AND A FLAG IS NOT A VALUE EITHER. `status --repo --fetch` set the slug + # to `--fetch` and left the flag unset, so the one thing the person asked + # for — a CURRENT answer — was the one thing that did not happen, and the + # refusal that followed was about a repository nobody named. The refusal + # is now the arm's own, and it names the spelling that does take a value + # beginning with `-`. + flagged = [(flag, run(STATUS, flag, "--fetch", home=home, cwd=atlas)) + for flag in ("--repo", "--name", "--project")] + for flag, result in flagged: + assert result.returncode == 2, result.stdout + result.stderr + assert f"REFUSED: {flag} needs a value:" in result.stderr + assert "`--fetch` begins with a `-`" in result.stderr + assert f"{flag}=--fetch" in result.stderr + assert result.stdout == "", ( + "nothing may be read before a flag given a flag is refused; " + f"stdout was {result.stdout!r}") + for result in (both, unknown, nope, valueless, + *(r for _, r in empty), *(r for _, r in flagged)): assert str(atlas) not in result.stdout, "a refusal read nothing" +def test_a_flag_takes_its_value_after_an_equals_sign_too(home, status_remotes): + """`--flag=value` IS `--flag value` here too, and `--flag=` is empty. + + `status` has no verb to run, so what the `=` spelling has to prove is that + the value RESOLVES THE ESTATE: `--name=Atlas`, and `--repo=` the origin + of the clone, both read Atlas from a directory that is not inside it, so + the walk-up cannot be what answered. Before this they reached the + catch-all — "status does not know --name=Atlas" — which is why refusing a + `-`-led value needed this spelling in the same commit. + """ + atlas = clone_root(home, status_remotes["Atlas"], "Atlas") + # The origin is re-spelled as the url a real root carries, because a clone + # whose origin is a bare repository ON DISK answers `` and the + # walk-up but never `--repo` — `test_park_resume_commands.py` re-spells it + # for the same reason. + git("remote", "set-url", "origin", f"https://github.com/{ORG}/Atlas.git", + cwd=atlas) + for spelling in ("--name=Atlas", "--project=Atlas", f"--repo={ORG}/Atlas"): + result = run(STATUS, spelling, home=home, cwd=home) + assert result.returncode == 0, ( + f"{spelling}: {result.stdout}{result.stderr}") + assert f"status: Atlas - the assembly root at {atlas}" in result.stdout + assert "reading every estate" not in result.stdout, ( + f"{spelling} fell past the arm into the bare form") + + for spelling in ("--name=", "--project=", "--repo="): + result = run(STATUS, spelling, home=home, cwd=atlas) + assert result.returncode == 2, result.stdout + result.stderr + assert f"REFUSED: {spelling[:-1]} needs a value:" in result.stderr + assert result.stdout == "", ( + f"nothing may be read before `{spelling}` is refused; stdout was " + f"{result.stdout!r}") + + # A VALUE THAT REALLY DOES BEGIN WITH `-` IS SAYABLE, which is the reason + # the spelling exists: read as a name, not as the flag it looks like. + weird = run(STATUS, "--name=--fetch", home=home, cwd=home) + assert weird.returncode == 2, weird.stdout + weird.stderr + assert "no estate named '--fetch'" in weird.stderr + assert "needs a value" not in weird.stderr + + def test_a_git_that_cannot_run_is_refused_not_reported(atlas, home, tmp_path): """Every git call is `|| true`-guarded so a missing ref is a finding, not a crash — so a git that refuses EVERY command would otherwise come back as