Source PR: merge commit 31309d62a918727c3e07daaeda28c52f0d3cb56e (3 substantive review rounds)
What the review rounds surfaced. The PR restructured a validation gate to enumerate what to exempt instead of what to check, so that the next unrecognized upstream source form defaults to blocked rather than silently passing. The reviewer endorsed that direction and confirmed every divergence between the gate and real upstream behavior fails closed. But the substantive finding was this: the PR transcribed the wrong parser. The upstream tree at the pinned SHA contains two source parsers that disagree with each other. One belongs to the plugin loader and runs at build time; the other belongs to the installer and is what actually writes the lockfile the gate validates. The gate mirrored the loader's grammar, which accepts a bare org/repo GitHub shorthand that the installer's grammar does not — the installer's branch list ends in a throw for that form.
The reusable learning. When a local gate mirrors an upstream grammar, the grammar that governs is the one belonging to the code path that produces the artifact you are validating — not the one that consumes it, and not whichever parser you found first. Multiple parsers for the same surface syntax routinely coexist in a codebase and drift apart. Before transcribing, trace backward from the artifact: which upstream function writes this file? That function's accepted forms are your contract.
Why the impact is subtle rather than catastrophic. Modeling the wrong parser here did not open a hole — the divergence was strictly stricter than the prior behavior, so nothing that previously errored now passes. The cost is diagnostic honesty: a bare org/repo source gets reported as "missing lock entry" when the truthful diagnosis is "the installer throws on this form." Wrong-but-fail-closed errors are the expensive kind, because they send whoever hits them chasing a lockfile problem that does not exist. The review also noted the PR pinned the real production config with a golden test against the actual files rather than a fixture, which is the pattern already documented in this repo's solutions directory — worth cross-referencing rather than re-deriving.
Proposed learning to author: a docs/solutions/ entry on mirroring upstream grammars — identify the producer of the artifact, not the consumer, and check whether multiple parsers exist before transcribing — with a secondary note that a fail-closed gate can still emit a misleading diagnosis, and that error messages deserve the same accuracy scrutiny as the accept/reject decision.
Source PR: merge commit
31309d62a918727c3e07daaeda28c52f0d3cb56e(3 substantive review rounds)What the review rounds surfaced. The PR restructured a validation gate to enumerate what to exempt instead of what to check, so that the next unrecognized upstream source form defaults to blocked rather than silently passing. The reviewer endorsed that direction and confirmed every divergence between the gate and real upstream behavior fails closed. But the substantive finding was this: the PR transcribed the wrong parser. The upstream tree at the pinned SHA contains two source parsers that disagree with each other. One belongs to the plugin loader and runs at build time; the other belongs to the installer and is what actually writes the lockfile the gate validates. The gate mirrored the loader's grammar, which accepts a bare
org/repoGitHub shorthand that the installer's grammar does not — the installer's branch list ends in athrowfor that form.The reusable learning. When a local gate mirrors an upstream grammar, the grammar that governs is the one belonging to the code path that produces the artifact you are validating — not the one that consumes it, and not whichever parser you found first. Multiple parsers for the same surface syntax routinely coexist in a codebase and drift apart. Before transcribing, trace backward from the artifact: which upstream function writes this file? That function's accepted forms are your contract.
Why the impact is subtle rather than catastrophic. Modeling the wrong parser here did not open a hole — the divergence was strictly stricter than the prior behavior, so nothing that previously errored now passes. The cost is diagnostic honesty: a bare
org/reposource gets reported as "missing lock entry" when the truthful diagnosis is "the installer throws on this form." Wrong-but-fail-closed errors are the expensive kind, because they send whoever hits them chasing a lockfile problem that does not exist. The review also noted the PR pinned the real production config with a golden test against the actual files rather than a fixture, which is the pattern already documented in this repo's solutions directory — worth cross-referencing rather than re-deriving.Proposed learning to author: a
docs/solutions/entry on mirroring upstream grammars — identify the producer of the artifact, not the consumer, and check whether multiple parsers exist before transcribing — with a secondary note that a fail-closed gate can still emit a misleading diagnosis, and that error messages deserve the same accuracy scrutiny as the accept/reject decision.