fix(cli): refuse an unknown --repo filter on add, commit, and push - #911
Conversation
An unknown --repo name matched zero repositories and the command reported success having done nothing. With a modified file present in the worktree, `gr add . --repo missing` printed "No changes to stage." and exited 0. There was a change to stage; gr said there was not. That is a false claim about the working tree, which is what justifies a fix rather than a documentation note. validate_repo_filters_known already existed in src/core/repo.rs, already produced the right diagnostic, and already carried a basename-matching branch that names the intended repository. It was not called from add, commit, or push. This calls it from all three. Wired to CliOutcomeError::refusal. outcome.rs defines EXIT_REFUSED = 2 and carries a test named refusal_is_distinct_from_operational_failure, so the codebase already separates a refusal from an operational failure. pr/merge uses the same mapping. Three regression tests assert that an unknown --repo name exits 2 with the diagnostic, one per verb. A control asserts that a KNOWN name still reaches the work: it writes a file, runs the command, and asserts that file is present in the repository's index. It asserts the destination rather than the absence of a message. Mutation-verified in both directions. Removing add.rs's validation call turns exactly the add rejection test red while commit and push stay green. Making run_add return early after validation turns exactly the control red while the three rejection tests stay green. Other commands accept a repo filter without validating it. Identifying which requires reading each command's behavior rather than grepping an identifier, and is deliberately left to a follow-on so each batch is reviewed on its own. Ref #886 — closes at promotion. Premium boundary: grip is OSS — local workspace orchestration, no identity, org, or entitlement semantics.
Review recordBound to head Atlas — reviewed v1 through v5. Seven findings across five rounds, every one on the prose describing the change rather than on the change itself; the three runtime hunks were byte-identical from v1 and cleared in every round. His last verdict on v5 was REQUEST CHANGES, over three items: a false claim that two outcomes were indistinguishable in the output (he disproved it from source — a clean-tree run prints the header and Stromus — reviewed v6, APPROVE. Re-derived all four frozen artifact hashes independently. Verified the three deletions with his own patterns and firing controls. Independently extracted and hashed the three RAN by the author at this head: NOT RUN by either reviewer: the full test suite. Both mutations were executed at an earlier head; the only change since is comment text, verified by diff. |
The defect
An unknown
--reponame matched zero repositories and the command reported success having done nothing.The precise failure is stronger than a scope miss. With a modified file present in the worktree, the command reported that there was nothing to stage and exited 0. There was a change to stage; it said there was not. That is a false claim about the working tree, and it is what justifies a fix rather than a documentation note.
The fix
validate_repo_filters_knownalready existed insrc/core/repo.rs, already produced the right diagnostic, and already carried a basename-matching branch that names the intended repository (Did you mean ... ?). It was not called fromadd,commit, orpush. This calls it from all three.It is wired to
CliOutcomeError::refusal.outcome.rsdefinesEXIT_REFUSED = 2and carries a test namedrefusal_is_distinct_from_operational_failure, so the codebase already separates a refusal from an operational failure.pr/mergeuses the same mapping.How it is witnessed
Three regression tests assert that an unknown
--reponame exits 2 with the diagnostic, one per verb.A control asserts that a known
--reponame still reaches the work: it writes a file into the repository, runs the command, and then asserts that file is present in that repository's index. It asserts the destination rather than the absence of a message.Both directions are mutation-verified:
add.rsturns exactlytest_add_unknown_repo_filter_is_refused_not_silently_emptyred, with a failure type matching the mutation, while thecommitandpushtests stay green.run_addreturn early after validation turns exactly the control red, while the three rejection tests stay green.cargo fmt --all --checkis clean.Scope
This wires three verbs. Other commands accept a repo filter without validating it; identifying which ones requires reading each command's behavior rather than grepping for an identifier, and that is deliberately left to a follow-on so each batch can be reviewed on its own.
Ref #886 — closes at promotion.
Premium boundary
grip is OSS: local workspace orchestration. No identity, org, or entitlement semantics.