fix(which-model): replace the self-interpreted data gate with an availability gate - #34
Merged
Merged
Conversation
…lability gate The old gate told the model to demand a citable approval -- data class, provider, retention terms -- before routing anything sensitive, and to fall back to "approved" routes when that approval could not be verified. No approval is ever on file, because the user never wrote one. So the rule fired every time and the model supplied the missing premise itself. Measured effect, from a session on a private repo: asked to route three PR reviews, the model refused the routing reference's lanes because they were openrouter IDs, and gave "I have no approval I can cite for routing it through a third-party aggregator" as the reason. The user had set no such policy. A search of that project's 80-odd memory files found no policy statement -- the gate came from this skill's own text. The replacement asks a question that has an answer: what can this harness select with the credentials it already has. It states that a missing approval is not a refusal reason, that provider approval and data movement are the user's calls, and that a stated constraint is honoured as a filter. It also says to map an unreachable lane to an available model rather than decline, which is the step the model skipped. Call sites follow the rename: loop-engineering's compose row and model-routing paragraph, council's tiering reference, the routing reference, the seeded openrouter caveat, and the manifest description. The static check is inverted, not deleted. "availability gate stays cold" pins the new section; "no self-interpreted approval gate" pins the absence of the two sentences that produced the behaviour, as two literals -- a pattern class would also reject a constraint the user states, which the skill must honour. Both proved red against main's SKILL.md and green after, and each fails alone under its own mutation. loop-engineering's routing assertion moves from "data-policy gate" to "availability gate". Deliberately not carried over from the source branch: - interrogate.md keeps "data policy" in its `--stop needs_human` boundary. That rule routes the question to the user instead of answering it, which is the opposite of the invented-refusal bug. - The resolver-block rewrite and the "When to use" section. Main trimmed the same resolver for budget in 3405033, and "When to use" restates the frontmatter while costing words. - A hunk that would have reverted main's bounded driver-probe paragraph from 2a9a39c, which the source branch predates. The root stays inside its 80-line/600-word budget at 74/600; the gate needs more words than the one it replaces, so Route first, Guideline, Output rules, and the resolver recovery sentence absorb the difference. Verified in this worktree: tests/run.sh -> 157 pass, 0 fail, exit 0, plus tests/run.sh packages -> 6 pass, 0 fail including worklog-memory-mcp e2e 5/5.
pr-query.sh merge-base needs a base-branch ref. On a pull_request event the default depth-1 checkout is a detached merge commit with no origin/main and no local main, so default_branch() falls through to the literal "main" and resolves nothing. That made "pr-review test_pr_review_bin / merge-base must print a SHA" fail on every PR while every push to main stayed green, because on a push HEAD is main. The repo has merged little through PRs lately, so the gap stayed hidden. Reproduced against the real script: a depth-1 detached clone with the base refs removed prints "no merge-base between origin/main and HEAD" and exits 1; the same clone at full depth prints a 40-character SHA and exits 0. Fixes the checkout, not the assertion -- the contract that merge-base resolves without a PR number is the thing worth keeping strict.
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.
What
which-model's## Data policy gatebecomes## Availability gate.Why
The old gate told the model to demand a citable approval — data class, provider, retention terms — before routing anything sensitive, and to fall back to "approved" routes when that approval could not be verified.
No approval is ever on file, because the user never wrote one. So the rule fired every time and the model supplied the missing premise itself.
Measured effect, from a session on a private repo: asked to route three PR reviews, the model refused the routing reference's lanes because they were openrouter IDs, and gave "I have no approval I can cite for routing it through a third-party aggregator" as the reason. The user had set no such policy. A search of that project's 80-odd memory files found no policy statement — the gate came from this skill's own text.
The replacement asks a question that has an answer: what can this harness select with the credentials it already has. A missing approval is not a refusal reason; provider approval and data movement are the user's calls; a stated constraint is honoured as a filter. It also says to map an unreachable lane to an available model rather than decline — the step the model skipped.
Provenance
This revives
fix/which-model-availability-gate(3 commits, never pushed, its remote 41 commits stale). That branch no longer rebases ontomain, so the surviving work is re-applied onto currentmainas one commit.Deliberately not carried over:
interrogate.mdkeepsdata policyin its--stop needs_humanboundary. That rule routes the question to the user instead of answering it — the opposite of the invented-refusal bug. The source branch's first commit said it was leaving this alone for exactly that reason, then its second commit changed it anyway.## When to usesection.maintrimmed the same resolver for budget in3405033, andWhen to userestates the frontmatter while costing words.main's bounded driver-probe paragraph from2a9a39c, which the source branch predates.Tests
The static check is inverted, not deleted:
availability gate stays coldpins the new section.no self-interpreted approval gatepins the absence of the two sentences that produced the behaviour, as two literals. A pattern class here would also reject a constraint the user states, which the skill must still honour.Both proved red against
main'sSKILL.mdand green after, and each fails alone under its own mutation.loop-engineering's routing assertion moves fromdata-policy gatetoavailability gate.Budget
The root stays inside its 80-line / 600-word cap at 74 / 600. The new gate needs more words than the one it replaces, so Route first, Guideline, Output rules, and the resolver recovery sentence absorb the difference. (The source branch was handed off stuck at 624 words;
main's later resolver trim is what made it fit.)Verification
Two environment skips in the worktree run, both pre-existing and unrelated:
loop-run wheel build(nopip) and the mcp e2e (nonode_modules) — the latter closed by thepackagesrun above afternpm install.