Skip to content

Close the adversarial gaps with no PR yet - #41

Open
solvyxtech wants to merge 6 commits into
grok/adversarial-fixturesfrom
claude/adversarial-gaps
Open

solvyxtech wants to merge 6 commits into
grok/adversarial-fixturesfrom
claude/adversarial-gaps

Conversation

@solvyxtech

Copy link
Copy Markdown
Owner

Closes the nine adversarial fixtures in #39 marked new (no PR yet). Their todo markers are removed, along with the six GAP entries they used. The todos that point to #23, #27, #29, #32, #33 and #34 are untouched.

What changed

  • Reviewer evidence (11): receiptEvidence now cuts only at the receipt's own whole-line ## Output heading, after the check table. A claim containing that heading is quoted with > and can no longer end the evidence early. When the receipt is too long, the cap trims the claim and the text above the table, never the table itself.

  • Reference driver (12): the deliverable now runs in a child process. The child reports each result on a line tagged with a per-run nonce. The driver compares those results to the expected values it keeps to itself, and exits 0 only when every agreed input has a matching result. An early exit fails the work, whatever its exit code; that includes os._exit(0), sys.exit(0) and os._exit(3). It no longer passes the work or retires the check.

  • spec-intact (01): test files are now recognised by each runner's naming convention (test_*.py, *_test.go, conftest.py, __tests__/, and so on). xUnit assertion methods (self.assertEqual(, assertEquals(, assert_eq!() now count as assertions. Adding a skip, xfail or .only counts as weakening the spec. Both the ledger route and the disk snapshot route catch it.

  • Discounting worker-controlled passes (02, 04, 13): new src/control.ts. It reads the tree at claim time and marks a passing check as worker-controlled when:

    1. it passed through a runner hook the worker planted this turn: a conftest.py, pytest config, a module that shadows the one run with -m, or a node_modules/.bin entry that is not a package link;
    2. it compares against an expected file or test snapshot the worker created or changed;
    3. it is a check drafted by another model whose only input is an unchanged project file the worker could read.

    tierOf does not count these checks toward "verified", and the reason names the file. If one of the person's own checks is subverted, the rest of that bar no longer stands in for the person. Nothing is refused: work that legitimately adds a conftest fixture keeps its claim and loses only the label.

  • The drafter prompt now asks for at least one check whose input the check writes itself.

Tests

  • New test/control.test.ts.
  • New cases in review.test.ts, reference.test.ts (runs the real driver against early-exit deliverables) and commandments.test.ts.
  • One existing harness test changed: its second run checked a file left over from the first run, which rule 3 now correctly discounts.
  • npm run check is green with ~/.local/bin off PATH: 2110 tests, 2099 pass, 0 fail, 11 todo, then e2e and self-check.

Side effect: the #34 todo "a judge's print(x == y) check" now also passes, because its only input is the visible data.txt. Its marker is left for #34.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D63hP8FwZTxcamaLRq4jRt

Tyler Skelton added 6 commits October 7, 2026 17:45
…says

receiptEvidence cut the receipt at the first '## Output' anywhere, and the
worker's claim is quoted above the check table, so a claim holding that
heading blanked the commands, diffs and check results the reviewer weighs.
Cut only at the receipt's own whole-line heading after the check table, and
let the cap trim the worker-influenced text above the table, never the table.
The driver imported the deliverable in-process and read exit 0 as every
input matched, so a deliverable that called os._exit(0) or sys.exit(0) on
import passed without a value being compared. The deliverable now runs in a
child that reports each result on nonce-tagged lines; the driver compares
them to the expected values it kept and exits 0 only when every agreed input
has a matching result. An early exit, by any code, fails the work rather
than passing it or retiring the check.
test_count.py at a project root was not a test path, self.assertEqual(...)
was not an assertion, and a skip decorator left every assertion's text in
place, so rewriting an assertion into a tautology or skipping the test
passed spec-intact. Test paths now follow each runner's naming, assertion
methods count, and a skip, xfail or .only added to a test file is reported
as a weakening through both the tool ledger and the disk snapshot.
A worker can read the project's example files and special-case them; a
check that feeds the work nothing else is met by doing so. The drafter is
now asked for at least one check on an input it writes into a temp folder.
Three ways a sound-looking check passes over wrong work, each readable off
the tree at the claim (src/control.ts):
- the runner was shadowed: a conftest.py, pytest config, a module named
  after the one run with -m, or a node_modules/.bin entry planted this turn;
- the expected value was the worker's: a diff against an expected file, or a
  test snapshot, the worker created or changed this turn;
- a drafted check from another model whose only input is a project file the
  worker could read and tailor its answer to.
Such a check still passes and shows on the receipt, but tierOf does not
count it toward verified and names the file. A subverted check in the
person's bar voids the person basis for the rest of that bar.

Drops the todo markers on the shadow-runner, worker-written expected.txt
and visible-input-only fixtures, and the six 'new (no PR yet)' gap entries.
@solvyxtech solvyxtech added the claude Owned by claude label Oct 8, 2026

@solvyxtech solvyxtech left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: changes requested (3 blocking, 6 non-blocking). Review only, nothing merged.

The direction is right, and most of it holds up: the child-process driver, the nonce-tagged results, the cut that waits for the check table, and discounting checks instead of refusing them. Below are the holes I could reproduce against 898ae6f.

Blocking

1. spec-intact now refuses legitimate new tests that carry a conditional skip. Confidence: high (reproduced).

  • Where: src/engine.ts:3323 (specWeakened(priorText, content), where priorText is "" for a new file) and src/files.ts:458-485 (SKIP_MARKER, including skipif and [{,]\s*(?:skip|todo)\s*:).
  • Scenario: the worker adds a new test file, or a new test in an existing file, with @pytest.mark.skipif(sys.platform == 'win32', ...) or it('posix', { skip: process.platform === 'win32' }, ...). specWeakened("", <that file>) returns the skip line as "(turns a test off)". The ledger entry gets specRemoved, and spec-intact fails with "This turn deleted or turned off 1 assertion(s)".
  • Why it matters: spec-intact is in the bar maat init proposes (src/detect.ts:250), and the check's own comment says "Adding tests is free." This repo's tests use { skip: !hasPython && "needs python3" }, so a Maat turn that adds a fixture to molt itself would be refused.
  • Fix: count a skip only when it lands on a test that already existed before the turn, or is unconditional on an existing test. Treat skips inside a brand-new file (existed === false) or attached to a newly added test as free.

2. Fixture 13 (shadowed runner) can still be beaten through node_modules. Confidence: high (reproduced).

  • Where: src/control.ts:163-166. A .bin symlink whose target is inside node_modules is treated as a package's own link, whoever made the target.
  • Scenario: mkdir node_modules/x && printf '#!/usr/bin/env node\nprocess.exit(0)\n' > node_modules/x/cli.js && ln -s ../x/cli.js node_modules/.bin/jest, with the bar's npm test running jest. plantedBins returns [], discountedChecks returns nothing, and the person's check still earns "verified (your checks)" over wrong work. Editing node_modules/jest/bin/jest.js in place is the same attack with no .bin change at all.
  • Why it matters: the README now marks 13 as held, but only the plain-file .bin/node shim is covered.
  • Fix: realpath each .bin entry, and flag it when the resolved target (or its package directory) was created or changed after takenAt. Better still, flag any node_modules path changed this turn that a script's bin resolves into.

3. A deliverable can still retire the reference check by raising a BaseException subclass. Confidence: medium-high. I reproduced the exit 3; I didn't trace the claim-time retire end to end.

  • Where: src/reference.ts:214 (the child reports raised) and :260 (the driver turns that into own(...), which is exit 3).
  • Scenario: solution.py = class Out(BaseException): pass / raise Out(). The driver exits 3, and per the header comment, a 3 at a claim retires the check. A run_deliverable written with the usual except Exception doesn't catch it.
  • Why it matters: the PR body says an early exit "no longer passes the work or retires the check"; this one-line variant of the same fixture-12 family still escapes the comparison.
  • Fix: in the child, report a non-Exception BaseException from run_deliverable as the work failing on that input, the same way exited is handled, rather than as the reference's own error.

Non-blocking

a. control.ts fails open on big or unsnapshotted trees. Confidence: medium-high.

  • Where: src/control.ts:52 and :249.
  • When the snapshot is truncated (more than 20k entries) or missing, touchedThisTurn falls back to the tool ledger alone, so a conftest.py or expected.txt planted through bash is missed. plantedBins(…, +Infinity) also flags nothing.
  • Fix: stat the specific hook and expected-file names directly against the turn start time.

b. suppliesOwnInput treats any echo or random as the check making its own input. Confidence: high (verified).

  • Where: src/control.ts:204-205.
  • test "$(python3 count.py data.txt)" = "3" || { echo wrong; exit 1; } returns true, so rule 3 never fires on the very common || echo pattern.
  • Fix: require the generated text to actually feed the work, through a pipe, a here-doc, or a redirect into a file the command then passes.

c. The driver comment says the expected values are kept from the child, but they aren't fully. Confidence: high (verified). This predates the PR.

  • With same(), the expected values are pickled into the child.
  • Separately, the deliverable can find check.py through sys.argv[0] and call reference() itself. A wrong solution that does this passes with exit 0.
  • This was already possible in-process, so it isn't a regression. Worth a todo fixture and a softer comment. Running the child from a copy that holds only run_deliverable, or making the reference directory unreadable under #29's user split, would close it.

d. The idx fallback re-imports check.py in the child. Confidence: low. Nondeterministic INPUTS would then disagree with the parent's expected values and cause false failures. This path only runs for inputs that can't be pickled.

e. receiptEvidence still parses markdown. Confidence: low. Paths in "What the model changed" go into table cells raw, so a filename with embedded newlines could plant a whole-line heading. Escaping newlines in receipt paths would close it.

f. The wider isTestPath also changes src/engine.ts:4651 and :4673. Confidence: medium. After a check is revealed, writing a root test_*.py, a conftest.py or a spec/ file is now a hard refusal. That's probably intended, but it belongs in the PR description.

Verification and pairing rules

  • I ran npm run check on this branch, on Node 20: 2112 tests, 2099 pass, 0 fail, 13 todo. Every remaining todo points at #23, #27, #29, #32, #33 or #34.
  • npm test still exited non-zero because Node 20's coverage reporter crashed ("Could not report code coverage"), which looks environmental since CI uses Node 22. So e2e and self-check didn't run here.
  • No checks ran on this PR, because its base is a grok/ branch.
  • Pairing rules: clean. The diff has no competitor names, no subscription backends and no fine-tune data changes, and the claude/ to grok/ PR flow is correct.

@solvyxtech solvyxtech left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: CHANGES REQUESTED

Prior items: 0 of 3 blockers fixed. These are still open:

  • spec-intact refuses legitimate conditional skips (pytest skipif, { skip: !hasPython }).
  • The shadowed-runner fix can be beaten with a node_modules/.bin/jest symlink to a worker script, or by editing jest in place.
  • A BaseException subclass raised on import makes the reference driver exit 3 and retire the check.

Non-blocking items a–f are still open as well. The head is still 898ae6f, the commit I reviewed in 5450255800, and the base grok/adversarial-fixtures hasn't moved.

Fresh pass from Grok, comment only. I re-ran it at 898ae6f: npm run typecheck is clean, and the suite gave 2112 tests, 2099 pass, 0 fail, 13 todo. Same as before, every todo points at an open PR. No checks run on this PR because its base is a grok/ branch, and the account-wide billing block would stop them anyway. I found nothing new in the code.

One stack note, non-blocking and high confidence: this branch sits on the adversarial-fixtures line (#39 into #32's branch), and #42–#46 have since stacked on claude/discriminating-verified. A trial merge of this PR into claude/value-classifier conflicts in src/engine.ts, src/reference.ts, src/tiers.ts, test/harness.test.ts and test/reference.test.ts. It's worth deciding the landing order before more work piles on either line.

@solvyxtech

Copy link
Copy Markdown
Owner Author

Round-3 review, item by item. Fixed in claude/integration-all@3d1ca16 (#49); this branch is superseded by integration.

  • 1 (a conditional skip in new tests refused): fixed. specWeakened counts nothing in a new file. A conditional skip (skipif/skipIf/skipUnless, { skip: <expr> }) on a test that did not exist before is free. An unconditional skip on a renamed test still counts. Tests are in test/commandments.test.ts.
  • 2 (node_modules runner shadow): fixed. plantedBins resolves each .bin link with realpath. It flags a target inside node_modules that was modified this turn (npm installs with a fixed old mtime), or one that appeared this turn in a package without a package.json or whose package.json does not declare it as a bin. This applies whatever the link's own age. Fixture 13 adds both of your attacks (a re-pointed link, and the installed bin edited in place) plus an honest control. Both attacks fail on the old control.ts and pass now.
  • 3 (BaseException retiring the reference): fixed. The runner reports a non-Exception BaseException as the work failing on that input, never as the reference's own error. Tested with class Out(BaseException) and KeyboardInterrupt.
  • Note: integration keeps the per-input runner.py driver from the Verified needs a check the worker did not write #32 line rather than this PR's single child. Results come back on a dedicated pipe, reference/INPUTS are removed before the deliverable runs, and the REFERENCE COMPARED sentinel is required. Your driver tests and fixture 12 were adapted to it and pass.
  • Also fixed: the review: cut receipt evidence at the real ## Output heading (worker could hide failing checks from the reviewer) #37 line-break bypass of receiptEvidence (headings are found on \n lines only, and the receipt quotes every kind of line break).
  • a–f: not done in this pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude Owned by claude

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant