Repository navigation
fix: a flag a probe arm did not run decides a reading (#113) - #119
Merged
Merged
Conversation
`checkRecord` read the flag names as well as the structure, so a record omitting `--strict-mcp-config` or carrying `--verbose` was a broken file rather than a failed probe. Only a wrong value reached the acceptance test, so ADR-0024's guarantee held for half the case it named. Allowlist membership and required-flag presence move to the value reading. `flagShapeProblems` keeps structural impossibility alone, which is what no revision of this collector could have produced. The principle: a check may refuse a record on a stable identity fact of the collector, and never on a protocol choice this repository versions. Operator ruling 2026-08-14, Option A on the issue's fork.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be793b7793
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review findings on the same root cause: the split moved more than membership. `armFlags` returns a literal array, so a floating element and a duplicated flag are impossible under every revision, whatever the set becomes. Both return to the shape reading. Duplication is now read before membership, so it is name-agnostic and `--verbose --verbose` no longer passes a rule that names no flag. The positional refusal names the position rather than the element, so a credential-shaped positional no longer withholds the whole line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
checkRecordread the flag NAMES as well as the structure. A record omitting--strict-mcp-config, or carrying--verbose, was a broken file rather than afailed probe. Only a wrong VALUE reached the acceptance test, so ADR-0024's
guarantee held for half the case it named, and the next move that adds or
removes a flag name would have made every committed record malformed.
Allowlist membership and required-flag presence move to the value reading.
isolationProblemsnow reads the names, the presence and the values, andderiveOutcomereports all three asisolated.flagShapeProblemskeepsstructural impossibility alone: flags that are not a non-empty array, an entry
that is not a string, a flag stated twice, a value-taking flag at the end of the
list, and a flag sitting where another flag's value belongs.
The principle the ADR amendment states: a check may refuse a record on a stable
identity fact of the collector, and never on a protocol choice this repository
versions. The pathway combination stays a shape refusal, untouched. Flag names,
required presence and
TRACE_LINE_LIMITdecide a reading and never a record'svalidity.
Operator ruling 2026-08-14, Option A on the fork #113 states.
The three committed records under
bench/probes/derive exactly what theyderived before, and
test/probe.test.jspins each tuple.Closes #113