From 89aa11ea6434424fe8a469ed820c965c0f5d5007 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:11:22 +0900 Subject: [PATCH 01/27] Make a delivered message something the recipient can act on A delivery told its recipient an event id, a relationship id, a generation and a revision hash, then listed file paths and their digests. All of it true, none of it saying which pull request to open, what was checked, what is still open, or what to do next, so a parent had to go and reconstruct the work before it could act on the message about it. The message now leads with the result, the repository and pull request, the base and head commit, the verification evidence, the unresolved items and the next action. The identifiers keep their place at the bottom: they are how the parent answers, not how it decides. None of that fits in a completion receipt, which is frozen with additionalProperties false, so a work report is a relay-owned record like the criteria set, bound to the event, generation and revision it describes. A report about one head cannot answer for a later one, and a pull request number is never stored or rendered without its repository, so the same number on two projects stays two pull requests. The second half is the seam with CXC. Both systems have a word for how an execution ended and the overlap is dangerous: CXC glosses DONE as verified success, meaning the child proved its own criteria, while the relay verified is the parent disposition written against registered criteria on an acknowledged event. A report status is therefore recorded beside the receipt outcome and checked against it, never used to derive it, and DONE, an open pull request, a review PASS and a green check are each listed as evidence for a verdict rather than one. An unknown status is refused by name with the contract version it was read against. The mapping points at the installed rules and copies none of them. Shortening a message is now something it admits to: headings survive with a count of what is missing, one omitted line names what was dropped and the command that shows the rest, and a budget too small for the required parts refuses rather than shipping a message that silently lost them. An event with no work report renders exactly what it rendered before. A receipt from before this contract has no pull request to centre a report on, and inventing one would be the guess this package refuses everywhere else. --- .../docs/cxc-contract-map.md | 111 ++++ .../codex-session-relay/docs/invariants.md | 15 + .../codex-session-relay/docs/protocol-v1.md | 33 + .../src/codex_session_relay/cxc.py | 341 +++++++++++ .../src/codex_session_relay/delivery.py | 27 +- .../src/codex_session_relay/report.py | 572 ++++++++++++++++++ .../src/codex_session_relay/store.py | 32 + .../tests/test_report_contract.py | 441 ++++++++++++++ 8 files changed, 1565 insertions(+), 7 deletions(-) create mode 100644 packages/codex-session-relay/docs/cxc-contract-map.md create mode 100644 packages/codex-session-relay/src/codex_session_relay/cxc.py create mode 100644 packages/codex-session-relay/src/codex_session_relay/report.py create mode 100644 packages/codex-session-relay/tests/test_report_contract.py diff --git a/packages/codex-session-relay/docs/cxc-contract-map.md b/packages/codex-session-relay/docs/cxc-contract-map.md new file mode 100644 index 0000000..ddf5dc2 --- /dev/null +++ b/packages/codex-session-relay/docs/cxc-contract-map.md @@ -0,0 +1,111 @@ +# The CXC report contract, as this relay reads it + +Jun's managed workflow runs CXC and this relay together, so both have a vocabulary for how +an execution ended. Where the two touch, one of them has to say what the other one means. +This page is that statement. It copies no CXC prose and rebuilds no part of its state +machine: every row points at the installed file that owns the rule, so a version bump is +re-read rather than trusted from a transcription that has quietly gone stale. + +Read against **codexclaw 0.2.28+codex.20260914090142**, installed under +`$CODEX_HOME/plugins/cache/codexclaw/`. The version string is reported identically by +`.codex-plugin/plugin.json` and `inventory.json`. `codex_session_relay.cxc.provenance()` +returns the same record at runtime, including the digest of every file below, and +`cxc.affected_by()` names which rules a changed install actually touches, so a bump +re-verifies those rules and their scenarios rather than all of them. + +| Rule | Owning file | Anchor | sha256 | +| --- | --- | --- | --- | +| report outcomes are not phases | `skills/loop/SKILL.md` | 146-148 | `61167d15...a44e5e2b` | +| terminal-state vocabulary | `skills/pabcd/references/loop-engineering.md` | 32-40 | `c5fb8c4e...5db67fc7` | +| REVIEW-SYNTHESIS-01 | `skills/pabcd/references/loop-engineering.md` | 41-60 | `c5fb8c4e...5db67fc7` | +| DISPATCH-TASK-01 | `skills/pabcd/references/delegation.md` | 20-22 | `ba773dbd...d0c578b2` | +| plan output, nine concepts | `skills/pabcd/references/plan-output.md` | 7-20 | `4535762a...366c7868` | +| REVIEW-OUTPUT-01 | `skills/dev-code-reviewer/SKILL.md` | 86-95 | `1d66f377...2e5d2d28` | +| ATTEST-EVIDENCE-01 | `skills/pabcd/references/phase-control.md` | 45-48 | `6982abdd...5a8b476a` | +| LOOP-WAIT-EVIDENCE-01 | `skills/loop/references/waiting.md` | 31-63 | `988451de...b15150c8` | +| DISPATCH-SURFACE-01 | `skills/pabcd/references/dispatch-surfaces.md` | 10-20 | `dd3e6721...611c26e98` | + +The full digests live in `cxc.SOURCES`; the table abbreviates only for width. + +## Report status against relay outcome + +A CXC report status does not choose a relay outcome. The outcome is asserted by the child's +completion receipt, which is the only thing the frozen contract lets assert one, and the +status is recorded beside it and checked for compatibility. A status that picked the outcome +could overrule the evidence; a status checked against it cannot. + +| CXC status | Means | Compatible relay outcome | +| --- | --- | --- | +| `DONE` | the child proved every recorded criterion | `ready_for_review` | +| `NOOP` | nothing needed doing | `ready_for_review` | +| `BLOCKED` | an external dependency is in the way | `blocked_needs_input` | +| `UNSAFE` | a human risk decision is required | `blocked_needs_input` | +| `NEEDS_HUMAN` | a judgment only the user can make | `blocked_needs_input` | +| `BUDGET_EXHAUSTED` | a stated bound ran out | `interrupted`, `failed` | + +Three statuses land on one outcome because the frozen enum has a single slot for a human +having to decide something, and adding a second is not available to this layer. Nothing is +lost: the original word and the reason the child gave are stored and rendered, so a reader +can still tell a blocked dependency from a refused risk. A status this build does not know +is refused by name, with the accepted set and the contract version it was read against, +rather than being mapped to whichever neighbour comes first in a dictionary. + +`NOOP` pairs with `ready_for_review` on purpose. A report that nothing needed doing is still +a finding, and the work that established it is the deliverable. This package already refuses +to complete a managed assignment verified against nothing; a no-change claim with nothing +behind it is the same shape of claim. + +## The word both systems use for different things + +CXC glosses `DONE` as verified success, meaning the child proved its own recorded criteria. +The relay's `verified` is the **parent's** disposition, written through `record_verdict` +against registered criteria on an acknowledged event. Same English word, two authorities. + +`cxc.NOT_VERIFICATION` lists the facts that resemble verification and are not, each with the +reason it does not carry: + +- a `DONE` report, the child proving its own criteria rather than the parent's verdict +- an opened pull request, a place to review rather than a review +- a review `PASS`, one reviewer's judgment rather than the parent's disposition +- a green required check, evidence for a verdict rather than a verdict +- a completed turn, the trigger to look, as protocol v1 section 2 already says +- a dispatched delivery, neither an acknowledgement nor a verification + +## Review verdicts + +REVIEW-OUTPUT-01 fixes the final line, and this relay renders exactly that: `VERDICT: PASS`, +`VERDICT: GO-WITH-FIXES (blockers=N)`, `VERDICT: FAIL`. A `GO-WITH-FIXES` without a blocker +count is refused, because it is a `PASS` wearing a hedge, and a count attached to `PASS` or +`FAIL` is refused as a number nobody can act on. A verdict line is rendered only for a +message that carries a review, so an ordinary progress or completion notice cannot be +dressed as a review judgment. + +## Waiting + +LOOP-WAIT-EVIDENCE-01 keeps five endings apart, and `cxc.classify_wait` returns exactly one +of them: `progress`, `suspected_stagnation`, `confirmed_failure`, `unobservable`, +`input_needed`, and `timed_out` for a wait that simply ended on its bound. A terminal error +outranks everything; a request for input is not a failure; fresh advancing evidence outranks +the clock. No result ever sets `authorisesRerun`, because running the work again is a +decision somebody makes from evidence, and these are the states in which that evidence does +not exist yet. This mirrors what the delivery layer has always done: protocol v1 section 3 +says no delivery state is reachable by a timeout, and section 4 says elapsed time is not +affirmative evidence. + +## Instruction shape + +A revision request is an instruction, so it carries the DISPATCH-TASK-01 fields, `TASK`, +`SCOPE`, `MUST DO`, `MUST NOT`, `PROOF` and `RETURN FORMAT`, plus a decision boundary. It +leads with the criterion that was violated and the anchor that reproduces it, because a +correction whose first line is an identifier is a correction the child has to research +before it can start. The acknowledgement asymmetry is unchanged: contract v1 defines no +acknowledgement for the parent-to-child direction, so the message says so and tells the +child what does work instead. + +## What this does not do + +It builds no dispatch mechanism, no state machine and no provider retry ladder; CXC owns all +three. It adds no value to any frozen enum: not the receipt outcomes, not the delivery +states, not the verdict dispositions. It does not require the recovery daemon to run a CXC +cycle. And it neither reads nor depends on an installed CXC runtime at delivery time, since +the provenance record above is pinned data rather than a live lookup. diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 618f874..fc146e4 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -101,6 +101,21 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-63 | Unchanged states stay quiet | a tick that changes nothing writes no journal rows | implemented | | I-64 | An unbounded daemon loop is not constructible | `run` requires a tick count, a deadline or a stop signal | implemented | +## Work reports and the CXC report contract + +| # | Invariant | Enforced in | Status | +|---|---|---|---| +| I-90 | A CXC report status never chooses a relay outcome; it is checked against the one the receipt asserted | `cxc.check_status`, called from `report.record` | implemented | +| I-91 | An unrecognised report status is refused by name, with the accepted set and the contract version | `cxc.check_status` | implemented | +| I-92 | DONE, an open pull request, a review PASS and a green check are never a relay verdict | `cxc.NOT_VERIFICATION`, `cxc.refuse_promotion`; no path writes a verdict outside `ack.record_verdict` | implemented | +| I-93 | A verdict line is rendered only for a message carrying a review, in the fixed PASS / GO-WITH-FIXES (blockers=N) / FAIL form | `cxc.verdict_line`, `cxc.assert_reviewed` | implemented | +| I-94 | A report is bound to one event, generation and revision, and cannot answer for a later head | `report.assert_current` | implemented | +| I-95 | A pull request number is never rendered or compared without its repository | `report.pr_ref`, `report.pr_key` | implemented | +| I-96 | A report naming a pull request names the head commit it is about | `report.record` | implemented | +| I-97 | A shortened message names what it dropped and where to read it; a budget too small to hold the required parts refuses | `report._compose` | implemented | +| I-98 | An event with no work report renders the pre-contract message unchanged | `delivery._render_completion`, `delivery._render_revision` | implemented | +| I-99 | A wait result never authorises a re-run, and a bare timeout is neither failure nor success | `cxc.classify_wait` | implemented | + ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/docs/protocol-v1.md b/packages/codex-session-relay/docs/protocol-v1.md index 3268332..3c507fa 100644 --- a/packages/codex-session-relay/docs/protocol-v1.md +++ b/packages/codex-session-relay/docs/protocol-v1.md @@ -172,3 +172,36 @@ revision may define this type; this implementation does not silently become that |---|---| | Terminal turns are detected by bounded polling rather than a receive-only notification connection | the contract itself lists as unresolved whether a client receives turn completion for threads it neither started nor resumed, and the transport cannot subscribe. Polling establishes the behaviour with the supported read path; a notification listener ships as an optional early-wake hint that is never evidence | | There is no operator release: an attempt with no affirmative evidence stays held | the contract lists three affirmative evidences and does not contemplate an operator override, so none is implemented. An attempt holding none of them stays held and reports what it is missing, and there is no flag, audited or otherwise, that releases it. An earlier revision of this table described an implementation behind an `allow_operator_release` switch; that is not in the delivered core. `RefusalReason.OPERATOR_RELEASE_DISABLED` is retained in the taxonomy and is unreachable, because no code path raises it | + +## 8. Work reports + +A delivered message has to be something the recipient can act on. The identifiers above are +how it answers, not how it decides, so the message leads with the result, the repository and +pull request, the base and head commit, what was verified, what is still unresolved, and +what to do next. + +None of that fits in a completion receipt. The five schemas are frozen with +`additionalProperties: false`, so a work report is a relay-owned record, like the criteria +set and the revision request, stored against the exact event, relationship, generation and +revision it describes. The repository is stored beside the pull request number and the two +are never rendered apart, so the same number on two projects stays two pull requests. A +report about one head cannot answer for another: a later push is a new report, not an +inherited one. + +An event with no work report renders exactly what it rendered before. A receipt from before +this contract has no pull request to centre a report on, and inventing one would be the same +guess this implementation refuses everywhere else. `report.version_of` names which of the +two an event is on, and the report-backed body says so in its own `contract:` line. + +When a message has to be shortened, the shortening is announced. Required action, scope and +unresolved items keep their headings and gain an explicit count of what is missing, every +reduction adds one `omitted:` line naming what was dropped and the command that shows the +whole record, and a budget too small to hold the required parts refuses rather than shipping +a message that silently lost them. A message that quietly drops its unresolved items reads +exactly like a message that had none. + +The CXC report vocabulary this maps onto is documented separately in +[the CXC contract map](cxc-contract-map.md). Its one load-bearing rule: a `DONE` report, an +opened pull request, a review `PASS` and a green required check are all evidence for a +verdict and none of them is one. Only the parent, from inside its own turn, against +registered criteria, writes `verified`. diff --git a/packages/codex-session-relay/src/codex_session_relay/cxc.py b/packages/codex-session-relay/src/codex_session_relay/cxc.py new file mode 100644 index 0000000..36a5e86 --- /dev/null +++ b/packages/codex-session-relay/src/codex_session_relay/cxc.py @@ -0,0 +1,341 @@ +"""The CXC report vocabulary this relay accepts, and the promotion it refuses. + +CXC owns the dispatch lifecycle, the PABCD state machine and the provider retry ladder. +None of that is rebuilt here and none of its prose is copied: every rule below points at +the installed skill that owns it, so a version bump is re-read rather than trusted from a +transcription that has quietly gone stale. + +What this module owns is the seam. Both systems have a word for how an execution ended, +and where the two vocabularies touch, one of them has to say what the other one means. +The dangerous overlap is "verified". CXC glosses DONE as verified success, meaning the +child proved its own recorded criteria. The relay's "verified" is the PARENT's +disposition, recorded through a verdict with criteria coverage and an acknowledged event. +Same English word, two different authorities, and nothing here may let the first become +the second. + +A CXC report status is therefore recorded BESIDE a relay outcome, never used to derive +one. The outcome still comes from the child's receipt, which is the only thing the frozen +contract lets assert it. That keeps the original status and its reason intact, adds no +value to any frozen enum, and gives NOOP somewhere honest to go instead of being forced +into a failure it is not. +""" + +from dataclasses import dataclass + +from .errors import ReceiptRefused, RefusalReason + +PACKAGE = "codexclaw" +VERSION = "0.2.28+codex.20260914090142" + + +@dataclass(frozen=True) +class Source: + """One rule, where it lives, and what the file said when we read it. + + The digest is what makes this a pointer rather than a copy. When the installed payload + changes, the digest stops matching and the mapping it supports is re-derived from the + file instead of being assumed to still hold. + """ + + rule: str + path: str + anchor: str + sha256: str + + +SOURCES = ( + Source( + "report-outcomes-are-not-phases", "skills/loop/SKILL.md", "146-148", + "61167d152d3c84f01b4d05ccb7ded5457db07ec42a84ae85e911eb75a44e5e2b", + ), + Source( + "terminal-state-vocabulary", "skills/pabcd/references/loop-engineering.md", "32-40", + "c5fb8c4e9fdd7dd9ce675a712cc9eca5b8e21301e8bfe49f7e27a9185db67fc7", + ), + Source( + "REVIEW-SYNTHESIS-01", "skills/pabcd/references/loop-engineering.md", "41-60", + "c5fb8c4e9fdd7dd9ce675a712cc9eca5b8e21301e8bfe49f7e27a9185db67fc7", + ), + Source( + "DISPATCH-TASK-01", "skills/pabcd/references/delegation.md", "20-22", + "ba773dbd5bbe3fd975bfca9f149205907ec9ca4c5a24bd2ce3c86d52d0c578b2", + ), + Source( + "plan-output-nine-concepts", "skills/pabcd/references/plan-output.md", "7-20", + "4535762ade768fad5aafb121e9c3b44b7cf6df17eb6b2a04cbfaa2dc366c7868", + ), + Source( + "REVIEW-OUTPUT-01", "skills/dev-code-reviewer/SKILL.md", "86-95", + "1d66f37c7754d852931bcf958063f30b2ab261a583ec080d280daf912e5d2d28", + ), + Source( + "ATTEST-EVIDENCE-01", "skills/pabcd/references/phase-control.md", "45-48", + "6982abddc1ceb52d046403b2a16a01ea22fab3fc30900d20c26a04a35a8b476a", + ), + Source( + "LOOP-WAIT-EVIDENCE-01", "skills/loop/references/waiting.md", "31-63", + "988451dee609d3949fa21f68fb2453a210f8fbbd06819db4219e710ab15150c8", + ), + Source( + "DISPATCH-SURFACE-01", "skills/pabcd/references/dispatch-surfaces.md", "10-20", + "dd3e6721718f05a45ac0031d861823441bba15592640dbe7785cae6611c26e98", + ), +) + + +def provenance() -> dict: + """What was read, from where, at which version. Recorded with every work report.""" + return { + "package": PACKAGE, + "version": VERSION, + "sources": [ + {"rule": s.rule, "path": s.path, "anchor": s.anchor, "sha256": s.sha256} + for s in SOURCES + ], + } + + +def sources_for(rule: str) -> tuple: + return tuple(s for s in SOURCES if s.rule == rule) + + +def affected_by(changed_paths) -> tuple: + """Which rules a changed install actually touches. + + A version bump does not invalidate the whole mapping, and treating it as though it did + is how a re-verification becomes expensive enough to skip. Only the rules whose files + moved need re-reading, and only the scenarios resting on those rules need re-running. + """ + changed = set(changed_paths) + return tuple(sorted({s.rule for s in SOURCES if s.path in changed})) + + +# ------------------------------------------------------------------ report status + +DONE = "DONE" +NOOP = "NOOP" +BLOCKED = "BLOCKED" +UNSAFE = "UNSAFE" +NEEDS_HUMAN = "NEEDS_HUMAN" +BUDGET_EXHAUSTED = "BUDGET_EXHAUSTED" + +REPORT_STATUSES = (DONE, NOOP, BLOCKED, UNSAFE, NEEDS_HUMAN, BUDGET_EXHAUSTED) + +# What each one means, in the relay's own words, so a recipient reading a collapsed mapping +# can still tell BLOCKED from UNSAFE from NEEDS_HUMAN. +MEANING = { + DONE: "the child proved every recorded criterion against its own work", + NOOP: "nothing needed doing, and the finding that established that is the deliverable", + BLOCKED: "an external dependency is in the way", + UNSAFE: "a human risk decision is required before this can proceed", + NEEDS_HUMAN: "a judgment only the user can make", + BUDGET_EXHAUSTED: "a bound the plan actually stated ran out; best-so-far is adopted", +} + +# The relay outcomes each status may accompany. This is a COMPATIBILITY relation, not a +# derivation: the outcome is asserted by the child's receipt and checked against the status +# here. Three statuses collapse onto blocked_needs_input because the frozen outcome enum has +# one slot for "a human has to decide something" and inventing a second is not available to +# this layer. Nothing is lost, because the status and its reason are stored alongside. +COMPATIBLE_OUTCOMES = { + DONE: ("ready_for_review",), + NOOP: ("ready_for_review",), + BLOCKED: ("blocked_needs_input",), + UNSAFE: ("blocked_needs_input",), + NEEDS_HUMAN: ("blocked_needs_input",), + BUDGET_EXHAUSTED: ("interrupted", "failed"), +} + + +def check_status(status: str, outcome: str) -> None: + """Refuse an unknown status by name, and an incompatible pair by both names. + + Silently defaulting an unrecognised status is the failure this exists to prevent: a + newer CXC reporting a word this build has never seen must be diagnosed, not mapped to + whichever neighbour happens to be first in a dict. + """ + if status not in COMPATIBLE_OUTCOMES: + raise ReceiptRefused( + RefusalReason.OUTCOME_INCONSISTENT, + f"{status!r} is not a CXC report status this build maps. Accepted: " + f"{', '.join(REPORT_STATUSES)}. Read against {PACKAGE} {VERSION}; a newer " + "contract needs the mapping extended rather than the value guessed at", + ) + allowed = COMPATIBLE_OUTCOMES[status] + if outcome not in allowed: + raise ReceiptRefused( + RefusalReason.OUTCOME_INCONSISTENT, + f"a {status} report cannot accompany outcome {outcome!r}: {status} means " + f"{MEANING[status]}, which this contract pairs with {', '.join(allowed)}", + ) + + +# --------------------------------------------------------------- the promotion ban + +# Facts that look like verification and are not. Each one is something a child, a forge or +# a CI run can produce on its own; the relay's verified disposition is something only the +# parent produces, from inside its own turn, against registered criteria. +NOT_VERIFICATION = { + "cxc_done": "a DONE report is the child proving its own criteria, not the parent's verdict", + "pull_request_opened": "an open pull request is a place to review, not a review", + "review_pass": "a review PASS is one reviewer's judgment, not the parent's disposition", + "required_checks_green": "a green required check is evidence for a verdict, not a verdict", + "turn_completed": "a completed turn is the trigger to look, as protocol v1 section 2 says", + "dispatched": "a dispatched delivery is not an acknowledgement and not a verification", +} + + +def refuse_promotion(fact: str) -> str: + """Why this fact does not become a relay verdict. Every caller gets a reason, not a bool.""" + if fact not in NOT_VERIFICATION: + raise KeyError( + f"{fact!r} is not a recorded non-verification fact; add it with its reason " + "rather than letting an unlisted fact through by omission" + ) + return NOT_VERIFICATION[fact] + + +# ------------------------------------------------------------------- review verdict + +PASS = "PASS" +GO_WITH_FIXES = "GO-WITH-FIXES" +FAIL = "FAIL" +VERDICT_KINDS = (PASS, GO_WITH_FIXES, FAIL) +PREFIX = "VERDICT: " + + +def verdict_line(kind: str, blockers=None) -> str: + """The machine-scannable final line REVIEW-OUTPUT-01 fixes. + + GO-WITH-FIXES without a blocker count is a PASS wearing a hedge, and a blocker count on + PASS or FAIL is a number nobody can act on. Both are refused rather than normalised. + """ + if kind not in VERDICT_KINDS: + raise ValueError( + f"{kind!r} is not a review verdict; REVIEW-OUTPUT-01 fixes " + f"{', '.join(VERDICT_KINDS)}" + ) + if kind == GO_WITH_FIXES: + if not isinstance(blockers, int) or isinstance(blockers, bool) or blockers < 1: + raise ValueError( + "GO-WITH-FIXES states how many blockers it is going ahead with; a count " + "below one is a PASS and should say so" + ) + return f"{PREFIX}{GO_WITH_FIXES} (blockers={blockers})" + if blockers is not None: + raise ValueError(f"a {kind} verdict carries no blocker count") + return f"{PREFIX}{kind}" + + +def parse_verdict_line(line: str): + """Read one back, or None. Used to tell a real verdict from prose that resembles one.""" + text = (line or "").strip() + if not text.startswith(PREFIX): + return None + body = text[len(PREFIX):].strip() + if body in (PASS, FAIL): + return {"kind": body, "blockers": None} + if body.startswith(GO_WITH_FIXES): + rest = body[len(GO_WITH_FIXES):].strip() + if rest.startswith("(blockers=") and rest.endswith(")"): + digits = rest[len("(blockers="):-1] + if digits.isdigit() and int(digits) >= 1: + return {"kind": GO_WITH_FIXES, "blockers": int(digits)} + return None + + +def assert_reviewed(has_review: bool) -> None: + """A verdict line belongs to a review. An ordinary progress notice may not wear one.""" + if not has_review: + raise ValueError( + "a verdict line states a review judgment; a progress or completion notice that " + "renders one is dressing an update as a review" + ) + + +# ------------------------------------------------------------------------- waiting + +PROGRESS = "progress" +SUSPECTED_STAGNATION = "suspected_stagnation" +CONFIRMED_FAILURE = "confirmed_failure" +UNOBSERVABLE = "unobservable" +INPUT_NEEDED = "input_needed" +TIMED_OUT = "timed_out" + +WAIT_STATES = ( + PROGRESS, SUSPECTED_STAGNATION, CONFIRMED_FAILURE, UNOBSERVABLE, INPUT_NEEDED, TIMED_OUT, +) + +# Nothing on this list is permission to run the work again. A re-run is a decision somebody +# makes from evidence; these four are the states in which that evidence does not exist yet. +NEVER_AUTHORISES_RERUN = (TIMED_OUT, SUSPECTED_STAGNATION, UNOBSERVABLE, INPUT_NEEDED) + + +def classify_wait(*, terminal_error=None, input_requested=False, advancing_evidence=False, + observable=True, stagnation_confirmed=False, timed_out=False) -> dict: + """Five endings that a wait can have, kept apart. + + The ordering is the point. A terminal error is a failure whatever else is true; a + request for input is not a failure at all; fresh advancing evidence outranks a clock. + A bare timeout lands on its own state and stays there, because elapsed time is not on + the list of things that establish anything, here or in reconciliation. + """ + if terminal_error: + return _wait(CONFIRMED_FAILURE, str(terminal_error)) + if input_requested: + return _wait(INPUT_NEEDED, "the work is waiting on an answer, not failing") + if advancing_evidence: + return _wait(PROGRESS, "observations advanced since the last look") + if not observable: + return _wait( + UNOBSERVABLE, + "available observations establish neither progress nor failure; the gap is the " + "finding", + ) + if stagnation_confirmed: + return _wait( + CONFIRMED_FAILURE, "no advancement at the stated review point, compared against " + "the prior observation", + ) + if timed_out: + return _wait( + TIMED_OUT, + "the wait ended on its own bound. That is a normal outcome: it is neither a " + "failure nor permission to start the work again", + ) + return _wait(SUSPECTED_STAGNATION, "comparable observations show no advancement yet") + + +def _wait(state: str, reason: str) -> dict: + return { + "state": state, + "reason": reason, + "authorisesRerun": False, + "isFailure": state == CONFIRMED_FAILURE, + } + + +# ------------------------------------------------------------------ packet vocabulary + +# DISPATCH-TASK-01. An instruction that omits one of these is an instruction the recipient +# has to guess at, which is the whole failure this shape prevents. +DISPATCH_FIELDS = ("TASK", "SCOPE", "MUST DO", "MUST NOT", "PROOF", "RETURN FORMAT") +DECISION_BOUNDARY = "DECISION BOUNDARY" + +# Which owner to re-read when a message resumes work, rather than reloading everything. +SKILL_POINTERS = { + "loop": "codexclaw:cxc-loop with codexclaw:cxc-pabcd", + "development": "codexclaw:cxc-dev", + "pull-request": "codexclaw:cxc-dev references/stacked-prs.md", + "review-repair": "REVIEW-SYNTHESIS-01, cxc-pabcd references/loop-engineering.md 11.3", + "lost-context": "codexclaw:cxc-recall", +} + + +def skill_pointer(activity: str) -> str: + if activity not in SKILL_POINTERS: + raise KeyError( + f"{activity!r} has no recorded owner; name the owning skill rather than sending " + "a recipient to reload everything" + ) + return SKILL_POINTERS[activity] diff --git a/packages/codex-session-relay/src/codex_session_relay/delivery.py b/packages/codex-session-relay/src/codex_session_relay/delivery.py index 9961764..d4ce1a0 100644 --- a/packages/codex-session-relay/src/codex_session_relay/delivery.py +++ b/packages/codex-session-relay/src/codex_session_relay/delivery.py @@ -28,6 +28,7 @@ classify_operation_receipt, ) from .policy import PUSH_CHANNEL_CLOSED, SUPERSEDED as SUPERSEDED_HOLD +from .report import read as read_work_report, render_completion, render_revision COMPLETION = "completion_event" REVISION = "revision_request" @@ -104,7 +105,7 @@ def enqueue_in(self, db, event_id, *, relationship_id, kind, recipient_task_id) ) self.store.journal("delivery_queued", event_id, {"kind": kind}, at=now) - def _render_for(self, row, record, request) -> str: + def _render_for(self, row, record, request, report=None) -> str: """Deterministic, directional, and carrying no turn id belonging to the recipient. The two directions are answered differently and must therefore be INSTRUCTED @@ -120,10 +121,13 @@ def _render_for(self, row, record, request) -> str: Pure by construction: every input is passed in, so this cannot read a counter that has moved since the attempt it is rendering for. + + The work report is passed in for the same reason. Whoever owns the transaction reads + it once and hands it over, so this stays a function of its arguments. """ if row["kind"] == REVISION: - return self._render_revision(row, record, request) - return self._render_completion(row, record, request) + return self._render_revision(row, record, request, report) + return self._render_completion(row, record, request, report) def preview_message(self, event_id: str) -> str: """What the NEXT attempt would say. Never evidence of what any attempt DID say. @@ -136,13 +140,18 @@ def preview_message(self, event_id: str) -> str: row = self.get(event_id) record = self.intake.get(event_id) or {} request = derive_request_id(event_id, row["attempt_count"] + 1) - return self._render_for(row, record, request) + return self._render_for(row, record, request, read_work_report(self.store, event_id)) def render_message(self, event_id: str) -> str: """Kept as the preview alias so no caller can mean 'what was sent' by accident.""" return self.preview_message(event_id) - def _render_completion(self, row, record, request) -> str: + def _render_completion(self, row, record, request, report=None) -> str: + # A report centred on a pull request needs a pull request. An event recorded before + # this contract has none, so it renders what it has always rendered rather than being + # dressed in a shape its own data cannot fill. + if report is not None: + return render_completion(row, record, request, report) lines = [ "[codex-session-relay] verification request", f"requestId: {request}", @@ -191,7 +200,9 @@ def _render_completion(self, row, record, request) -> str: ] return NEWLINE.join(lines) - def _render_revision(self, row, record, request) -> str: + def _render_revision(self, row, record, request, report=None) -> str: + if report is not None: + return render_revision(row, record, request, report) lines = [ "[codex-session-relay] revision request", f"requestId: {request}", @@ -290,7 +301,9 @@ def _claim(self, event_id: str, *, now: float, owner: str, recipient: str): f"request id {request_id!r} already belongs to event {clash['event_id']!r}", ) record = self.intake.get(event_id) or {} - message = self._render_for(row, record, request_id) + message = self._render_for( + row, record, request_id, read_work_report(self.store, event_id) + ) db.execute( "INSERT INTO attempts (request_id, event_id, attempt_no, kind, internal_state," " state, sent_at, observed_at) VALUES (?,?,?,?,?,?,?,?)", diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py new file mode 100644 index 0000000..c13cce3 --- /dev/null +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -0,0 +1,572 @@ +"""What the recipient actually needs, put where they will read it first. + +The delivered message used to open with an event id, a relationship id, a generation and a +revision hash, and then list file paths and their digests. All of that is true and none of +it tells a parent which pull request to open, what was checked, what is still unresolved, +or what to do next. A recipient had to go and reconstruct the work before it could act. + +A work report is the part contract v1 has no room for. The five schemas are frozen with +additionalProperties false, so a pull request cannot be added to a completion receipt; this +is a relay-owned record, like the criteria set and the revision request, bound to the exact +event, relationship, generation and revision it describes. A repository slug is stored +beside the pull request number and the two are never rendered apart, so the same number on +two projects stays two different pull requests. + +An event with no work report renders exactly what it rendered before. That is not a +fallback bolted on for safety: a receipt from before this contract has no pull request to +centre a report on, and inventing one would be the same guess this package refuses +everywhere else. +""" + +import json + +from . import cxc +from .errors import DeliveryRefused, ReceiptRefused, RefusalReason + +NEWLINE = chr(10) +VERSION = "relay-report/1" +LEGACY = "relay-message/legacy" + +# Generous enough that an ordinary report is never touched, small enough that a runaway one +# is elided here, on purpose and in the open, rather than cut by whatever reads it later. +BUDGET = 6000 + + +def show_command(event_id: str) -> str: + return f"codex-session-relay show --event {event_id}" + + +# ------------------------------------------------------------------------ recording + +def record(store, clock, *, event_id, relationship_id, execution_generation, revision_hash, + outcome, repository, cxc_status, cxc_reason, summary, next_action, + pr_number=None, pr_url=None, pr_state=None, base_ref=None, base_sha=None, + head_sha=None, criteria_digest=None, evidence=None, unresolved=None, review=None, + restore=None, submission_no=1) -> dict: + """Store one report, validated, bound to the revision it describes. + + The CXC status is checked against the outcome the child's receipt already asserted. It + is not allowed to choose that outcome: the receipt is the only thing the frozen contract + lets assert one, and a status that picked it could quietly overrule the evidence. + """ + cxc.check_status(cxc_status, outcome) + repository = _required(repository, "repository") + summary = _required(summary, "summary") + next_action = _required(next_action, "next_action") + reason = _required(cxc_reason, "cxc_reason") + if pr_number is not None: + if isinstance(pr_number, bool) or not isinstance(pr_number, int) or pr_number < 1: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a pull request number is a positive integer, not {pr_number!r}", + ) + if not head_sha: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + "a report naming a pull request names the head commit it is about; without " + "one, a later push silently inherits this report", + ) + review = _check_review(review) + row = { + "eventId": event_id, + "relationshipId": relationship_id, + "executionGeneration": int(execution_generation), + "revisionHash": revision_hash, + "submissionNo": int(submission_no), + "repository": repository, + "prNumber": pr_number, + "prUrl": pr_url, + "prState": pr_state, + "baseRef": base_ref, + "baseSha": base_sha, + "headSha": head_sha, + "criteriaDigest": criteria_digest, + "cxcStatus": cxc_status, + "cxcReason": reason, + "contractVersion": cxc.VERSION, + "summary": summary, + "evidence": list(evidence or []), + "unresolved": list(unresolved or []), + "nextAction": next_action, + "review": review, + "restore": dict(restore or {}), + } + now = clock.iso() + with store.transaction() as db: + db.execute( + "INSERT INTO work_reports (event_id, relationship_id, execution_generation," + " revision_hash, submission_no, repository, pr_number, pr_url, pr_state, base_ref," + " base_sha, head_sha, criteria_digest, cxc_status, cxc_reason, contract_version," + " summary, evidence, unresolved, next_action, review, restore, recorded_at)" + " VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)" + " ON CONFLICT(event_id) DO UPDATE SET" + " submission_no = excluded.submission_no, repository = excluded.repository," + " pr_number = excluded.pr_number, pr_url = excluded.pr_url," + " pr_state = excluded.pr_state, base_ref = excluded.base_ref," + " base_sha = excluded.base_sha, head_sha = excluded.head_sha," + " criteria_digest = excluded.criteria_digest, cxc_status = excluded.cxc_status," + " cxc_reason = excluded.cxc_reason, contract_version = excluded.contract_version," + " summary = excluded.summary, evidence = excluded.evidence," + " unresolved = excluded.unresolved, next_action = excluded.next_action," + " review = excluded.review, restore = excluded.restore," + " recorded_at = excluded.recorded_at", + ( + event_id, relationship_id, row["executionGeneration"], revision_hash, + row["submissionNo"], repository, pr_number, pr_url, pr_state, base_ref, + base_sha, head_sha, criteria_digest, cxc_status, reason, cxc.VERSION, summary, + json.dumps(row["evidence"]), json.dumps(row["unresolved"]), next_action, + json.dumps(review) if review else None, json.dumps(row["restore"]), now, + ), + ) + store.journal( + "work_report_recorded", event_id, + {"repository": repository, "prNumber": pr_number, "cxcStatus": cxc_status, + "headSha": head_sha, "submissionNo": row["submissionNo"]}, + at=now, + ) + row["recordedAt"] = now + return row + + +def read(store, event_id: str): + row = store.one("SELECT * FROM work_reports WHERE event_id = ?", (event_id,)) + if row is None: + return None + return { + "eventId": row["event_id"], + "relationshipId": row["relationship_id"], + "executionGeneration": row["execution_generation"], + "revisionHash": row["revision_hash"], + "submissionNo": row["submission_no"], + "repository": row["repository"], + "prNumber": row["pr_number"], + "prUrl": row["pr_url"], + "prState": row["pr_state"], + "baseRef": row["base_ref"], + "baseSha": row["base_sha"], + "headSha": row["head_sha"], + "criteriaDigest": row["criteria_digest"], + "cxcStatus": row["cxc_status"], + "cxcReason": row["cxc_reason"], + "contractVersion": row["contract_version"], + "summary": row["summary"], + "evidence": json.loads(row["evidence"]) if row["evidence"] else [], + "unresolved": json.loads(row["unresolved"]) if row["unresolved"] else [], + "nextAction": row["next_action"], + "review": json.loads(row["review"]) if row["review"] else None, + "restore": json.loads(row["restore"]) if row["restore"] else {}, + "recordedAt": row["recorded_at"], + } + + +def version_of(report) -> str: + """Which message contract an event is on. Absence is an answer, not a missing value.""" + return LEGACY if report is None else VERSION + + +def _required(value, field): + text = str(value or "").strip() + if not text: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a work report states its {field}; a report the recipient cannot act on is the " + "thing this record exists to replace", + ) + return text + + +def _check_review(review): + if review is None: + return None + kind = review.get("kind") + blockers = review.get("blockers") + # Raises on an unknown kind, on GO-WITH-FIXES without a count, and on a count attached + # to PASS or FAIL. Rendering the line here is what makes those refusals reachable. + cxc.verdict_line(kind, blockers) + findings = [] + for item in review.get("findings") or []: + identifier = str(item.get("id") or "").strip() + if not identifier: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, "each review finding names a criterion id" + ) + findings.append({ + "id": identifier, + "verdict": item.get("verdict"), + "note": str(item.get("note") or "").strip(), + "anchor": str(item.get("anchor") or "").strip(), + }) + return {"kind": kind, "blockers": blockers, "findings": findings} + + +# -------------------------------------------------------------------- identity guards + +def pr_ref(report) -> str: + """Always repository-qualified. A bare number belongs to whoever reads it first.""" + if report is None or not report.get("prNumber"): + return "" + return f"{report['repository']}#{report['prNumber']}" + + +def pr_key(report): + """The identity two reports are compared on. The number alone is not one.""" + if report is None or not report.get("prNumber"): + return None + return (report["relationshipId"], report["repository"], report["prNumber"]) + + +def assert_current(report, *, execution_generation, head_sha=None) -> None: + """A report describes one revision of one head. It cannot answer for a later one. + + Without this, the cheapest way to pass a review is to push again and let the previous + report stand for the new bytes. + """ + if report is None: + return + if report["executionGeneration"] != execution_generation: + raise DeliveryRefused( + RefusalReason.STALE_GENERATION, + f"this report is generation {report['executionGeneration']} and the assignment " + f"is on generation {execution_generation}; it cannot answer for the current one", + ) + if head_sha and report.get("headSha") and report["headSha"] != head_sha: + raise DeliveryRefused( + RefusalReason.STALE_MARK_CONTEXT, + f"this report is about head {report['headSha']} and the current head is " + f"{head_sha}; re-report against the head under review", + ) + + +# ------------------------------------------------------------------------ composing + +class _Section: + """One block of the message, with how badly it is needed. + + rank orders removal, lowest number last to go. essential blocks are never dropped + whole; when they have to shrink they keep their heading and say how much is missing. + """ + + def __init__(self, name, lines, rank, *, essential=False, keep=0): + self.name = name + self.lines = [line for line in lines if line is not None] + self.rank = rank + self.essential = essential + self.keep = keep + + +def _compose(sections, event_id, *, budget) -> str: + """Fit the message, and say out loud whatever did not fit. + + Silence is the failure mode being designed against. A message that quietly loses its + unresolved items reads exactly like a message that had none, and the recipient acts on + the wrong one. Every removal here leaves a mark and a command that shows the whole + record. + """ + blocks = [list(section.lines) for section in sections] + removed = [] + + def rendered(extra_note=True): + body = [line for block in blocks for line in block] + if removed and extra_note: + body += ["", _omission_line(removed, event_id)] + return NEWLINE.join(body) + + order = sorted(range(len(sections)), key=lambda i: -sections[i].rank) + for index in order: + if len(rendered()) <= budget: + break + section = sections[index] + if section.essential or not blocks[index]: + continue + removed.append(section.name) + blocks[index] = [] + + for index in order: + section = sections[index] + if not section.essential or len(section.lines) <= section.keep: + continue + # kept shrinks by one every pass and the marker is rebuilt from it rather than + # appended to it. Popping a line and then appending a marker leaves the block the + # same length, which is a loop that never ends. + kept = list(section.lines) + while len(rendered()) > budget and len(kept) > section.keep: + kept.pop() + dropped = len(section.lines) - len(kept) + blocks[index] = kept + [f" ... {dropped} more, see the full record"] + if section.name not in removed: + removed.append(section.name) + + out = rendered() + if len(out) > budget: + raise ValueError( + f"a message budget of {budget} cannot hold this report even reduced to its " + "required parts; raise the budget rather than shipping a message that lost them" + ) + return out + + +def _omission_line(removed, event_id) -> str: + return f"omitted: {', '.join(removed)} - read in full with {show_command(event_id)}" + + +# ------------------------------------------------------------------------ rendering + +def render_completion(row, receipt, request, report, *, budget=BUDGET) -> str: + """Child to parent, led by what the parent has to decide. + + Result, then the pull request, then the evidence, then what is still open, then what to + do. The identifiers keep their place at the bottom: they are how the parent answers, not + how it decides. + """ + event_id = row["event_id"] + sections = [ + _Section("header", [ + "[codex-session-relay] verification request", + f"result: {report['summary']}", + f"cxc: {report['cxcStatus']} - {report['cxcReason']}", + f" meaning: {cxc.MEANING[report['cxcStatus']]}", + " this is the child reporting on its own work. It is not a verification:" + f" {cxc.refuse_promotion('cxc_done')}", + ], rank=0, essential=True, keep=5), + # keep counts from the top of the block, and these blocks open with a blank line, so + # a floor of two is what keeps the heading attached to whatever survives under it. + _Section("pull request", _pr_lines(report), rank=1, essential=True, keep=2), + _Section("verification", _evidence_lines(report), rank=4), + _Section("unresolved", _unresolved_lines(report), rank=2, essential=True, keep=2), + _Section("next", [f"next: {report['nextAction']}"], rank=0, essential=True, keep=1), + _Section("workflow restore", _restore_lines(report), rank=3), + _Section("deliverables", _manifest_lines(receipt, event_id), rank=6), + _Section("relay record", [ + "", + "relay record:", + f" contract: {version_of(report)}", + f" requestId: {request}", + f" eventId: {event_id}", + f" relationshipId: {row['relationship_id']}", + f" executionGeneration: {receipt.get('executionGeneration')}", + f" attempt: {receipt.get('attempt')} submission: {report['submissionNo']}", + f" outcome: {receipt.get('outcome')}", + f" revisionHash: {receipt.get('revisionHash')}", + ], rank=5), + _Section("respond", _ack_lines(event_id), rank=0, essential=True, keep=7), + ] + return _compose(sections, event_id, budget=budget) + + +def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: + """Parent to child, shaped as an instruction the child can execute. + + DISPATCH-TASK-01 fixes the fields. What leads is the thing that was violated and the + anchor that reproduces it, because a correction whose first line is an identifier is a + correction the child has to go and research before it can start. + """ + event_id = row["event_id"] + generation = receipt.get("executionGeneration") + review = report.get("review") + head = [ + "[codex-session-relay] revision request", + ] + if review: + cxc.assert_reviewed(True) + head.append(cxc.verdict_line(review["kind"], review.get("blockers"))) + head.append(f"TASK: {report['summary']}") + + sections = [ + _Section("header", head, rank=0, essential=True, keep=2), + _Section("violated criteria", _finding_lines(review), rank=0, essential=True, keep=2), + _Section("SCOPE", _scope_lines(report, generation), rank=1, essential=True, keep=2), + _Section("MUST DO", [ + "MUST DO:", + f" {report['nextAction']}", + " answer every finding above with a fix, a reasoned rebuttal, or an explicit" + " out-of-scope split, and reply on its thread", + ], rank=0, essential=True, keep=2), + _Section("MUST NOT", [ + "MUST NOT:", + " discard work outside the findings above, rewrite another task history, or" + " force-push a shared branch", + " treat this request as an acknowledgeable message; see the note below", + ], rank=1, essential=True, keep=2), + _Section("PROOF", _proof_lines(report), rank=2, essential=True, keep=2), + _Section("RETURN FORMAT", [ + "RETURN FORMAT:", + " result summary, repository and pull request, base and head SHA, verification" + " evidence, unresolved items, next action, and the CXC report status", + ], rank=2, essential=True, keep=2), + _Section("DECISION BOUNDARY", [ + "DECISION BOUNDARY:", + " fix what the findings name. Anything wider, anything that would discard" + " preserved work, and anything needing authority you were not given comes back" + " here instead of being decided locally", + ], rank=1, essential=True, keep=2), + _Section("workflow restore", _restore_lines(report), rank=3), + _Section("answer", [ + "", + "There is nothing to acknowledge. Contract v1 defines no acknowledgement for this", + "direction and the relay refuses one by kind, so there is no proof to compute and", + "no acknowledgement to send.", + "Answer with your next completion receipt under the new generation:", + f" emit --relationship {row['relationship_id']}" + f" --generation {generation} --attempt ", + " --outcome ready_for_review --turn-thread " + " --turn-id ", + " --artifact [--continues-anchor ]", + "", + f"relay record: requestId {request}, eventId {event_id}," + f" contract {version_of(report)}", + f"Full record: {show_command(event_id)}", + ], rank=0, essential=True, keep=9), + ] + return _compose(sections, event_id, budget=budget) + + +def _pr_lines(report): + reference = pr_ref(report) + if not reference: + return [ + "", + f"repository: {report['repository']}", + "pull request: none recorded for this event", + ] + lines = [ + "", + f"pull request: {reference}" + + (f" ({report['prState']})" if report.get("prState") else ""), + ] + if report.get("prUrl"): + lines.append(f" url: {report['prUrl']}") + base = report.get("baseRef") or "" + if report.get("baseSha"): + lines.append(f" base: {base} {report['baseSha']}".rstrip()) + lines.append(f" head: {report.get('headSha')}") + if report.get("criteriaDigest"): + lines.append(f" criteria: {report['criteriaDigest']}") + return lines + + +def _evidence_lines(report): + entries = report.get("evidence") or [] + if not entries: + return ["", "verification: none recorded"] + lines = ["", "verification:"] + for item in entries: + if isinstance(item, str): + lines.append(f" {item}") + continue + check = item.get("check", "") + code = item.get("exitCode") + detail = item.get("detail") + rendered = f" {check}" + if code is not None: + rendered += f" -> exit {code}" + if detail: + rendered += f" {detail}" + lines.append(rendered) + return lines + + +def _unresolved_lines(report): + entries = report.get("unresolved") or [] + if not entries: + return ["", "unresolved: none"] + lines = ["", "unresolved:"] + for item in entries: + if isinstance(item, str): + lines.append(f" - {item}") + else: + note = item.get("note") or "" + lines.append(f" - {item.get('id')}: {note}".rstrip(": ")) + return lines + + +def _finding_lines(review): + if not review or not review.get("findings"): + return ["", "violated criteria: no per-criterion findings were recorded"] + lines = ["", "violated criteria:"] + for item in review["findings"]: + rendered = f" {item['id']}: {item.get('verdict')}" + if item.get("note"): + rendered += f" - {item['note']}" + lines.append(rendered) + if item.get("anchor"): + lines.append(f" anchor: {item['anchor']}") + return lines + + +def _scope_lines(report, generation): + lines = ["", "SCOPE:"] + reference = pr_ref(report) + lines.append(f" {reference}" if reference else f" {report['repository']}") + if report.get("baseSha"): + lines.append(f" base {report.get('baseRef') or ''} {report['baseSha']}".rstrip()) + if report.get("headSha"): + lines.append(f" head {report['headSha']}") + if report.get("criteriaDigest"): + lines.append(f" criteria {report['criteriaDigest']}") + lines.append(f" execution generation {generation} (new)") + lines.append(" preserve: everything outside the findings above, including work this") + lines.append(" request does not mention and any other task in-flight beside it") + return lines + + +def _proof_lines(report): + lines = ["", "PROOF:"] + entries = report.get("evidence") or [] + if entries: + lines.append(" re-run what this review ran, and report command, exit code and result:") + for item in entries: + if isinstance(item, str): + lines.append(f" {item}") + else: + lines.append(f" {item.get('check', '')}") + else: + lines.append(" state the command, its exit code and what it showed, for each finding") + lines.append(" a passing string match is not a passing behaviour") + return lines + + +def _restore_lines(report): + restore = report.get("restore") or {} + if not restore: + return [] + lines = ["", "workflow restore:"] + for key, label in ( + ("mode", "mode"), ("scope", "scope"), ("phase", "phase"), + ("phaseObservedAt", "phase observed"), ("plan", "plan"), ("evidence", "evidence"), + ("remaining", "remaining"), + ): + if restore.get(key): + lines.append(f" {label}: {restore[key]}") + for activity in restore.get("skills") or []: + lines.append(f" read: {cxc.skill_pointer(activity)}") + return lines + + +def _manifest_lines(receipt, event_id): + manifest = receipt.get("manifest") + if not manifest: + return ["", "deliverables: none (execution-only outcome)"] + lines = ["", f"deliverables: {len(manifest)}"] + for entry in manifest: + size = entry.get("bytes") + lines.append( + f" {entry['path']} sha256={entry['sha256']}" + + (f" bytes={size}" if size is not None else "") + ) + return lines + + +def _ack_lines(event_id): + return [ + "", + "To respond, from inside your own turn:", + f" claim --event {event_id} --turn ", + f" ack-proof --event {event_id} --turn ", + f" ack --event {event_id} --ack-turn --ack-proof ", + f" verdict --event {event_id} --verdict --verdict-turn ", + "", + "The proof is sha256(eventId|). This message does not and cannot", + "contain that turn id, which is what distinguishes acknowledging from echoing.", + f"Full record: {show_command(event_id)}", + ] diff --git a/packages/codex-session-relay/src/codex_session_relay/store.py b/packages/codex-session-relay/src/codex_session_relay/store.py index 6cc0996..00ed11d 100644 --- a/packages/codex-session-relay/src/codex_session_relay/store.py +++ b/packages/codex-session-relay/src/codex_session_relay/store.py @@ -240,6 +240,38 @@ recorded_at TEXT NOT NULL ); +-- What a delivered message leads with: which pull request this is about, which commit it is +-- about, what was checked, what is still open and what to do next. A table rather than fields +-- on the receipt, because completion-receipt.json is frozen with additionalProperties false. +-- Bound to the event, generation and revision it describes, so a later push cannot inherit an +-- earlier report, and the repository is stored beside the number so the same pull request +-- number on two projects stays two pull requests. +CREATE TABLE IF NOT EXISTS work_reports ( + event_id TEXT PRIMARY KEY, + relationship_id TEXT NOT NULL, + execution_generation INTEGER NOT NULL, + revision_hash TEXT NOT NULL, + submission_no INTEGER NOT NULL DEFAULT 1, + repository TEXT NOT NULL, + pr_number INTEGER, + pr_url TEXT, + pr_state TEXT, + base_ref TEXT, + base_sha TEXT, + head_sha TEXT, + criteria_digest TEXT, + cxc_status TEXT NOT NULL, + cxc_reason TEXT NOT NULL, + contract_version TEXT NOT NULL, + summary TEXT NOT NULL, + evidence TEXT, + unresolved TEXT, + next_action TEXT NOT NULL, + review TEXT, + restore TEXT, + recorded_at TEXT NOT NULL +); + -- How an acknowledgement's own turn was established. host_read is an App Server read of the -- recipient's real turn list; unverified is recorded intent still awaiting that read. There is -- deliberately no tier derived from what the relay itself sent: a stored dispatch proves a send diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py new file mode 100644 index 0000000..d06bb87 --- /dev/null +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -0,0 +1,441 @@ +"""JUN-131: can the recipient act on the message, and do the two vocabularies still agree. + +Every case here is written from the recipient's side. The question is never whether a +string appears, but whether somebody holding only this message could do the next thing. +""" + +import unittest + +from codex_session_relay import cxc, report +from codex_session_relay.errors import RefusalReason +from codex_session_relay.models import Endpoint + +from .support import CHILD, HOST, PARENT, DeliveryTestCase + + +def a_report(**overrides): + base = { + "repository": "thisisjun786/codex-relay-workflow", + "pr_number": 12, + "pr_url": "https://github.com/thisisjun786/codex-relay-workflow/pull/12", + "pr_state": "ready", + "base_ref": "dev", + "base_sha": "c56576d5be412b5bc352dd93b9eb37ab279a12f6", + "head_sha": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "criteria_digest": "d1e2f3", + "cxc_status": cxc.DONE, + "cxc_reason": "every recorded criterion has fresh proof on this head", + "summary": "delivery messages now lead with the pull request", + "next_action": "review the diff and record a verdict", + "evidence": [{"check": "python3 -m pytest", "exitCode": 0, "detail": "214 passed"}], + "unresolved": ["the CLI verb lands after PR 8 merges"], + } + base.update(overrides) + return base + + +class Recording(DeliveryTestCase): + def recorded(self, **overrides): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + fields = a_report(**overrides) + stored = report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **fields + ) + return relationship, event_id, stored + + def test_the_message_leads_with_what_the_parent_has_to_decide(self): + _relationship, event_id, _stored = self.recorded() + message = self.delivery.render_message(event_id) + # The result, the pull request and the next action all arrive before the identifiers + # that used to open the message. + self.assertLess(message.index("result:"), message.index("eventId:")) + self.assertLess( + message.index("pull request:"), message.index("relationshipId:"), + "a recipient reads the pull request before it reads a relationship id", + ) + self.assertLess(message.index("next:"), message.index("revisionHash:")) + self.assertIn("thisisjun786/codex-relay-workflow#12", message) + self.assertIn("c56576d5be412b5bc352dd93b9eb37ab279a12f6", message) + self.assertIn("a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", message) + self.assertIn("python3 -m pytest -> exit 0", message) + self.assertIn("the CLI verb lands after PR 8 merges", message) + self.assertIn("review the diff and record a verdict", message) + # And it still says how to answer, which is the parent's required action. + self.assertIn("ack-proof", message) + + def test_a_done_report_is_not_allowed_to_read_as_a_verification(self): + _relationship, event_id, _stored = self.recorded() + message = self.delivery.render_message(event_id) + self.assertIn("DONE", message) + self.assertIn("It is not a verification", message) + # The relay's own verified disposition is untouched by anything in the report. + self.assertIsNone( + self.store.one("SELECT 1 FROM verdicts WHERE event_id = ?", (event_id,)) + ) + for fact in ("cxc_done", "pull_request_opened", "review_pass", "required_checks_green"): + self.assertTrue(cxc.refuse_promotion(fact)) + + def test_an_unknown_cxc_status_is_diagnosed_rather_than_defaulted(self): + error = self.assertRefused( + RefusalReason.OUTCOME_INCONSISTENT, + lambda: cxc.check_status("SHIPPED", "ready_for_review"), + ) + self.assertIn("SHIPPED", error.detail) + self.assertIn(cxc.VERSION, error.detail, "it names the contract it was read against") + self.assertIn("DONE", error.detail, "and the set it does accept") + + def test_a_status_cannot_contradict_the_outcome_the_receipt_asserted(self): + error = self.assertRefused( + RefusalReason.OUTCOME_INCONSISTENT, + lambda: cxc.check_status(cxc.BLOCKED, "ready_for_review"), + ) + self.assertIn("BLOCKED", error.detail) + self.assertIn("blocked_needs_input", error.detail) + + def test_the_three_human_decision_statuses_collapse_without_losing_which_one_it_was(self): + human = (cxc.BLOCKED, cxc.UNSAFE, cxc.NEEDS_HUMAN) + for status in human: + self.assertEqual(cxc.COMPATIBLE_OUTCOMES[status], ("blocked_needs_input",)) + self.assertEqual( + len({cxc.MEANING[status] for status in human}), 3, + "they share one outcome, so the words have to stay distinguishable", + ) + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + # The stored report keeps the original word and its reason, which is what makes the + # collapse lossless for a reader. + stored = report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], + **a_report(cxc_status=cxc.DONE) + ) + self.assertEqual(report.read(self.store, event_id)["cxcReason"], stored["cxcReason"]) + + def test_a_report_about_an_older_head_cannot_answer_for_the_current_one(self): + _relationship, event_id, _stored = self.recorded() + stored = report.read(self.store, event_id) + error = self.assertRefused( + RefusalReason.STALE_MARK_CONTEXT, + lambda: report.assert_current( + stored, execution_generation=stored["executionGeneration"], + head_sha="9999999999999999999999999999999999999999", + ), + ) + self.assertIn("re-report against the head under review", error.detail) + + def test_a_report_from_an_earlier_generation_cannot_answer_for_the_current_one(self): + _relationship, event_id, _stored = self.recorded() + stored = report.read(self.store, event_id) + self.assertRefused( + RefusalReason.STALE_GENERATION, + lambda: report.assert_current(stored, execution_generation=99), + ) + + def test_a_pull_request_number_never_travels_without_its_repository(self): + _relationship, event_id, _stored = self.recorded() + mine = report.read(self.store, event_id) + other = dict(mine) + other["repository"] = "someone-else/other-project" + other["relationshipId"] = "rel-different" + self.assertEqual(report.pr_ref(mine), "thisisjun786/codex-relay-workflow#12") + self.assertEqual(report.pr_ref(other), "someone-else/other-project#12") + self.assertNotEqual( + report.pr_key(mine), report.pr_key(other), + "the same number on two projects is two pull requests", + ) + self.assertIn(report.pr_ref(mine), self.delivery.render_message(event_id)) + + def test_a_report_naming_a_pull_request_names_the_commit_it_is_about(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], + **a_report(head_sha=None) + ), + ) + self.assertIn("a later push silently inherits this report", error.detail) + + def test_a_report_with_nothing_to_act_on_is_refused(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + for field in ("summary", "next_action", "repository"): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field: report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], + **a_report(**{field: " "}) + ), + ) + + +class Elision(DeliveryTestCase): + def test_more_than_ten_findings_are_never_dropped_in_silence(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + unresolved = [f"finding {n}: something specific that still needs doing" for n in range(24)] + report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], + **a_report(unresolved=unresolved) + ) + message = self.delivery.render_message(event_id) + self.assertIn("finding 23", message, "a generous budget keeps all of them") + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + tight = report.render_completion( + row, receipt, "del-x-a1", stored, budget=1400, + ) + self.assertIn("omitted:", tight, "a shortened message says so") + self.assertIn("show --event", tight, "and says where to read the rest") + self.assertIn("unresolved:", tight, "the heading survives") + self.assertIn("... ", tight, "and the count of what is missing survives with it") + self.assertIn("next:", tight, "the required next action is never what gets dropped") + self.assertIn("ack-proof", tight, "nor the instruction for answering") + + def test_an_impossible_budget_refuses_instead_of_shipping_a_gutted_message(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + ) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + with self.assertRaises(ValueError) as caught: + report.render_completion(row, receipt, "del-x-a1", stored, budget=120) + self.assertIn("rather than shipping a message that lost them", str(caught.exception)) + + def test_the_preview_is_still_not_evidence_of_what_was_sent(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + ) + before = self.delivery.preview_message(event_id) + record = self.attempt(event_id) + sent = self.delivery.sent_message(record["requestId"]) + self.assertIn(record["requestId"], sent) + # Before the first attempt the preview predicts attempt 1 correctly, so the two agree. + self.assertEqual(before, sent) + # Afterwards it describes the attempt that has NOT run, which is the whole reason a + # preview is never quoted as evidence of what was delivered. + after = self.delivery.preview_message(event_id) + self.assertNotEqual(after, sent) + self.assertNotIn(record["requestId"], after) + self.assertEqual(sent, self.delivery.sent_message(record["requestId"])) + + +class Legacy(DeliveryTestCase): + def test_an_event_with_no_report_renders_exactly_what_it_always_did(self): + _relationship, event_id = self.queued_event() + message = self.delivery.render_message(event_id) + receipt = self.intake.get(event_id) + self.assertIn("verification request", message) + self.assertIn(receipt["manifest"][0]["sha256"], message) + self.assertNotIn("pull request:", message) + self.assertNotIn("relay-report/1", message) + self.assertIsNone(report.read(self.store, event_id)) + self.assertEqual(report.version_of(None), report.LEGACY) + + def test_the_two_message_versions_are_told_apart_explicitly(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + self.assertEqual(report.version_of(report.read(self.store, event_id)), report.LEGACY) + report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + ) + stored = report.read(self.store, event_id) + self.assertEqual(report.version_of(stored), report.VERSION) + self.assertIn("contract: relay-report/1", self.delivery.render_message(event_id)) + + +class Verdicts(unittest.TestCase): + def test_the_verdict_line_is_the_one_the_reviewer_contract_fixes(self): + self.assertEqual(cxc.verdict_line(cxc.PASS), "VERDICT: PASS") + self.assertEqual(cxc.verdict_line(cxc.FAIL), "VERDICT: FAIL") + self.assertEqual( + cxc.verdict_line(cxc.GO_WITH_FIXES, 2), "VERDICT: GO-WITH-FIXES (blockers=2)" + ) + + def test_a_go_with_fixes_without_a_blocker_count_is_refused(self): + for bad in (None, 0, -1, True): + with self.assertRaises(ValueError): + cxc.verdict_line(cxc.GO_WITH_FIXES, bad) + + def test_a_blocker_count_cannot_be_attached_to_pass_or_fail(self): + for kind in (cxc.PASS, cxc.FAIL): + with self.assertRaises(ValueError): + cxc.verdict_line(kind, 3) + + def test_prose_that_resembles_a_verdict_does_not_parse_as_one(self): + self.assertIsNone(cxc.parse_verdict_line("we think this is a PASS")) + self.assertIsNone(cxc.parse_verdict_line("VERDICT: LOOKS FINE")) + self.assertIsNone(cxc.parse_verdict_line("VERDICT: GO-WITH-FIXES")) + self.assertEqual( + cxc.parse_verdict_line("VERDICT: GO-WITH-FIXES (blockers=4)"), + {"kind": cxc.GO_WITH_FIXES, "blockers": 4}, + ) + + def test_a_progress_notice_may_not_wear_a_verdict(self): + with self.assertRaises(ValueError): + cxc.assert_reviewed(False) + + +class Waiting(unittest.TestCase): + def test_a_bare_timeout_is_neither_a_failure_nor_permission_to_run_it_again(self): + result = cxc.classify_wait(timed_out=True) + self.assertEqual(result["state"], cxc.TIMED_OUT) + self.assertFalse(result["isFailure"]) + self.assertFalse(result["authorisesRerun"]) + + def test_the_five_endings_stay_apart(self): + self.assertEqual( + cxc.classify_wait(terminal_error="exit 1")["state"], cxc.CONFIRMED_FAILURE + ) + self.assertEqual(cxc.classify_wait(input_requested=True)["state"], cxc.INPUT_NEEDED) + self.assertEqual( + cxc.classify_wait(advancing_evidence=True, timed_out=True)["state"], cxc.PROGRESS, + "fresh evidence outranks the clock", + ) + self.assertEqual( + cxc.classify_wait(observable=False, timed_out=True)["state"], cxc.UNOBSERVABLE + ) + self.assertEqual( + cxc.classify_wait(stagnation_confirmed=True)["state"], cxc.CONFIRMED_FAILURE + ) + self.assertEqual(cxc.classify_wait()["state"], cxc.SUSPECTED_STAGNATION) + + def test_nothing_this_module_returns_authorises_a_rerun(self): + for kwargs in ({"timed_out": True}, {"observable": False}, {"input_requested": True}, + {"terminal_error": "boom"}, {"advancing_evidence": True}, {}): + self.assertFalse(cxc.classify_wait(**kwargs)["authorisesRerun"]) + + +class Provenance(unittest.TestCase): + def test_the_mapping_points_at_the_install_rather_than_copying_it(self): + record = cxc.provenance() + self.assertEqual(record["version"], "0.2.28+codex.20260914090142") + rules = {source["rule"] for source in record["sources"]} + for expected in ("DISPATCH-TASK-01", "REVIEW-OUTPUT-01", "LOOP-WAIT-EVIDENCE-01", + "ATTEST-EVIDENCE-01", "REVIEW-SYNTHESIS-01"): + self.assertIn(expected, rules) + for source in record["sources"]: + self.assertEqual(len(source["sha256"]), 64) + self.assertTrue(source["path"].startswith("skills/")) + + def test_a_changed_install_re_verifies_only_what_it_touched(self): + touched = cxc.affected_by(["skills/loop/references/waiting.md"]) + self.assertEqual(touched, ("LOOP-WAIT-EVIDENCE-01",)) + self.assertEqual(cxc.affected_by(["skills/nothing/here.md"]), ()) + + +class Directions(DeliveryTestCase): + """The revision request is an instruction, so it is shaped like one.""" + + def _revision(self): + from codex_session_relay import identity + + _relationship, event_id = self.queued_event(recipients=[PARENT, CHILD]) + self.attempt(event_id) + self.clock.advance(5) + turn = self.adapter.start_turn(PARENT, turn_id="ack-turn", status="inProgress") + self.ack.acknowledge( + event_id, ack_turn_id=turn.turn_id, + ack_proof=identity.ack_proof(event_id, turn.turn_id), accepted=True, + adapter=self.adapter, + ) + self.ack.record_verdict( + event_id, verdict="needs_changes", verdict_turn_id="verdict-1", + criteria=[{"id": "c-1", "verdict": "needs_changes", + "note": "the migration script is missing from the manifest"}], + ) + row = self.store.one("SELECT * FROM deliveries WHERE kind = 'revision_request'") + return event_id, row["event_id"] + + def test_a_correction_carries_every_field_the_dispatch_contract_names(self): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) or {} + row = self.delivery.get(revision_event) + stored = report.record( + self.store, self.clock, event_id=revision_event, + relationship_id=row["relationship_id"], + execution_generation=receipt.get("executionGeneration") or 2, + revision_hash=receipt.get("revisionHash") or "0" * 64, + outcome="blocked_needs_input", + **a_report( + cxc_status=cxc.NEEDS_HUMAN, + cxc_reason="the parent judged the manifest incomplete", + summary="add the migration script and re-submit", + next_action="add the migration script to the manifest and emit generation 2", + review={ + "kind": cxc.GO_WITH_FIXES, "blockers": 1, + "findings": [{ + "id": "c-1", "verdict": "needs_changes", + "note": "the migration script is missing from the manifest", + "anchor": "migrations/004_add_reports.sql", + }], + }, + ) + ) + message = report.render_revision(row, receipt, "del-y-a1", stored) + for field in cxc.DISPATCH_FIELDS: + self.assertIn(field + ":", message, f"{field} is missing from the instruction") + self.assertIn(cxc.DECISION_BOUNDARY + ":", message) + self.assertIn("VERDICT: GO-WITH-FIXES (blockers=1)", message) + # What was violated, and how to reproduce it, lead the message. + self.assertLess(message.index("violated criteria:"), message.index("SCOPE:")) + self.assertIn("anchor: migrations/004_add_reports.sql", message) + self.assertIn("preserve:", message) + # And the asymmetry the contract actually has is preserved. + self.assertIn("nothing to acknowledge", message) + self.assertNotIn("--ack-proof", message) + self.assertIn("emit --relationship", message) + + +class Isolation(DeliveryTestCase): + def test_two_parents_holding_the_same_pull_request_number_stay_apart(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + report.record( + self.store, self.clock, event_id=event_id, + relationship_id=relationship["relationshipId"], + execution_generation=receipt["executionGeneration"], + revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + ) + other = self.registry.register( + parent=Endpoint("01other-parent", HOST, cwd="/other"), + child=Endpoint(CHILD, HOST, cwd=self.root), + issue_key="REL-2", artifact_roots=[self.root], + allowed_recipients=["01other-parent"], dispatch_request_id="dispatch-2", + dispatch_turn_id="turn-dispatch-2", + ) + mine = report.read(self.store, event_id) + theirs = dict(mine) + theirs["relationshipId"] = other["relationshipId"] + theirs["repository"] = "another-org/another-repo" + self.assertNotEqual(report.pr_key(mine), report.pr_key(theirs)) + self.assertNotIn("another-org", self.delivery.render_message(event_id)) From 0bea954262bb453e396f40801e0db1e403154c3f Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:22:51 +0900 Subject: [PATCH 02/27] Stop a work report from believing whatever its caller said it was Review of #10 found the hole: record() took the relationship, generation, revision and outcome as arguments and never compared them with the events row for that event id. A mistyped or stale caller could file a report describing a different execution, and delivery would then hand the recipient another pull request and another instruction. Identity is now READ from the stored event and there is no argument left through which a caller could say otherwise. A report against an event nobody accepted is refused. Six smaller things found in the same review: A revision request carries no child receipt, so pairing its status with an asserted outcome forced the caller to invent one. That direction now checks the status is known and stops there. Evidence and unresolved entries are validated where they are recorded. Rendering runs inside the delivery claim, so a shape that only fails at render time rolled the claim back and the delivery never went out. The non-verification sentence is chosen by status. Every report was told it was the child proving its own criteria, which is false for BLOCKED and the opposite of what it means. The review verdict moves to the final line, where REVIEW-OUTPUT-01 puts it and where a scanner reading the tail will find it. The message budget counts UTF-8 bytes. A transport limit is a byte limit, and this workflow writes Korean, which costs about three bytes a character, so counting characters let exactly the messages it actually sends overrun a budget that looked comfortable. provenance() no longer claims every report stores its source digests; it stores the version string, and the digests are pinned in source. --- .../src/codex_session_relay/cxc.py | 29 +++- .../src/codex_session_relay/report.py | 121 ++++++++++++-- .../tests/test_report_contract.py | 152 ++++++++++++++---- 3 files changed, 251 insertions(+), 51 deletions(-) diff --git a/packages/codex-session-relay/src/codex_session_relay/cxc.py b/packages/codex-session-relay/src/codex_session_relay/cxc.py index 36a5e86..487cf73 100644 --- a/packages/codex-session-relay/src/codex_session_relay/cxc.py +++ b/packages/codex-session-relay/src/codex_session_relay/cxc.py @@ -84,7 +84,12 @@ class Source: def provenance() -> dict: - """What was read, from where, at which version. Recorded with every work report.""" + """What was read, from where, at which version. + + A work report stores the version string only. The digests live here, pinned in source, + because copying nine of them onto every row would store the same constant many times and + still not prove anything a reader could not get from this function. + """ return { "package": PACKAGE, "version": VERSION, @@ -147,12 +152,13 @@ def affected_by(changed_paths) -> tuple: } -def check_status(status: str, outcome: str) -> None: - """Refuse an unknown status by name, and an incompatible pair by both names. +def check_known(status: str) -> None: + """Is this a status this build understands, without pairing it to a receipt outcome. - Silently defaulting an unrecognised status is the failure this exists to prevent: a - newer CXC reporting a word this build has never seen must be diagnosed, not mapped to - whichever neighbour happens to be first in a dict. + The parent-to-child direction has no child receipt to pair with: a revision request is + the parent stating a judgment, not a child asserting how its execution ended. Reusing the + compatibility check there would force the caller to invent an outcome, and an invented + outcome is exactly what the rest of this package refuses. """ if status not in COMPATIBLE_OUTCOMES: raise ReceiptRefused( @@ -161,6 +167,16 @@ def check_status(status: str, outcome: str) -> None: f"{', '.join(REPORT_STATUSES)}. Read against {PACKAGE} {VERSION}; a newer " "contract needs the mapping extended rather than the value guessed at", ) + + +def check_status(status: str, outcome: str) -> None: + """Refuse an unknown status by name, and an incompatible pair by both names. + + Silently defaulting an unrecognised status is the failure this exists to prevent: a + newer CXC reporting a word this build has never seen must be diagnosed, not mapped to + whichever neighbour happens to be first in a dict. + """ + check_known(status) allowed = COMPATIBLE_OUTCOMES[status] if outcome not in allowed: raise ReceiptRefused( @@ -177,6 +193,7 @@ def check_status(status: str, outcome: str) -> None: # parent produces, from inside its own turn, against registered criteria. NOT_VERIFICATION = { "cxc_done": "a DONE report is the child proving its own criteria, not the parent's verdict", + "cxc_report": "a child report is the child describing its own execution, not a verdict", "pull_request_opened": "an open pull request is a place to review, not a review", "review_pass": "a review PASS is one reviewer's judgment, not the parent's disposition", "required_checks_green": "a green required check is evidence for a verdict, not a verdict", diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index c13cce3..2004293 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -26,30 +26,59 @@ NEWLINE = chr(10) VERSION = "relay-report/1" LEGACY = "relay-message/legacy" +REVISION_OUTCOME = "revision_request" +# UTF-8 BYTES, not characters. A transport limit is a byte limit, and a report written in +# Korean costs roughly three bytes per character, so measuring characters would let exactly +# the messages this workflow actually sends overrun a budget that looked comfortable. # Generous enough that an ordinary report is never touched, small enough that a runaway one # is elided here, on purpose and in the open, rather than cut by whatever reads it later. BUDGET = 6000 +def _size(text) -> int: + return len(text.encode("utf-8")) + + def show_command(event_id: str) -> str: return f"codex-session-relay show --event {event_id}" # ------------------------------------------------------------------------ recording -def record(store, clock, *, event_id, relationship_id, execution_generation, revision_hash, - outcome, repository, cxc_status, cxc_reason, summary, next_action, +def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summary, next_action, pr_number=None, pr_url=None, pr_state=None, base_ref=None, base_sha=None, head_sha=None, criteria_digest=None, evidence=None, unresolved=None, review=None, restore=None, submission_no=1) -> dict: """Store one report, validated, bound to the revision it describes. - The CXC status is checked against the outcome the child's receipt already asserted. It - is not allowed to choose that outcome: the receipt is the only thing the frozen contract - lets assert one, and a status that picked it could quietly overrule the evidence. + Identity is READ from the stored event, never accepted from the caller. Taking the + relationship, generation, revision and outcome as arguments meant a mistyped or stale + caller could file a report whose metadata described a different execution entirely, and + delivery would then hand the recipient another pull request and another instruction. The + event row is the only thing that knows what this event is. + + The CXC status is then checked against the outcome that event actually carries. It is not + allowed to choose that outcome: the receipt is the only thing the frozen contract lets + assert one, and a status that picked it could quietly overrule the evidence. """ - cxc.check_status(cxc_status, outcome) + event = store.one("SELECT * FROM events WHERE event_id = ?", (event_id,)) + if event is None: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"no event {event_id!r} is recorded, so there is nothing for this report to be " + "about; a report is written against an accepted event, not ahead of one", + ) + relationship_id = event["relationship_id"] + execution_generation = event["execution_generation"] + revision_hash = event["revision_hash"] + outcome = event["outcome"] + if outcome == REVISION_OUTCOME: + # The parent-to-child direction carries no child receipt, so there is no asserted + # outcome to pair the status with. Demanding one would make the caller invent it. + cxc.check_known(cxc_status) + else: + cxc.check_status(cxc_status, outcome) repository = _required(repository, "repository") summary = _required(summary, "summary") next_action = _required(next_action, "next_action") @@ -67,6 +96,8 @@ def record(store, clock, *, event_id, relationship_id, execution_generation, rev "one, a later push silently inherits this report", ) review = _check_review(review) + evidence = _check_evidence(evidence) + unresolved = _check_unresolved(unresolved) row = { "eventId": event_id, "relationshipId": relationship_id, @@ -199,6 +230,50 @@ def _check_review(review): return {"kind": kind, "blockers": blockers, "findings": findings} +def _check_evidence(entries): + """Reject a malformed entry HERE, where a caller can fix it. + + Rendering happens inside the delivery claim transaction, so an entry that only blows up + at render time rolls the claim back and the delivery never goes out. A shape that cannot + be rendered must therefore be refused at the point it is recorded. + """ + checked = [] + for item in entries or []: + if isinstance(item, str): + checked.append(item) + continue + if not isinstance(item, dict) or not str(item.get("check") or "").strip(): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"each verification entry is a string or an object naming its check, not " + f"{item!r}", + ) + checked.append({ + "check": str(item["check"]).strip(), + "exitCode": item.get("exitCode"), + "detail": str(item.get("detail") or "").strip() or None, + }) + return checked + + +def _check_unresolved(entries): + checked = [] + for item in entries or []: + if isinstance(item, str): + checked.append(item) + continue + if not isinstance(item, dict) or not str(item.get("id") or "").strip(): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"each unresolved entry is a string or an object naming its id, not {item!r}", + ) + checked.append({ + "id": str(item["id"]).strip(), + "note": str(item.get("note") or "").strip(), + }) + return checked + + # -------------------------------------------------------------------- identity guards def pr_ref(report) -> str: @@ -273,7 +348,7 @@ def rendered(extra_note=True): order = sorted(range(len(sections)), key=lambda i: -sections[i].rank) for index in order: - if len(rendered()) <= budget: + if _size(rendered()) <= budget: break section = sections[index] if section.essential or not blocks[index]: @@ -289,7 +364,7 @@ def rendered(extra_note=True): # appended to it. Popping a line and then appending a marker leaves the block the # same length, which is a loop that never ends. kept = list(section.lines) - while len(rendered()) > budget and len(kept) > section.keep: + while _size(rendered()) > budget and len(kept) > section.keep: kept.pop() dropped = len(section.lines) - len(kept) blocks[index] = kept + [f" ... {dropped} more, see the full record"] @@ -297,9 +372,9 @@ def rendered(extra_note=True): removed.append(section.name) out = rendered() - if len(out) > budget: + if _size(out) > budget: raise ValueError( - f"a message budget of {budget} cannot hold this report even reduced to its " + f"a message budget of {budget} bytes cannot hold this report even reduced to its " "required parts; raise the budget rather than shipping a message that lost them" ) return out @@ -309,6 +384,15 @@ def _omission_line(removed, event_id) -> str: return f"omitted: {', '.join(removed)} - read in full with {show_command(event_id)}" +def _non_verification(status: str) -> str: + """The reason this particular report is not a verdict. + + Reusing the DONE sentence for every status told a BLOCKED report it was proving its own + criteria, which is both false and the opposite of what BLOCKED means. + """ + return cxc.refuse_promotion("cxc_done" if status == cxc.DONE else "cxc_report") + + # ------------------------------------------------------------------------ rendering def render_completion(row, receipt, request, report, *, budget=BUDGET) -> str: @@ -319,6 +403,8 @@ def render_completion(row, receipt, request, report, *, budget=BUDGET) -> str: how it decides. """ event_id = row["event_id"] + assert_current(report, execution_generation=receipt.get("executionGeneration") + or report["executionGeneration"]) sections = [ _Section("header", [ "[codex-session-relay] verification request", @@ -326,7 +412,7 @@ def render_completion(row, receipt, request, report, *, budget=BUDGET) -> str: f"cxc: {report['cxcStatus']} - {report['cxcReason']}", f" meaning: {cxc.MEANING[report['cxcStatus']]}", " this is the child reporting on its own work. It is not a verification:" - f" {cxc.refuse_promotion('cxc_done')}", + f" {_non_verification(report['cxcStatus'])}", ], rank=0, essential=True, keep=5), # keep counts from the top of the block, and these blocks open with a blank line, so # a floor of two is what keeps the heading attached to whatever survives under it. @@ -362,13 +448,11 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: """ event_id = row["event_id"] generation = receipt.get("executionGeneration") + assert_current(report, execution_generation=generation or report["executionGeneration"]) review = report.get("review") head = [ "[codex-session-relay] revision request", ] - if review: - cxc.assert_reviewed(True) - head.append(cxc.verdict_line(review["kind"], review.get("blockers"))) head.append(f"TASK: {report['summary']}") sections = [ @@ -417,6 +501,15 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: f"Full record: {show_command(event_id)}", ], rank=0, essential=True, keep=9), ] + if review: + # REVIEW-OUTPUT-01 puts the machine-scannable judgment on the FINAL line, so a + # scanner reading the tail finds it. assert_reviewed is what stops an ordinary + # progress notice from reaching this branch at all. + cxc.assert_reviewed(True) + sections.append(_Section( + "verdict", ["", cxc.verdict_line(review["kind"], review.get("blockers"))], + rank=0, essential=True, keep=2, + )) return _compose(sections, event_id, budget=budget) diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index d06bb87..706edf8 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -41,9 +41,7 @@ def recorded(self, **overrides): fields = a_report(**overrides) stored = report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **fields + **fields ) return relationship, event_id, stored @@ -110,9 +108,6 @@ def test_the_three_human_decision_statuses_collapse_without_losing_which_one_it_ # collapse lossless for a reader. stored = report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report(cxc_status=cxc.DONE) ) self.assertEqual(report.read(self.store, event_id)["cxcReason"], stored["cxcReason"]) @@ -158,9 +153,6 @@ def test_a_report_naming_a_pull_request_names_the_commit_it_is_about(self): RefusalReason.MALFORMED_RECEIPT, lambda: report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report(head_sha=None) ), ) @@ -174,9 +166,6 @@ def test_a_report_with_nothing_to_act_on_is_refused(self): RefusalReason.MALFORMED_RECEIPT, lambda field=field: report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report(**{field: " "}) ), ) @@ -189,9 +178,6 @@ def test_more_than_ten_findings_are_never_dropped_in_silence(self): unresolved = [f"finding {n}: something specific that still needs doing" for n in range(24)] report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report(unresolved=unresolved) ) message = self.delivery.render_message(event_id) @@ -213,9 +199,7 @@ def test_an_impossible_budget_refuses_instead_of_shipping_a_gutted_message(self) receipt = self.intake.get(event_id) report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + **a_report() ) stored = report.read(self.store, event_id) row = self.delivery.get(event_id) @@ -228,9 +212,7 @@ def test_the_preview_is_still_not_evidence_of_what_was_sent(self): receipt = self.intake.get(event_id) report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + **a_report() ) before = self.delivery.preview_message(event_id) record = self.attempt(event_id) @@ -264,9 +246,7 @@ def test_the_two_message_versions_are_told_apart_explicitly(self): self.assertEqual(report.version_of(report.read(self.store, event_id)), report.LEGACY) report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + **a_report() ) stored = report.read(self.store, event_id) self.assertEqual(report.version_of(stored), report.VERSION) @@ -382,10 +362,6 @@ def test_a_correction_carries_every_field_the_dispatch_contract_names(self): row = self.delivery.get(revision_event) stored = report.record( self.store, self.clock, event_id=revision_event, - relationship_id=row["relationship_id"], - execution_generation=receipt.get("executionGeneration") or 2, - revision_hash=receipt.get("revisionHash") or "0" * 64, - outcome="blocked_needs_input", **a_report( cxc_status=cxc.NEEDS_HUMAN, cxc_reason="the parent judged the manifest incomplete", @@ -422,9 +398,7 @@ def test_two_parents_holding_the_same_pull_request_number_stay_apart(self): receipt = self.intake.get(event_id) report.record( self.store, self.clock, event_id=event_id, - relationship_id=relationship["relationshipId"], - execution_generation=receipt["executionGeneration"], - revision_hash=receipt["revisionHash"], outcome=receipt["outcome"], **a_report() + **a_report() ) other = self.registry.register( parent=Endpoint("01other-parent", HOST, cwd="/other"), @@ -439,3 +413,119 @@ def test_two_parents_holding_the_same_pull_request_number_stay_apart(self): theirs["repository"] = "another-org/another-repo" self.assertNotEqual(report.pr_key(mine), report.pr_key(theirs)) self.assertNotIn("another-org", self.delivery.render_message(event_id)) + + +class Identity(DeliveryTestCase): + """Raised in review of PR 10: a report used to believe whatever its caller said it was.""" + + def test_a_report_cannot_be_filed_against_an_event_that_does_not_exist(self): + self.register() + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record( + self.store, self.clock, event_id="0" * 32, **a_report() + ), + ) + self.assertIn("nothing for this report to be about", error.detail) + + def test_identity_is_read_from_the_event_rather_than_taken_from_the_caller(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + stored = report.record(self.store, self.clock, event_id=event_id, **a_report()) + # There is no argument through which a caller could have said otherwise. + self.assertEqual(stored["relationshipId"], relationship["relationshipId"]) + self.assertEqual(stored["executionGeneration"], receipt["executionGeneration"]) + self.assertEqual(stored["revisionHash"], receipt["revisionHash"]) + read_back = report.read(self.store, event_id) + self.assertEqual(read_back["revisionHash"], receipt["revisionHash"]) + + def test_a_malformed_entry_is_refused_where_the_caller_can_still_fix_it(self): + relationship, event_id = self.queued_event() + for field, value in (("evidence", [1]), ("unresolved", [1]), + ("evidence", [{"detail": "no check named"}])): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field, value=value: report.record( + self.store, self.clock, event_id=event_id, + **a_report(**{field: value}) + ), + ) + # Left unchecked this surfaced inside the delivery claim, where a render failure + # rolls the transaction back and the delivery never goes out at all. + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.assertIsNotNone(self.attempt(event_id)) + + def test_a_blocked_report_does_not_tell_the_reader_it_proved_anything(self): + relationship = self.register() + payload = self.execution_payload(relationship, "interrupted") + self.accept(payload) + event_id = payload["eventId"] + self.delivery.enqueue(event_id) + report.record( + self.store, self.clock, event_id=event_id, + **a_report(cxc_status=cxc.BUDGET_EXHAUSTED, + cxc_reason="the stated token bound ran out", + pr_number=None, pr_url=None, head_sha=None) + ) + message = self.delivery.render_message(event_id) + self.assertIn("BUDGET_EXHAUSTED", message) + self.assertIn("a bound the plan actually stated ran out", message) + self.assertNotIn("proving its own criteria", message) + self.assertIn("pull request: none recorded", message) + + def test_the_budget_counts_bytes_because_a_transport_limit_does(self): + relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + korean = "전달 메시지가 풀리퀘스트를 먼저 말하도록 바꿉니다. " * 12 + report.record(self.store, self.clock, event_id=event_id, **a_report(summary=korean)) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + budget = 2200 + message = report.render_completion(row, receipt, "del-z-a1", stored, budget=budget) + self.assertLessEqual( + len(message.encode("utf-8")), budget, + "counting characters would let a Korean report overrun a byte budget", + ) + + +class FinalLine(Directions): + def test_the_review_verdict_is_the_last_line_a_scanner_reads(self): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) or {} + row = self.delivery.get(revision_event) + stored = report.record( + self.store, self.clock, event_id=revision_event, + **a_report( + cxc_status=cxc.NEEDS_HUMAN, + cxc_reason="the parent judged the manifest incomplete", + summary="add the migration script and re-submit", + next_action="add the migration script and emit generation 2", + review={"kind": cxc.GO_WITH_FIXES, "blockers": 1, "findings": [{ + "id": "c-1", "verdict": "needs_changes", "note": "manifest is short", + "anchor": "migrations/004.sql"}]}, + ) + ) + message = report.render_revision(row, receipt, "del-y-a1", stored) + self.assertEqual( + message.splitlines()[-1], "VERDICT: GO-WITH-FIXES (blockers=1)", + "REVIEW-OUTPUT-01 puts the machine-scannable judgment on the final line", + ) + self.assertEqual( + cxc.parse_verdict_line(message.splitlines()[-1]), + {"kind": cxc.GO_WITH_FIXES, "blockers": 1}, + ) + + def test_a_revision_report_does_not_have_to_invent_a_receipt_outcome(self): + _source, revision_event = self._revision() + event = self.store.one( + "SELECT outcome FROM events WHERE event_id = ?", (revision_event,) + ) + self.assertEqual(event["outcome"], "revision_request") + # No child receipt exists in this direction, so pairing the status with an asserted + # outcome would force the caller to make one up. + stored = report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="manifest incomplete") + ) + self.assertEqual(stored["cxcStatus"], cxc.NEEDS_HUMAN) + self.assertIn("NEEDS_HUMAN", str(report.read(self.store, revision_event))) From 1d95e19ac98dd311f913eaffb49fc4cb50e6d5fe Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:29:13 +0900 Subject: [PATCH 03/27] Let the recorded verdict decide what a correction corrects Second review round on #10, three real findings. A revision event already carries the parent findings in its own receipt, written by record_verdict in the transaction that opened the new generation. render_revision was reading only the work report review, so a report with no review replaced those findings with nothing, and one naming a different set replaced them with something else. Either way the child was corrected against instructions the parent never gave. The receipt now leads, the review adds notes and source anchors by id, and anything the review raises on its own is kept and labelled as not part of the recorded verdict. An oversized summary or next action passed record() and then failed every render. Rendering happens inside the delivery claim, so each claim rolled back and the delivery never went out: the same failure mode as the malformed entries in the previous round, which is the argument for bounding these fields where the caller can still fix them rather than where nobody can. Replacing a report in place after a delivery had already been attempted let a retry carry different instructions under the same event and the same stated submission, with no way for the recipient to tell which one it was answering. The frozen attempt bytes always kept the history; what was missing was the report admitting it had changed. Correcting a report before anything is sent is still free, and afterwards it has to be a new submission, which the message already shows. --- .../codex-session-relay/docs/invariants.md | 4 + .../src/codex_session_relay/report.py | 111 +++++++++++++++--- .../tests/test_report_contract.py | 75 ++++++++++++ 3 files changed, 176 insertions(+), 14 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index fc146e4..1abcf90 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -115,6 +115,10 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-97 | A shortened message names what it dropped and where to read it; a budget too small to hold the required parts refuses | `report._compose` | implemented | | I-98 | An event with no work report renders the pre-contract message unchanged | `delivery._render_completion`, `delivery._render_revision` | implemented | | I-99 | A wait result never authorises a re-run, and a bare timeout is neither failure nor success | `cxc.classify_wait` | implemented | +| I-100 | A report reads its relationship, generation, revision and outcome from the stored event; no caller supplies them | `report.record` | implemented | +| I-101 | A correction names the criteria the recorded verdict names; a review only adds notes and anchors, and anything it raises alone is labelled | `report._finding_lines` | implemented | +| I-102 | A shape or length that could only fail at render time is refused at record time, because rendering runs inside the delivery claim | `report._check_evidence`, `_check_unresolved`, `_bounded` | implemented | +| I-103 | A report already delivered cannot be replaced in place; a changed report is a new submission and says so | `report._check_resubmission` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 2004293..b99fd76 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -35,6 +35,16 @@ # is elided here, on purpose and in the open, rather than cut by whatever reads it later. BUDGET = 6000 +# Per-field byte ceilings, enforced when a report is RECORDED. The composer can drop an +# optional section and shorten a list, but it cannot shorten a single required line without +# losing the thing that line exists to say. An oversized summary therefore used to pass +# record() and then fail every render, and because rendering happens inside the delivery +# claim, every claim rolled back and the delivery never went out. Refusing here keeps the +# failure where the caller can still fix it. +SUMMARY_MAX = 1200 +ACTION_MAX = 1200 +REASON_MAX = 600 + def _size(text) -> int: return len(text.encode("utf-8")) @@ -79,10 +89,10 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar cxc.check_known(cxc_status) else: cxc.check_status(cxc_status, outcome) - repository = _required(repository, "repository") - summary = _required(summary, "summary") - next_action = _required(next_action, "next_action") - reason = _required(cxc_reason, "cxc_reason") + repository = _bounded(_required(repository, "repository"), "repository", 200) + summary = _bounded(_required(summary, "summary"), "summary", SUMMARY_MAX) + next_action = _bounded(_required(next_action, "next_action"), "next_action", ACTION_MAX) + reason = _bounded(_required(cxc_reason, "cxc_reason"), "cxc_reason", REASON_MAX) if pr_number is not None: if isinstance(pr_number, bool) or not isinstance(pr_number, int) or pr_number < 1: raise ReceiptRefused( @@ -98,6 +108,7 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar review = _check_review(review) evidence = _check_evidence(evidence) unresolved = _check_unresolved(unresolved) + _check_resubmission(store, event_id, submission_no) row = { "eventId": event_id, "relationshipId": relationship_id, @@ -206,6 +217,47 @@ def _required(value, field): return text +def _bounded(text, field, limit): + if _size(text) > limit: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} is {_size(text)} bytes and the limit is {limit}; a required line cannot " + "be shortened at render time without losing what it exists to say, and a render " + "that fails inside the delivery claim is a delivery that never goes out. Put the " + "detail in the deliverables or the evidence and keep this line to the point", + ) + return text + + +def _check_resubmission(store, event_id, submission_no) -> None: + """Once a message has gone out, a changed report is a new submission and says so. + + The bytes of every attempt stay frozen in attempt_messages, so history is never lost. + What this stops is the quieter thing: replacing a report in place after a delivery has + already been attempted, so a retry carries different instructions under the same event + and the same stated submission. The recipient would have no way to tell which one it was + answering. Correcting a report before anything is sent stays free. + """ + existing = store.one( + "SELECT submission_no FROM work_reports WHERE event_id = ?", (event_id,) + ) + if existing is None: + return + attempted = store.one( + "SELECT 1 FROM attempts WHERE event_id = ? LIMIT 1", (event_id,) + ) + if attempted is None: + return + if int(submission_no) <= existing["submission_no"]: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"submission {existing['submission_no']} of this report has already been " + f"delivered, so replacing it in place would change what a retry says without " + f"changing what it calls itself; record this as submission " + f"{existing['submission_no'] + 1} or higher", + ) + + def _check_review(review): if review is None: return None @@ -457,7 +509,8 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: sections = [ _Section("header", head, rank=0, essential=True, keep=2), - _Section("violated criteria", _finding_lines(review), rank=0, essential=True, keep=2), + _Section("violated criteria", _finding_lines(receipt, review), rank=0, essential=True, + keep=2), _Section("SCOPE", _scope_lines(report, generation), rank=1, essential=True, keep=2), _Section("MUST DO", [ "MUST DO:", @@ -572,20 +625,50 @@ def _unresolved_lines(report): return lines -def _finding_lines(review): - if not review or not review.get("findings"): +def _finding_lines(receipt, review): + """The recorded verdict decides WHICH criteria; the review only enriches them. + + A revision event already carries the parent findings in its own receipt, written by + record_verdict in the transaction that opened the new generation. Rendering only the work + report review meant a report with no review, or one naming a different set, replaced the + authoritative findings with nothing or with something else, and the child was corrected + against instructions the parent never gave. So the receipt leads, the review adds notes + and source anchors by id, and anything the review raises on its own is kept but labelled + as not part of the recorded verdict. + """ + authoritative = [item for item in (receipt.get("criteria") or []) if item.get("id")] + enrichment = {} + for item in (review or {}).get("findings") or []: + enrichment[item["id"]] = item + if not authoritative and not enrichment: return ["", "violated criteria: no per-criterion findings were recorded"] + lines = ["", "violated criteria:"] - for item in review["findings"]: - rendered = f" {item['id']}: {item.get('verdict')}" - if item.get("note"): - rendered += f" - {item['note']}" - lines.append(rendered) - if item.get("anchor"): - lines.append(f" anchor: {item['anchor']}") + seen = set() + for item in authoritative or list(enrichment.values()): + extra = enrichment.get(item["id"], {}) + seen.add(item["id"]) + lines += _one_finding(item, extra) + unrecorded = [item for key, item in enrichment.items() if key not in seen] + if authoritative and unrecorded: + lines.append(" also raised in review, not part of the recorded verdict:") + for item in unrecorded: + lines += _one_finding(item, {}) return lines +def _one_finding(item, extra): + note = item.get("note") or extra.get("note") + rendered = f" {item['id']}: {item.get('verdict')}" + if note: + rendered += f" - {note}" + out = [rendered] + anchor = extra.get("anchor") or item.get("anchor") + if anchor: + out.append(f" anchor: {anchor}") + return out + + def _scope_lines(report, generation): lines = ["", "SCOPE:"] reference = pr_ref(report) diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 706edf8..5e6aec2 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -529,3 +529,78 @@ def test_a_revision_report_does_not_have_to_invent_a_receipt_outcome(self): ) self.assertEqual(stored["cxcStatus"], cxc.NEEDS_HUMAN) self.assertIn("NEEDS_HUMAN", str(report.read(self.store, revision_event))) + + def test_the_recorded_verdict_decides_which_criteria_a_correction_names(self): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) + row = self.delivery.get(revision_event) + # No review on the work report at all. The findings the parent actually recorded are + # in the revision receipt and must still be what the child is corrected against. + stored = report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="manifest incomplete", + review=None) + ) + message = report.render_revision(row, receipt, "del-y-a1", stored) + self.assertIn("c-1", message) + self.assertIn("the migration script is missing from the manifest", message) + self.assertNotIn("no per-criterion findings were recorded", message) + + def test_a_review_only_finding_is_kept_but_marked_as_outside_the_verdict(self): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) + row = self.delivery.get(revision_event) + stored = report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="manifest incomplete", + review={"kind": cxc.GO_WITH_FIXES, "blockers": 1, "findings": [ + {"id": "c-1", "verdict": "needs_changes", "note": "", + "anchor": "migrations/004.sql"}, + {"id": "c-9", "verdict": "needs_changes", + "note": "a reviewer noticed this separately"}]}) + ) + message = report.render_revision(row, receipt, "del-y-a1", stored) + # The recorded criterion leads and picks up the review anchor. + self.assertIn("anchor: migrations/004.sql", message) + self.assertLess(message.index("c-1"), message.index("c-9")) + self.assertIn("also raised in review, not part of the recorded verdict", message) + self.assertIn("a reviewer noticed this separately", message) + + +class Bounds(DeliveryTestCase): + def test_a_required_line_too_long_to_render_is_refused_when_it_is_recorded(self): + _relationship, event_id = self.queued_event() + for field in ("summary", "next_action"): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field: report.record( + self.store, self.clock, event_id=event_id, + **a_report(**{field: "x" * 4000}) + ), + ) + self.assertIn("a delivery that never goes out", error.detail) + # Otherwise this passed record and then failed every render, and because rendering + # happens inside the claim, every claim rolled back unsent. + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.assertIsNotNone(self.attempt(event_id)) + + def test_a_report_already_delivered_cannot_be_replaced_in_place(self): + _relationship, event_id = self.queued_event() + report.record(self.store, self.clock, event_id=event_id, **a_report()) + # Before anything is sent, correcting a report is free. + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="corrected before sending")) + self.assertEqual( + report.read(self.store, event_id)["summary"], "corrected before sending" + ) + self.attempt(event_id) + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="quietly different now")), + ) + self.assertIn("record this as submission 2 or higher", error.detail) + # Saying so out loud is allowed, and the message carries the number. + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="openly revised", submission_no=2)) + self.assertIn("submission: 2", self.delivery.preview_message(event_id)) From 7a9b75d277767e4d813cc642d718f3d5ccecf475 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:34:42 +0900 Subject: [PATCH 04/27] Keep the verdict last and the frozen manifest pointer present Third review round on #10. The omission notice was appended after every section, including the deliberately last verdict section, so any correction that had to drop something ended on omitted: instead of its judgment. A consumer following the final-line contract would stop finding the verdict in exactly the messages that had to shorten. A section can now declare itself last and the notice goes before it. A revision event exists because the parent ruled needs_changes, and the review half of a report is still caller-supplied even now that identity is not. A PASS verdict on one would approve the work and demand changes in the same message, so it is refused. manifestRef is back in the report-backed message. The pre-contract message always carried it, it is the stable location a receipt points at when live artifact paths may move, and adding a work report was silently removing it even with no elision in play. --- .../codex-session-relay/docs/invariants.md | 3 + .../src/codex_session_relay/report.py | 42 +++++++++++-- .../tests/test_report_contract.py | 60 +++++++++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 1abcf90..6dffa72 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -119,6 +119,9 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-101 | A correction names the criteria the recorded verdict names; a review only adds notes and anchors, and anything it raises alone is labelled | `report._finding_lines` | implemented | | I-102 | A shape or length that could only fail at render time is refused at record time, because rendering runs inside the delivery claim | `report._check_evidence`, `_check_unresolved`, `_bounded` | implemented | | I-103 | A report already delivered cannot be replaced in place; a changed report is a new submission and says so | `report._check_resubmission` | implemented | +| I-104 | An omission notice is placed before the final verdict, so an elided correction still ends on its judgment | `report._compose` | implemented | +| I-105 | A revision request cannot carry a PASS verdict | `report.record` | implemented | +| I-106 | A report-backed message keeps the receipt manifestRef the pre-contract message carried | `report._manifest_lines` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index b99fd76..b35629a 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -89,6 +89,17 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar cxc.check_known(cxc_status) else: cxc.check_status(cxc_status, outcome) + if outcome == REVISION_OUTCOME and (review or {}).get("kind") == cxc.PASS: + # A revision event exists because the parent ruled needs_changes. A PASS verdict on + # it would tell the child its work was approved in the same message that demands + # changes, and the review half of a report is caller-supplied even now that identity + # is not. + raise ReceiptRefused( + RefusalReason.DISPOSITION_CONFLICT, + "a revision request cannot carry a PASS verdict: this event exists because the " + "parent ruled needs_changes, and the message would approve and demand changes " + "at the same time", + ) repository = _bounded(_required(repository, "repository"), "repository", 200) summary = _bounded(_required(summary, "summary"), "summary", SUMMARY_MAX) next_action = _bounded(_required(next_action, "next_action"), "next_action", ACTION_MAX) @@ -373,12 +384,15 @@ class _Section: whole; when they have to shrink they keep their heading and say how much is missing. """ - def __init__(self, name, lines, rank, *, essential=False, keep=0): + def __init__(self, name, lines, rank, *, essential=False, keep=0, last=False): self.name = name self.lines = [line for line in lines if line is not None] self.rank = rank self.essential = essential self.keep = keep + # A section that must stay at the end of the message even when something was + # elided. The omission notice goes BEFORE it, not after. + self.last = last def _compose(sections, event_id, *, budget) -> str: @@ -391,11 +405,19 @@ def _compose(sections, event_id, *, budget) -> str: """ blocks = [list(section.lines) for section in sections] removed = [] + tail = next((i for i, section in enumerate(sections) if section.last), None) def rendered(extra_note=True): - body = [line for block in blocks for line in block] + body = [ + line for index, block in enumerate(blocks) if index != tail for line in block + ] if removed and extra_note: body += ["", _omission_line(removed, event_id)] + if tail is not None: + # Appending the notice after this would make "omitted: ..." the final line, and + # a consumer following the final-line verdict contract would stop finding the + # verdict in exactly the messages that had to drop something. + body += blocks[tail] return NEWLINE.join(body) order = sorted(range(len(sections)), key=lambda i: -sections[i].rank) @@ -561,7 +583,7 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: cxc.assert_reviewed(True) sections.append(_Section( "verdict", ["", cxc.verdict_line(review["kind"], review.get("blockers"))], - rank=0, essential=True, keep=2, + rank=0, essential=True, keep=2, last=True, )) return _compose(sections, event_id, budget=budget) @@ -719,9 +741,19 @@ def _restore_lines(report): def _manifest_lines(receipt, event_id): + """Including manifestRef, which the pre-contract message always carried. + + It is the stable location a receipt points at when the live artifact paths may move, so + dropping it would leave the recipient verifying against files that had relocated. Adding + a work report must not quietly take it away. + """ + reference = receipt.get("manifestRef") manifest = receipt.get("manifest") if not manifest: - return ["", "deliverables: none (execution-only outcome)"] + lines = ["", "deliverables: none (execution-only outcome)"] + if reference: + lines.append(f"manifestRef: {reference}") + return lines lines = ["", f"deliverables: {len(manifest)}"] for entry in manifest: size = entry.get("bytes") @@ -729,6 +761,8 @@ def _manifest_lines(receipt, event_id): f" {entry['path']} sha256={entry['sha256']}" + (f" bytes={size}" if size is not None else "") ) + if reference: + lines.append(f"manifestRef: {reference}") return lines diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 5e6aec2..8bc5bcf 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -604,3 +604,63 @@ def test_a_report_already_delivered_cannot_be_replaced_in_place(self): report.record(self.store, self.clock, event_id=event_id, **a_report(summary="openly revised", submission_no=2)) self.assertIn("submission: 2", self.delivery.preview_message(event_id)) + + def test_a_report_backed_message_still_carries_the_frozen_manifest_pointer(self): + _relationship, event_id = self.queued_event() + receipt = dict(self.intake.get(event_id)) + receipt["manifestRef"] = "/var/lib/relay/frozen/abc123" + report.record(self.store, self.clock, event_id=event_id, **a_report()) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + message = report.render_completion(row, receipt, "del-m-a1", stored) + self.assertIn( + "manifestRef: /var/lib/relay/frozen/abc123", message, + "the pre-contract message carried it, so adding a report must not take it away", + ) + + +class VerdictPosition(Directions): + def _revision_report(self, **overrides): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) + row = self.delivery.get(revision_event) + fields = dict( + cxc_status=cxc.NEEDS_HUMAN, cxc_reason="manifest incomplete", + review={"kind": cxc.GO_WITH_FIXES, "blockers": 2, "findings": [ + {"id": "c-1", "verdict": "needs_changes", "note": "", + "anchor": "migrations/004.sql"}]}, + ) + fields.update(overrides) + stored = report.record( + self.store, self.clock, event_id=revision_event, **a_report(**fields) + ) + return row, receipt, stored + + def test_an_elided_correction_still_ends_on_its_verdict(self): + row, receipt, stored = self._revision_report( + evidence=[{"check": f"a long check name number {n} that takes up room", + "exitCode": 0} for n in range(30)], + ) + message = report.render_revision(row, receipt, "del-y-a1", stored, budget=2400) + self.assertIn("omitted:", message, "something had to go") + self.assertEqual( + message.splitlines()[-1], "VERDICT: GO-WITH-FIXES (blockers=2)", + "the omission notice goes before the verdict, not after it", + ) + self.assertLess( + message.index("omitted:"), message.index("VERDICT:"), + "otherwise a consumer stops finding the verdict in exactly the messages that " + "had to drop something", + ) + + def test_a_correction_cannot_approve_and_demand_changes_at_once(self): + _source, revision_event = self._revision() + error = self.assertRefused( + RefusalReason.DISPOSITION_CONFLICT, + lambda: report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="incomplete", + review={"kind": cxc.PASS, "findings": []}) + ), + ) + self.assertIn("ruled needs_changes", error.detail) From 570340bed6c170f53599cae5a7d4c75351bbd30f Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:34:59 +0900 Subject: [PATCH 05/27] Record what a work report cannot know about a head commit --- packages/codex-session-relay/docs/invariants.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 6dffa72..5eaad57 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -134,3 +134,4 @@ status. Every row below is implemented and carries a test; the suite is the proo | The JSON date-time format is unvalidated | the available validator has no working format checker, so timestamp format is unverified rather than implied | | Terminal turns are polled, not subscribed | the transport cannot subscribe, so automatic invocation is a bounded poll that then dispatches | | Archive state can be unknown | an inconclusive listing withholds rather than guessing, and a later observation releases it | +| A head commit is not observable from here | the relay cannot watch a forge, so `assert_current` enforces generation on the delivery path and takes `head_sha` only from a caller that already knows the current head. A push that changes the declared manifest is structurally a new event, because the revision hash and therefore the event id change with it; a push that changes nothing declared is not, and `_check_resubmission` is what stops an old report standing for it silently | From 66e62f03f6a2e2e6727d4d56e88640155a56f347 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:38:01 +0900 Subject: [PATCH 06/27] Make the command the omission notice names actually return the report An elided message told its recipient to run show --event for the rest, and cmd_show never read work_reports, so exactly the fields the message had dropped were the fields that command could not return. The notice was a false promise rather than an incomplete one, which is worse than eliding nothing. show now carries the whole report. This touches cli.py, which the parallel JUN-103 branch is also editing: it was reported before being changed, the added lines sit inside cmd_show about forty lines from that branch nearest hunk, and the import is function-local so the module import block it does edit stays untouched. Also from the same round: a restore section could name a skill owner nobody has, and cxc.skill_pointer refuses an unknown name on purpose, so the typo surfaced as a failed render inside the delivery claim instead of as something a caller could fix. Validated at record time now, like the other shapes that can only fail where nobody can act on them. --- .../codex-session-relay/docs/invariants.md | 2 + .../src/codex_session_relay/cli.py | 7 +++ .../src/codex_session_relay/report.py | 22 ++++++++ .../tests/test_report_contract.py | 50 +++++++++++++++++++ 4 files changed, 81 insertions(+) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 5eaad57..2ee89cb 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -122,6 +122,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-104 | An omission notice is placed before the final verdict, so an elided correction still ends on its judgment | `report._compose` | implemented | | I-105 | A revision request cannot carry a PASS verdict | `report.record` | implemented | | I-106 | A report-backed message keeps the receipt manifestRef the pre-contract message carried | `report._manifest_lines` | implemented | +| I-107 | The command an omission notice names returns the whole report, so every elided field stays recoverable | `cli.cmd_show` | implemented | +| I-108 | A restore section naming a skill owner nobody has is refused at record time | `report._check_restore` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/cli.py b/packages/codex-session-relay/src/codex_session_relay/cli.py index cd0e051..2bbc18d 100644 --- a/packages/codex-session-relay/src/codex_session_relay/cli.py +++ b/packages/codex-session-relay/src/codex_session_relay/cli.py @@ -535,6 +535,13 @@ def cmd_show(services, args) -> dict: "acknowledgementVerified": ack["verified"] if ack else None, "verdict": json.loads(verdict["record"]) if verdict else None, } + # The work report, whole. A delivered message may have had to elide part of it, and its + # omission notice sends the recipient here, so this is the one place that must always + # carry every field the message could have dropped. Imported locally to keep this change + # out of the module import block, which a parallel branch is editing. + from .report import read as read_work_report + + payload["workReport"] = read_work_report(services.store, args.event) if delivery is not None and args.message: # The bytes each attempt actually froze, with how far they got. A preview is offered # only when nothing has been prepared, and it is labelled a preview, because the old diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index b35629a..a84c7ff 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -119,6 +119,7 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar review = _check_review(review) evidence = _check_evidence(evidence) unresolved = _check_unresolved(unresolved) + restore = _check_restore(restore) _check_resubmission(store, event_id, submission_no) row = { "eventId": event_id, @@ -269,6 +270,27 @@ def _check_resubmission(store, event_id, submission_no) -> None: ) +def _check_restore(restore): + """A skill pointer nobody owns is another render-time failure inside the claim. + + _restore_lines resolves each named activity through cxc.skill_pointer, which refuses an + unknown name on purpose, so an unvalidated name recorded here would surface as a failed + render and a rolled-back delivery rather than as a typo somebody could fix. + """ + restore = dict(restore or {}) + unknown = [ + name for name in restore.get("skills") or [] if name not in cxc.SKILL_POINTERS + ] + if unknown: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"no recorded skill owner for {unknown}; known activities are " + f"{sorted(cxc.SKILL_POINTERS)}. Naming an owner nobody has would fail at render " + "time, inside the delivery claim, instead of here", + ) + return restore + + def _check_review(review): if review is None: return None diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 8bc5bcf..6ad104a 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -664,3 +664,53 @@ def test_a_correction_cannot_approve_and_demand_changes_at_once(self): ), ) self.assertIn("ruled needs_changes", error.detail) + + +class Recovery(DeliveryTestCase): + """The omission notice promises a command; that command has to deliver.""" + + def test_the_command_the_omission_notice_names_returns_the_whole_report(self): + from codex_session_relay import cli + + _relationship, event_id = self.queued_event() + unresolved = [f"finding {n}: something that still needs doing" for n in range(30)] + report.record(self.store, self.clock, event_id=event_id, + **a_report(unresolved=unresolved)) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + receipt = self.intake.get(event_id) + tight = report.render_completion(row, receipt, "del-r-a1", stored, budget=1400) + self.assertIn("omitted:", tight) + self.assertIn(report.show_command(event_id), tight) + + services = type("S", (), {"store": self.store, "delivery": self.delivery, + "intake": self.intake})() + args = type("A", (), {"event": event_id, "message": False})() + payload = cli.cmd_show(services, args) + self.assertIsNotNone(payload["workReport"], "show must carry the report") + # Every field the message was able to drop is recoverable there. + self.assertEqual(payload["workReport"]["unresolved"], unresolved) + self.assertEqual(payload["workReport"]["evidence"], stored["evidence"]) + self.assertEqual(payload["workReport"]["nextAction"], stored["nextAction"]) + + def test_a_skill_pointer_nobody_owns_is_refused_where_it_can_be_fixed(self): + _relationship, event_id = self.queued_event() + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record( + self.store, self.clock, event_id=event_id, + **a_report(restore={"mode": "CXC Loop", "skills": ["loop", "telepathy"]}) + ), + ) + self.assertIn("telepathy", error.detail) + # A known set renders the owners rather than telling the reader to reload everything. + report.record( + self.store, self.clock, event_id=event_id, + **a_report(restore={"mode": "CXC Loop, HOTL", "phase": "C", + "plan": "devlog/_plan/260916_jun131", + "skills": ["loop", "pull-request"]}) + ) + message = self.delivery.render_message(event_id) + self.assertIn("workflow restore:", message) + self.assertIn("mode: CXC Loop, HOTL", message) + self.assertIn(cxc.skill_pointer("pull-request"), message) From c0bfa9ffef8145d297cb15211ef2a14101f391cf Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:42:20 +0900 Subject: [PATCH 07/27] Keep the verification pointer when the file listing goes manifestRef lived inside the deliverables block, which is the first thing the composer drops, so the reports most likely to be shortened were exactly the ones that lost the stable location their artifacts can still be verified against. It is a verification pointer rather than a file listing, so it is now its own essential section and the listing can go without it. _check_restore tested entries for dictionary membership before checking their shape, so an unhashable entry raised TypeError and the caller got a host failure instead of a named refusal. A validator that crashes on malformed input is not validating it. Shape first now, for the outer object, the skills list and each entry, and every invalid form is MALFORMED_RECEIPT. --- .../codex-session-relay/docs/invariants.md | 2 + .../src/codex_session_relay/report.py | 55 +++++++++++++++---- .../tests/test_report_contract.py | 16 ++++++ 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 2ee89cb..24319a2 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -124,6 +124,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-106 | A report-backed message keeps the receipt manifestRef the pre-contract message carried | `report._manifest_lines` | implemented | | I-107 | The command an omission notice names returns the whole report, so every elided field stays recoverable | `cli.cmd_show` | implemented | | I-108 | A restore section naming a skill owner nobody has is refused at record time | `report._check_restore` | implemented | +| I-109 | The frozen-manifest pointer is its own section, so shortening the file listing never drops it | `report._manifest_ref_lines` | implemented | +| I-110 | Every malformed report shape is a named refusal, never a host exception from the validator itself | `report._check_restore`, `_check_evidence`, `_check_unresolved` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index a84c7ff..389d5a4 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -276,11 +276,37 @@ def _check_restore(restore): _restore_lines resolves each named activity through cxc.skill_pointer, which refuses an unknown name on purpose, so an unvalidated name recorded here would surface as a failed render and a rolled-back delivery rather than as a typo somebody could fix. + + Shape is checked before membership. Testing an unhashable entry against a dict raises + TypeError, which reaches the caller as a host failure instead of a named refusal, and a + validator that crashes on malformed input is not validating it. """ - restore = dict(restore or {}) - unknown = [ - name for name in restore.get("skills") or [] if name not in cxc.SKILL_POINTERS - ] + if restore is None: + return {} + if not isinstance(restore, dict): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a restore section is an object of named fields, not {type(restore).__name__}", + ) + skills = restore.get("skills") + if skills is None: + names = [] + elif isinstance(skills, (list, tuple)): + names = list(skills) + else: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"restore skills is a list of activity names, not {type(skills).__name__}", + ) + unknown = [] + for name in names: + if not isinstance(name, str): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"each restore skill is the name of a recorded activity, not {name!r}", + ) + if name not in cxc.SKILL_POINTERS: + unknown.append(name) if unknown: raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, @@ -518,6 +544,8 @@ def render_completion(row, receipt, request, report, *, budget=BUDGET) -> str: _Section("next", [f"next: {report['nextAction']}"], rank=0, essential=True, keep=1), _Section("workflow restore", _restore_lines(report), rank=3), _Section("deliverables", _manifest_lines(receipt, event_id), rank=6), + _Section("manifest reference", _manifest_ref_lines(receipt), rank=2, essential=True, + keep=2), _Section("relay record", [ "", "relay record:", @@ -769,13 +797,9 @@ def _manifest_lines(receipt, event_id): dropping it would leave the recipient verifying against files that had relocated. Adding a work report must not quietly take it away. """ - reference = receipt.get("manifestRef") manifest = receipt.get("manifest") if not manifest: - lines = ["", "deliverables: none (execution-only outcome)"] - if reference: - lines.append(f"manifestRef: {reference}") - return lines + return ["", "deliverables: none (execution-only outcome)"] lines = ["", f"deliverables: {len(manifest)}"] for entry in manifest: size = entry.get("bytes") @@ -783,11 +807,20 @@ def _manifest_lines(receipt, event_id): f" {entry['path']} sha256={entry['sha256']}" + (f" bytes={size}" if size is not None else "") ) - if reference: - lines.append(f"manifestRef: {reference}") return lines +def _manifest_ref_lines(receipt): + """Its own section, because it is a verification pointer and not a file listing. + + Living inside the deliverables block meant the first thing the composer dropped took the + pointer with it, so precisely the long reports most likely to be shortened lost the + stable location their artifacts can still be verified against. + """ + reference = receipt.get("manifestRef") + return ["", f"manifestRef: {reference}"] if reference else [] + + def _ack_lines(event_id): return [ "", diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 6ad104a..4c40523 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -617,6 +617,22 @@ def test_a_report_backed_message_still_carries_the_frozen_manifest_pointer(self) "manifestRef: /var/lib/relay/frozen/abc123", message, "the pre-contract message carried it, so adding a report must not take it away", ) + # And it survives the elision that drops the file listing, because it is the pointer + # those files can still be verified against once they have moved. + tight = report.render_completion(row, receipt, "del-m-a1", stored, budget=1400) + self.assertIn("omitted:", tight) + self.assertIn("manifestRef: /var/lib/relay/frozen/abc123", tight) + + def test_a_malformed_restore_section_is_refused_rather_than_crashing(self): + _relationship, event_id = self.queued_event() + for bad in ({"skills": [{"name": "loop"}]}, {"skills": [["loop"]]}, + {"skills": "loop"}, ["loop"], "loop"): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, **a_report(restore=bad) + ), + ) class VerdictPosition(Directions): From 010af47d1de63547a4d6cd9d20a3e09a37fab8c3 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:46:43 +0900 Subject: [PATCH 08/27] Bound every report field that lands on a line nobody can shorten pr_state was copied without a limit onto the second, non-elidable line of the pull request section, so a 10 KB value was accepted and then made every preview and every delivery claim raise. The same was true of pr_url, base_ref, base_sha, head_sha and criteria_digest. All six are bounded at record time now, and a non-string value in any of them is a named refusal rather than whatever str() would have produced. The malformed-shape guarantee did not hold at the top level either. A mapping passed as evidence or unresolved iterated as its own keys and was silently accepted as a list of strings; a scalar raised TypeError out of the validator. Both are MALFORMED_RECEIPT now. --- .../codex-session-relay/docs/invariants.md | 2 + .../src/codex_session_relay/report.py | 42 ++++++++++++++++++- .../tests/test_report_contract.py | 30 +++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 24319a2..f4a7605 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -126,6 +126,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-108 | A restore section naming a skill owner nobody has is refused at record time | `report._check_restore` | implemented | | I-109 | The frozen-manifest pointer is its own section, so shortening the file listing never drops it | `report._manifest_ref_lines` | implemented | | I-110 | Every malformed report shape is a named refusal, never a host exception from the validator itself | `report._check_restore`, `_check_evidence`, `_check_unresolved` | implemented | +| I-111 | Every report field that lands on a line the composer cannot shorten is length-bounded at record time | `report._bounded`, `_bounded_optional` | implemented | +| I-112 | A collection field that is not a list is refused rather than iterated, so a mapping never becomes a list of its own keys | `report._sequence` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 389d5a4..9dc31b9 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -44,6 +44,9 @@ SUMMARY_MAX = 1200 ACTION_MAX = 1200 REASON_MAX = 600 +# The short identifying fields. Each one lands on a line the composer cannot shorten, so an +# unbounded value there is the same undeliverable report by a different route. +LABEL_MAX = 300 def _size(text) -> int: @@ -104,6 +107,12 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar summary = _bounded(_required(summary, "summary"), "summary", SUMMARY_MAX) next_action = _bounded(_required(next_action, "next_action"), "next_action", ACTION_MAX) reason = _bounded(_required(cxc_reason, "cxc_reason"), "cxc_reason", REASON_MAX) + pr_state = _bounded_optional(pr_state, "pr_state") + pr_url = _bounded_optional(pr_url, "pr_url") + base_ref = _bounded_optional(base_ref, "base_ref") + base_sha = _bounded_optional(base_sha, "base_sha") + head_sha = _bounded_optional(head_sha, "head_sha") + criteria_digest = _bounded_optional(criteria_digest, "criteria_digest") if pr_number is not None: if isinstance(pr_number, bool) or not isinstance(pr_number, int) or pr_number < 1: raise ReceiptRefused( @@ -241,6 +250,18 @@ def _bounded(text, field, limit): return text +def _bounded_optional(value, field, limit=LABEL_MAX): + """The short fields, bounded too. They sit on lines that cannot be shortened either.""" + if value is None: + return None + if not isinstance(value, str): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} is a string when it is given at all, not {type(value).__name__}", + ) + return _bounded(value, field, limit) + + def _check_resubmission(store, event_id, submission_no) -> None: """Once a message has gone out, a changed report is a new submission and says so. @@ -341,6 +362,23 @@ def _check_review(review): return {"kind": kind, "blockers": blockers, "findings": findings} +def _sequence(entries, field): + """A mapping iterates as its keys and a scalar does not iterate at all. + + Without this, a dict quietly became a list of its own key strings and a number raised + TypeError out of the validator, so the one guarantee this layer makes, that every + malformed shape comes back as a named refusal, did not hold at the top level. + """ + if entries is None: + return [] + if not isinstance(entries, (list, tuple)): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} is a list of entries, not {type(entries).__name__}", + ) + return list(entries) + + def _check_evidence(entries): """Reject a malformed entry HERE, where a caller can fix it. @@ -349,7 +387,7 @@ def _check_evidence(entries): be rendered must therefore be refused at the point it is recorded. """ checked = [] - for item in entries or []: + for item in _sequence(entries, "evidence"): if isinstance(item, str): checked.append(item) continue @@ -369,7 +407,7 @@ def _check_evidence(entries): def _check_unresolved(entries): checked = [] - for item in entries or []: + for item in _sequence(entries, "unresolved"): if isinstance(item, str): checked.append(item) continue diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 4c40523..3607573 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -634,6 +634,36 @@ def test_a_malformed_restore_section_is_refused_rather_than_crashing(self): ), ) + def test_a_collection_that_is_not_a_list_is_refused_not_iterated(self): + _relationship, event_id = self.queued_event() + # A mapping iterates as its own keys, so this used to be silently accepted as a list + # of strings, and a scalar raised TypeError out of the validator. + for field, bad in (("evidence", {"check": "python3 -m pytest"}), + ("evidence", 7), ("unresolved", {"id": "c-1"}), + ("unresolved", 7)): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field, bad=bad: report.record( + self.store, self.clock, event_id=event_id, **a_report(**{field: bad}) + ), + ) + self.assertIn("is a list of entries", error.detail) + + def test_every_line_the_composer_cannot_shorten_is_bounded(self): + _relationship, event_id = self.queued_event() + for field in ("pr_state", "pr_url", "base_ref", "base_sha", "head_sha", + "criteria_digest"): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field: report.record( + self.store, self.clock, event_id=event_id, + **a_report(**{field: "x" * 10000}) + ), + ) + # Left unbounded, a 10 KB pr_state was accepted and then made every claim raise. + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.assertIsNotNone(self.attempt(event_id)) + class VerdictPosition(Directions): def _revision_report(self, **overrides): From de72bb851cf5dde34b06e85a784b32b389c3744e Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:51:26 +0900 Subject: [PATCH 09/27] Keep every submission of a report, not just the newest A report was keyed on the event alone, so recording submission 2 overwrote the only full copy of submission 1. That breaks the promise the earlier message made: its omission notice sent the recipient to show --event for the evidence and unresolved items it had to elide, and after the overwrite those were gone, while the frozen attempt bytes hold only the elided text. work_reports is now keyed on the event and the submission, read() returns the current one, read_all() returns all of them oldest first, and show carries both. Two more from the same round: A review passed as a bare string reached .get() and raised AttributeError, so the named-refusal guarantee did not hold for that container either. The review object, its findings list and each finding are shape-checked now. pr_url was bounded at the label limit, which is the ceiling for lines the composer cannot shorten. The url is not one of those: it sits on a line that can be dropped, so a real forge url over 300 bytes was being rejected for a report that would have rendered fine. It gets a storage ceiling instead. A tuple stays accepted for evidence and unresolved. The hazard the shape check exists for is a mapping iterating as its keys and a string iterating as characters; a tuple has the same semantics as the list it normalises to. The invariant wording now says ordered sequence, which is what the code does. --- .../codex-session-relay/docs/invariants.md | 3 +- .../src/codex_session_relay/cli.py | 5 +- .../src/codex_session_relay/report.py | 56 +++++++++++++++-- .../src/codex_session_relay/store.py | 11 +++- .../tests/test_report_contract.py | 60 +++++++++++++++++++ 5 files changed, 124 insertions(+), 11 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index f4a7605..2785f6a 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -127,7 +127,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-109 | The frozen-manifest pointer is its own section, so shortening the file listing never drops it | `report._manifest_ref_lines` | implemented | | I-110 | Every malformed report shape is a named refusal, never a host exception from the validator itself | `report._check_restore`, `_check_evidence`, `_check_unresolved` | implemented | | I-111 | Every report field that lands on a line the composer cannot shorten is length-bounded at record time | `report._bounded`, `_bounded_optional` | implemented | -| I-112 | A collection field that is not a list is refused rather than iterated, so a mapping never becomes a list of its own keys | `report._sequence` | implemented | +| I-112 | A collection field that is not an ordered sequence is refused rather than iterated, so a mapping never becomes a list of its own keys and a string never becomes a list of characters | `report._sequence` | implemented | +| I-113 | Recording a later submission preserves the earlier one, so a recipient holding an older elided message can still recover what it promised | `report.record` keyed on (event, submission); `report.read_all`; `cli.cmd_show` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/cli.py b/packages/codex-session-relay/src/codex_session_relay/cli.py index 2bbc18d..6fbc5ad 100644 --- a/packages/codex-session-relay/src/codex_session_relay/cli.py +++ b/packages/codex-session-relay/src/codex_session_relay/cli.py @@ -539,9 +539,12 @@ def cmd_show(services, args) -> dict: # omission notice sends the recipient here, so this is the one place that must always # carry every field the message could have dropped. Imported locally to keep this change # out of the module import block, which a parallel branch is editing. - from .report import read as read_work_report + from .report import read as read_work_report, read_all as read_work_reports payload["workReport"] = read_work_report(services.store, args.event) + # Every submission, because an earlier message may have elided part of its report and + # sent its recipient here for the rest. + payload["workReportSubmissions"] = read_work_reports(services.store, args.event) if delivery is not None and args.message: # The bytes each attempt actually froze, with how far they got. A preview is offered # only when nothing has been prepared, and it is labelled a preview, because the old diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 9dc31b9..bc6f22b 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -47,6 +47,10 @@ # The short identifying fields. Each one lands on a line the composer cannot shorten, so an # unbounded value there is the same undeliverable report by a different route. LABEL_MAX = 300 +# A url sits on a line the composer CAN drop, so it only needs a storage ceiling rather than +# a fits-on-one-line ceiling. Bounding it like a label rejected real forge urls that would +# have rendered perfectly well. +URL_MAX = 2000 def _size(text) -> int: @@ -108,7 +112,7 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar next_action = _bounded(_required(next_action, "next_action"), "next_action", ACTION_MAX) reason = _bounded(_required(cxc_reason, "cxc_reason"), "cxc_reason", REASON_MAX) pr_state = _bounded_optional(pr_state, "pr_state") - pr_url = _bounded_optional(pr_url, "pr_url") + pr_url = _bounded_optional(pr_url, "pr_url", URL_MAX) base_ref = _bounded_optional(base_ref, "base_ref") base_sha = _bounded_optional(base_sha, "base_sha") head_sha = _bounded_optional(head_sha, "head_sha") @@ -162,8 +166,8 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar " base_sha, head_sha, criteria_digest, cxc_status, cxc_reason, contract_version," " summary, evidence, unresolved, next_action, review, restore, recorded_at)" " VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)" - " ON CONFLICT(event_id) DO UPDATE SET" - " submission_no = excluded.submission_no, repository = excluded.repository," + " ON CONFLICT(event_id, submission_no) DO UPDATE SET" + " repository = excluded.repository," " pr_number = excluded.pr_number, pr_url = excluded.pr_url," " pr_state = excluded.pr_state, base_ref = excluded.base_ref," " base_sha = excluded.base_sha, head_sha = excluded.head_sha," @@ -192,9 +196,31 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar def read(store, event_id: str): - row = store.one("SELECT * FROM work_reports WHERE event_id = ?", (event_id,)) + """The current submission. Earlier ones are still there; see read_all.""" + row = store.one( + "SELECT * FROM work_reports WHERE event_id = ?" + " ORDER BY submission_no DESC LIMIT 1", + (event_id,), + ) if row is None: return None + return _row(row) + + +def read_all(store, event_id: str) -> list: + """Every submission, oldest first. + + A message that had to elide part of its report points its recipient at the full record. + If a later submission replaced the only stored copy, that promise would break for anyone + still holding the older message, so the rows are kept and this is how they are read. + """ + rows = store.all( + "SELECT * FROM work_reports WHERE event_id = ? ORDER BY submission_no", (event_id,) + ) + return [_row(row) for row in rows] + + +def _row(row) -> dict: return { "eventId": row["event_id"], "relationshipId": row["relationship_id"], @@ -272,9 +298,10 @@ def _check_resubmission(store, event_id, submission_no) -> None: answering. Correcting a report before anything is sent stays free. """ existing = store.one( - "SELECT submission_no FROM work_reports WHERE event_id = ?", (event_id,) + "SELECT MAX(submission_no) AS submission_no FROM work_reports WHERE event_id = ?", + (event_id,), ) - if existing is None: + if existing is None or existing["submission_no"] is None: return attempted = store.one( "SELECT 1 FROM attempts WHERE event_id = ? LIMIT 1", (event_id,) @@ -341,6 +368,18 @@ def _check_restore(restore): def _check_review(review): if review is None: return None + if not isinstance(review, dict): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a review is an object with a kind and its findings, not " + f"{type(review).__name__}; a bare verdict word is not a review", + ) + findings_in = review.get("findings") + if findings_in is not None and not isinstance(findings_in, (list, tuple)): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"review findings is a list, not {type(findings_in).__name__}", + ) kind = review.get("kind") blockers = review.get("blockers") # Raises on an unknown kind, on GO-WITH-FIXES without a count, and on a count attached @@ -348,6 +387,11 @@ def _check_review(review): cxc.verdict_line(kind, blockers) findings = [] for item in review.get("findings") or []: + if not isinstance(item, dict): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"each review finding is an object naming a criterion, not {item!r}", + ) identifier = str(item.get("id") or "").strip() if not identifier: raise ReceiptRefused( diff --git a/packages/codex-session-relay/src/codex_session_relay/store.py b/packages/codex-session-relay/src/codex_session_relay/store.py index 00ed11d..aa195ac 100644 --- a/packages/codex-session-relay/src/codex_session_relay/store.py +++ b/packages/codex-session-relay/src/codex_session_relay/store.py @@ -247,11 +247,11 @@ -- earlier report, and the repository is stored beside the number so the same pull request -- number on two projects stays two pull requests. CREATE TABLE IF NOT EXISTS work_reports ( - event_id TEXT PRIMARY KEY, + event_id TEXT NOT NULL, + submission_no INTEGER NOT NULL DEFAULT 1, relationship_id TEXT NOT NULL, execution_generation INTEGER NOT NULL, revision_hash TEXT NOT NULL, - submission_no INTEGER NOT NULL DEFAULT 1, repository TEXT NOT NULL, pr_number INTEGER, pr_url TEXT, @@ -269,7 +269,12 @@ next_action TEXT NOT NULL, review TEXT, restore TEXT, - recorded_at TEXT NOT NULL + recorded_at TEXT NOT NULL, + -- Keyed on the submission too, so recording a later one preserves the earlier row. An + -- earlier message may have elided part of its report and told its recipient to read the + -- rest with show; overwriting the only full copy would break that promise for anyone + -- still holding the older message. + PRIMARY KEY (event_id, submission_no) ); -- How an acknowledgement's own turn was established. host_read is an App Server read of the diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 3607573..77a394b 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -664,6 +664,66 @@ def test_every_line_the_composer_cannot_shorten_is_bounded(self): report.record(self.store, self.clock, event_id=event_id, **a_report()) self.assertIsNotNone(self.attempt(event_id)) + def test_a_forge_url_is_bounded_for_storage_not_for_one_line(self): + _relationship, event_id = self.queued_event() + # The url sits on a line the composer CAN drop, so a label-sized ceiling rejected + # real urls that would have rendered perfectly well. + long_url = "https://example.invalid/" + "a" * 400 + report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_url=long_url)) + self.assertIn(long_url, self.delivery.render_message(event_id)) + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_url="https://x.invalid/" + "a" * 3000)), + ) + + def test_a_bare_verdict_word_is_not_a_review(self): + _relationship, event_id = self.queued_event() + for bad in ("PASS", 1, ["c-1"], {"kind": cxc.PASS, "findings": "c-1"}, + {"kind": cxc.PASS, "findings": ["c-1"]}): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, **a_report(review=bad) + ), + ) + + def test_a_tuple_is_an_ordered_sequence_and_stays_accepted(self): + _relationship, event_id = self.queued_event() + # The hazard the shape check exists for is a mapping iterating as its keys and a + # string iterating as characters. A tuple has the same semantics as the list it + # normalises to, so refusing it would be pedantry rather than protection. + report.record(self.store, self.clock, event_id=event_id, + **a_report(evidence=("pytest passed",), unresolved=("one thing",))) + stored = report.read(self.store, event_id) + self.assertEqual(stored["evidence"], ["pytest passed"]) + self.assertEqual(stored["unresolved"], ["one thing"]) + + def test_a_later_submission_does_not_erase_what_an_earlier_message_promised(self): + from codex_session_relay import cli + + _relationship, event_id = self.queued_event() + first = [f"finding {n}: something specific" for n in range(30)] + report.record(self.store, self.clock, event_id=event_id, + **a_report(unresolved=first, summary="first submission")) + self.attempt(event_id) + report.record(self.store, self.clock, event_id=event_id, + **a_report(unresolved=["only this now"], summary="second submission", + submission_no=2)) + # The current report is the second one. + self.assertEqual(report.read(self.store, event_id)["summary"], "second submission") + # The first is still whole, which is what its own elided message pointed at. + every = report.read_all(self.store, event_id) + self.assertEqual([r["submissionNo"] for r in every], [1, 2]) + self.assertEqual(every[0]["unresolved"], first) + services = type("S", (), {"store": self.store, "delivery": self.delivery, + "intake": self.intake})() + args = type("A", (), {"event": event_id, "message": False})() + payload = cli.cmd_show(services, args) + self.assertEqual(len(payload["workReportSubmissions"]), 2) + self.assertEqual(payload["workReportSubmissions"][0]["unresolved"], first) + class VerdictPosition(Directions): def _revision_report(self, **overrides): From a22c02024061a7cbd8b207e41d2e0960c990975b Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:57:47 +0900 Subject: [PATCH 10/27] Decide a resubmission inside the lock that writes it _check_resubmission ran before BEGIN IMMEDIATE, so two recorders could both approve the same next submission number and a delivery could open an attempt between the check and the upsert. The rest of this package reads inside the caller transaction for exactly this reason; this now does too. The write also stopped naming a conflict target. A target has to match a constraint in the schema the database was actually created with, and CREATE TABLE IF NOT EXISTS never reshapes an existing table, so naming one made every report write depend on which revision of this table a store started life on. Delete then insert inside the same transaction has no such dependency. The limit that remains is recorded: no released version has this table, so a store built from an intermediate revision of this change is recreated rather than upgraded. Two ordering bugs in the validation: The PASS conflict check read review.get() before any shape check, so a revision report carrying a bare string raised AttributeError instead of a named refusal. Shape is checked first now. A completion report accepted a review that render_completion never reads, so a supplied PASS or FAIL was stored and silently never delivered, with no omission notice either. Only a correction carries a review. --- .../codex-session-relay/docs/invariants.md | 1 + .../src/codex_session_relay/report.py | 72 ++++++++++++------- .../tests/test_report_contract.py | 25 +++++++ 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 2785f6a..96b58bb 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -142,3 +142,4 @@ status. Every row below is implemented and carries a test; the suite is the proo | Terminal turns are polled, not subscribed | the transport cannot subscribe, so automatic invocation is a bounded poll that then dispatches | | Archive state can be unknown | an inconclusive listing withholds rather than guessing, and a later observation releases it | | A head commit is not observable from here | the relay cannot watch a forge, so `assert_current` enforces generation on the delivery path and takes `head_sha` only from a caller that already knows the current head. A push that changes the declared manifest is structurally a new event, because the revision hash and therefore the event id change with it; a push that changes nothing declared is not, and `_check_resubmission` is what stops an old report standing for it silently | +| `work_reports` ships with its composite key | the schema is applied with `CREATE TABLE IF NOT EXISTS`, which never reshapes an existing table, so a store created from an intermediate revision of this change that used an event-only key cannot hold a second submission. No released version has this table, so there is nothing to migrate; a store built from such a revision is recreated rather than upgraded. The write itself no longer names a conflict target, so it does not depend on which revision created the table | diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index bc6f22b..2261da8 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -96,16 +96,31 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar cxc.check_known(cxc_status) else: cxc.check_status(cxc_status, outcome) - if outcome == REVISION_OUTCOME and (review or {}).get("kind") == cxc.PASS: - # A revision event exists because the parent ruled needs_changes. A PASS verdict on - # it would tell the child its work was approved in the same message that demands - # changes, and the review half of a report is caller-supplied even now that identity - # is not. + # Shape before meaning. Reading review.get() ahead of _check_review assumed every truthy + # review was a mapping, so a revision report carrying a bare string raised AttributeError + # out of the validator instead of coming back as a named refusal. + review = _check_review(review) + if outcome == REVISION_OUTCOME: + if (review or {}).get("kind") == cxc.PASS: + # A revision event exists because the parent ruled needs_changes. A PASS verdict + # on it would tell the child its work was approved in the same message that + # demands changes, and the review half of a report is caller-supplied even now + # that identity is not. + raise ReceiptRefused( + RefusalReason.DISPOSITION_CONFLICT, + "a revision request cannot carry a PASS verdict: this event exists because " + "the parent ruled needs_changes, and the message would approve and demand " + "changes at the same time", + ) + elif review is not None: + # Only a correction renders a verdict line. Storing a review on a completion would + # accept a PASS or FAIL that the delivered message never shows and never says it + # dropped, which is the silent loss the omission notice exists to prevent. raise ReceiptRefused( RefusalReason.DISPOSITION_CONFLICT, - "a revision request cannot carry a PASS verdict: this event exists because the " - "parent ruled needs_changes, and the message would approve and demand changes " - "at the same time", + f"a {outcome!r} report carries no review: a verdict line belongs to a correction, " + "so this judgment would be stored and never delivered. Put the reviewers findings " + "in the unresolved items, or record the review on the revision request", ) repository = _bounded(_required(repository, "repository"), "repository", 200) summary = _bounded(_required(summary, "summary"), "summary", SUMMARY_MAX) @@ -129,11 +144,9 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar "a report naming a pull request names the head commit it is about; without " "one, a later push silently inherits this report", ) - review = _check_review(review) evidence = _check_evidence(evidence) unresolved = _check_unresolved(unresolved) restore = _check_restore(restore) - _check_resubmission(store, event_id, submission_no) row = { "eventId": event_id, "relationshipId": relationship_id, @@ -160,23 +173,25 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar } now = clock.iso() with store.transaction() as db: + # Re-read inside the write lock. Two recorders can both pass a preflight check and + # both claim the same next submission, and a delivery can open an attempt between a + # preflight read and this commit. Everything this decides can move, which is why the + # rest of this package reads inside the caller transaction rather than before it. + _assert_resubmission(db, event_id, row["submissionNo"]) + # Delete then insert, rather than an upsert with a conflict target. A conflict target + # has to match a constraint in the schema the database was actually created with, and + # CREATE TABLE IF NOT EXISTS never changes an existing one, so naming one here made + # the write depend on which version of this table a store happened to start life on. + db.execute( + "DELETE FROM work_reports WHERE event_id = ? AND submission_no = ?", + (event_id, row["submissionNo"]), + ) db.execute( "INSERT INTO work_reports (event_id, relationship_id, execution_generation," " revision_hash, submission_no, repository, pr_number, pr_url, pr_state, base_ref," " base_sha, head_sha, criteria_digest, cxc_status, cxc_reason, contract_version," " summary, evidence, unresolved, next_action, review, restore, recorded_at)" - " VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)" - " ON CONFLICT(event_id, submission_no) DO UPDATE SET" - " repository = excluded.repository," - " pr_number = excluded.pr_number, pr_url = excluded.pr_url," - " pr_state = excluded.pr_state, base_ref = excluded.base_ref," - " base_sha = excluded.base_sha, head_sha = excluded.head_sha," - " criteria_digest = excluded.criteria_digest, cxc_status = excluded.cxc_status," - " cxc_reason = excluded.cxc_reason, contract_version = excluded.contract_version," - " summary = excluded.summary, evidence = excluded.evidence," - " unresolved = excluded.unresolved, next_action = excluded.next_action," - " review = excluded.review, restore = excluded.restore," - " recorded_at = excluded.recorded_at", + " VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", ( event_id, relationship_id, row["executionGeneration"], revision_hash, row["submissionNo"], repository, pr_number, pr_url, pr_state, base_ref, @@ -288,7 +303,7 @@ def _bounded_optional(value, field, limit=LABEL_MAX): return _bounded(value, field, limit) -def _check_resubmission(store, event_id, submission_no) -> None: +def _assert_resubmission(db, event_id, submission_no) -> None: """Once a message has gone out, a changed report is a new submission and says so. The bytes of every attempt stay frozen in attempt_messages, so history is never lost. @@ -296,16 +311,19 @@ def _check_resubmission(store, event_id, submission_no) -> None: already been attempted, so a retry carries different instructions under the same event and the same stated submission. The recipient would have no way to tell which one it was answering. Correcting a report before anything is sent stays free. + + Takes the transaction handle rather than the store, so this cannot be satisfied by a + read that was already stale by the time the row was written. """ - existing = store.one( + existing = db.execute( "SELECT MAX(submission_no) AS submission_no FROM work_reports WHERE event_id = ?", (event_id,), - ) + ).fetchone() if existing is None or existing["submission_no"] is None: return - attempted = store.one( + attempted = db.execute( "SELECT 1 FROM attempts WHERE event_id = ? LIMIT 1", (event_id,) - ) + ).fetchone() if attempted is None: return if int(submission_no) <= existing["submission_no"]: diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 77a394b..375642f 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -530,6 +530,20 @@ def test_a_revision_report_does_not_have_to_invent_a_receipt_outcome(self): self.assertEqual(stored["cxcStatus"], cxc.NEEDS_HUMAN) self.assertIn("NEEDS_HUMAN", str(report.read(self.store, revision_event))) + def test_a_malformed_review_on_a_correction_is_refused_not_a_crash(self): + _source, revision_event = self._revision() + # The PASS conflict check used to read review.get() before any shape check, so a + # truthy non-mapping raised AttributeError out of the validator on this path only. + for bad in ("PASS", 1, ["c-1"]): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="incomplete", + review=bad) + ), + ) + def test_the_recorded_verdict_decides_which_criteria_a_correction_names(self): _source, revision_event = self._revision() receipt = self.intake.get(revision_event) @@ -689,6 +703,17 @@ def test_a_bare_verdict_word_is_not_a_review(self): ), ) + def test_a_completion_cannot_carry_a_review_nobody_would_ever_see(self): + _relationship, event_id = self.queued_event() + error = self.assertRefused( + RefusalReason.DISPOSITION_CONFLICT, + lambda: report.record( + self.store, self.clock, event_id=event_id, + **a_report(review={"kind": cxc.PASS, "findings": []}) + ), + ) + self.assertIn("stored and never delivered", error.detail) + def test_a_tuple_is_an_ordered_sequence_and_stays_accepted(self): _relationship, event_id = self.queued_event() # The hazard the shape check exists for is a mapping iterating as its keys and a From 60f3163121dd929c9027c2871d8c1e0942904998 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:01:57 +0900 Subject: [PATCH 11/27] Say which submission a message came from, and keep the commit context show now returns every preserved submission, so a recipient holding an older message needs to know which one it is reading. The completion message already said so; the revision message did not. Both do now, and since the bytes are frozen per attempt, the message itself is the attempt-to-submission record. A report with no pull request dropped its baseRef, baseSha, headSha and criteriaDigest on an early return, with no omission notice. Blocked and budget-exhausted work is exactly the case that has no PR yet and still has a branch point and a commit worth naming, so the parent was losing the context it needed to act. The commit lines are shared between both branches now. --- .../codex-session-relay/docs/invariants.md | 2 ++ .../src/codex_session_relay/report.py | 20 ++++++++++++++----- .../tests/test_report_contract.py | 16 +++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 96b58bb..e769504 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -129,6 +129,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-111 | Every report field that lands on a line the composer cannot shorten is length-bounded at record time | `report._bounded`, `_bounded_optional` | implemented | | I-112 | A collection field that is not an ordered sequence is refused rather than iterated, so a mapping never becomes a list of its own keys and a string never becomes a list of characters | `report._sequence` | implemented | | I-113 | Recording a later submission preserves the earlier one, so a recipient holding an older elided message can still recover what it promised | `report.record` keyed on (event, submission); `report.read_all`; `cli.cmd_show` | implemented | +| I-114 | Every delivered message states its report submission, so the frozen bytes identify which stored submission produced them | `report.render_completion`, `render_revision` | implemented | +| I-115 | A report with no pull request still renders its base, head and criteria digest rather than dropping them unannounced | `report._commit_lines` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 2261da8..6223cfc 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -722,7 +722,7 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: " --artifact [--continues-anchor ]", "", f"relay record: requestId {request}, eventId {event_id}," - f" contract {version_of(report)}", + f" submission {report['submissionNo']}, contract {version_of(report)}", f"Full record: {show_command(event_id)}", ], rank=0, essential=True, keep=9), ] @@ -741,11 +741,14 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: def _pr_lines(report): reference = pr_ref(report) if not reference: + # No pull request does not mean no revision context. A blocked or budget-exhausted + # report can still name the branch point and the commit it got to, and dropping + # those silently left the parent without what it needed to act. return [ "", f"repository: {report['repository']}", "pull request: none recorded for this event", - ] + ] + _commit_lines(report) lines = [ "", f"pull request: {reference}" @@ -753,10 +756,17 @@ def _pr_lines(report): ] if report.get("prUrl"): lines.append(f" url: {report['prUrl']}") + return lines + _commit_lines(report) + + +def _commit_lines(report): + """The branch point, the commit and the criteria set, whenever they are known.""" + lines = [] base = report.get("baseRef") or "" - if report.get("baseSha"): - lines.append(f" base: {base} {report['baseSha']}".rstrip()) - lines.append(f" head: {report.get('headSha')}") + if report.get("baseSha") or base: + lines.append(f" base: {base} {report.get('baseSha') or ''}".rstrip()) + if report.get("headSha"): + lines.append(f" head: {report['headSha']}") if report.get("criteriaDigest"): lines.append(f" criteria: {report['criteriaDigest']}") return lines diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 375642f..03169a0 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -472,6 +472,19 @@ def test_a_blocked_report_does_not_tell_the_reader_it_proved_anything(self): self.assertIn("a bound the plan actually stated ran out", message) self.assertNotIn("proving its own criteria", message) self.assertIn("pull request: none recorded", message) + # A report with no pull request still has revision context, and dropping it silently + # left the parent without what it needed to act. + report.record( + self.store, self.clock, event_id=event_id, + **a_report(cxc_status=cxc.BUDGET_EXHAUSTED, cxc_reason="the bound ran out", + pr_number=None, pr_url=None, head_sha="f" * 40, + base_ref="dev", base_sha="e" * 40, criteria_digest="d1e2f3") + ) + message = self.delivery.render_message(event_id) + self.assertIn("pull request: none recorded", message) + self.assertIn("base: dev " + "e" * 40, message) + self.assertIn("head: " + "f" * 40, message) + self.assertIn("criteria: d1e2f3", message) def test_the_budget_counts_bytes_because_a_transport_limit_does(self): relationship, event_id = self.queued_event() @@ -514,6 +527,9 @@ def test_the_review_verdict_is_the_last_line_a_scanner_reads(self): cxc.parse_verdict_line(message.splitlines()[-1]), {"kind": cxc.GO_WITH_FIXES, "blockers": 1}, ) + # The frozen bytes have to say which submission produced them, because show now + # returns several and a recipient holding an older message has only the message. + self.assertIn("submission 1", message) def test_a_revision_report_does_not_have_to_invent_a_receipt_outcome(self): _source, revision_event = self._revision() From 5aa00c2f1ef7fb284a546cef598a14faf0e94242 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:06:35 +0900 Subject: [PATCH 12/27] Protect the submission identifier from the shortening that removed it Consequence of the previous commit. The revision message identity sat at the end of the answer block, whose keep floor was two lines short of it, so shortening removed the submission number first. That is the one line that tells a recipient holding these bytes which of several stored submissions produced them, which was the whole point of adding it. It is its own section now, with a floor covering every line in it, and the elision test asserts it survives at two budgets. --- packages/codex-session-relay/docs/invariants.md | 2 +- .../codex-session-relay/src/codex_session_relay/report.py | 8 +++++++- .../codex-session-relay/tests/test_report_contract.py | 7 +++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index e769504..40f184c 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -129,7 +129,7 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-111 | Every report field that lands on a line the composer cannot shorten is length-bounded at record time | `report._bounded`, `_bounded_optional` | implemented | | I-112 | A collection field that is not an ordered sequence is refused rather than iterated, so a mapping never becomes a list of its own keys and a string never becomes a list of characters | `report._sequence` | implemented | | I-113 | Recording a later submission preserves the earlier one, so a recipient holding an older elided message can still recover what it promised | `report.record` keyed on (event, submission); `report.read_all`; `cli.cmd_show` | implemented | -| I-114 | Every delivered message states its report submission, so the frozen bytes identify which stored submission produced them | `report.render_completion`, `render_revision` | implemented | +| I-114 | Every delivered message states its report submission and survives elision doing so, so the frozen bytes identify which stored submission produced them | `report.render_completion`, `render_revision`; the identity is its own section with a floor covering it | implemented | | I-115 | A report with no pull request still renders its base, head and criteria digest rather than dropping them unannounced | `report._commit_lines` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 6223cfc..13d8ef7 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -720,11 +720,17 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: " --outcome ready_for_review --turn-thread " " --turn-id ", " --artifact [--continues-anchor ]", + ], rank=0, essential=True, keep=8), + # Its own section, with a floor that covers every line in it. Left at the end of the + # answer block, the submission identifier was the first thing shortening removed, and + # it is what tells a recipient which of several stored submissions produced the bytes + # it is holding. + _Section("relay record", [ "", f"relay record: requestId {request}, eventId {event_id}," f" submission {report['submissionNo']}, contract {version_of(report)}", f"Full record: {show_command(event_id)}", - ], rank=0, essential=True, keep=9), + ], rank=0, essential=True, keep=3), ] if review: # REVIEW-OUTPUT-01 puts the machine-scannable judgment on the FINAL line, so a diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 03169a0..dddb449 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -799,6 +799,13 @@ def test_an_elided_correction_still_ends_on_its_verdict(self): "otherwise a consumer stops finding the verdict in exactly the messages that " "had to drop something", ) + # And the identity survives too, because it is what selects one submission out of + # read_all for a recipient holding only these bytes. + self.assertIn("submission 1", message) + self.assertIn("requestId del-y-a1", message) + tighter = report.render_revision(row, receipt, "del-y-a1", stored, budget=2000) + self.assertIn("submission 1", tighter) + self.assertEqual(tighter.splitlines()[-1], "VERDICT: GO-WITH-FIXES (blockers=2)") def test_a_correction_cannot_approve_and_demand_changes_at_once(self): _source, revision_event = self._revision() From 9bb11cba0862dbf67d8cdaa4ad458ee2f8051469 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:11:10 +0900 Subject: [PATCH 13/27] Treat a first report after a legacy delivery as a change too _assert_resubmission read MAX(submission_no) first and returned early when it was null, so an event whose pre-contract message had already gone out could accept a first report as submission 1 and switch a retry from the legacy message to report-backed instructions without saying anything had changed. A delivered pre-contract message now counts as submission 1, so the first report after one has to announce itself as submission 2. An invalid verdict in an otherwise well-shaped review raised ValueError out of cxc.verdict_line instead of a named refusal. Every other producer mistake in a report comes back as MALFORMED_RECEIPT; that one escaped as a host exception. It is translated now, keeping the original message. --- .../src/codex_session_relay/report.py | 44 ++++++++++++++----- .../tests/test_report_contract.py | 27 ++++++++++++ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 13d8ef7..e69e05c 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -315,24 +315,38 @@ def _assert_resubmission(db, event_id, submission_no) -> None: Takes the transaction handle rather than the store, so this cannot be satisfied by a read that was already stale by the time the row was written. """ - existing = db.execute( - "SELECT MAX(submission_no) AS submission_no FROM work_reports WHERE event_id = ?", - (event_id,), - ).fetchone() - if existing is None or existing["submission_no"] is None: - return attempted = db.execute( "SELECT 1 FROM attempts WHERE event_id = ? LIMIT 1", (event_id,) ).fetchone() if attempted is None: return - if int(submission_no) <= existing["submission_no"]: + existing = db.execute( + "SELECT MAX(submission_no) AS submission_no FROM work_reports WHERE event_id = ?", + (event_id,), + ).fetchone() + stored = existing["submission_no"] if existing else None + if stored is None: + # A message has already gone out for this event and it was the pre-contract one, so + # the FIRST report is also a change to what a retry will say. Counting the delivered + # legacy message as submission 1 is what makes that change announce itself; the null + # MAX used to return early here and let the switch happen silently. + delivered = 1 + detail = ( + "a pre-contract message has already been delivered for this event, so a first " + "report would change what a retry says without changing what it calls itself; " + "record it as submission 2 or higher" + ) + else: + delivered = stored + detail = ( + f"submission {stored} of this report has already been delivered, so replacing it " + f"in place would change what a retry says without changing what it calls itself; " + f"record this as submission {stored + 1} or higher" + ) + if int(submission_no) <= delivered: raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, - f"submission {existing['submission_no']} of this report has already been " - f"delivered, so replacing it in place would change what a retry says without " - f"changing what it calls itself; record this as submission " - f"{existing['submission_no'] + 1} or higher", + detail, ) @@ -402,7 +416,13 @@ def _check_review(review): blockers = review.get("blockers") # Raises on an unknown kind, on GO-WITH-FIXES without a count, and on a count attached # to PASS or FAIL. Rendering the line here is what makes those refusals reachable. - cxc.verdict_line(kind, blockers) + # Translated, because everything else a producer can get wrong here comes back as a + # named refusal and a bare ValueError would be the one typo that escapes as a host + # exception instead. + try: + cxc.verdict_line(kind, blockers) + except ValueError as invalid: + raise ReceiptRefused(RefusalReason.MALFORMED_RECEIPT, str(invalid)) from invalid findings = [] for item in review.get("findings") or []: if not isinstance(item, dict): diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index dddb449..3f00299 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -635,6 +635,33 @@ def test_a_report_already_delivered_cannot_be_replaced_in_place(self): **a_report(summary="openly revised", submission_no=2)) self.assertIn("submission: 2", self.delivery.preview_message(event_id)) + def test_a_first_report_after_a_legacy_delivery_is_also_a_change(self): + _relationship, event_id = self.queued_event() + # Nothing recorded, so this attempt froze the pre-contract message. + self.attempt(event_id) + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, **a_report()), + ) + self.assertIn("pre-contract message has already been delivered", error.detail) + # Announced, it is allowed, and the retry says which submission it is. + report.record(self.store, self.clock, event_id=event_id, + **a_report(submission_no=2)) + self.assertIn("submission: 2", self.delivery.preview_message(event_id)) + + def test_an_invalid_verdict_is_a_refusal_not_a_host_exception(self): + _source = self.queued_event() + _relationship, revision_like = _source + for bad in ({"kind": "LOOKS FINE"}, {"kind": cxc.GO_WITH_FIXES}, + {"kind": cxc.GO_WITH_FIXES, "blockers": 0}, + {"kind": cxc.FAIL, "blockers": 3}): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=revision_like, **a_report(review=bad) + ), + ) + def test_a_report_backed_message_still_carries_the_frozen_manifest_pointer(self): _relationship, event_id = self.queued_event() receipt = dict(self.intake.get(event_id)) From e05a1c2fec64925ad5165a245d3c6aecdaa783c9 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:17:47 +0900 Subject: [PATCH 14/27] Protect the completion identity and stop counting sends that never happened Three from the same round, all consequences of the last two commits. The completion relay-record block was still nonessential, so a long report lost the request, event and submission identifiers before anything else, which is the same loss the revision renderer was fixed for. It is essential now, reordered so its floor protects exactly the lines that let a recipient pick its own submission out of the several that show returns. manifestRef arrives from the receipt, where the frozen contract sets no length, and it now lands on an essential line. A value larger than the whole message budget therefore made every delivery claim raise. It gets a bounded representation here, truncated visibly with the full value in the record, rather than a length limit imposed on a contract field this layer does not own. An attempt row is not a delivery. A settled attempt whose record says sendAttempted is no was refused before the send, so counting it as a delivered submission left a gap in the numbering for a message nobody received. Attempts still in flight, or whose record cannot be read, stay counted, which is the same conservative reading reconciliation uses: an unfinished receipt is never proof of non-delivery. --- .../codex-session-relay/docs/invariants.md | 2 + .../src/codex_session_relay/report.py | 51 ++++++++++++++++--- .../tests/test_report_contract.py | 39 ++++++++++++-- 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 40f184c..e5f7971 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -131,6 +131,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-113 | Recording a later submission preserves the earlier one, so a recipient holding an older elided message can still recover what it promised | `report.record` keyed on (event, submission); `report.read_all`; `cli.cmd_show` | implemented | | I-114 | Every delivered message states its report submission and survives elision doing so, so the frozen bytes identify which stored submission produced them | `report.render_completion`, `render_revision`; the identity is its own section with a floor covering it | implemented | | I-115 | A report with no pull request still renders its base, head and criteria digest rather than dropping them unannounced | `report._commit_lines` | implemented | +| I-116 | An attempt that is proven never to have sent is not counted as a delivered submission; anything unproven is | `report._may_have_reached` | implemented | +| I-117 | A manifest reference too long to render is truncated visibly rather than making the event unsendable | `report._manifest_ref_lines` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index e69e05c..fb120b3 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -51,6 +51,10 @@ # a fits-on-one-line ceiling. Bounding it like a label rejected real forge urls that would # have rendered perfectly well. URL_MAX = 2000 +# A manifestRef arrives from the receipt, where the frozen contract sets no length. It lands +# on an essential line now, so it is given a bounded REPRESENTATION here rather than a limit +# imposed on a contract field this layer does not own. +REF_SHOWN = 240 def _size(text) -> int: @@ -316,9 +320,9 @@ def _assert_resubmission(db, event_id, submission_no) -> None: read that was already stale by the time the row was written. """ attempted = db.execute( - "SELECT 1 FROM attempts WHERE event_id = ? LIMIT 1", (event_id,) - ).fetchone() - if attempted is None: + "SELECT record FROM attempts WHERE event_id = ?", (event_id,) + ).fetchall() + if not _may_have_reached(attempted): return existing = db.execute( "SELECT MAX(submission_no) AS submission_no FROM work_reports WHERE event_id = ?", @@ -350,6 +354,28 @@ def _assert_resubmission(db, event_id, submission_no) -> None: ) +def _may_have_reached(attempt_rows) -> bool: + """Could any of these attempts have put bytes in front of the recipient. + + An attempt row is not a delivery. A settled attempt whose record says sendAttempted is + no was refused before the send, so its frozen bytes never reached anyone, and counting + it as a delivered submission would leave a gap in the numbering for nothing. Anything + else, including an attempt still in flight or one whose record cannot be read, is + treated as possibly delivered, which is the reading reconciliation already uses: an + unfinished receipt is never proof of non-delivery. + """ + for row in attempt_rows or []: + if row["record"] is None: + return True + try: + record = json.loads(row["record"]) + except (TypeError, ValueError): + return True + if record.get("sendAttempted") != "no": + return True + return False + + def _check_restore(restore): """A skill pointer nobody owns is another render-time failure inside the claim. @@ -669,15 +695,18 @@ def render_completion(row, receipt, request, report, *, budget=BUDGET) -> str: _Section("relay record", [ "", "relay record:", - f" contract: {version_of(report)}", f" requestId: {request}", f" eventId: {event_id}", + f" submission: {report['submissionNo']} contract: {version_of(report)}", f" relationshipId: {row['relationship_id']}", f" executionGeneration: {receipt.get('executionGeneration')}", - f" attempt: {receipt.get('attempt')} submission: {report['submissionNo']}", + f" attempt: {receipt.get('attempt')}", f" outcome: {receipt.get('outcome')}", f" revisionHash: {receipt.get('revisionHash')}", - ], rank=5), + # The first five lines are ordered so the floor protects exactly what lets a + # recipient pick its own submission out of the several that show returns. The rest + # of the record can shorten. + ], rank=5, essential=True, keep=5), _Section("respond", _ack_lines(event_id), rank=0, essential=True, keep=7), ] return _compose(sections, event_id, budget=budget) @@ -954,7 +983,15 @@ def _manifest_ref_lines(receipt): stable location their artifacts can still be verified against. """ reference = receipt.get("manifestRef") - return ["", f"manifestRef: {reference}"] if reference else [] + if not reference: + return [] + text = str(reference) + if _size(text) > REF_SHOWN: + # Truncated visibly, never quietly. A pointer nobody can read is still better than a + # message that cannot be sent, and the whole value is in the record. + shown = text.encode("utf-8")[:REF_SHOWN].decode("utf-8", "ignore") + return ["", f"manifestRef: {shown}... (truncated; full value in the record)"] + return ["", f"manifestRef: {text}"] def _ack_lines(event_id): diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 3f00299..0c9a55d 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -185,7 +185,7 @@ def test_more_than_ten_findings_are_never_dropped_in_silence(self): stored = report.read(self.store, event_id) row = self.delivery.get(event_id) tight = report.render_completion( - row, receipt, "del-x-a1", stored, budget=1400, + row, receipt, "del-x-a1", stored, budget=1700, ) self.assertIn("omitted:", tight, "a shortened message says so") self.assertIn("show --event", tight, "and says where to read the rest") @@ -193,6 +193,37 @@ def test_more_than_ten_findings_are_never_dropped_in_silence(self): self.assertIn("... ", tight, "and the count of what is missing survives with it") self.assertIn("next:", tight, "the required next action is never what gets dropped") self.assertIn("ack-proof", tight, "nor the instruction for answering") + self.assertIn("submission: 1", tight, "nor which submission produced these bytes") + self.assertIn("requestId: del-x-a1", tight) + + def test_a_long_manifest_reference_is_truncated_visibly_not_left_unsendable(self): + _relationship, event_id = self.queued_event() + receipt = dict(self.intake.get(event_id)) + receipt["manifestRef"] = "/var/lib/relay/frozen/" + "z" * 9000 + report.record(self.store, self.clock, event_id=event_id, **a_report()) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + # The receipt field has no contract length limit, and the section is essential, so + # without a bounded representation this raised inside every delivery claim. + message = report.render_completion(row, receipt, "del-q-a1", stored) + self.assertIn("manifestRef: /var/lib/relay/frozen/", message) + self.assertIn("(truncated; full value in the record)", message) + self.assertLessEqual(len(message.encode("utf-8")), report.BUDGET) + + def test_an_attempt_that_never_sent_is_not_a_delivered_submission(self): + _relationship, event_id = self.queued_event() + # A scripted pre-send refusal DOES settle an attempt row, with sendAttempted no. Its + # frozen bytes never reached the recipient, so the first report is still submission 1 + # and the numbering keeps no gap for nothing. + self.adapter.script("busy") + record = self.attempt(event_id) + self.assertEqual(record["sendAttempted"], "no") + self.assertTrue(self.attempts_for(event_id), "the attempt row exists") + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.assertEqual(report.read(self.store, event_id)["submissionNo"], 1) + # The contrast, that a DISPATCHED attempt does count, is what + # Bounds.test_a_report_already_delivered_cannot_be_replaced_in_place asserts. So the + # distinction is the sendAttempted flag, not the mere presence of an attempt row. def test_an_impossible_budget_refuses_instead_of_shipping_a_gutted_message(self): relationship, event_id = self.queued_event() @@ -493,7 +524,7 @@ def test_the_budget_counts_bytes_because_a_transport_limit_does(self): report.record(self.store, self.clock, event_id=event_id, **a_report(summary=korean)) stored = report.read(self.store, event_id) row = self.delivery.get(event_id) - budget = 2200 + budget = 2500 message = report.render_completion(row, receipt, "del-z-a1", stored, budget=budget) self.assertLessEqual( len(message.encode("utf-8")), budget, @@ -676,7 +707,7 @@ def test_a_report_backed_message_still_carries_the_frozen_manifest_pointer(self) ) # And it survives the elision that drops the file listing, because it is the pointer # those files can still be verified against once they have moved. - tight = report.render_completion(row, receipt, "del-m-a1", stored, budget=1400) + tight = report.render_completion(row, receipt, "del-m-a1", stored, budget=1700) self.assertIn("omitted:", tight) self.assertIn("manifestRef: /var/lib/relay/frozen/abc123", tight) @@ -860,7 +891,7 @@ def test_the_command_the_omission_notice_names_returns_the_whole_report(self): stored = report.read(self.store, event_id) row = self.delivery.get(event_id) receipt = self.intake.get(event_id) - tight = report.render_completion(row, receipt, "del-r-a1", stored, budget=1400) + tight = report.render_completion(row, receipt, "del-r-a1", stored, budget=1700) self.assertIn("omitted:", tight) self.assertIn(report.show_command(event_id), tight) From 89c12eb30b45e1985b0c926939fdb9302cef9c25 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:25:05 +0900 Subject: [PATCH 15/27] Protect only the submissions that were actually frozen The resubmission guard compared against the highest STORED submission, so after submission 1 was delivered, recording submission 2 and then correcting it before any further attempt was refused even though every frozen attempt still carried submission 1. That made a correction impossible in the one window where it is harmless. I argued a round ago that the frozen bytes already carried the association, and for the recipient they do. The relay needs it as data, not prose, so attempt_report_submissions records which submission each attempt froze and the guard compares against that. An attempt with no row predates the contract and counts as submission 1, which keeps the legacy case working. Two more from the same round: A submission number is half the primary key and is printed in the frozen bytes, and int() was coercing True to 1 and 1.9 to 1, storing 0 and -1, and letting None and a string escape as TypeError and ValueError. Positive non-boolean integer or a named refusal now. An exit code could be any JSON value, so a mapping rendered as an exit code nobody can interpret and an arbitrary object failed inside json.dumps as a host exception. Integer or absent now: evidence a reader cannot interpret is not evidence. --- .../codex-session-relay/docs/invariants.md | 4 +- .../src/codex_session_relay/delivery.py | 12 ++- .../src/codex_session_relay/report.py | 87 ++++++++++++++----- .../src/codex_session_relay/store.py | 12 +++ .../tests/test_report_contract.py | 47 ++++++++++ 5 files changed, 136 insertions(+), 26 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index e5f7971..ffefa04 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -118,7 +118,7 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-100 | A report reads its relationship, generation, revision and outcome from the stored event; no caller supplies them | `report.record` | implemented | | I-101 | A correction names the criteria the recorded verdict names; a review only adds notes and anchors, and anything it raises alone is labelled | `report._finding_lines` | implemented | | I-102 | A shape or length that could only fail at render time is refused at record time, because rendering runs inside the delivery claim | `report._check_evidence`, `_check_unresolved`, `_bounded` | implemented | -| I-103 | A report already delivered cannot be replaced in place; a changed report is a new submission and says so | `report._check_resubmission` | implemented | +| I-103 | A submission already frozen into a delivered attempt cannot be replaced in place; one never sent stays correctable | `report._assert_resubmission` against `attempt_report_submissions` | implemented | | I-104 | An omission notice is placed before the final verdict, so an elided correction still ends on its judgment | `report._compose` | implemented | | I-105 | A revision request cannot carry a PASS verdict | `report.record` | implemented | | I-106 | A report-backed message keeps the receipt manifestRef the pre-contract message carried | `report._manifest_lines` | implemented | @@ -133,6 +133,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-115 | A report with no pull request still renders its base, head and criteria digest rather than dropping them unannounced | `report._commit_lines` | implemented | | I-116 | An attempt that is proven never to have sent is not counted as a delivered submission; anything unproven is | `report._may_have_reached` | implemented | | I-117 | A manifest reference too long to render is truncated visibly rather than making the event unsendable | `report._manifest_ref_lines` | implemented | +| I-118 | A submission number is a positive integer or a named refusal, never a coerced one, because it is half the identity and is printed in frozen bytes | `report._submission` | implemented | +| I-119 | An exit code is an integer or absent, so evidence a reader cannot interpret is refused rather than delivered | `report._exit_code` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/delivery.py b/packages/codex-session-relay/src/codex_session_relay/delivery.py index d4ce1a0..f6dcf24 100644 --- a/packages/codex-session-relay/src/codex_session_relay/delivery.py +++ b/packages/codex-session-relay/src/codex_session_relay/delivery.py @@ -301,8 +301,9 @@ def _claim(self, event_id: str, *, now: float, owner: str, recipient: str): f"request id {request_id!r} already belongs to event {clash['event_id']!r}", ) record = self.intake.get(event_id) or {} + report = read_work_report(self.store, event_id) message = self._render_for( - row, record, request_id, read_work_report(self.store, event_id) + row, record, request_id, report ) db.execute( "INSERT INTO attempts (request_id, event_id, attempt_no, kind, internal_state," @@ -317,6 +318,15 @@ def _claim(self, event_id: str, *, now: float, owner: str, recipient: str): " message, rendered_at) VALUES (?,?,?,?,?,?)", (request_id, event_id, attempt_no, row["kind"], message, self.clock.iso()), ) + if report is not None: + # Which submission these bytes came from. The message says so for its reader; + # this is the same fact in a form the relay can compare against, so a + # submission that has never been frozen stays correctable in place. + db.execute( + "INSERT OR REPLACE INTO attempt_report_submissions (request_id," + " event_id, submission_no, frozen_at) VALUES (?,?,?,?)", + (request_id, event_id, report["submissionNo"], self.clock.iso()), + ) # Capacity is reserved in the SAME transaction as the claim. Counting after the # send let two interleaved callers both pass a cap of one. self._count_send(db, recipient, now) diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index fb120b3..3f54c5a 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -151,12 +151,13 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar evidence = _check_evidence(evidence) unresolved = _check_unresolved(unresolved) restore = _check_restore(restore) + submission_no = _submission(submission_no) row = { "eventId": event_id, "relationshipId": relationship_id, "executionGeneration": int(execution_generation), "revisionHash": revision_hash, - "submissionNo": int(submission_no), + "submissionNo": submission_no, "repository": repository, "prNumber": pr_number, "prUrl": pr_url, @@ -295,6 +296,22 @@ def _bounded(text, field, limit): return text +def _submission(value): + """A positive integer, refused by name otherwise. + + This is half of the primary key and it is printed in frozen message bytes, so coercing + True to 1, or 1.9 to 1, or storing 0, would give an attempt an identity that means + something other than what it says. + """ + if isinstance(value, bool) or not isinstance(value, int) or value < 1: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a submission number is a positive integer, not {value!r}; it is part of this " + "report identity and is printed in the bytes that get frozen", + ) + return value + + def _bounded_optional(value, field, limit=LABEL_MAX): """The short fields, bounded too. They sit on lines that cannot be shortened either.""" if value is None: @@ -320,38 +337,42 @@ def _assert_resubmission(db, event_id, submission_no) -> None: read that was already stale by the time the row was written. """ attempted = db.execute( - "SELECT record FROM attempts WHERE event_id = ?", (event_id,) + "SELECT a.record, s.submission_no" + " FROM attempts a" + " LEFT JOIN attempt_report_submissions s ON s.request_id = a.request_id" + " WHERE a.event_id = ?", + (event_id,), ).fetchall() - if not _may_have_reached(attempted): + delivered = 0 + legacy = False + for row in attempted: + if not _may_have_reached([row]): + continue + if row["submission_no"] is None: + # A pre-contract message reached the recipient. The first report is therefore + # also a change to what a retry will say, so it counts as submission 1. + legacy = True + delivered = max(delivered, 1) + else: + delivered = max(delivered, row["submission_no"]) + if delivered == 0: return - existing = db.execute( - "SELECT MAX(submission_no) AS submission_no FROM work_reports WHERE event_id = ?", - (event_id,), - ).fetchone() - stored = existing["submission_no"] if existing else None - if stored is None: - # A message has already gone out for this event and it was the pre-contract one, so - # the FIRST report is also a change to what a retry will say. Counting the delivered - # legacy message as submission 1 is what makes that change announce itself; the null - # MAX used to return early here and let the switch happen silently. - delivered = 1 + if submission_no > delivered: + return + if legacy and delivered == 1: detail = ( "a pre-contract message has already been delivered for this event, so a first " "report would change what a retry says without changing what it calls itself; " "record it as submission 2 or higher" ) else: - delivered = stored detail = ( - f"submission {stored} of this report has already been delivered, so replacing it " - f"in place would change what a retry says without changing what it calls itself; " - f"record this as submission {stored + 1} or higher" - ) - if int(submission_no) <= delivered: - raise ReceiptRefused( - RefusalReason.MALFORMED_RECEIPT, - detail, + f"submission {delivered} of this report has already been frozen into a delivered " + f"attempt, so replacing it in place would change what a retry says without " + f"changing what it calls itself; record this as submission {delivered + 1} or " + "higher. A submission that has never been sent can still be corrected in place" ) + raise ReceiptRefused(RefusalReason.MALFORMED_RECEIPT, detail) def _may_have_reached(attempt_rows) -> bool: @@ -507,12 +528,30 @@ def _check_evidence(entries): ) checked.append({ "check": str(item["check"]).strip(), - "exitCode": item.get("exitCode"), + "exitCode": _exit_code(item.get("exitCode")), "detail": str(item.get("detail") or "").strip() or None, }) return checked +def _exit_code(value): + """An integer or nothing. Anything else is either not proof or not storable. + + A mapping here rendered as an exit code nobody can interpret, which is verification + evidence that says nothing, and an arbitrary object failed later inside json.dumps as a + host exception rather than a named refusal. + """ + if value is None: + return None + if isinstance(value, bool) or not isinstance(value, int): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"an exit code is an integer or absent, not {value!r}; evidence a reader cannot " + "interpret is not evidence", + ) + return value + + def _check_unresolved(entries): checked = [] for item in _sequence(entries, "unresolved"): diff --git a/packages/codex-session-relay/src/codex_session_relay/store.py b/packages/codex-session-relay/src/codex_session_relay/store.py index aa195ac..ff5d5d2 100644 --- a/packages/codex-session-relay/src/codex_session_relay/store.py +++ b/packages/codex-session-relay/src/codex_session_relay/store.py @@ -277,6 +277,18 @@ PRIMARY KEY (event_id, submission_no) ); +-- Which report submission the bytes frozen for one attempt were rendered from. The message +-- itself says so, which is what a recipient needs, but the relay needs it programmatically: +-- a submission that has never been frozen into an attempt can still be corrected in place, +-- and one that has cannot. An attempt with no row here predates the work report contract and +-- carries the pre-contract message. +CREATE TABLE IF NOT EXISTS attempt_report_submissions ( + request_id TEXT PRIMARY KEY, + event_id TEXT NOT NULL, + submission_no INTEGER NOT NULL, + frozen_at TEXT NOT NULL +); + -- How an acknowledgement's own turn was established. host_read is an App Server read of the -- recipient's real turn list; unverified is recorded intent still awaiting that read. There is -- deliberately no tier derived from what the relay itself sent: a stored dispatch proves a send diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 0c9a55d..8c1ebc7 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -693,6 +693,53 @@ def test_an_invalid_verdict_is_a_refusal_not_a_host_exception(self): ), ) + def test_a_submission_number_is_not_coerced_into_something_it_is_not(self): + _relationship, event_id = self.queued_event() + for bad in (True, 1.9, 0, -1, None, "bad"): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, + **a_report(submission_no=bad) + ), + ) + + def test_an_exit_code_a_reader_cannot_interpret_is_not_evidence(self): + _relationship, event_id = self.queued_event() + for bad in ({"code": 1}, "0", True, [1]): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, + **a_report(evidence=[{"check": "pytest", "exitCode": bad}]) + ), + ) + self.assertIn("an exit code is an integer or absent", error.detail) + report.record(self.store, self.clock, event_id=event_id, + **a_report(evidence=[{"check": "pytest"}, + {"check": "ruff", "exitCode": 1}])) + self.assertIn("ruff -> exit 1", self.delivery.render_message(event_id)) + + def test_a_submission_never_frozen_into_an_attempt_stays_correctable(self): + _relationship, event_id = self.queued_event() + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.attempt(event_id) + # Submission 1 is frozen into a delivered attempt, so 2 is required. + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="second", submission_no=2)) + # Nothing has frozen submission 2 yet, so correcting it in place is still free. + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="second, corrected", submission_no=2)) + self.assertEqual(report.read(self.store, event_id)["summary"], "second, corrected") + self.assertEqual([r["submissionNo"] for r in report.read_all(self.store, event_id)], + [1, 2]) + # Submission 1 was frozen, so it still cannot be rewritten. + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="rewriting history", submission_no=1)), + ) + def test_a_report_backed_message_still_carries_the_frozen_manifest_pointer(self): _relationship, event_id = self.queued_event() receipt = dict(self.intake.get(event_id)) From b67b876fe70d7dcf3f7c0dedb26193f7fc2c1891 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:34:22 +0900 Subject: [PATCH 16/27] Stop an inbox item from being rewritten behind the recipient _may_have_reached excluded every attempt whose record says sendAttempted no, and an inbox_only attempt says exactly that: the push was refused before any resume. But its frozen message is the durable inbox item and the recipient can read it, which protocol v1 section 3 calls the guarantee of the whole dual channel. So the report behind an inbox message could be rewritten in place, and anything that message had elided was then gone. Three more from the same round: A line break in summary, next_action, repository or cxc_reason was spliced straight into the message, where it does not wrap, it adds a line to the protocol. A summary ending in a second line reading VERDICT: PASS put a standalone verdict into a completion that carried no review, which is the exact separation the rest of this module is built to keep. Those fields are one line now, and so are the optional label fields. pr_url and pr_state were accepted with no pr_number and then never rendered, because the no-pull-request branch has nowhere to hang them. Stored and never shown, with no omission notice, is the failure this change exists to remove, so the combination is refused. cxc.check_known tested an unhashable status for dict membership and raised TypeError, so a structured status escaped the named-refusal contract. --- .../codex-session-relay/docs/invariants.md | 3 + .../src/codex_session_relay/cxc.py | 2 +- .../src/codex_session_relay/report.py | 51 ++++++++++++++- .../tests/test_report_contract.py | 63 ++++++++++++++++++- 4 files changed, 114 insertions(+), 5 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index ffefa04..ba54c56 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -135,6 +135,9 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-117 | A manifest reference too long to render is truncated visibly rather than making the event unsendable | `report._manifest_ref_lines` | implemented | | I-118 | A submission number is a positive integer or a named refusal, never a coerced one, because it is half the identity and is printed in frozen bytes | `report._submission` | implemented | | I-119 | An exit code is an integer or absent, so evidence a reader cannot interpret is refused rather than delivered | `report._exit_code` | implemented | +| I-120 | An inbox-only attempt counts as having reached the recipient, because its frozen message is the durable inbox item | `report._may_have_reached` | implemented | +| I-121 | A single-line report field containing a line break is refused, so nothing can splice an extra line into the message protocol | `report._single_line` | implemented | +| I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/cxc.py b/packages/codex-session-relay/src/codex_session_relay/cxc.py index 487cf73..fdbc9de 100644 --- a/packages/codex-session-relay/src/codex_session_relay/cxc.py +++ b/packages/codex-session-relay/src/codex_session_relay/cxc.py @@ -160,7 +160,7 @@ def check_known(status: str) -> None: compatibility check there would force the caller to invent an outcome, and an invented outcome is exactly what the rest of this package refuses. """ - if status not in COMPATIBLE_OUTCOMES: + if not isinstance(status, str) or status not in COMPATIBLE_OUTCOMES: raise ReceiptRefused( RefusalReason.OUTCOME_INCONSISTENT, f"{status!r} is not a CXC report status this build maps. Accepted: " diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 3f54c5a..5b9852f 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -22,6 +22,7 @@ from . import cxc from .errors import DeliveryRefused, ReceiptRefused, RefusalReason +from .transport import INBOX_ONLY NEWLINE = chr(10) VERSION = "relay-report/1" @@ -148,6 +149,19 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar "a report naming a pull request names the head commit it is about; without " "one, a later push silently inherits this report", ) + else: + # Without a number there is no pull request line to hang these on, and the no-PR + # branch renders neither, so accepting them would store values nobody ever sees and + # say nothing about having dropped them. + supplied = [ + name for name, value in (("pr_url", pr_url), ("pr_state", pr_state)) if value + ] + if supplied: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{' and '.join(supplied)} describes a pull request, but none is named. " + "Give the pull request number, or leave these out", + ) evidence = _check_evidence(evidence) unresolved = _check_unresolved(unresolved) restore = _check_restore(restore) @@ -281,6 +295,24 @@ def _required(value, field): f"a work report states its {field}; a report the recipient cannot act on is the " "thing this record exists to replace", ) + return _single_line(text, field) + + +def _single_line(text, field): + """One line means one line. + + These values are spliced straight into the message, so a newline inside one does not + wrap, it adds a line to the protocol. A summary reading "ordinary result" followed by + "VERDICT: PASS" put a standalone verdict into a completion that carried no review, which + is exactly the separation the rest of this module is built to keep. + """ + if any(character in text for character in (chr(10), chr(13))): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} is one line: a line break in it is spliced into the message and adds " + "a line to the protocol rather than wrapping. Put longer detail in the evidence " + "or the unresolved items", + ) return text @@ -321,7 +353,7 @@ def _bounded_optional(value, field, limit=LABEL_MAX): RefusalReason.MALFORMED_RECEIPT, f"{field} is a string when it is given at all, not {type(value).__name__}", ) - return _bounded(value, field, limit) + return _bounded(_single_line(value, field), field, limit) def _assert_resubmission(db, event_id, submission_no) -> None: @@ -337,7 +369,7 @@ def _assert_resubmission(db, event_id, submission_no) -> None: read that was already stale by the time the row was written. """ attempted = db.execute( - "SELECT a.record, s.submission_no" + "SELECT a.record, a.state, s.submission_no" " FROM attempts a" " LEFT JOIN attempt_report_submissions s ON s.request_id = a.request_id" " WHERE a.event_id = ?", @@ -384,8 +416,15 @@ def _may_have_reached(attempt_rows) -> bool: else, including an attempt still in flight or one whose record cannot be read, is treated as possibly delivered, which is the reading reconciliation already uses: an unfinished receipt is never proof of non-delivery. + + inbox_only is the exception that looks like the rule. It carries sendAttempted no, + because the push was refused before any resume, but its frozen message IS the durable + inbox item and the recipient can read it. Protocol v1 section 3 calls that channel the + guarantee. So it reached someone, and the submission behind it is not rewritable. """ for row in attempt_rows or []: + if _state_of(row) == INBOX_ONLY: + return True if row["record"] is None: return True try: @@ -397,6 +436,14 @@ def _may_have_reached(attempt_rows) -> bool: return False +def _state_of(row): + """The delivery state this attempt settled at, when the row carries one.""" + try: + return row["state"] + except (IndexError, KeyError): + return None + + def _check_restore(restore): """A skill pointer nobody owns is another render-time failure inside the claim. diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 8c1ebc7..611dc25 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -225,6 +225,29 @@ def test_an_attempt_that_never_sent_is_not_a_delivered_submission(self): # Bounds.test_a_report_already_delivered_cannot_be_replaced_in_place asserts. So the # distinction is the sendAttempted flag, not the mere presence of an attempt row. + def test_an_inbox_only_attempt_did_reach_someone(self): + from codex_session_relay.transport import INBOX_ONLY + + _relationship, event_id = self.queued_event() + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.adapter.script("approval_policy") + record = self.attempt(event_id) + # It carries sendAttempted no, like a retryable pre-send refusal, but its frozen + # message IS the durable inbox item and the recipient can read it. Protocol v1 + # section 3 calls that channel the guarantee. + self.assertEqual(record["deliveryState"], INBOX_ONLY) + self.assertEqual(record["sendAttempted"], "no") + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="rewritten behind the inbox item")), + ) + # Announced as a new submission it is allowed, and the old one stays whole. + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="openly revised", submission_no=2)) + self.assertEqual([r["submissionNo"] for r in report.read_all(self.store, event_id)], + [1, 2]) + def test_an_impossible_budget_refuses_instead_of_shipping_a_gutted_message(self): relationship, event_id = self.queued_event() receipt = self.intake.get(event_id) @@ -496,7 +519,7 @@ def test_a_blocked_report_does_not_tell_the_reader_it_proved_anything(self): self.store, self.clock, event_id=event_id, **a_report(cxc_status=cxc.BUDGET_EXHAUSTED, cxc_reason="the stated token bound ran out", - pr_number=None, pr_url=None, head_sha=None) + pr_number=None, pr_url=None, pr_state=None, head_sha=None) ) message = self.delivery.render_message(event_id) self.assertIn("BUDGET_EXHAUSTED", message) @@ -508,7 +531,7 @@ def test_a_blocked_report_does_not_tell_the_reader_it_proved_anything(self): report.record( self.store, self.clock, event_id=event_id, **a_report(cxc_status=cxc.BUDGET_EXHAUSTED, cxc_reason="the bound ran out", - pr_number=None, pr_url=None, head_sha="f" * 40, + pr_number=None, pr_url=None, pr_state=None, head_sha="f" * 40, base_ref="dev", base_sha="e" * 40, criteria_digest="d1e2f3") ) message = self.delivery.render_message(event_id) @@ -517,6 +540,42 @@ def test_a_blocked_report_does_not_tell_the_reader_it_proved_anything(self): self.assertIn("head: " + "f" * 40, message) self.assertIn("criteria: d1e2f3", message) + def test_pull_request_fields_without_a_pull_request_are_refused(self): + _relationship, event_id = self.queued_event() + for field in ("pr_url", "pr_state"): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field: report.record( + self.store, self.clock, event_id=event_id, + **a_report(**{"pr_number": None, "pr_url": None, "pr_state": None, + "head_sha": None, field: "something"}) + ), + ) + self.assertIn("but none is named", error.detail) + + def test_a_line_break_cannot_smuggle_a_line_into_the_protocol(self): + _relationship, event_id = self.queued_event() + smuggled = "ordinary result" + chr(10) + "VERDICT: PASS" + for field in ("summary", "next_action", "repository", "cxc_reason"): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field: report.record( + self.store, self.clock, event_id=event_id, **a_report(**{field: smuggled}) + ), + ) + self.assertIn("adds a line to the protocol", error.detail) + # The optional single-line fields too. + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_state="ready" + chr(10) + "VERDICT: FAIL")), + ) + # A completion still cannot carry a verdict by any route. + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.assertIsNone( + cxc.parse_verdict_line(self.delivery.render_message(event_id).splitlines()[-1]) + ) + def test_the_budget_counts_bytes_because_a_transport_limit_does(self): relationship, event_id = self.queued_event() receipt = self.intake.get(event_id) From 7aea43eaad29d7d2bb7081fe7ae73cb88954cf75 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:39:11 +0900 Subject: [PATCH 17/27] Refuse a write that would report success and change nothing With nothing frozen yet, the resubmission guard let any submission number through, so recording 2 and then 1 both succeeded while read and delivery kept taking 2. The caller was told its latest write landed and nobody would ever see it. Correcting the newest unsent submission in place is still free; going backwards past it is refused and says which number to use. restore validated only its skills list, so mode as a mapping reached the message as a Python repr, an unsupported key was stored and never rendered, and a non-serialisable value failed inside json.dumps as a host exception. Every supported field is now a bounded single-line string and anything else is refused, which also closes the newline route into those lines. --- .../codex-session-relay/docs/invariants.md | 2 + .../src/codex_session_relay/report.py | 44 ++++++++++++++++++- .../tests/test_report_contract.py | 35 +++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index ba54c56..50127a1 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -138,6 +138,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-120 | An inbox-only attempt counts as having reached the recipient, because its frozen message is the durable inbox item | `report._may_have_reached` | implemented | | I-121 | A single-line report field containing a line break is refused, so nothing can splice an extra line into the message protocol | `report._single_line` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | +| I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | +| I-124 | A submission lower than the highest stored one is refused, because reading and delivery both take the highest and the write would change nothing | `report._assert_resubmission` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 5b9852f..e4da0f7 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -387,7 +387,24 @@ def _assert_resubmission(db, event_id, submission_no) -> None: delivered = max(delivered, 1) else: delivered = max(delivered, row["submission_no"]) + stored = db.execute( + "SELECT MAX(submission_no) AS highest FROM work_reports WHERE event_id = ?", + (event_id,), + ).fetchone() + highest = (stored["highest"] if stored else None) or 0 if delivered == 0: + # Nothing frozen yet, so the newest stored submission can still be corrected in + # place. Going BACKWARDS is the case worth refusing: read and delivery both select + # the highest submission, so writing an older number returns success and then has no + # effect on anything anyone will see. + if submission_no < highest: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"submission {highest} of this report already exists, and both reading and " + f"delivery take the highest, so recording {submission_no} would report " + f"success and change nothing anyone sees. Correct submission {highest}, or " + f"record {highest + 1}", + ) return if submission_no > delivered: return @@ -444,6 +461,9 @@ def _state_of(row): return None +RESTORE_FIELDS = ("mode", "scope", "phase", "phaseObservedAt", "plan", "evidence", "remaining") + + def _check_restore(restore): """A skill pointer nobody owns is another render-time failure inside the claim. @@ -488,7 +508,29 @@ def _check_restore(restore): f"{sorted(cxc.SKILL_POINTERS)}. Naming an owner nobody has would fail at render " "time, inside the delivery claim, instead of here", ) - return restore + checked = {"skills": names} if names else {} + for key, value in restore.items(): + if key == "skills": + continue + if key not in RESTORE_FIELDS: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{key!r} is not a restore field this build renders; supported fields are " + f"{list(RESTORE_FIELDS)} plus skills. An unrendered field is one the " + "recipient never sees and is never told was dropped", + ) + if value is None or value == "": + continue + if not isinstance(value, str): + # A mapping here reached the message as a Python repr, and an arbitrary object + # failed later inside json.dumps as a host exception rather than a refusal. + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"restore {key} is a single line of text, not {type(value).__name__}", + ) + checked[key] = _bounded(_single_line(value.strip(), f"restore {key}"), + f"restore {key}", LABEL_MAX) + return checked def _check_review(review): diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 611dc25..d73baa0 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -1032,3 +1032,38 @@ def test_a_skill_pointer_nobody_owns_is_refused_where_it_can_be_fixed(self): self.assertIn("workflow restore:", message) self.assertIn("mode: CXC Loop, HOTL", message) self.assertIn(cxc.skill_pointer("pull-request"), message) + + def test_every_restore_field_is_validated_not_just_the_skills(self): + _relationship, event_id = self.queued_event() + for bad in ({"mode": {"phase": "C"}}, {"mode": 7}, {"plan": ["a", "b"]}, + {"unsupported": "value"}): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, **a_report(restore=bad) + ), + ) + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record( + self.store, self.clock, event_id=event_id, + **a_report(restore={"mode": "loop" + chr(10) + "VERDICT: PASS"}) + ), + ) + + def test_a_submission_older_than_the_stored_one_changes_nothing_and_says_so(self): + _relationship, event_id = self.queued_event() + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="second", submission_no=2)) + # Reading and delivery both take the highest, so writing 1 now would report success + # and change nothing anyone sees. + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="first, late", submission_no=1)), + ) + self.assertIn("change nothing anyone sees", error.detail) + # Correcting the newest unsent submission in place is still free. + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="second, corrected", submission_no=2)) + self.assertEqual(report.read(self.store, event_id)["summary"], "second, corrected") From b22ca124623559b29fc189d766bad55fe774c85a Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:44:22 +0900 Subject: [PATCH 18/27] Make a submission clear both floors, not one of them The delivered floor and the highest-stored floor were checked in different branches, so the band between them was open: with submission 1 delivered and 3 stored, submission 2 passed the delivered check, sat under the stored one, and was accepted into history where reading and delivery would never select it. Both floors apply now regardless of the other, and the refusal names the number that clears both. A whitespace-only restore value survived as an empty string, which kept the restore dict non-empty and left the render emitting a workflow-restore heading with no fields under it. Trimmed first, dropped when empty. --- .../codex-session-relay/docs/invariants.md | 2 +- .../src/codex_session_relay/report.py | 43 +++++++++++-------- .../tests/test_report_contract.py | 24 +++++++++++ 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 50127a1..289710f 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -139,7 +139,7 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-121 | A single-line report field containing a line break is refused, so nothing can splice an extra line into the message protocol | `report._single_line` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | -| I-124 | A submission lower than the highest stored one is refused, because reading and delivery both take the highest and the write would change nothing | `report._assert_resubmission` | implemented | +| I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | ## Recorded limits, so a row above is not read as more than it is diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index e4da0f7..8153c4a 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -392,22 +392,21 @@ def _assert_resubmission(db, event_id, submission_no) -> None: (event_id,), ).fetchone() highest = (stored["highest"] if stored else None) or 0 - if delivered == 0: - # Nothing frozen yet, so the newest stored submission can still be corrected in - # place. Going BACKWARDS is the case worth refusing: read and delivery both select - # the highest submission, so writing an older number returns success and then has no - # effect on anything anyone will see. - if submission_no < highest: - raise ReceiptRefused( - RefusalReason.MALFORMED_RECEIPT, - f"submission {highest} of this report already exists, and both reading and " - f"delivery take the highest, so recording {submission_no} would report " - f"success and change nothing anyone sees. Correct submission {highest}, or " - f"record {highest + 1}", - ) - return - if submission_no > delivered: + # Two independent floors, and both apply whatever the other one says. A submission at or + # below the delivered one would rewrite bytes somebody already has. A submission below + # the highest stored one would be accepted and then never selected, because reading and + # delivery both take the highest, so the caller would be told a write landed that nobody + # will ever see. Checking only one of them left the band between them open. + if submission_no >= highest and submission_no > delivered: return + if submission_no < highest: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"submission {highest} of this report already exists, and both reading and " + f"delivery take the highest, so recording {submission_no} would report success " + f"and change nothing anyone sees. Correct submission {highest}, or record " + f"{max(highest, delivered) + 1}", + ) if legacy and delivered == 1: detail = ( "a pre-contract message has already been delivered for this event, so a first " @@ -418,7 +417,8 @@ def _assert_resubmission(db, event_id, submission_no) -> None: detail = ( f"submission {delivered} of this report has already been frozen into a delivered " f"attempt, so replacing it in place would change what a retry says without " - f"changing what it calls itself; record this as submission {delivered + 1} or " + f"changing what it calls itself; record this as submission " + f"{max(highest, delivered) + 1} or " "higher. A submission that has never been sent can still be corrected in place" ) raise ReceiptRefused(RefusalReason.MALFORMED_RECEIPT, detail) @@ -519,16 +519,21 @@ def _check_restore(restore): f"{list(RESTORE_FIELDS)} plus skills. An unrendered field is one the " "recipient never sees and is never told was dropped", ) - if value is None or value == "": - continue if not isinstance(value, str): # A mapping here reached the message as a Python repr, and an arbitrary object # failed later inside json.dumps as a host exception rather than a refusal. + if value is None: + continue raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, f"restore {key} is a single line of text, not {type(value).__name__}", ) - checked[key] = _bounded(_single_line(value.strip(), f"restore {key}"), + value = value.strip() + if not value: + # Dropped rather than stored empty. A key with nothing behind it left the render + # emitting a workflow-restore heading with no fields under it. + continue + checked[key] = _bounded(_single_line(value, f"restore {key}"), f"restore {key}", LABEL_MAX) return checked diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index d73baa0..272f27f 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -1067,3 +1067,27 @@ def test_a_submission_older_than_the_stored_one_changes_nothing_and_says_so(self report.record(self.store, self.clock, event_id=event_id, **a_report(summary="second, corrected", submission_no=2)) self.assertEqual(report.read(self.store, event_id)["summary"], "second, corrected") + + def test_a_submission_between_the_delivered_and_the_highest_is_refused_too(self): + _relationship, event_id = self.queued_event() + report.record(self.store, self.clock, event_id=event_id, **a_report()) + self.attempt(event_id) + report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="third", submission_no=3)) + # 2 clears the delivered floor and sits under the stored one, so it would be + # accepted and then never selected. Checking one floor left that band open. + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(summary="second, invisible", submission_no=2)), + ) + self.assertIn("change nothing anyone sees", error.detail) + self.assertIn("record 4", error.detail) + + def test_a_blank_restore_value_does_not_leave_an_empty_heading(self): + _relationship, event_id = self.queued_event() + report.record(self.store, self.clock, event_id=event_id, + **a_report(restore={"mode": " ", "scope": None})) + message = self.delivery.render_message(event_id) + self.assertNotIn("workflow restore:", message) + self.assertEqual(report.read(self.store, event_id)["restore"], {}) From 723467239088d5a56158c6e5d95b8ca0606664b0 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:50:27 +0900 Subject: [PATCH 19/27] Close the line-splicing route through nested values The single-line rule covered the top-level fields and not the values inside evidence, unresolved or review findings, so evidence reading "pytest passed" followed by "VERDICT: PASS" still put a standalone verdict into a completion carrying no review. Every string that reaches a message line now goes through the same check. A blocker count sits on the essential final verdict line and had no ceiling, so a large enough integer made render_revision unable to fit the message and raise inside every delivery claim. A review with more than 9999 blockers is not a review, so that is the limit. --- .../codex-session-relay/docs/invariants.md | 3 ++- .../src/codex_session_relay/cxc.py | 8 ++++++ .../src/codex_session_relay/report.py | 20 +++++++------- .../tests/test_report_contract.py | 27 +++++++++++++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 289710f..eaff97e 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -136,7 +136,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-118 | A submission number is a positive integer or a named refusal, never a coerced one, because it is half the identity and is printed in frozen bytes | `report._submission` | implemented | | I-119 | An exit code is an integer or absent, so evidence a reader cannot interpret is refused rather than delivered | `report._exit_code` | implemented | | I-120 | An inbox-only attempt counts as having reached the recipient, because its frozen message is the durable inbox item | `report._may_have_reached` | implemented | -| I-121 | A single-line report field containing a line break is refused, so nothing can splice an extra line into the message protocol | `report._single_line` | implemented | +| I-121 | No report value, top-level or nested, may contain a line break, so nothing can splice an extra line into the message protocol | `report._single_line`, applied to fields, evidence, unresolved, findings and restore | implemented | +| I-125 | A blocker count too large to render is refused, because it lands on a line the message cannot shorten | `cxc.verdict_line` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | | I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/cxc.py b/packages/codex-session-relay/src/codex_session_relay/cxc.py index fdbc9de..0dd8947 100644 --- a/packages/codex-session-relay/src/codex_session_relay/cxc.py +++ b/packages/codex-session-relay/src/codex_session_relay/cxc.py @@ -219,6 +219,9 @@ def refuse_promotion(fact: str) -> str: FAIL = "FAIL" VERDICT_KINDS = (PASS, GO_WITH_FIXES, FAIL) PREFIX = "VERDICT: " +# A review with more blockers than this is not a review, and the count lands on a line the +# message cannot shorten, so an unbounded one made it unrenderable. +BLOCKERS_MAX = 9999 def verdict_line(kind: str, blockers=None) -> str: @@ -238,6 +241,11 @@ def verdict_line(kind: str, blockers=None) -> str: "GO-WITH-FIXES states how many blockers it is going ahead with; a count " "below one is a PASS and should say so" ) + if blockers > BLOCKERS_MAX: + raise ValueError( + f"a blocker count of {blockers} is past the point of being a review, and it " + f"sits on a line the message cannot shorten; the limit is {BLOCKERS_MAX}" + ) return f"{PREFIX}{GO_WITH_FIXES} (blockers={blockers})" if blockers is not None: raise ValueError(f"a {kind} verdict carries no blocker count") diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 8153c4a..b233f63 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -577,10 +577,11 @@ def _check_review(review): RefusalReason.MALFORMED_RECEIPT, "each review finding names a criterion id" ) findings.append({ - "id": identifier, + "id": _single_line(identifier, "a finding id"), "verdict": item.get("verdict"), - "note": str(item.get("note") or "").strip(), - "anchor": str(item.get("anchor") or "").strip(), + "note": _single_line(str(item.get("note") or "").strip(), "a finding note"), + "anchor": _single_line(str(item.get("anchor") or "").strip(), + "a finding anchor"), }) return {"kind": kind, "blockers": blockers, "findings": findings} @@ -612,7 +613,7 @@ def _check_evidence(entries): checked = [] for item in _sequence(entries, "evidence"): if isinstance(item, str): - checked.append(item) + checked.append(_single_line(item, "an evidence entry")) continue if not isinstance(item, dict) or not str(item.get("check") or "").strip(): raise ReceiptRefused( @@ -621,9 +622,10 @@ def _check_evidence(entries): f"{item!r}", ) checked.append({ - "check": str(item["check"]).strip(), + "check": _single_line(str(item["check"]).strip(), "an evidence check"), "exitCode": _exit_code(item.get("exitCode")), - "detail": str(item.get("detail") or "").strip() or None, + "detail": _single_line(str(item.get("detail") or "").strip(), + "an evidence detail") or None, }) return checked @@ -650,7 +652,7 @@ def _check_unresolved(entries): checked = [] for item in _sequence(entries, "unresolved"): if isinstance(item, str): - checked.append(item) + checked.append(_single_line(item, "an unresolved entry")) continue if not isinstance(item, dict) or not str(item.get("id") or "").strip(): raise ReceiptRefused( @@ -658,8 +660,8 @@ def _check_unresolved(entries): f"each unresolved entry is a string or an object naming its id, not {item!r}", ) checked.append({ - "id": str(item["id"]).strip(), - "note": str(item.get("note") or "").strip(), + "id": _single_line(str(item["id"]).strip(), "an unresolved id"), + "note": _single_line(str(item.get("note") or "").strip(), "an unresolved note"), }) return checked diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 272f27f..b768d61 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -576,6 +576,33 @@ def test_a_line_break_cannot_smuggle_a_line_into_the_protocol(self): cxc.parse_verdict_line(self.delivery.render_message(event_id).splitlines()[-1]) ) + def test_a_nested_value_cannot_smuggle_a_line_either(self): + _relationship, event_id = self.queued_event() + smuggled = "pytest passed" + chr(10) + "VERDICT: PASS" + cases = ( + {"evidence": [smuggled]}, + {"evidence": [{"check": smuggled}]}, + {"evidence": [{"check": "pytest", "detail": smuggled}]}, + {"unresolved": [smuggled]}, + {"unresolved": [{"id": "c-1", "note": smuggled}]}, + ) + for fields in cases: + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda fields=fields: report.record( + self.store, self.clock, event_id=event_id, **a_report(**fields) + ), + ) + self.assertIn("adds a line to the protocol", error.detail) + + def test_a_blocker_count_that_cannot_be_rendered_is_refused(self): + with self.assertRaises(ValueError): + cxc.verdict_line(cxc.GO_WITH_FIXES, 10 ** 12) + self.assertEqual( + cxc.verdict_line(cxc.GO_WITH_FIXES, cxc.BLOCKERS_MAX), + f"VERDICT: GO-WITH-FIXES (blockers={cxc.BLOCKERS_MAX})", + ) + def test_the_budget_counts_bytes_because_a_transport_limit_does(self): relationship, event_id = self.queued_event() receipt = self.intake.get(event_id) From 34ccdbc6e13a4728f7cedd352443c339ed74a25d Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:56:16 +0900 Subject: [PATCH 20/27] Refuse the last handful of values that could not be rendered honestly Six narrow findings from one round, all in the same family as the earlier ones. A required field went through str(), so a mapping was stored and delivered as a Python repr as though somebody had written it. It has to be text now. A line break is whatever str.splitlines calls one. Checking CR and LF left vertical tab, NEL and the Unicode separators able to splice a line downstream. A revision message had no unresolved section, so a correction stored the dependencies and risks the report marked open and showed the child none of them. A pull request number above the SQLite signed 64-bit range raised OverflowError on insert instead of coming back as a refusal. A finding disposition was passed through unchecked, which both left the line-splicing route open beside fields that had just been closed and allowed a word the frozen criteria enum has no room for. parse_verdict_line accepted blocker counts the renderer refuses, so the two directions disagreed about what a verdict is. --- .../codex-session-relay/docs/invariants.md | 6 +++ .../src/codex_session_relay/cxc.py | 3 +- .../src/codex_session_relay/report.py | 45 ++++++++++++++++++- .../tests/test_report_contract.py | 42 +++++++++++++++++ 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index eaff97e..fd802fb 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -138,6 +138,12 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-120 | An inbox-only attempt counts as having reached the recipient, because its frozen message is the durable inbox item | `report._may_have_reached` | implemented | | I-121 | No report value, top-level or nested, may contain a line break, so nothing can splice an extra line into the message protocol | `report._single_line`, applied to fields, evidence, unresolved, findings and restore | implemented | | I-125 | A blocker count too large to render is refused, because it lands on a line the message cannot shorten | `cxc.verdict_line` | implemented | +| I-126 | A required report field must be text, not a value coerced through `str()` into a Python repr | `report._required` | implemented | +| I-127 | A line break is anything `str.splitlines` treats as one, so a separator other than CR or LF cannot splice a line either | `report._single_line` | implemented | +| I-128 | A correction renders the unresolved items the report marked open, rather than storing them unseen | `report.render_revision` | implemented | +| I-129 | A pull request number outside what the store can hold is refused, not left to raise `OverflowError` on insert | `report.record` | implemented | +| I-130 | A finding disposition is one of the frozen criteria values, checked rather than passed through | `report._disposition` | implemented | +| I-131 | The verdict parser and the verdict renderer accept the same language, including the blocker ceiling | `cxc.parse_verdict_line` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | | I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/cxc.py b/packages/codex-session-relay/src/codex_session_relay/cxc.py index 0dd8947..bb77a43 100644 --- a/packages/codex-session-relay/src/codex_session_relay/cxc.py +++ b/packages/codex-session-relay/src/codex_session_relay/cxc.py @@ -264,7 +264,8 @@ def parse_verdict_line(line: str): rest = body[len(GO_WITH_FIXES):].strip() if rest.startswith("(blockers=") and rest.endswith(")"): digits = rest[len("(blockers="):-1] - if digits.isdigit() and int(digits) >= 1: + # The same ceiling the renderer applies, so both directions speak one language. + if digits.isdigit() and 1 <= int(digits) <= BLOCKERS_MAX: return {"kind": GO_WITH_FIXES, "blockers": int(digits)} return None diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index b233f63..78f859c 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -56,6 +56,8 @@ # on an essential line now, so it is given a bounded REPRESENTATION here rather than a limit # imposed on a contract field this layer does not own. REF_SHOWN = 240 +# SQLite stores a signed 64-bit integer and raises OverflowError above it. +SQLITE_MAX_INT = 2 ** 63 - 1 def _size(text) -> int: @@ -143,6 +145,13 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar RefusalReason.MALFORMED_RECEIPT, f"a pull request number is a positive integer, not {pr_number!r}", ) + if pr_number > SQLITE_MAX_INT: + # Past this the insert raises OverflowError, a host exception escaping the + # refusal path rather than a producer being told what it got wrong. + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a pull request number of {pr_number} is outside what the store can hold", + ) if not head_sha: raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, @@ -288,6 +297,13 @@ def version_of(report) -> str: def _required(value, field): + if value is not None and not isinstance(value, str): + # str() on a mapping or a list produces a Python repr, which was then stored and + # delivered as though somebody had written it. + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} is a line of text, not {type(value).__name__}", + ) text = str(value or "").strip() if not text: raise ReceiptRefused( @@ -305,8 +321,13 @@ def _single_line(text, field): wrap, it adds a line to the protocol. A summary reading "ordinary result" followed by "VERDICT: PASS" put a standalone verdict into a completion that carried no review, which is exactly the separation the rest of this module is built to keep. + + Boundaries are whatever str.splitlines treats as one, so vertical tab, NEL and the + Unicode line and paragraph separators count too. Checking only CR and LF would leave + the same splice available through a character that still breaks the line downstream. """ - if any(character in text for character in (chr(10), chr(13))): + parts = text.splitlines() + if len(parts) > 1 or (parts and parts[0] != text): raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, f"{field} is one line: a line break in it is spliced into the message and adds " @@ -578,7 +599,7 @@ def _check_review(review): ) findings.append({ "id": _single_line(identifier, "a finding id"), - "verdict": item.get("verdict"), + "verdict": _disposition(item.get("verdict")), "note": _single_line(str(item.get("note") or "").strip(), "a finding note"), "anchor": _single_line(str(item.get("anchor") or "").strip(), "a finding anchor"), @@ -586,6 +607,23 @@ def _check_review(review): return {"kind": kind, "blockers": blockers, "findings": findings} +def _disposition(value): + """One of the frozen criteria dispositions, checked rather than passed through. + + It is rendered onto the violated-criteria line, so an unchecked value carried the same + line-splicing route as the fields beside it, and a word outside the enum would describe + a judgment the contract has no room for. + """ + from .criteria import DISPOSITIONS + + if value not in DISPOSITIONS: + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{value!r} is not one of {DISPOSITIONS}; the criteria enum is frozen", + ) + return value + + def _sequence(entries, field): """A mapping iterates as its keys and a scalar does not iterate at all. @@ -868,6 +906,9 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: _Section("violated criteria", _finding_lines(receipt, review), rank=0, essential=True, keep=2), _Section("SCOPE", _scope_lines(report, generation), rank=1, essential=True, keep=2), + # A correction that hides the dependencies and risks the report marked open sends the + # child at the findings without telling it what else is in the way. + _Section("unresolved", _unresolved_lines(report), rank=2, essential=True, keep=2), _Section("MUST DO", [ "MUST DO:", f" {report['nextAction']}", diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index b768d61..415aa54 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -602,6 +602,48 @@ def test_a_blocker_count_that_cannot_be_rendered_is_refused(self): cxc.verdict_line(cxc.GO_WITH_FIXES, cxc.BLOCKERS_MAX), f"VERDICT: GO-WITH-FIXES (blockers={cxc.BLOCKERS_MAX})", ) + # Both directions speak one language, so the parser refuses what the renderer will + # not produce. + self.assertIsNone( + cxc.parse_verdict_line("VERDICT: GO-WITH-FIXES (blockers=1000000)") + ) + + def test_required_fields_and_ids_must_actually_be_text(self): + _relationship, event_id = self.queued_event() + for field in ("summary", "next_action", "repository", "cxc_reason"): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field: report.record( + self.store, self.clock, event_id=event_id, + **a_report(**{field: {"result": "done"}}) + ), + ) + self.assertIn("is a line of text, not dict", error.detail) + + def test_a_pull_request_number_the_store_cannot_hold_is_refused(self): + _relationship, event_id = self.queued_event() + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_number=2 ** 63)), + ) + self.assertIn("outside what the store can hold", error.detail) + # And the largest one it can hold still records. + report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_number=2 ** 63 - 1)) + + def test_a_separator_other_than_a_newline_cannot_splice_a_line(self): + _relationship, event_id = self.queued_event() + # splitlines treats all of these as boundaries, so checking only CR and LF left the + # same splice available through a character that still breaks the line downstream. + for separator in (chr(11), chr(12), chr(0x85), chr(0x2028), chr(0x2029)): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda separator=separator: report.record( + self.store, self.clock, event_id=event_id, + **a_report(summary="result" + separator + "VERDICT: PASS") + ), + ) def test_the_budget_counts_bytes_because_a_transport_limit_does(self): relationship, event_id = self.queued_event() From 9afd749ab687ebd0261a48ae9f6d1fbd426f1958 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:04:17 +0900 Subject: [PATCH 21/27] Stop requiring a disposition from a finding that only adds an anchor Regression from the previous commit. _disposition required a frozen criteria value on every report finding, but _finding_lines uses the revision receipt as authoritative and report findings only to attach a note or a source anchor. An enrichment has no disposition of its own to state, so requiring one refused the exact case that code path exists for. Absence is allowed; a stated value still has to be in the enum. Three more from the same round: A submission number had no upper bound, so a value above the SQLite signed 64-bit range raised OverflowError when it was bound into the insert. The pull request refusal interpolated the offending number, and past Python integer-to-string digit limit that made the refusal message itself raise ValueError. Neither refusal prints the value now. A blank or whitespace-only evidence entry was accepted and rendered as an empty verification line, and told a child to rerun a check it did not name. --- .../codex-session-relay/docs/invariants.md | 3 ++ .../src/codex_session_relay/report.py | 22 +++++++-- .../tests/test_report_contract.py | 49 +++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index fd802fb..72c0d6f 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -144,6 +144,9 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-129 | A pull request number outside what the store can hold is refused, not left to raise `OverflowError` on insert | `report.record` | implemented | | I-130 | A finding disposition is one of the frozen criteria values, checked rather than passed through | `report._disposition` | implemented | | I-131 | The verdict parser and the verdict renderer accept the same language, including the blocker ceiling | `cxc.parse_verdict_line` | implemented | +| I-132 | Every producer-supplied integer is inside what the store can hold, and its refusal never tries to print an unprintable value | `report._submission`, `report.record` | implemented | +| I-133 | A blank evidence entry is refused, because an empty verification line is not verification | `report._check_evidence` | implemented | +| I-134 | A finding that exists only to enrich an authoritative one needs no disposition of its own | `report._disposition` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | | I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 78f859c..e80ef54 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -150,7 +150,9 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar # refusal path rather than a producer being told what it got wrong. raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, - f"a pull request number of {pr_number} is outside what the store can hold", + # The value is deliberately not interpolated: past Python integer-to-string + # digit limit, rendering it raises ValueError out of the refusal itself. + "a pull request number is outside what the store can hold", ) if not head_sha: raise ReceiptRefused( @@ -356,11 +358,12 @@ def _submission(value): True to 1, or 1.9 to 1, or storing 0, would give an attempt an identity that means something other than what it says. """ - if isinstance(value, bool) or not isinstance(value, int) or value < 1: + if (isinstance(value, bool) or not isinstance(value, int) or value < 1 + or value > SQLITE_MAX_INT): raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, - f"a submission number is a positive integer, not {value!r}; it is part of this " - "report identity and is printed in the bytes that get frozen", + "a submission number is a positive integer the store can hold; it is part of " + "this report identity and is printed in the bytes that get frozen", ) return value @@ -613,9 +616,16 @@ def _disposition(value): It is rendered onto the violated-criteria line, so an unchecked value carried the same line-splicing route as the fields beside it, and a word outside the enum would describe a judgment the contract has no room for. + + Absence is allowed. A report finding is also used purely to enrich an authoritative + finding from the revision receipt with a note or a source anchor, and that use has no + disposition of its own to state; requiring one refused exactly the enrichment case + _finding_lines exists for. """ from .criteria import DISPOSITIONS + if value is None: + return None if value not in DISPOSITIONS: raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, @@ -651,7 +661,9 @@ def _check_evidence(entries): checked = [] for item in _sequence(entries, "evidence"): if isinstance(item, str): - checked.append(_single_line(item, "an evidence entry")) + checked.append( + _single_line(_required(item, "an evidence entry"), "an evidence entry") + ) continue if not isinstance(item, dict) or not str(item.get("check") or "").strip(): raise ReceiptRefused( diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 415aa54..a8790e8 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -631,6 +631,39 @@ def test_a_pull_request_number_the_store_cannot_hold_is_refused(self): # And the largest one it can hold still records. report.record(self.store, self.clock, event_id=event_id, **a_report(pr_number=2 ** 63 - 1)) + # A number past the integer-to-string digit limit must not make the refusal itself + # raise while trying to print it. + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_number=10 ** 6000)), + ) + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(submission_no=2 ** 63)), + ) + + def test_a_blank_evidence_entry_is_not_verification(self): + _relationship, event_id = self.queued_event() + for bad in ("", " "): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, **a_report(evidence=[bad]) + ), + ) + + def test_an_enrichment_finding_needs_no_disposition_of_its_own(self): + _relationship, event_id = self.queued_event() + # The revision receipt owns the disposition; a report finding may exist purely to + # attach a source anchor to it, so requiring one refused the enrichment case. + report.record( + self.store, self.clock, event_id=event_id, + **a_report(review=None, unresolved=[{"id": "c-1", "note": "still open"}]) + ) + stored = report.read(self.store, event_id) + self.assertEqual(stored["unresolved"][0]["id"], "c-1") def test_a_separator_other_than_a_newline_cannot_splice_a_line(self): _relationship, event_id = self.queued_event() @@ -755,6 +788,22 @@ def test_a_review_only_finding_is_kept_but_marked_as_outside_the_verdict(self): self.assertIn("also raised in review, not part of the recorded verdict", message) self.assertIn("a reviewer noticed this separately", message) + def test_an_anchor_only_enrichment_is_accepted_and_rendered(self): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) + row = self.delivery.get(revision_event) + # The receipt owns c-1 and its disposition. This finding exists only to attach the + # source anchor, so it has no disposition of its own to state. + stored = report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="manifest incomplete", + review={"kind": cxc.GO_WITH_FIXES, "blockers": 1, "findings": [ + {"id": "c-1", "anchor": "migrations/004.sql"}]}) + ) + message = report.render_revision(row, receipt, "del-y-a1", stored) + self.assertIn("c-1: needs_changes", message, "the receipt disposition still leads") + self.assertIn("anchor: migrations/004.sql", message) + class Bounds(DeliveryTestCase): def test_a_required_line_too_long_to_render_is_refused_when_it_is_recorded(self): From 48bafc1fbd34c0e77e8a67c96572565d99706e87 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:08:44 +0900 Subject: [PATCH 22/27] Render an absent disposition as absent, and never print a rejected number Two consequences of the previous commit. Allowing an enrichment finding to omit its disposition meant a review-only finding, one the receipt does not own, rendered as "c-9: None", telling the reader a criterion had a judgment called None. The line now omits the disposition when there is none, so an enrichment reads as an id and its note. The positive-integer branch of the pull request check still interpolated the value, and a huge negative number reaches that branch, so formatting it raised ValueError out of the refusal. Both branches name the type and the range instead of printing the value. --- .../src/codex_session_relay/report.py | 11 +++++++++-- .../codex-session-relay/tests/test_report_contract.py | 6 ++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index e80ef54..c0dd312 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -143,7 +143,11 @@ def record(store, clock, *, event_id, repository, cxc_status, cxc_reason, summar if isinstance(pr_number, bool) or not isinstance(pr_number, int) or pr_number < 1: raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, - f"a pull request number is a positive integer, not {pr_number!r}", + # The value is named by TYPE, never printed. Past the integer-to-string + # digit limit, formatting it raises ValueError out of the refusal itself, + # and a huge negative number reaches this branch. + "a pull request number is a positive integer the store can hold, not a " + f"{type(pr_number).__name__} outside that range", ) if pr_number > SQLITE_MAX_INT: # Past this the insert raises OverflowError, a host exception escaping the @@ -1084,7 +1088,10 @@ def _finding_lines(receipt, review): def _one_finding(item, extra): note = item.get("note") or extra.get("note") - rendered = f" {item['id']}: {item.get('verdict')}" + # An enrichment-only finding has no disposition of its own, and rendering the absence as + # "None" told the reader a criterion had a judgment named None. + disposition = item.get("verdict") + rendered = f" {item['id']}: {disposition}" if disposition else f" {item['id']}" if note: rendered += f" - {note}" out = [rendered] diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index a8790e8..00e8839 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -638,6 +638,12 @@ def test_a_pull_request_number_the_store_cannot_hold_is_refused(self): lambda: report.record(self.store, self.clock, event_id=event_id, **a_report(pr_number=10 ** 6000)), ) + # Negative reaches the positive-integer branch, which must not print it either. + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(pr_number=-(10 ** 6000))), + ) self.assertRefused( RefusalReason.MALFORMED_RECEIPT, lambda: report.record(self.store, self.clock, event_id=event_id, From 2da7f17d85696cd720e3e1d6260e54ddb200a9c8 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:15:03 +0900 Subject: [PATCH 23/27] Carry the status into corrections and refuse what cannot be encoded The contract map says a report carries its CXC status and reason, and the revision renderer built its header without either, so a correction dropped both and produced no omission notice for them. It carries them now, like a completion does. A lone surrogate is one line by every line rule and still cannot be encoded as UTF-8, so a required field raised UnicodeEncodeError out of _size while a nested value was stored and then failed inside every delivery claim. Every line value passes through _single_line, so the encodability check lives there. --- .../codex-session-relay/docs/invariants.md | 2 ++ .../src/codex_session_relay/report.py | 17 +++++++++++++++- .../tests/test_report_contract.py | 20 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 72c0d6f..2b2a9d9 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -147,6 +147,8 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-132 | Every producer-supplied integer is inside what the store can hold, and its refusal never tries to print an unprintable value | `report._submission`, `report.record` | implemented | | I-133 | A blank evidence entry is refused, because an empty verification line is not verification | `report._check_evidence` | implemented | | I-134 | A finding that exists only to enrich an authoritative one needs no disposition of its own | `report._disposition` | implemented | +| I-135 | A line value that cannot be encoded as UTF-8 is refused where it is recorded, not where it is measured or sent | `report._single_line` | implemented | +| I-136 | A correction carries the CXC status and its reason, like a completion does | `report.render_revision` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | | I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index c0dd312..1d35416 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -340,6 +340,17 @@ def _single_line(text, field): "a line to the protocol rather than wrapping. Put longer detail in the evidence " "or the unresolved items", ) + try: + text.encode("utf-8") + except UnicodeEncodeError as unencodable: + # A lone surrogate is one line by every line rule and still cannot be encoded, so + # it reached _size and raised there instead, or was stored and then failed inside + # every delivery claim. Every line value passes through here, so this is the place. + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} contains a character that cannot be encoded as UTF-8, so it cannot " + "be measured against the message budget or sent", + ) from unencodable return text @@ -916,9 +927,13 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: "[codex-session-relay] revision request", ] head.append(f"TASK: {report['summary']}") + # The status and its reason belong here too. The contract map says a report carries them + # and the correction direction was dropping both without saying it had. + head.append(f"cxc: {report['cxcStatus']} - {report['cxcReason']}") + head.append(f" meaning: {cxc.MEANING[report['cxcStatus']]}") sections = [ - _Section("header", head, rank=0, essential=True, keep=2), + _Section("header", head, rank=0, essential=True, keep=4), _Section("violated criteria", _finding_lines(receipt, review), rank=0, essential=True, keep=2), _Section("SCOPE", _scope_lines(report, generation), rank=1, essential=True, keep=2), diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index 00e8839..b524f37 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -435,6 +435,10 @@ def test_a_correction_carries_every_field_the_dispatch_contract_names(self): for field in cxc.DISPATCH_FIELDS: self.assertIn(field + ":", message, f"{field} is missing from the instruction") self.assertIn(cxc.DECISION_BOUNDARY + ":", message) + # The status and its reason belong in this direction too. + self.assertIn("cxc: NEEDS_HUMAN", message) + self.assertIn("the parent judged the manifest incomplete", message) + self.assertIn(cxc.MEANING[cxc.NEEDS_HUMAN], message) self.assertIn("VERDICT: GO-WITH-FIXES (blockers=1)", message) # What was violated, and how to reproduce it, lead the message. self.assertLess(message.index("violated criteria:"), message.index("SCOPE:")) @@ -684,6 +688,22 @@ def test_a_separator_other_than_a_newline_cannot_splice_a_line(self): ), ) + def test_a_value_that_cannot_be_encoded_is_refused_where_it_is_recorded(self): + _relationship, event_id = self.queued_event() + lone_surrogate = "result " + chr(0xD800) + # One line by every line rule, and still unsendable: it used to raise + # UnicodeEncodeError out of _size, or be stored and fail inside every delivery claim. + for fields in ({"summary": lone_surrogate}, + {"evidence": [lone_surrogate]}, + {"unresolved": [lone_surrogate]}): + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda fields=fields: report.record( + self.store, self.clock, event_id=event_id, **a_report(**fields) + ), + ) + self.assertIn("cannot be encoded as UTF-8", error.detail) + def test_the_budget_counts_bytes_because_a_transport_limit_does(self): relationship, event_id = self.queued_event() receipt = self.intake.get(event_id) From 06d38a6c614bb9e95ad4692ea441b7263320e646 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:21:57 +0900 Subject: [PATCH 24/27] Say where a finding came from when there is no recorded verdict A legacy relationship can reach needs_changes with no registered criteria, so the review findings are all there is and _finding_lines fell back to them unlabelled, under the same violated-criteria heading a recorded verdict uses. The child would read them as a verdict it could look up. The fallback now says they come from the review and that the assignment has no recorded criteria set. An evidence check name went through str(), so a mapping was stored and presented as verification of a Python repr, and the revision direction told the recipient to rerun that repr as though it were a command. --- .../src/codex_session_relay/report.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 1d35416..2948e4c 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -680,14 +680,15 @@ def _check_evidence(entries): _single_line(_required(item, "an evidence entry"), "an evidence entry") ) continue - if not isinstance(item, dict) or not str(item.get("check") or "").strip(): + if not isinstance(item, dict) or not isinstance(item.get("check"), str) \ + or not item["check"].strip(): raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, f"each verification entry is a string or an object naming its check, not " f"{item!r}", ) checked.append({ - "check": _single_line(str(item["check"]).strip(), "an evidence check"), + "check": _single_line(item["check"].strip(), "an evidence check"), "exitCode": _exit_code(item.get("exitCode")), "detail": _single_line(str(item.get("detail") or "").strip(), "an evidence detail") or None, @@ -1089,6 +1090,11 @@ def _finding_lines(receipt, review): lines = ["", "violated criteria:"] seen = set() + if not authoritative: + # A legacy relationship can reach needs_changes with no registered criteria, so the + # review findings are all there is. Saying where they came from still matters: the + # child should not read them as a recorded verdict it can look up. + lines.append(" from the review; this assignment has no recorded criteria set:") for item in authoritative or list(enrichment.values()): extra = enrichment.get(item["id"], {}) seen.add(item["id"]) From 3582516272e6b4a5c196f7a4bf1d599008d7e6cc Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:26:44 +0900 Subject: [PATCH 25/27] Count the message instead of rebuilding it, and finish the unresolved checks Shortening pops one line at a time and re-joined and re-encoded every remaining line on each pass, which is quadratic in the length of the list being shortened. The joined size is the sum of the encoded lines plus one separator between each pair, so it is counted directly now. The unresolved path still had the two holes the evidence path had already had closed: a blank entry was accepted and rendered as an empty bullet, and a mapping supplied as id or note went through str() and was stored as a Python repr. Both are refused. The evidence check name had the same coercion left in its mapping form and now has to be a string. --- .../src/codex_session_relay/report.py | 36 +++++++++++++++---- .../tests/test_report_contract.py | 16 +++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 2948e4c..fda0407 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -718,16 +718,25 @@ def _check_unresolved(entries): checked = [] for item in _sequence(entries, "unresolved"): if isinstance(item, str): - checked.append(_single_line(item, "an unresolved entry")) + checked.append( + _single_line(_required(item, "an unresolved entry"), "an unresolved entry") + ) continue - if not isinstance(item, dict) or not str(item.get("id") or "").strip(): + if (not isinstance(item, dict) or not isinstance(item.get("id"), str) + or not item["id"].strip()): raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, f"each unresolved entry is a string or an object naming its id, not {item!r}", ) + note = item.get("note") + if note is not None and not isinstance(note, str): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"an unresolved note is a line of text, not {type(note).__name__}", + ) checked.append({ - "id": _single_line(str(item["id"]).strip(), "an unresolved id"), - "note": _single_line(str(item.get("note") or "").strip(), "an unresolved note"), + "id": _single_line(item["id"].strip(), "an unresolved id"), + "note": _single_line((note or "").strip(), "an unresolved note"), }) return checked @@ -815,9 +824,24 @@ def rendered(extra_note=True): body += blocks[tail] return NEWLINE.join(body) + def over_budget(): + """Byte length counted, not rebuilt. + + Shortening pops one line at a time, and re-joining and re-encoding every remaining + line on each pass made that quadratic in the length of the list being shortened. + The joined size is the sum of the encoded lines plus one separator between each + pair, so it can be counted directly. + """ + count = sum(len(block) for block in blocks) + total = sum(_size(line) for block in blocks for line in block) + if removed: + count += 2 + total += _size(_omission_line(removed, event_id)) + return total + max(count - 1, 0) > budget + order = sorted(range(len(sections)), key=lambda i: -sections[i].rank) for index in order: - if _size(rendered()) <= budget: + if not over_budget(): break section = sections[index] if section.essential or not blocks[index]: @@ -833,7 +857,7 @@ def rendered(extra_note=True): # appended to it. Popping a line and then appending a marker leaves the block the # same length, which is a loop that never ends. kept = list(section.lines) - while _size(rendered()) > budget and len(kept) > section.keep: + while over_budget() and len(kept) > section.keep: kept.pop() dropped = len(section.lines) - len(kept) blocks[index] = kept + [f" ... {dropped} more, see the full record"] diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index b524f37..e96fb89 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -663,6 +663,22 @@ def test_a_blank_evidence_entry_is_not_verification(self): self.store, self.clock, event_id=event_id, **a_report(evidence=[bad]) ), ) + # The unresolved path had the same two holes: a blank entry, and a mapping coerced + # through str() into a Python repr. + for bad in ([""], [" "], [{"id": {"criterion": "c-1"}}], + [{"id": "c-1", "note": ["a", "b"]}]): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, **a_report(unresolved=bad) + ), + ) + # A non-string evidence check name too. + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record(self.store, self.clock, event_id=event_id, + **a_report(evidence=[{"check": {"cmd": "pytest"}}])), + ) def test_an_enrichment_finding_needs_no_disposition_of_its_own(self): _relationship, event_id = self.queued_event() From 78fb02ed380e338e9766e802aa9bdb2163641694 Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:37:15 +0900 Subject: [PATCH 26/27] Keep one finding per criterion and one pass over the lines Four findings from the review round on the delivered head. The renderer keys enrichment on the criterion id, so two findings for the same criterion meant the second replaced the first and its note or anchor vanished with no omission notice. One criterion carries one finding now, refused at record time the way a registered criteria set already refuses a duplicate id. over_budget still summed every remaining line on each pop, so shortening a long list was still quadratic, and it runs inside the claim transaction where that holds the single SQLite writer for the duration. The byte total and line count are carried and adjusted by each mutation instead. SCOPE mixed variable data with fixed protocol prose, and its floor let the preserve boundary be shortened away. The omission marker points at show, which returns the receipt and the work report but not template text, so those lines were recoverable nowhere. They are their own section with a floor covering them. A finding note or anchor still went through str(), so a mapping rendered as a Python repr and a falsey non-text value such as 0 silently became absent. --- .../codex-session-relay/docs/invariants.md | 3 + .../src/codex_session_relay/report.py | 69 +++++++++++++++---- .../tests/test_report_contract.py | 65 +++++++++++++++++ 3 files changed, 124 insertions(+), 13 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index 2b2a9d9..aa567f6 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -149,6 +149,9 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-134 | A finding that exists only to enrich an authoritative one needs no disposition of its own | `report._disposition` | implemented | | I-135 | A line value that cannot be encoded as UTF-8 is refused where it is recorded, not where it is measured or sent | `report._single_line` | implemented | | I-136 | A correction carries the CXC status and its reason, like a completion does | `report.render_revision` | implemented | +| I-137 | One criterion carries one finding; a duplicate id is refused rather than silently replacing the first | `report._check_review` | implemented | +| I-138 | Shortening carries a running byte total rather than recounting, so it stays linear inside the claim transaction | `report._compose` | implemented | +| I-139 | Fixed protocol prose is never shortened away, because the advertised command returns records and not template text | `report._preserve_lines` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | | I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index fda0407..214c9e8 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -615,12 +615,26 @@ def _check_review(review): raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, "each review finding names a criterion id" ) + if identifier in {finding["id"] for finding in findings}: + # The renderer keys enrichment on the id, so a second entry replaced the first + # and its note or anchor vanished with no omission notice. + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"two findings name criterion {identifier!r}; one criterion carries one " + "finding, so the second would silently replace the first", + ) + for field in ("note", "anchor"): + value = item.get(field) + if value is not None and not isinstance(value, str): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"a finding {field} is a line of text, not {type(value).__name__}", + ) findings.append({ "id": _single_line(identifier, "a finding id"), "verdict": _disposition(item.get("verdict")), - "note": _single_line(str(item.get("note") or "").strip(), "a finding note"), - "anchor": _single_line(str(item.get("anchor") or "").strip(), - "a finding anchor"), + "note": _single_line((item.get("note") or "").strip(), "a finding note"), + "anchor": _single_line((item.get("anchor") or "").strip(), "a finding anchor"), }) return {"kind": kind, "blockers": blockers, "findings": findings} @@ -825,20 +839,25 @@ def rendered(extra_note=True): return NEWLINE.join(body) def over_budget(): - """Byte length counted, not rebuilt. + """Byte length from a running total, never recounted. - Shortening pops one line at a time, and re-joining and re-encoding every remaining - line on each pass made that quadratic in the length of the list being shortened. - The joined size is the sum of the encoded lines plus one separator between each - pair, so it can be counted directly. + Shortening pops one line at a time. Summing every remaining line on each pass was + still quadratic in the length of the list being shortened, and this runs inside the + claim transaction, where a long report would hold the single SQLite writer for the + duration. The totals below are adjusted by each mutation instead. """ - count = sum(len(block) for block in blocks) - total = sum(_size(line) for block in blocks for line in block) + count, total = state["lines"], state["bytes"] if removed: count += 2 total += _size(_omission_line(removed, event_id)) return total + max(count - 1, 0) > budget + sizes = [[_size(line) for line in block] for block in blocks] + state = { + "lines": sum(len(block) for block in blocks), + "bytes": sum(size for block in sizes for size in block), + } + order = sorted(range(len(sections)), key=lambda i: -sections[i].rank) for index in order: if not over_budget(): @@ -847,7 +866,10 @@ def over_budget(): if section.essential or not blocks[index]: continue removed.append(section.name) + state["lines"] -= len(blocks[index]) + state["bytes"] -= sum(sizes[index]) blocks[index] = [] + sizes[index] = [] for index in order: section = sections[index] @@ -857,10 +879,19 @@ def over_budget(): # appended to it. Popping a line and then appending a marker leaves the block the # same length, which is a loop that never ends. kept = list(section.lines) + kept_sizes = list(sizes[index]) + marker_size = 0 while over_budget() and len(kept) > section.keep: kept.pop() + state["bytes"] -= kept_sizes.pop() + state["lines"] -= 1 dropped = len(section.lines) - len(kept) - blocks[index] = kept + [f" ... {dropped} more, see the full record"] + marker = f" ... {dropped} more, see the full record" + state["bytes"] += _size(marker) - marker_size + state["lines"] += 0 if marker_size else 1 + marker_size = _size(marker) + blocks[index] = kept + [marker] + sizes[index] = kept_sizes + [marker_size] if section.name not in removed: removed.append(section.name) @@ -962,6 +993,7 @@ def render_revision(row, receipt, request, report, *, budget=BUDGET) -> str: _Section("violated criteria", _finding_lines(receipt, review), rank=0, essential=True, keep=2), _Section("SCOPE", _scope_lines(report, generation), rank=1, essential=True, keep=2), + _Section("preserve", _preserve_lines(), rank=0, essential=True, keep=2), # A correction that hides the dependencies and risks the report marked open sends the # child at the findings without telling it what else is in the way. _Section("unresolved", _unresolved_lines(report), rank=2, essential=True, keep=2), @@ -1157,11 +1189,22 @@ def _scope_lines(report, generation): if report.get("criteriaDigest"): lines.append(f" criteria {report['criteriaDigest']}") lines.append(f" execution generation {generation} (new)") - lines.append(" preserve: everything outside the findings above, including work this") - lines.append(" request does not mention and any other task in-flight beside it") return lines +def _preserve_lines(): + """Fixed protocol prose, kept out of the shortenable part of SCOPE. + + Mixed in with the variable data, the preserve boundary could be shortened away, and the + omission marker points at show, which returns the receipt and the work report but not + template text. So those lines were not recoverable anywhere once dropped. + """ + return [ + " preserve: everything outside the findings above, including work this", + " request does not mention and any other task in-flight beside it", + ] + + def _proof_lines(report): lines = ["", "PROOF:"] entries = report.get("evidence") or [] diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index e96fb89..ab73404 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -196,6 +196,27 @@ def test_more_than_ten_findings_are_never_dropped_in_silence(self): self.assertIn("submission: 1", tight, "nor which submission produced these bytes") self.assertIn("requestId: del-x-a1", tight) + def test_shortening_a_long_list_stays_correct_and_does_not_rescan(self): + _relationship, event_id = self.queued_event() + receipt = self.intake.get(event_id) + # The byte total is carried incrementally now, so the risk this covers is the + # accounting drifting from what the message actually measures. + report.record( + self.store, self.clock, event_id=event_id, + **a_report(unresolved=[f"open item {n} with text" for n in range(2000)]) + ) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + for budget in (1700, 2500, 4000): + message = report.render_completion(row, receipt, "del-x-a1", stored, budget=budget) + self.assertLessEqual( + len(message.encode("utf-8")), budget, + "the running total has to agree with the message it describes", + ) + self.assertIn("omitted:", message) + self.assertIn("unresolved:", message) + self.assertIn("next:", message) + def test_a_long_manifest_reference_is_truncated_visibly_not_left_unsendable(self): _relationship, event_id = self.queued_event() receipt = dict(self.intake.get(event_id)) @@ -846,6 +867,50 @@ def test_an_anchor_only_enrichment_is_accepted_and_rendered(self): self.assertIn("c-1: needs_changes", message, "the receipt disposition still leads") self.assertIn("anchor: migrations/004.sql", message) + def test_two_findings_for_one_criterion_are_refused_not_silently_merged(self): + _source, revision_event = self._revision() + error = self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="incomplete", + review={"kind": cxc.GO_WITH_FIXES, "blockers": 1, "findings": [ + {"id": "c-1", "note": "the first thing"}, + {"id": "c-1", "anchor": "migrations/004.sql"}]}) + ), + ) + self.assertIn("would silently replace the first", error.detail) + + def test_a_finding_note_or_anchor_must_be_text(self): + _source, revision_event = self._revision() + for field in ("note", "anchor"): + for bad in ({"text": "x"}, ["x"], 0): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda field=field, bad=bad: report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="incomplete", + review={"kind": cxc.FAIL, "findings": [ + {"id": "c-1", field: bad}]}) + ), + ) + + def test_the_fixed_preserve_boundary_is_never_shortened_away(self): + _source, revision_event = self._revision() + receipt = self.intake.get(revision_event) + row = self.delivery.get(revision_event) + stored = report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="incomplete", + unresolved=[f"open item {n} with some length to it" for n in range(40)]) + ) + message = report.render_revision(row, receipt, "del-p-a1", stored, budget=2600) + self.assertIn("omitted:", message, "something had to go") + # show returns the receipt and the work report, not template prose, so these lines + # are recoverable nowhere once dropped. + self.assertIn("preserve: everything outside the findings above", message) + self.assertIn("request does not mention", message) + class Bounds(DeliveryTestCase): def test_a_required_line_too_long_to_render_is_refused_when_it_is_recorded(self): From 7354c00645f82c971bd6a89574fade5c426510cb Mon Sep 17 00:00:00 2001 From: thisisjun786 <259586770+thisisjun786@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:44:52 +0900 Subject: [PATCH 27/27] Require text where text was being manufactured Four more coercion routes, all on producer input. An evidence detail and a review finding id both went through str(), so a mapping became a repr and a falsey value such as 0 or False silently became absence. A fake identifier is worse than a rejected one: it cannot enrich the authoritative finding it was meant for, so it arrives as a separate review item. An exit code had no range, and past the integer-to-string digit limit json.dumps raised ValueError while serialising the row. No process exits with a number that size, so the bound says that rather than naming a serialisation limit. A manifestRef carrying a lone surrogate made _size raise while measuring, so every delivery claim rolled back once a report was attached. The receipt is contract-validated and has no encodability rule for that field, so refusing the delivery would punish the recipient for the producer. It renders as present and not renderable, with the record as the place to read it. --- .../codex-session-relay/docs/invariants.md | 3 ++ .../src/codex_session_relay/report.py | 35 +++++++++++++-- .../tests/test_report_contract.py | 43 +++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/packages/codex-session-relay/docs/invariants.md b/packages/codex-session-relay/docs/invariants.md index aa567f6..2bcd421 100644 --- a/packages/codex-session-relay/docs/invariants.md +++ b/packages/codex-session-relay/docs/invariants.md @@ -152,6 +152,9 @@ status. Every row below is implemented and carries a test; the suite is the proo | I-137 | One criterion carries one finding; a duplicate id is refused rather than silently replacing the first | `report._check_review` | implemented | | I-138 | Shortening carries a running byte total rather than recounting, so it stays linear inside the claim transaction | `report._compose` | implemented | | I-139 | Fixed protocol prose is never shortened away, because the advertised command returns records and not template text | `report._preserve_lines` | implemented | +| I-140 | Every producer-supplied text field is required to be text, never coerced through `str()` into a representation of itself | `report._required`, `_text_or_none`, and the evidence, unresolved and finding checks | implemented | +| I-141 | An exit code is a number a process could have exited with, so it can always be serialised | `report._exit_code` | implemented | +| I-142 | An unrenderable receipt manifestRef is reported as present rather than blocking the delivery, because the receipt is contract-validated and the recipient is not at fault | `report._manifest_ref_lines` | implemented | | I-122 | Pull-request fields are refused when no pull request is named, rather than stored and never rendered | `report.record` | implemented | | I-123 | Every restore field is a supported, bounded, single-line string; an unsupported or unrenderable one is refused | `report._check_restore` | implemented | | I-124 | A submission must clear both floors, the delivered one and the highest stored one, so no write is accepted that nobody would ever see | `report._assert_resubmission` | implemented | diff --git a/packages/codex-session-relay/src/codex_session_relay/report.py b/packages/codex-session-relay/src/codex_session_relay/report.py index 214c9e8..c26cf92 100644 --- a/packages/codex-session-relay/src/codex_session_relay/report.py +++ b/packages/codex-session-relay/src/codex_session_relay/report.py @@ -58,6 +58,8 @@ REF_SHOWN = 240 # SQLite stores a signed 64-bit integer and raises OverflowError above it. SQLITE_MAX_INT = 2 ** 63 - 1 +# Wider than any real exit status or signal, and far inside what can be serialised. +EXIT_CODE_MAX = 2 ** 31 def _size(text) -> int: @@ -610,11 +612,11 @@ def _check_review(review): RefusalReason.MALFORMED_RECEIPT, f"each review finding is an object naming a criterion, not {item!r}", ) - identifier = str(item.get("id") or "").strip() - if not identifier: + if not isinstance(item.get("id"), str) or not item["id"].strip(): raise ReceiptRefused( RefusalReason.MALFORMED_RECEIPT, "each review finding names a criterion id" ) + identifier = item["id"].strip() if identifier in {finding["id"] for finding in findings}: # The renderer keys enrichment on the id, so a second entry replaced the first # and its note or anchor vanished with no omission notice. @@ -704,12 +706,24 @@ def _check_evidence(entries): checked.append({ "check": _single_line(item["check"].strip(), "an evidence check"), "exitCode": _exit_code(item.get("exitCode")), - "detail": _single_line(str(item.get("detail") or "").strip(), + "detail": _single_line(_text_or_none(item.get("detail"), "an evidence detail"), "an evidence detail") or None, }) return checked +def _text_or_none(value, field) -> str: + """A string or nothing. Coercion here turned a mapping into a repr and 0 into absence.""" + if value is None: + return "" + if not isinstance(value, str): + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + f"{field} is a line of text when it is given at all, not {type(value).__name__}", + ) + return value.strip() + + def _exit_code(value): """An integer or nothing. Anything else is either not proof or not storable. @@ -725,6 +739,14 @@ def _exit_code(value): f"an exit code is an integer or absent, not {value!r}; evidence a reader cannot " "interpret is not evidence", ) + if not -EXIT_CODE_MAX <= value <= EXIT_CODE_MAX: + # Past the integer-to-string digit limit, json.dumps raises ValueError while + # serialising the row, which is a host exception rather than a named refusal, and a + # process never exited with a number this size anyway. + raise ReceiptRefused( + RefusalReason.MALFORMED_RECEIPT, + "an exit code is a number a process could actually have exited with", + ) return value @@ -1269,6 +1291,13 @@ def _manifest_ref_lines(receipt): if not reference: return [] text = str(reference) + try: + text.encode("utf-8") + except UnicodeEncodeError: + # The receipt is contract-validated and this field has no encodability rule there, + # so an unencodable one reached here and made every delivery claim raise while + # measuring. Say it exists and where to read it rather than refusing the delivery. + return ["", "manifestRef: present but not renderable; read it in the record"] if _size(text) > REF_SHOWN: # Truncated visibly, never quietly. A pointer nobody can read is still better than a # message that cannot be sent, and the whole value is in the record. diff --git a/packages/codex-session-relay/tests/test_report_contract.py b/packages/codex-session-relay/tests/test_report_contract.py index ab73404..13fd499 100644 --- a/packages/codex-session-relay/tests/test_report_contract.py +++ b/packages/codex-session-relay/tests/test_report_contract.py @@ -700,6 +700,37 @@ def test_a_blank_evidence_entry_is_not_verification(self): lambda: report.record(self.store, self.clock, event_id=event_id, **a_report(evidence=[{"check": {"cmd": "pytest"}}])), ) + # A detail coerced through str() turned a mapping into a repr and 0 into absence. + for bad in ({"result": "passed"}, ["passed"], 0, False): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=event_id, + **a_report(evidence=[{"check": "pytest", "detail": bad}]) + ), + ) + # And an exit code no process could have produced, which json.dumps could not + # serialise either. + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda: report.record( + self.store, self.clock, event_id=event_id, + **a_report(evidence=[{"check": "pytest", "exitCode": 10 ** 5000}]) + ), + ) + + def test_an_unrenderable_manifest_reference_does_not_block_the_delivery(self): + _relationship, event_id = self.queued_event() + receipt = dict(self.intake.get(event_id)) + receipt["manifestRef"] = "/frozen/" + chr(0xD800) + report.record(self.store, self.clock, event_id=event_id, **a_report()) + stored = report.read(self.store, event_id) + row = self.delivery.get(event_id) + # The receipt is contract-validated and has no encodability rule for this field, so + # refusing the delivery would punish the recipient for the producer. + message = report.render_completion(row, receipt, "del-u-a1", stored) + self.assertIn("manifestRef: present but not renderable", message) + self.assertIn("show --event", message) def test_an_enrichment_finding_needs_no_disposition_of_its_own(self): _relationship, event_id = self.queued_event() @@ -895,6 +926,18 @@ def test_a_finding_note_or_anchor_must_be_text(self): ), ) + def test_a_finding_id_must_be_text_not_a_coerced_repr(self): + _source, revision_event = self._revision() + for bad in ({"criterion": "c-1"}, ["c-1"], 1, True): + self.assertRefused( + RefusalReason.MALFORMED_RECEIPT, + lambda bad=bad: report.record( + self.store, self.clock, event_id=revision_event, + **a_report(cxc_status=cxc.NEEDS_HUMAN, cxc_reason="incomplete", + review={"kind": cxc.FAIL, "findings": [{"id": bad}]}) + ), + ) + def test_the_fixed_preserve_boundary_is_never_shortened_away(self): _source, revision_event = self._revision() receipt = self.intake.get(revision_event)