fix(drift): require real drift resolution, not fixture relaxation, before PR/merge by jpr5 · Pull Request #310 · CopilotKit/aimock

jpr5 · 2026-07-17T18:16:33Z

The hole

scripts/fix-drift.ts's abort guard accepted any auto-fix run that changed a
file under src/ (builderFiles.length > 0 || testFiles.length > 0) and exited
0. The drift tests are three-way triangulations (SDK vs real API vs mock), and
the SDK leg is literally the repo fixture src/__tests__/drift/sdk-shapes.ts.
So the LLM could "resolve" drift by relaxing that fixture: deleting the
field makes the SDK leg null → the critical branch is unreachable → the
collector re-reports clean (exit 0) with the production mock untouched →
WS-1's hardened merge gate sees green CI and auto-merges. Re-running the
collector alone can't catch this because its own SDK leg reads the relaxed
fixture.

The fix — a 3-signal predicate

New scripts/drift-success-predicate.ts (pure evaluateDriftResolved() + thin
CLI). resolved: true requires ALL of:

Authoritative — post-fix collector re-run clean (exit 0 AND
criticalCount 0). Distinct exits 2/5/1 → STILL_DIRTY / QUARANTINE_AFTER_FIX
/ COLLECTOR_INFRA.
Production change present — ≥1 file under src/** excluding
src/__tests__/. A relaxation NEVER changes one. Empty → NO_PRODUCTION_CHANGE;
only comparison-leg files → COMPARISON_LEG_ONLY (the headline cheat).
No standalone comparison-leg relaxation — a gameable-leg edit
(sdk-shapes.ts / providers.ts / ws-providers.ts / harness helpers.ts)
must be accompanied by a production change; and schema.ts (its
ALLOWLISTED_PATHS) or any *.drift.ts assertion edit is always blocked
as SUPPRESSION_SUSPECTED, even alongside a production change.

Legit fixture targets (model-registry.ts, model-family.ts, voice-models.ts)
are sanctioned only via the report's builderFile/typesFile (canary routing).
An off-target production change → PRODUCTION_CHANGE_OFF_TARGET (WARNING, still
blocks). Distinct exit codes (0, 10–16, 2) route each failure to a named
needs-human Slack reason.

Wiring

fix-drift.ts createPr(): the predicate replaces the weak guard and runs
before any git add/commit (blocked verdict → no PR → no merge). Legacy
exit-4 guard retained as a fallback when the workflow doesn't pass
--post-fix-report/--post-fix-exit.
.github/workflows/fix-drift.yml: fresh authoritative re-collect to
drift-report.post-fix.json, uploaded as an artifact; new "Assert drift truly
resolved" step; new reasons wired into the Slack case block.
Fixed a latent bug in the assert step: PIPESTATUS[0] after a
VAR="$(pipeline)" assignment reports the assignment's status (always 0), not
the predicate's — so the step would never have blocked a cheat. Now the
predicate runs to predicate.log, its real exit is captured directly, and the
reason is grepped from the log.

Red-green proof

Unit suite (src/__tests__/drift-success-predicate.test.ts, 28 tests, no LLM):

RED — stubbing evaluateDriftResolved to always accept (== the old
testFiles>0 guard) fails all 10 cheat-detection cases, including the headline:

❯ src/__tests__/drift-success-predicate.test.ts (28 tests | 10 failed)
   × HEADLINE: fixture-relaxation-only (sdk-shapes.ts) → COMPARISON_LEG_ONLY, and the OLD guard would have ACCEPTED it
   × schema/allowlist edit + real builder change → SUPPRESSION_SUSPECTED
   × *.drift.ts assertion loosened only → SUPPRESSION_SUSPECTED
   × no changes at all → NO_PRODUCTION_CHANGE
   × production change but collector still dirty (exit 2) → STILL_DIRTY
   ... (10 total)

GREEN — real implementation restored: 28/28 pass.

CLI transcript (the exact cheat vs a real fix):

############## RED: fixture-relaxation-only (sdk-shapes.ts) ##############
DRIFT NOT RESOLVED [comparison-leg-only]: Fix changed ONLY comparison-leg files
(src/__tests__/drift/sdk-shapes.ts) with no production mock-builder change —
this relaxes the drift detector instead of fixing the mock. The exact cheat this gate blocks.
Offending files: src/__tests__/drift/sdk-shapes.ts
reason=comparison-leg-only
RED exit code: 11

############## GREEN: real production fix (src/helpers.ts) ##############
Drift genuinely resolved: post-fix collector clean, real production change
(src/helpers.ts), no comparison-leg relaxation.
reason=resolved
GREEN exit code: 0

Full suite (pnpm test): 4533 passed / 44 skipped. prettier --check, eslint,
tsc --noEmit, pnpm build, actionlint (incl. shellcheck) all green.

Out of scope — residual WS-2b (flagged)

Hybrid gaming: a fix that relaxes sdk-shapes.ts AND makes a token edit to
the named production builder passes both signal 1 (collector clean — its own
SDK leg reads the relaxed fixture) and signal 2 (production change present). Fully
closing this needs an SDK leg sourced independently of the repo fixture (e.g.
generated from the vendored SDK's .d.ts at collect time). That is tracked as
WS-2b and is out of this PR's scope; SUPPRESSION_SUSPECTED on schema/allowlist
plus the OFF_TARGET warning are the partial mitigations here.

Update (CR fix round 1, `75c42ab`) — WS-2b CLOSED for auto-merge

The "residual WS-2b" hybrid above is now closed for the auto-merge path. The
predicate no longer ignores a gameable-leg edit when a production file also
changed: any edit to a gameable leg (sdk-shapes.ts, the real-API/harness
files incl. voice-models.ts, providers.ts/ws-providers.ts, schema.ts,
*.drift.ts) now blocks regardless of how many production files also changed.
So relaxing sdk-shapes.ts + a trivial on-target production edit → needs-human
(SUPPRESSION_SUSPECTED), never auto-merge.

The independently-sourced SDK leg remains a good future hardening for the
detection side, but is no longer required to keep the hybrid cheat out of
auto-merge — a leg edit can never reach a PR/merge unattended.

Also in this round: dual-classified voice-models.ts blocks (precedence);
empty sanctioned-target set fails closed; --changed-file lists are cross-checked
against git (fail-closed on mismatch); the no-post-fix-args legacy PR path is
removed (fail-closed); exit 5/1 get distinct reasons ahead of criticalCount;
readReport rejects a missing timestamp. Real-predicate red→green locks added
(WS-2b headline flips resolved→SUPPRESSION_SUSPECTED) plus a revert guard.

…fore PR/merge The auto-fix abort guard in fix-drift.ts accepted any run that changed a file under src/ (builderFiles>0 || testFiles>0) and exited 0. Because the drift tests are three-way triangulations whose SDK leg is the repo fixture src/__tests__/drift/sdk-shapes.ts, the LLM could "resolve" drift by RELAXING that fixture: the critical branch goes unreachable, the collector re-reports clean, and WS-1's merge gate auto-merges with the production mock untouched. Add scripts/drift-success-predicate.ts (a pure evaluateDriftResolved fn plus a thin CLI) that requires THREE independent signals for resolved:true: 1. post-fix collector clean (exit 0 AND criticalCount 0), 2. >=1 PRODUCTION file changed (src/** excluding src/__tests__/), 3. no standalone comparison-leg relaxation; allowlist / *.drift.ts assertion edits ALWAYS block (SUPPRESSION_SUSPECTED). Legit fixture targets (model-registry.ts, model-family.ts, voice-models.ts) are sanctioned via the report's builderFile/typesFile. Distinct exit codes (10-16, 2) route each failure to a named needs-human Slack reason. Replace the weak guard in fix-drift.ts createPr() with the predicate (gated before any git add/commit) and wire the workflow: a fresh authoritative re-collect to drift-report.post-fix.json, an "Assert drift truly resolved" step, and the new reasons into the Slack case block.

pkg-pr-new · 2026-07-17T18:17:01Z

Open in StackBlitz

npm i https://pkg.pr.new/@copilotkit/aimock@310

commit: 71856a3

The predicate ignored comparison/SDK/harness-leg edits once any production file also changed, so relaxing sdk-shapes.ts plus one trivial on-target production edit returned resolved:true and auto-merged (the WS-2b hybrid cheat). Now any gameable-leg edit always blocks regardless of production files; schema/*.drift.ts always map to SUPPRESSION_SUSPECTED, other legs to SUPPRESSION_SUSPECTED when paired with a production change and COMPARISON_LEG_ONLY when standalone. Also: dual-classified voice-models.ts blocks (precedence), empty sanctioned-target set fails closed, --changed-file lists are cross-checked against git, the no-post-fix-args legacy PR path is removed (fail-closed), exit 5/1 get distinct reasons over criticalCount, and readReport rejects a missing timestamp.

…t-deny) The round-2 adversarial CR showed the "block known gameable legs" denylist still leaks: any editable collector input NOT on the static list (package.json/lockfile pinning a vendored SDK, tsconfig, an imported sub-fixture, an unknown path, a drift-dir *.test.ts) could accompany a token on-target production edit and reach resolved:true. Invert to an allowlist: a fix is RESOLVED only when EVERY changed file is (a) production source (src/** not under src/__tests__/) or (b) a fixture explicitly named for a drift entry in the report (builderFile/typesFile). Anything else blocks with a new UNSANCTIONED_CHANGE reason (exit 17). This closes CR F1/F2/F3 and the drift-dir .test.ts gap by default-deny rather than a stale denylist. Also: - Canonicalize every changed path before classification (strip ./, collapse //, resolve ./.. , fail closed on repo-root escape) so a spelling variant cannot sneak a leg past the exact-string matchers; read git with -c core.quotePath=false (slot-1/slot-2 path findings). - F6: reject empty/whitespace --post-fix-exit (Number("")===0 must not masquerade as a clean exit 0) — fail closed. - F8: catch a malformed post-fix report that would crash countCriticalDiffs and map it to a named config-error, in both runCli and the fix-drift.ts createPr in-script gate, instead of a bare stacktrace with an empty reason= line. - F4: bind the scored reports to the in-workflow re-collect output (a clarifying comment — neither report is repo-committed). - Wire unsanctioned-change into the workflow Slack reason routing. - Signal 2 now accepts a report-named fixture target as a valid fix (canary model-list case), not just production source. - Retain: crossCheckChangedFiles, empty-sanctioned fail-closed, legacy no-post-fix-args fail-closed, voice-models->block, exit-code lock. Red-green: reverting the predicate to the denylist flips 18 tests (allowlist/canonicalization/F6/F8) from green to red; the inverted predicate passes all 67. Full suite 4621 pass, tsc clean.

…rt validation CR round-3 report-integrity + workflow-wiring hardening for the drift-success allowlist predicate. The allowlist LOGIC was already sound; this closes the last layer — the report the sanctioned set is derived from, the files createPr actually commits, and the entry-level validation of that report. F-A — pin the PRE-fix report so the autofix LLM cannot forge the allowlist. The sanctioned-target set is derived from drift-report.json, which the autofix Claude Code subprocess has Write/Edit on (its cwd is the repo checkout). A new "Pin pre-fix drift report" workflow step copies it to $RUNNER_TEMP (outside the LLM-writable scope) BEFORE the autofix runs; the Assert and Create PR steps now pass --report from that pinned copy, never the in-repo file. sanctionedTargets is a pure function of the passed report, so a forged on-disk file cannot widen the allowlist. F-C / F2 — createPr now derives its changed-file set from the predicate's own gitChangedFiles() (git -c core.quotePath=false) + canonicalizePath, so staging matches the verdict byte-for-byte. A new pure gatedCommitFiles() partitions the set into the ONLY groups permitted to stage (production source + report-named fixture targets); the "remaining"/straggler catch-all git add is removed, so no unclassified file can land in the PR after the verdict. The version bump + CHANGELOG remain an explicit, exact-path-staged part of the set. F3 — the predicate's readReport now validates per-entry builderFile/typesFile/ diffs, mirroring fix-drift.ts:readDriftReport, so a malformed report fails closed with a DISTINCT named CONFIG_ERROR instead of a bare TypeError caught as an unnamed error. F1 — locked the already-wired post-fix re-collect + args and corrected the now-stale comments (no legacy no-post-fix fallback remains) in fix-drift.ts. Tests: red-green proven by stashing the source and confirming the new F-A/F-C/ F3 locks fail on baseline, then pass after the fix. Added a workflow-shape test (fix-drift-workflow.test.ts) asserting the pin step ordering and that both gate invocations read --report from the pinned copy; end-to-end canonicalization (../ and absolute) and unrecognized-collector-exit-code locks. Full suite, format, lint, tsc, actionlint, build all green.

…oles The drift-success predicate becomes a strong AUTO-FILTER, not a provable merge gate: the in-workflow re-collect is not independent of the fix (its SDK/expected leg reads the fixtures the autofix touched — WS-2b), so a clean post-fix signal can filter out cheats but cannot prove the mock was genuinely fixed. Drift-fix PRs therefore require ONE HUMAN APPROVAL to merge — no unattended auto-merge on the drift path. - Remove the in-workflow auto-merge step and every `gh pr merge`; the drift path now opens a PR (verdict recorded in the PR body) and STOPS. A human reviews CI + the diff + the verdict and merges (branch ruleset 1-approval is the gate). Slack success copy says "PR opened — needs human review + merge", never "merged to main"; failure Slack drops the removed merge-step reasons. - F3 (happy-path break): the collector outputs (pre-fix, re-collect, predicate log) were written into the repo cwd, so they showed as untracked and the predicate scored them UNSANCTIONED — always fail-closed. Write all three to $RUNNER_TEMP (outside the checkout) and broaden .gitignore to drift-report*.json as belt-and-suspenders. - F2 (canary-only bypass): a fixture-target-only change (no production change) reached resolved:true despite the ">=1 production change" invariant. Require >=1 production mock-builder change for RESOLVED; fixture-only diffs route to NO_PRODUCTION_CHANGE (needs-human). - F7 (fail-open): fix-drift.ts PR path accepted Number("")===0 as a clean exit 0. Add parsePostFixExit, mirroring the predicate CLI's non-empty integer guard; empty/invalid post-fix exit fails closed. - F5: createPr asserts the stragglers set is empty on a RESOLVED verdict and fail-closes (UNSANCTIONED_CHANGE) rather than silently dropping an unclassified file from the diff. - Test-tightness: lock the runCli CONFIG_ERROR path and the machine- readable reason= stdout line the workflow greps for Slack routing. - Document accepted residuals (F6 TOCTOU, WS-2b independent SDK leg) as mitigated by the human-approval backstop.

…d code The buildPrompt() skill-update instruction told the LLM to edit skills/write-fixtures/SKILL.md, but the drift-success predicate allowlist only sanctions src/** production code and report-named fixtures. Any SKILL.md edit therefore tripped UNSANCTIONED_CHANGE and rejected the whole run, blocking legitimate fixes. Remove the instruction so the autofix only touches production mock code (the allowlist stays tight — skills/ is deliberately not added). With the prompt no longer touching skills and the allowlist never sanctioning skills/**, the skillFiles staging group in gatedCommitFiles/createPr is unreachable dead code; remove it along with the now-unused SKILL_FILE constant and the stale prompt test. Also remove scripts/ci-merge-gate.sh and its test: the workflow no longer auto-merges, so the green-gate script has zero references.

jpr5 force-pushed the harden/drift-success-predicate branch 2 times, most recently from 75c42ab to 2f7522b Compare July 17, 2026 18:43

jpr5 force-pushed the harden/drift-success-predicate branch from 2f7522b to 36fec1b Compare July 17, 2026 18:44

jpr5 added 4 commits July 17, 2026 11:57

jpr5 marked this pull request as ready for review July 17, 2026 19:55

jpr5 merged commit 6a5e679 into main Jul 17, 2026
29 checks passed

jpr5 deleted the harden/drift-success-predicate branch July 17, 2026 19:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(drift): require real drift resolution, not fixture relaxation, before PR/merge#310

fix(drift): require real drift resolution, not fixture relaxation, before PR/merge#310
jpr5 merged 6 commits into
mainfrom
harden/drift-success-predicate

jpr5 commented Jul 17, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jpr5 commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The hole

The fix — a 3-signal predicate

Wiring

Red-green proof

Out of scope — residual WS-2b (flagged)

Update (CR fix round 1, 75c42ab) — WS-2b CLOSED for auto-merge

Uh oh!

pkg-pr-new Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jpr5 commented Jul 17, 2026 •

edited

Loading

Update (CR fix round 1, `75c42ab`) — WS-2b CLOSED for auto-merge

pkg-pr-new Bot commented Jul 17, 2026 •

edited

Loading