The pattern, not the bug
assert_merge_args_allowed decides whether a merge invocation is safe by modelling how gh-axi parses arguments. That model has now been wrong twice, and each time the error produced a real defect:
- It refused a legitimate
--sha abc123 because the value was read as an unknown flag. The repair taught the allow-list that value-taking flags consume the next word.
- That repair then let
-- --subject --auto through: --subject consumed --auto as its value, so nothing was refused, and gh-axi - which finds flags by scanning, not positionally - armed deferred execution anyway. The immediate-execution guarantee the allow-list exists to provide was defeated by argument order.
Both are being fixed. This issue is about the shape that produced them.
The two failures point in opposite directions
This is the part that makes it a design question rather than a bug count. One failure refused something legitimate; the other admitted something forbidden. That is the signature of a model that is wrong, not merely incomplete - a stricter version of the same model would have failed the first case, and a looser version would have failed the second. There is no calibration of this approach that avoids both.
Concretely, the second failure exists because gh-axi 0.1.34 (dist/src/commands/pr.js) takes --auto at line 486 before it takes --subject at line 489, and its flag reader returns undefined rather than erroring on a valueless flag. Our guard assumed positional consumption. That is a version-observed fact about a third-party tool, and the guard's correctness silently depends on it.
Why it recurs
The guard duplicates another program's parser in order to decide safety. That coupling is invisible in the code: the allow-list looks like a self-contained list of permitted strings, and nothing in it says "this is only correct while gh-axi parses the way I assumed". It is also a dated dependency - a change to gh-axi's parsing silently invalidates the guard with no failing test anywhere.
A guard whose correctness depends on an assumption it does not state, about software it does not control, will keep producing this class.
The alternative worth considering
Verify the outcome rather than police the input: rather than proving no forbidden argument can reach the forge, confirm after the call that the merge actually executed immediately and was not queued or deferred. The forge's own state is authority; our model of a CLI's parser is not.
That is a different design with its own costs - it acts and then checks, which is only acceptable where the wrong outcome is detectable and recoverable - and it may well not be right here. It is written down so the option is on the table when someone next repairs this guard.
Deliberately not done now
Not opened inside the merge-path task. Redesigning a guard mid-validation is how a third defect gets introduced, and the two concrete holes are being closed on their own merits first.
Reopen trigger
The next defect traced to the allow-list's model of gh-axi parsing. Two is a pattern; three should force the design question rather than a fourth repair.
The pattern, not the bug
assert_merge_args_alloweddecides whether a merge invocation is safe by modelling howgh-axiparses arguments. That model has now been wrong twice, and each time the error produced a real defect:--sha abc123because the value was read as an unknown flag. The repair taught the allow-list that value-taking flags consume the next word.-- --subject --autothrough:--subjectconsumed--autoas its value, so nothing was refused, andgh-axi- which finds flags by scanning, not positionally - armed deferred execution anyway. The immediate-execution guarantee the allow-list exists to provide was defeated by argument order.Both are being fixed. This issue is about the shape that produced them.
The two failures point in opposite directions
This is the part that makes it a design question rather than a bug count. One failure refused something legitimate; the other admitted something forbidden. That is the signature of a model that is wrong, not merely incomplete - a stricter version of the same model would have failed the first case, and a looser version would have failed the second. There is no calibration of this approach that avoids both.
Concretely, the second failure exists because
gh-axi0.1.34 (dist/src/commands/pr.js) takes--autoat line 486 before it takes--subjectat line 489, and its flag reader returns undefined rather than erroring on a valueless flag. Our guard assumed positional consumption. That is a version-observed fact about a third-party tool, and the guard's correctness silently depends on it.Why it recurs
The guard duplicates another program's parser in order to decide safety. That coupling is invisible in the code: the allow-list looks like a self-contained list of permitted strings, and nothing in it says "this is only correct while
gh-axiparses the way I assumed". It is also a dated dependency - a change togh-axi's parsing silently invalidates the guard with no failing test anywhere.A guard whose correctness depends on an assumption it does not state, about software it does not control, will keep producing this class.
The alternative worth considering
Verify the outcome rather than police the input: rather than proving no forbidden argument can reach the forge, confirm after the call that the merge actually executed immediately and was not queued or deferred. The forge's own state is authority; our model of a CLI's parser is not.
That is a different design with its own costs - it acts and then checks, which is only acceptable where the wrong outcome is detectable and recoverable - and it may well not be right here. It is written down so the option is on the table when someone next repairs this guard.
Deliberately not done now
Not opened inside the merge-path task. Redesigning a guard mid-validation is how a third defect gets introduced, and the two concrete holes are being closed on their own merits first.
Reopen trigger
The next defect traced to the allow-list's model of
gh-axiparsing. Two is a pattern; three should force the design question rather than a fourth repair.