Skip to content

test(relay): the pin test's message says what it pins, not what the bytes are not - #1033

Merged
cameri merged 2 commits into
rewrite/mainfrom
factory/020-pin-test-message
Nov 2, 2026
Merged

cameri merged 2 commits into
rewrite/mainfrom
factory/020-pin-test-message

Conversation

@chappie-daemon

Copy link
Copy Markdown
Collaborator

Closes #1028 — but see the note at the end: the keyword will not close it on this base.

The pin test's failure message said:

"the bytes are the wire contract"

They are not. The refusal word never crosses the wire: it is a process-local AtomicU8 on Account, written only by Account::refuse and read only by Account::refusal. The type's own doc says so twenty lines above (sink.rs:92-98, "Not reachable from the wire"), so the test's explanatory text contradicted the type it explains.

The message now states the invariant the assertion actually holds:

-            "the bytes are the wire contract"
+            "the refusal bytes are pinned to 0, 1, 2 — the numbers word() returns and the enum stores"

Two files, +12/−1: that line, and an empty-fronted changeset.

Verification

cargo fmt --all --check                                    exit 0
cargo clippy --workspace --all-targets --all-features -D warnings   exit 0
cargo clippy --workspace --lib --bins (strict five lints)  exit 0
cargo test --workspace --all-features                      exit 0   nostream-relay 259 passed / 0 failed
cargo build --workspace --all-features --release           exit 0
grep -rn "wire contract" crates docs .changeset            exit 1   (one line at base)
git merge-tree --write-tree beaacde HEAD                   exit 0

No test observes this string, and the job's spec says so rather than dressing a green suite as proof: changing only the message leaves the suite at 259 passed / 0 failed. The sentence's truth is a read, and the evidence for it is: the assertion compares the three constants to the literals (0, 1, 2); word()'s arms return those constants; the type is #[repr(u8)] with variants assigned from them; and the loop below holds the stored byte to word(). Re-measured rather than argued — with QUEUE_FULL set to 4 in a scratch copy the suite goes 258 passed / 1 failed and the new sentence prints verbatim with left: (0, 4, 2).

Observations from this job (first run on factory 0.12.9)

Recorded because they are findings about the pipeline, not caveats about this diff.

  • The lenses cannot start two seconds apart. The skill says "about two seconds apart"; measured, it is one model turn per start — 10.8 s in round 1, 31 s in round 2. Moving in the safe direction (the earlier lens's first-turn cache write lands first), but the instruction reads as though the interval were tunable by the orchestrator. It is not: it would take a dispatcher starting both from one process.
  • A plan can record "oracle": "passed" when the only falsifiable claim is a reading. scope-plan.ts checks a plan's shape, not whether its oracle is mechanical; on this job it accepted passed on the strength of a grep and a one-test selector. Correct by its rules, and worth naming, because a prose job is exactly where that reads stronger than it is. Filed as cameri/skills#224.
  • confirmation was unreachable on 0.12.9 for any plan that stands a lens down. scope-plan.ts:256 requires every lens not run to be PASS, which does not distinguish a lens the gate held back from one the plan stood down. Round 3 therefore ran three lenses where one would have done — four sessions instead of two, a 2x on that round — and the fix is in 0.12.11, which accepts the second case using the round's plan line.
  • A lens returned CHANGES on job state, not on the change. Round 2's architecture review opened "My round-1 finding is closed on all four grounds, and the code half of this round needs no further change", then blocked on spec.md — untracked, unshipped, invisible to anyone reading this diff. In round 3 the same lens named it: "a CHANGES that the builder can only satisfy by editing an untracked file is a gate no code change opens; the correct instrument was a non-blocking finding." The finding was real and is fixed; the verdict was the wrong instrument.
  • A one-line change took three review rounds. Round 1 caught something real — an operator-suggested const for the message had one consumer, deduplicated nothing, and contradicted every other message in that module; it was deleted. Round 2's finding was record repair, and only round 3 could confirm it. The cost is stated plainly because it is the number that argues for the rules above.

One follow-up filed from the review: #1032 — a byte re-pin now needs three edits (constants, tuple, sentence) and only two are test-held, so grep -n 'pinned to 0, 1, 2' is the only reminder of the third.

Note on the keyword

Closes #1028 does nothing on this repository: GitHub closes by keyword only when the pull request targets the default branch, and this targets rewrite/main. Whoever merges closes the issue by hand and reads the state back.

@changeset-bot

changeset-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 712cce5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 576c6a28-f20b-437f-a700-970c695dac47

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@greptile-apps

greptile-apps Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

Greptile CLI A Greptile CLI review exists for this PR, but a new review ran because auto-approve needs this PR's own review.

[Low impact] This change appears safe to merge.

Summary

Updates the failure message in every_refusal_word_is_pinned_to_its_byte to describe the byte values the test checks, rather than calling them a wire contract.

  • The refusal pin test names the bytes it checks.

Reviews (1) · Last reviewed commit: "test(relay): inline the pin test's failu..." · Reviewed by Greptile

@cameri
cameri merged commit 7f491f4 into rewrite/main Nov 2, 2026
17 checks passed
@cameri
cameri deleted the factory/020-pin-test-message branch November 2, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants