Skip to content

✨ feat: extensions: check an extension's answer under --json - #42

Merged
pascalandy merged 1 commit into
mainfrom
m01/t3-answer-check
Oct 12, 2026
Merged

pascalandy merged 1 commit into
mainfrom
m01/t3-answer-check

Conversation

@pascalandy

@pascalandy pascalandy commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Why

M01-T3 asks core to hold an extension's answer under --json and pass it on only when the eight rules of Answer check hold, as D050 decides, so a broken answer never reaches the caller.

Scope

  • internal/contract/answercheck.go: CheckAnswer applies the eight rules to a Reply, and Reply.OnStdout places the answer: on stdout for a success and for a report with exit 78
  • internal/cli/dispatch.go: under --json, core holds stdout and passes every stderr line on except the latest, which may be the answer. A text result passes through
  • contract.DetectMode takes the resolved --json, as D046 decides
  • ManifestError becomes RuleError, because the answer check uses it too
  • compat_test.go gains the consumer case that M00 left for this milestone: a code that the consumer does not know is read by its exit code

Verification

  • extension_protocol.txtar: 15 broken answers, each RUNTIME with exit 1, stdout empty, and nothing held passed on, and 8 valid ones passed unchanged, including a failed report on stdout, a report that fails itself, a text result, diagnostic lines in order, and a last line without a newline
  • just check passes

Checklist

  • 1. Spec: Answer check and Modes in contract.md cover the rules and D046
  • 2. Scenario: extension_protocol.txtar fails without the check
  • 3. Code: the scenario passes
  • 4. Text: one extension.answer_* message per rule is a catalog entry
  • 5. Checks: just check passes
  • 6. Docs: M01-T3 is done, and README.md says that ncly checks the answer
  • 7. Sign off: just signoff runs at the merge, as the Merge section of AGENTS.md says

by Opus-5.5 via Claude Code

@pascalandy pascalandy self-assigned this Oct 11, 2026
@pascalandy

Copy link
Copy Markdown
Owner Author

2nd pass: Manifest.codePrefix holds the prefix of an extension's own codes (such as HELLO_), so the manifest check and the answer check no longer each derive it. No behavior change; just check passes on every layer.

by Opus-5.5 via Claude Code

@pascalandy

Copy link
Copy Markdown
Owner Author

Review round 1 (Codex GPT-6.1-Sol, xhigh): 3 findings, all fixed in this layer.

  • Fixed (medium): a batch item with "ok":null passed, because Unmarshal reads null as false. jsonBool now accepts only true or false, for the top-level ok too.
  • Fixed (medium): core wrote its own failure answer right after a last stderr line without a newline, so the answer did not start a line. That forwarded line now ends with a newline (the unterminated case).
  • Fixed (low): rule 4 asks for strings, not non-empty ones, so an empty hint no longer turns a valid answer into RUNTIME. null is still rejected (the nullhint case guards it).
  • Also here, from the ✨ feat: extensions: forward Ctrl-C and SIGTERM to the process tree #43 review: cautious ranked 130 before 143, but Output in contract.md puts them on one rank ("130 or 143, then 1..."). An answer led by TERMINATED that keeps an INTERRUPTED error was therefore rejected, including core's own re-rendered answer when another extension composes it. They now share a rank (the twosignals case).

Each new case was checked against the old binary: every behavior case fails on the old code, and the nullhint guard holds before and after.

by Opus-5.5 via Claude Code

@pascalandy
pascalandy force-pushed the m01/t2-dispatch branch 2 times, most recently from 27f9347 to ffe4195 Compare October 11, 2026 15:52
@pascalandy
pascalandy force-pushed the m01/t3-answer-check branch 2 times, most recently from 0359ce5 to e13039f Compare October 11, 2026 17:13
@pascalandy

Copy link
Copy Markdown
Owner Author

Cross-stack review, round 1 (Codex GPT-6.1-Sol, xhigh), kept in this layer: the answer check and the manifest share one regex for the form of a code, so the two rules cannot drift, and badCode became invalidCode, whose boolean is no longer inverted. TestAConsumerActsOnTheExitCodeOfACodeItDoesNotKnow now passes each answer through the real CheckAnswer and checks that core keeps its bytes, instead of testing only its own logic.

by Opus-5.5 via Claude Code

- Purpose: M01-T3 asks core to hold an extension's stdout and the last line of its stderr until it exits, and to pass the answer on only when the eight rules of Answer check hold, as D050 decides
- Impact: a broken answer becomes RUNTIME with exit 1 and never reaches the caller, a report answers on stdout with exit 78, a text result passes through, an unknown code keeps the meaning of its exit code, and --json selects non-interactive mode, as D046 decides

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@pascalandy

Copy link
Copy Markdown
Owner Author

Fixed PRIS1's P2, found in pass 2, in this layer's commit, now 5de8dad. Core dropped the answer's data when it could not write a valid answer to stdout.

Before: when CheckAnswer accepted an answer and the write to stdout failed, core answered RUNTIME with no other key. The caller lost output_dir, completed results, and warnings, although the work had happened.

Now: core writes the answer to stderr with ok false and the write failure leading, and keeps every other key as the extension sent it. This follows Output in contract.md: a failure after partial work keeps the data keys that still apply, and an unwritable output exits 1 with RUNTIME on stderr. When stderr refuses the answer too, the exit code stays 1. A protocol rejection still drops the unchecked output.

  • contract.ExtensionAnswer.Failed(lead) renders the rewrite, next to the answer check. ✨ feat: extensions: forward Ctrl-C and SIGTERM to the process tree #43's signal path used to call its own copy, Stopped. It now uses Failed, so a single function renders both rewrites.
  • reportKept in internal/cli/dispatch.go writes the rewritten answer for both paths: a stdout that refuses the answer here, and a signal from ✨ feat: extensions: forward Ctrl-C and SIGTERM to the process tree #43 on.
  • The exit code is 1 even when the extension's own verdict was 78, as for a report whose findings need a human. An unwritable output exits 1 with RUNTIME, RUNTIME maps to exit 1, ok false needs a nonzero exit, and exit 1 is more cautious than 78 in Output's order. The SDK's own --json write failure answers the same way.

Scenario (testdata/script/extension_protocol.txtar, new section "When stdout refuses a valid answer…"). It uses the existing closedpipe helper, which works on macOS too:

  • hello case artifact writes artifact.txt and answers with output_dir, a results item, and a warning. Under closedpipe, it exits 1, leads with RUNTIME on stderr, matches want-lost.txt with every key kept, and leaves artifact.txt on disk.
  • hello check failing, a report with exit 78, exits 1 with RUNTIME and keeps checks.
  • On Linux only, with stdout and stderr both on /dev/full, it still exits 1.
  • Without the fix, the cmp fails: stderr holds only {"ok":false,"contract_version":1,"errors":[{"code":"RUNTIME",…}]}.

The restack touched #43 to #50. Each layer from #43 up changes only dispatch.go, answercheck.go, and this scenario. Every layer passes just check on Linux and on macOS:

PR Branch Head Linux macOS
#40 m01/t1-manifest 80bcdf4 ok ok
#41 m01/t2-dispatch 68c8db1 ok ok
#42 m01/t3-answer-check 5de8dad ok ok
#43 m01/t4-cancellation f4d3540 ok ok
#44 m01/t5-stubborn b4d99f7 ok ok
#45 m02/t1-sdk-packages 824ff91 ok ok
#46 m02/t2-import-boundary 776e4b8 ok ok
#47 m02/t3-extension-entry 0e653ba ok ok
#48 m02/t4-bundled-transport 7936b26 ok ok
#50 m02/docs bbe6e9e ok ok

by Opus-5.5 via Claude Code

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