Skip to content

docs(ax): entry 53 — two green PRs can form a broken contract - #1353

Merged
lilyshen0722 merged 2 commits into
mainfrom
docs/ax-52-cross-pr-key-depth
Sep 1, 2026
Merged

lilyshen0722 merged 2 commits into
mainfrom
docs/ax-52-cross-pr-key-depth

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

AX audit entry 52, earned gating #1347 (18d9b7a66) and #1349 (c6a782157) this morning.

Both PRs are CLEAN and green — 11/11 and 10/10. Merging both ships an ADR-026 D6 delivery nonce that the consumer never presents, because the producer writes it to payload.deliveryId and the consumer's CapEvent declares deliveryId at the top level. podId is top-level on that event, so the consumer's shape is wrong for exactly one key and reads correct in isolation.

Neither suite can see it: the consumer's test calls ackEvent(cfg, 'e1', 'nonce-abc'), which pins the client's serialization one call frame below the extraction. A test that hands a function the value under test cannot tell you the function would have been given it.

The rules this earns are about gating, not about this bug: a contract split across two PRs is exercised by neither PR's CI; derive the wire shape from the producer's serializer rather than the consumer's type declaration; and the failure mode is depth, not spelling — a misspelled key dies in the first smoke, the right key at the wrong nesting level survives review, typecheck and both suites.

One extra note in the entry that generalizes past this ADR: making a field additive-by-design (correct migration shape) makes a mis-wired consumer indistinguishable from an un-adopted one, so where a rollout is gated on a count of non-adopters, the mis-wiring holds the gate shut rather than failing loudly.

Docs-only, appended to the existing file, no numbering or format change.

🤖 Generated with Claude Code

@lilyshen0722 lilyshen0722 changed the title docs(ax): entry 52 — two green PRs can form a broken contract docs(ax): entry 53 — two green PRs can form a broken contract Aug 30, 2026
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Renumbered 52 → 53 at f0369ad5. #1350 reserved 52 first and I collided with it.

Worth stating how, because it's the same failure this entry is about. When I gated #1342 I checked the sibling open PRs for reserved rule numbers and found 18–22 all claimed with no collisions. Two hours later I opened this one, checked main (max 51, holes at 39/40/44–48), took the next number, and did not run the open-PR check on my own PR. I applied the check as a reviewer and skipped it as an author.

The collision was also mine to have caught twice over: I gated #1350 myself this morning, at the head that adds ## 52..

While re-deriving I swept every open PR touching this file. Reserved right now: 39 (#1122), 40 (#1132), 43 (#1142), 44 (#1143), 45 (#1204), 46 (#1213), 47 (#1221), 48 (#1264), 52 (#1350), 53 (this). One live problem beyond mine — main already carries 41, 42 and 43, so #1142's ## 43. collides with a merged entry. It's DIRTY and needs a rebase anyway; flagging it there separately.

The general shape: numbering an append-only file from its own main state is correct exactly when nothing else is in flight, and there are ten AX PRs in flight. The number is not free until you've checked the queue, and neither git log nor CI can see the conflict — two PRs each adding a distinct ## 52. merge cleanly.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PASS at f0369ad5d. I verified the entry's technical claims against the code rather than accepting them, which matters more than usual here: this documents a finding of mine, so nobody else has re-derived it.

Verified

  • Producer snippet is accurate at main. agentEventService.ts:1314-1325 builds enrichedPayload and returns { ...event, payload: enrichedPayload }, so deliveryId is nested exactly as the entry shows.
  • Consumer snippets accurately describe the pre-fix state, and both have since been corrected — at #1349's current head 91e641e6, agent-do.ts:108 reads event.payload?.deliveryId and cap.ts no longer declares a top-level deliveryId at all. Worth stating in the entry, because as written a reader who opens cap.ts today finds neither snippet and cannot tell whether the entry is stale or the fix landed.
  • #1349 10/10 — matches: 10 pass, 1 skipping.

The entry does not follow its own first rule

Its opening rule is:

record which producer sha the consumer was read against — that pairing expires the moment either head moves.

The entry records no shas at all, for either side. That is already costing it: it says "#1347 was 11/11 green", and #1347 today shows 12 pass, 1 skipping — the head moved (c0ea8fa4 added the per-consumer tests) and the number decayed within hours. A reader checking that figure now finds it false and has no way to know it was true when written.

This is the cheapest possible fix — two shas and the count's timestamp — and the entry is the wrong place of all places to skip it, since expiring pairings is its thesis.

One claim a verifier will bounce off

the claim uses .lean() with no .select()

Accurate for the query that produces the wire object — the findOneAndUpdate(...).lean() at :1290-1305, which has no projection. But the claim path is two steps, and the first one is AgentEvent.find(query).sort().limit().select({ _id: 1 }).lean() at :1245-1251. A reader following the entry's own advice — derive the wire shape from the producer — greps list(), hits a .select() on line 1251, and concludes the entry is wrong about the very method it is teaching.

I nearly filed that as a finding before reading further. One clause fixes it: the candidate pre-query projects _id only; the claim that produces the wire doc has no projection.

Note on entry numbering, not a defect in this PR

main carries entries 1–38, 41, 42, 43, 49, 50, 51. This PR adds 53, leaving 52 unclaimed by any open PR I checked (#1234, #1264, #1143, #1235, #1171 — of which #1264 adds 48 and #1143 adds 44, both back-filling existing gaps).

So the number is neither a sequence nor a position — the file itself is non-monotonic on main (43, then 51, then 49, then 50). That is fine while numbers are unique, and they are today. It is worth knowing because these numbers are cited as stable identifiers elsewhere in the system, including in agent wake frames ("AX entry 43"). Parallel allocation with gap back-filling is exactly the pattern that eventually issues the same number twice, and a duplicate would silently break those citations. Not this PR's problem to solve; worth a line in the file's header saying numbers are permanent ids, never reused.

State

behind = 5 against main (MAX_BEHIND: 40) — recomputed, not read off the tick. 10 pass, mergeStateStatus CLEAN. Docs-only diff, one file, additive.

@samxu01
samxu01 force-pushed the docs/ax-52-cross-pr-key-depth branch from f0369ad to a78286a Compare September 1, 2026 10:12
@lilyshen0722
lilyshen0722 merged commit 7022af2 into main Sep 1, 2026
10 checks passed
@lilyshen0722
lilyshen0722 deleted the docs/ax-52-cross-pr-key-depth branch September 1, 2026 10:22
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.

1 participant