Skip to content

Add explicit retained-column Draft document validation - #337

Merged
kikashy merged 5 commits into
mainfrom
codex/agent011-retained-column-runtime
Oct 26, 2026
Merged

kikashy merged 5 commits into
mainfrom
codex/agent011-retained-column-runtime

Conversation

@kikashy

@kikashy kikashy commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Agent's retained-column authoring path needs an explicitly selected Runtime document reader. This adds experimental CLI/MCP selection pinned to reviewed RFC0029 source ade3914b0b81ef299e5b9a8d6147f37ff3f0976c, byte-exact schema/corpus fixtures, discovery metadata, bounded document/frame validation and exact retained-byte digests. Ordinary paths retain their existing behavior.

Selected Draft MCP serialization preserves both text and structured representations. The unpublished standalone frame report keeps its complete original document and digest, plus bounded frameSummary counts; the typed frame remains internal rather than duplicating every decoded value on the wire. This closes review finding R1: raw Unicode separators and mandatory quote/backslash escaping previously exceeded the 16 MiB client envelope. No accepted input domain or transport limit is reduced or enlarged.

Supports Judgment-Pack/judgment-pack-agent#171 and Judgment-Pack/judgment-pack-spec#271. This PR implements the document-validation role only. Normative adoption, producer authenticity, preparation/offline roles, final Agent compatibility and published dependency support remain separate gates.

Material-decision impact: caller-selected experimental document admission and MCP response handling. Independent clean-room Codex review under the maintainer's temporary same-vendor exception found one MEDIUM (R1). The original reviewer confirmed R1 closed at the current head after all five exact-head CI jobs passed; no new actionable findings. The verbatim review and disposition are recorded here. This is not cross-vendor compliance.

Checks

One full local checklist at original candidate c15fe243c9f14388c4f20334f1847d8db72c6b7a, Go 1.26.10 / Python 3.12.11:

PASS     local-paths
PASS     agent-docs
PASS     diff-check
PASS     go-fmt
PASS     go-vet
PASS     go-test
PASS     conformance
PASS     build
PASS     registry-template
== 9 passed, 0 failed, 0 skipped, 0 not run ==

After R1, focused validation covers 33 actual CLI/MCP transport witnesses, complete reconstruction, typed-frame preservation, nine byte-exact comparisons (five ordinary and four unaffected selected surfaces) and both original reviewer requests. The largest tested Base response is 12,584,841 bytes; both raw-separator reviewer requests return 9,089,880-byte consumable, equivalent responses. Restoring only duplicate frame serialization fails at the intended bounded-client assertion (18,177,185 bytes); restored production passes. Complete original values, declaration order, null/absence, raw spelling and digest are retained.

Current candidate: 735566bc12915a3c40d612424ba8aa7701363857. The coordinator verified 1,349 source hashes and 29 artifacts before committing the four-file fix. Main and GitHub's automated carry changed only three Devkit provenance comments; merging the automatic carry preserved the final tree exactly. No repeated full suite was run for this bounded fix. All five exact-head CI jobs passed in run 40403014277; the original independent reviewer confirmed the fix. Normative adoption and publication/artifact gates remain separate.

@kikashy

kikashy commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of Runtime PR #337

Recommendation: request changes for R1 before approval. One MEDIUM finding; no HIGH or LOW findings. The completed source review found no other actionable defects in this candidate.

Reviewed head: c15fe243c9f14388c4f20334f1847d8db72c6b7a.
Base: a0a9c770f7078877d2fe4febc9d0a0726f5aa5de.
Complete binary diff SHA-256: 7ab5f24d3ee96468bf5c002d182101449a4994c2dfa34a12808bcc702e1b1236.

Reviewer: OpenAI Codex, host launch selection gpt-6.1-sol, reasoning effort high. The parent reports the explicit selector was accepted by the host; this record does not assert separate runtime model telemetry. High effort was selected for the untrusted document/frame parser, admission limits and CLI/MCP transport boundary. This is a separate reviewer session under the maintainer's authorized temporary same-vendor exception for Agent v0.11 and its required upstream work. It is not cross-vendor review or cross-vendor policy compliance. No builder reasoning or prior review report was supplied. The review used the issue, exact diff, pinned Draft source and existing verification evidence.

R1 — MEDIUM: literal U+2028/U+2029 still make an admitted Base response exceed the client envelope

Location: internal/mcp/dialect.go:291, particularly the selected text encoding and dual response returned at lines 295–298. The test gap is in internal/cli/dialect_retained_transport_test.go:322 through its frame construction at line 327.

SetEscapeHTML(false) leaves Go's unconditional U+2028 and U+2029 escapes enabled. A caller can supply either character literally in the UTF-8 frame, using three bytes per character. The structured report emits six-byte escapes. The text report does the same, and the outer JSON string escapes those backslashes again. The report also carries the original document and its inspected frame, so those repeated copies amplify the complete dual response.

The existing mixed-character stress test first constructs its input with Go's JSON encoder. That encoder has already expanded both characters before the frame byte ceiling is measured. The test therefore does not cover literal UTF-8 separator input even though it names both characters.

Handwritten independent probes supplied a complete generated frame with values:{}, declaration:null and one supported field whose opaque valueJson consists of literal U+2028, then independently U+2029. Each complete frame is exactly 2,097,152 UTF-8 bytes, with at most two trailing spaces used to reach the exact cap. The exact current Draft selector and base profile were supplied.

Observation U+2028 U+2029
Complete frame bytes 2,097,152 2,097,152
Complete MCP request bytes 2,097,414 2,097,414
CLI exit/status 0 / retained-frame-valid 0 / retained-frame-valid
CLI report bytes 8,389,331 8,389,331
MCP isError/status false / retained-frame-valid false / retained-frame-valid
Complete dual MCP response bytes 18,176,947 18,176,947
Client line limit 16,777,216 16,777,216

Expected: the supported Base frame has a complete response consumable by the bounded 16 MiB client while retaining both equivalent report forms and the exact retained data/digest. Actual: the server accepts the small request and reports success, but the response line exceeds that client limit by 1,399,731 bytes. The first newline is outside the permitted line window. Both decoded report forms agree and retained bytes/digests remain correct; the defect is complete response admission, not data corruption or an authorization bypass.

Disposition recommendation: fix this selected response path in the builder's fix round and add raw literal UTF-8 U+2028/U+2029 regression fixtures that do not pre-escape their frame through Go's encoder. Verify the complete serialized response through the bounded client reader, preserve ordinary serialization and exact data, and validate the fix's full escaping behavior. Do not silently reduce the supported Base frame envelope, remove a report form, clip data or raise a global limit. Re-read the fix against this finding after exact-head CI; no unrelated broad review is requested.

Evidence: <scratch>/probe.py, <scratch>/evidence/probe-results.json, and the two request/response JSONL pairs. The probe executed the actual pinned development binary through both its CLI and MCP commands. Binary SHA-256: 970b55389de9746802557e9c4b48d8184b1c8f5ec39810487ba37db55654b60c; Go 1.26.10; development version 0.39.0-retained171-test.1; no release claim. Before execution, all 1,349 files in its build-source manifest matched the isolated exact-head checkout. The binary itself also matched the recorded digest. Both probes ran with the shared heavy-check lock held and the required 2,576,980,378-byte free-space reserve verified.

Scope and evidence assessment

The complete sixteen-file diff was reviewed, including the CLI and MCP routes, shared ordinary reader changes, frame/declaration parser, selected discovery/input schemas, documentation and added tests. The byte-exact 673,006-byte corpus and schema were checked by SHA-256 and matched independently fetched Git blobs at the pinned immutable Spec revision. Runtime's use of these cases is explicitly document/frame admission; it does not claim to run their conversion or preparation expectations.

The exact trusted Draft tuple is revision ade3914b0b81ef299e5b9a8d6147f37ff3f0976c, schema 996cec17473987eebf1aa553536df0ce5a740971532567f411282ee70036879f, corpus e2ac53f56cd5ab779e032d2847b49fa04438ffb9df57d456f75f2ba058dc12d2. Neither caller hashes nor document leaves install a revision. The public selector is resolved before CLI file reads or artifact validation. MCP exact-member checks plus the existing dispatch duplicate/scalar checks reject extra, null, repeated and malformed selector arguments. No filesystem, network, acquisition or evaluation effect was added by the selected reader.

Source inspection and the reused tests cover base document/frame ceilings of 1/2 MiB, explicit document-only Extended selection with 2/4 MiB ceilings, profile mismatch refusal, independent parser budgets, fixed retained spelling/declaration limits, duplicate and surrogate refusal, target ownership and source ordering. Raw valueJson is opaque; declaration parsing uses json.Number and never float-rounds its original numeric spelling. Retained strings, absence/null distinctions, ordered declaration entries and exact supplied frame digests are preserved. Canonicalize/digest check encoding only, as documented. Check reuses the ordinary bounded pack/inventory walk and keeps execution/type claims false. Ordinary reader limits/defaults and ordinary tool serialization remain unchanged by this diff.

All sixteen source hashes from the completed stable-batch checklist matched the exact reviewed commit. Its recorded uncommitted candidate diff hash equals this commit's complete diff hash; its base-head field correctly reflects that precommit execution. Reused local evidence: nine checks passed, zero failed/skipped/not run, exit 0, sources unchanged. Reused coordinator-verified CI evidence: run 40392882932, all five jobs green on the exact reviewed head (check plus macOS stable, Ubuntu Go 1.24/stable and Windows stable). CI was not watched or rerun. No broad suite was repeated; only the two targeted CLI/MCP probes ran independently.

The PR correctly limits its claims to an experimental document-validation role. The Extended capability does not establish Runner Extended preparation. Merged Draft source does not establish normative adoption; the development binary and this source PR do not establish a released supported dependency. Agent #171's final certified two-language tool integration, actual consumer compatibility and upstream adoption/release gates remain separate. This review supplies no live provider or whole Desk execution evidence.

Finding disposition

ID Severity Proposed disposition State
R1 MEDIUM Fix selected response serialization and add literal separator boundary regressions in this PR's fix round Open; builder/maintainer disposition pending

Source was clean and unchanged when this report was frozen. No implementation edits, Git publication, installs, cleanup, CI mutation or delegation were performed. The reviewer stops here so the parent can reuse the builder for the bounded fix.


Runtime PR #337: original-reviewer confirmation of R1

R1 is closed at 735566bc12915a3c40d612424ba8aa7701363857. No new actionable findings were found in this bounded fix review. Recommend accepting the fix and recording R1 as fixed in this PR. The parent retains merge, publication and release authority.

This is a continuation of the original independent reviewer session, with its original OpenAI gpt-6.1-sol / high launch selection retained. It reuses the original review of c15fe243c9f14388c4f20334f1847d8db72c6b7a and its frozen report SHA-256 7b5ab48d0d381bec0ec50cfd1581504ad5f77aa0b63e229a76b7a23999ecb0e7. The authorized temporary same-vendor exception still applies; this is not cross-vendor review. No independent model telemetry beyond the recorded host launch selection is asserted.

Change assessed

I inspected the complete original-head-to-confirmed-head diff relevant to R1. Fix commit 37c809f838043507cce356cdf56d54662954bee7 changes the unpublished standalone-frame wire report: the complete exact original frame remains in document with its exact-byte digest; frameSummary adds bounded version, origin and inventory counts. Frame remains a complete typed inspection for internal Go callers and is excluded from JSON. Both MCP report forms remain present and contain the same complete report. Clients can recover the full frame from its exact retained document, including ordered declarations, opaque numeric/JSON spelling, statuses, absent members and null distinctions.

This is a documented layout adjustment to the selected unpublished report, not a reduction of accepted input or loss of source data. No output schema in this change promises the previous duplicated typed frame. Ordinary reports, default selection, trusted pins, input schemas/corpus, input and parser limits, and both JSON encoders are unchanged. Three carried Devkit provenance comments are the only other changes from the original review; their executable text is unchanged.

The serialization bound now follows the representation: a valid Base frame of F bytes is retained once per report. Its JSON string encoding requires at most 2F plus quotes, and quoting that report text again at most 4F plus fixed overhead. Both forms therefore require at most 6F plus bounded report/summary and normal fixed request-ID framing overhead. At F=2 MiB this leaves approximately 4 MiB beneath the 16 MiB client window. This bound is for the selected Base report under the exercised client framing; it does not establish a general Extended-response or arbitrary JSON-RPC-envelope guarantee.

Verification

I verified the fix manifest SHA-256 070a7c73ee6e480624df012fc6dab1edc8bb0bfbe8791199524357aad7af7f7e, all 1,349 candidate source-manifest files against the clean exact-head source checkout, and all 29 recorded fix artifacts. The existing binary is the actual development build 0.39.0-retained171-test.1, Go 1.26.10, SHA-256 da04bb7bb92a32c489232d49fbdf6da106cab64797f6d7251b29f108830129c3. Its build identity is preserved: it is not relabeled as a released binary or as built from the later comment-only carry. The source manifest, inspected carry and coordinator verification establish the relevant source equivalence.

To avoid duplicate application runs, I independently read the recorded actual-binary responses to both original reviewer requests through a bounded 16,777,216-byte client line reader. Both complete lines fit, contain no trailing second response, and parse successfully. I checked equivalent text/structured reports, successful status, exact original document/digest, full frame reconstruction and summary counts.

Original witness Before fix Fixed response Bounded client
Literal U+2028, exact 2 MiB Base frame 18,176,947 bytes 9,089,880 bytes PASS
Literal U+2029, exact 2 MiB Base frame 18,176,947 bytes 9,089,880 bytes PASS

I inspected and reused the verified 33 focused transport witnesses. They cover actual CLI/MCP paths, literal and escaped separators, mandatory quote/backslash expansion, literal backslash-u spellings, mixed Unicode/escaping, full-document reports, asserted/generated frame families, exact limits and one-over refusals. Their largest complete dual response is 12,584,841 bytes; the full-document maximum witness is 6,292,617 bytes. The reconstruction fixture preserves noncanonical source spacing/order, historical null, absence, opaque malformed typed JSON and ordered declarations. The internal typed-frame preservation check passes. Nine exact-byte comparisons cover five ordinary outputs and four unaffected selected contract/document outputs; all are equal.

The verified safeguard-removal control restores the duplicated typed-frame wire representation and fails at the intended bounded-client assertion (bufio.Scanner: token too long, 18,177,185-byte response). Restoring the fix passes at 9,089,880 bytes. This is an intended regression assertion failure, not a setup/compile/timeout result.

Reused exact-head CI record: run 40403014277, all five jobs successful for 735566bc12915a3c40d612424ba8aa7701363857. The earlier nine-PASS stable-batch checklist remains accurately attributed to its original candidate; current exact-head CI covers the corrected head. I neither reran the full checklist nor watched/retriggered CI. Independent artifact decoding was serialized under the shared heavy-check lock, with 4,399,529,984 free bytes checked against the required 2,576,980,378-byte reserve. No application execution was repeated in this confirmation.

Disposition and limits

Finding Severity Disposition
R1: admitted Base literal separator frame exceeds client response envelope MEDIUM Fixed in PR #337; confirmed closed at 735566bc12915a3c40d612424ba8aa7701363857

This confirmation covers R1, its representation change, regression coverage and relevant interactions. It is not a restarted whole-PR review. The original review supplies the remaining source assessment. No new actionable issue was found within this scope. Downstream consumer inspection statements in builder/coordinator evidence are supporting context, not an independently rerun Agent/Runner interoperability test.

The selected capability remains experimental document-validation only. Draft publication/merge is not normative adoption. The development binary and source PR do not satisfy immutable artifact publication, actual complete consumer compatibility or release gates. The parent owns those remaining decisions.

No source, Git or public mutation, install, cleanup, delegation, broad test run or additional binary build was performed. The original review and failing probe evidence remain unchanged. This finding-specific record and its hash manifest are frozen separately under <scratch>/r1-confirmation/.

Maintainer disposition

Finding Severity Disposition Evidence
R1 MEDIUM Fixed in this PR by commit 37c809f838043507cce356cdf56d54662954bee7; original independent reviewer confirmed closed at 735566bc12915a3c40d612424ba8aa7701363857. Both original requests fit; complete retained data and both report forms preserved; intended safeguard-removal failure and restored pass verified; all five exact-head CI jobs passed in run 40403014277.

No new actionable finding in the bounded confirmation. This is the authorized same-vendor Codex review; it is not cross-vendor compliance. Normative adoption and published dependency/artifact gates remain separate.

@kikashy
kikashy marked this pull request as ready for review October 26, 2026 21:36
@kikashy
kikashy merged commit 2fbcd13 into main Oct 26, 2026
7 checks passed
@kikashy
kikashy deleted the codex/agent011-retained-column-runtime branch October 26, 2026 21:36
@github-actions

Copy link
Copy Markdown
Contributor

🎉 Thank you for your contribution to Judgment Pack!

If you’d like another task, choose a path that interests you:

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant