Refresh current Claude prompt surface for v1.2.0

[lancekrogers]

Summary

• align the SDK with the current Claude -p/--print flag surface and stop emitting removed flags
• wire plugins, budget tracking, and subagent execution into the real prompt execution path
• update README, release notes, and examples for the v1.2.0 -p scope and remove stale top-level MaxTurns usage

Testing

• go test ./pkg/claude/...
• go test ./...
• just release check

[lancekrogers]

Objective review findings from obey-agent:

1. OnToolCall hooks for RunPromptCtx / RunFromStdinCtx only run after cmd.Run() has already completed. That means ToolFilterPlugin and any security or approval plugins cannot actually prevent tool execution on the JSON/non-streaming path; they only observe a completed transcript after the fact.

2. applyCompletionHooks suppresses ErrBudgetExceeded from BudgetTracker. Shared budget overruns now surface as successful calls, so callers cannot reliably stop when actual spend exceeds the tracker budget.

3. The new automatic plugin-manager wrapping changes plugin lifecycle semantics from one-time initialization/shutdown to per-call initialization/shutdown unless the caller pre-initializes manually. That contradicts the Plugin interface contract and can break custom plugins that allocate resources or expect manager state to stay live across calls.

Validation run for this review:

• go test ./...
• go vet ./...

[obey-agent]

Staff-Level Re-Review — Approve

Verdict: Approve

Overview: Turn-around from the first review pass (CHANGES_REQUESTED on f911ab2). Re-reviewed against ee76685 — all five inline findings have concrete code responses backed by tests, and the race-detector suite passes cleanly. This is exactly the kind of review response I want to see: each point addressed with either the exact fix requested or a reasoned alternative documented in code.

Verification Done

Cloned the PR head at ee76685 into a fresh worktree:

• go build ./... — clean
• go vet ./... — clean
• go test ./pkg/claude/... — all pass
• go test -race ./pkg/claude/... — clean, including the new TestRunPromptCtx_DoesNotMutateDefaultOptions and TestRunPromptCtx_HandlesLargeStreamJSONMessages

Verified each of my five inline comments against current source.

Per-Finding Disposition

1. Deprecated fields silent-ignore — Fully addressed. New validateDeprecatedOptions in options.go:358–375 rejects all six fields (PermissionTool, MaxTurns, ConfigFile, DisableAutoUpdate, Theme, PermissionCallback) with clear NewValidationError messages. TestPreprocessOptions_RejectsDeprecatedFields in enhanced_test.go:88–128 covers every case. Fail-loud option 1 — the stronger choice — was taken.
2. --file / --betas arg shape — Partially addressed. The code now has intent-describing comments (// Variadic Cobra flag: one --betas flag followed by one or more values.) and unit tests asserting the expected shape (claude_test.go:873–883, :938–947). No integration test against a real claude binary was added, so if the upstream CLI's flag type doesn't accept this form the drift won't be caught automatically. Happy to land as-is given the author's domain knowledge, but an integration smoke test would make this resilient to upstream flag-type changes. Not blocking.
3. bufio.Scanner 64KB default — Fully addressed. structured_run.go:105–107 now applies a 4MiB line cap with the exact pattern suggested. TestRunPromptCtx_HandlesLargeStreamJSONMessages in execution_hooks_test.go:394 pipes a 128KB JSON message through a mock binary, locking in the behavior.
4. v0.1.1 version labeling — Fully addressed. File renamed from RELEASE_NOTES_v0.1.1.md to PROMPT_SURFACE_NOTES_0.1.1.md. First line inside the doc clarifies 0.1.1 is a planning label and not a Go module tag. Option 2 from my comment, cleanly executed.
5. SubagentConfig.ToRunOptions silent behavior change — Fully addressed. Godoc on ToRunOptions (subagent.go:55–58) now states explicitly that the method preserves the subagent in Agents and does not flatten into SystemPrompt/AllowedTools. PROMPT_SURFACE_NOTES_0.1.1.md line 40 calls out the behavior change in a dedicated section. Both asks met.

Bonus: the PreprocessOptions thread-safety concern I raised in the same comment thread was also closed by the new prepareRunOptions clone-before-preprocess at every entry point (claude.go:54–67, also applied via buildStreamJSONRunOptions in structured_run.go:15–21). Regression test TestRunPromptCtx_DoesNotMutateDefaultOptions locks in that the caller's ParsedAllowedTools/ParsedDisallowedTools stay nil. The race detector is clean on the full test suite.

What's Done Well

• Fail-loud over silent-ignore on deprecated fields is the staff-level move. Every dropped field now gives a user immediate feedback on the first run.
• Clone-before-mutate is now the consistent contract across all entry points (RunPromptCtx, RunFromStdinCtx, StreamPrompt via buildStreamJSONRunOptions). The helpers resolveRunOptions / prepareRunOptions make this obvious to future callers.
• Regression tests at the exact failure surface — a 128KB mock JSON line for the bufio cap, a mutation-check for defaultOpts fields — each finding has a test that would fire if the fix regressed.
• Clear explanatory doc comments at the changed call sites and in the release notes. The PROMPT_SURFACE_NOTES_0.1.1.md disambiguation of planning label vs. module tag is particularly nice — exactly the kind of thing that prevents confusion downstream.

Remaining (Non-Blocking)

• Consider a single integration-style smoke test that invokes a stub claude binary asserting the received argv for --betas and --file. Would catch any future upstream flag-type drift and lock in the current intent at the CLI boundary.

Staff Standard

Yes. I would be comfortable submitting the response to this review as my own work. The diff quality, test discipline, and per-comment disposition are all staff-level. Approve for v1.2.0 tagging once ready.

[obey-agent]

Review — Refresh current Claude prompt surface for 0.1.1

Verdict: REQUEST_CHANGES — 4 inline comments posted. The mechanics of the prompt-surface refresh are well-executed, but two issues block merge as-is.

---

Semver — the headline concern

This is labeled 0.1.1 (patch), but it contains breaking runtime behavior changes. Callers on current 0.1.0 who set any of MaxTurns, ConfigFile, Theme, DisableAutoUpdate, PermissionTool, or PermissionCallback will receive a hard validation error after upgrading — their code compiled and ran fine before. That is a minor-version break at minimum (0.2.0), arguably a major one. The PROMPT_SURFACE_NOTES doc clarifies this is a planning label, not the release tag, but that needs to be resolved before tagging.

Options: (a) soft-warn and ignore deprecated fields, reserving errors for 1.0, or (b) bump to v0.2.0 with a migration guide.

---

Top 3 findings

1. Breaking behavior on deprecated fields (, ): silent success → hard runtime error is a semver violation for a patch release. See inline.

2. SubagentConfig.ToRunOptions hardcodes agent key (): always registers under "subagent", so any caller that previously set a custom agent name via the returned opts will silently mismatch. This is both a breaking change and a usability trap. See inline.

3. False-negative validation for IncludeHookEvents/IncludePartialMessages (): these flags are validated against Format != StreamJSONOutput in PreprocessOptions, but runPromptWithStructuredHooks upgrades JSONOutput → StreamJSONOutput internally — so valid option combinations are incorrectly rejected. See inline.

---

Minor issues

• fmt.Errorf in executeStreamJSON (): two fmt.Errorf calls where the rest of the package uses typed ClaudeError. Callers lose the ability to type-assert. See inline.
• Missing Godoc on EffortLevel constants: EffortLow through EffortMax have no per-value comments, which renders poorly on pkg.go.dev. See inline.

---

What's good

The core mechanics are solid: cloneRunOptions prevents mutation of caller-owned DefaultOptions (confirmed by TestRunPromptCtx_DoesNotMutateDefaultOptions), executeStreamJSON correctly drains stderr before returning errors, the scanner buffer is sized to 4 MB to handle large messages, budget tracking is now automatic and synchronized (callback refactor in budget.go removes the goroutine race), and the test suite is meaningfully extended with lifecycle hook and budget-exceeded coverage.

[obey-agent]

Re-review — Refresh current Claude prompt surface for 0.1.1

Prior findings status:

1. Semver (hard deprecated-field errors) — Resolved. validateDeprecatedOptions was replaced with warnDeprecatedOptions, which emits a one-time log.Printf via the injectable deprecatedOptionWarningf hook and leaves deprecated fields intact. PreprocessOptions no longer returns errors for MaxTurns, ConfigFile, Theme, DisableAutoUpdate, PermissionTool, or PermissionCallback. Runtime behavior is preserved for callers who set these fields; they get a one-time warning. TestPreprocessOptions_WarnsOnDeprecatedFields locks in the contract and the log-once behavior.

2. SubagentConfig.ToRunOptions hardcodes "subagent" — Resolved. ToNamedRunOptions(agentName, parentOpts) was introduced. ToRunOptions now delegates to it with "subagent" as the synthetic default, and its godoc explicitly states the default key. subagent_test.go tests ToNamedRunOptions("security", nil) and asserts opts.Agent == "security", Agents["security"] != nil, and Agents["subagent"] == nil.

3. False-negative validation on IncludeHookEvents / IncludePartialMessages — Resolved. supportsStreamJSONOutput now returns true for Format == JSONOutput && PluginManager != nil, which matches the upgrade path inside buildStreamJSONRunOptions. TestPreprocessOptions_AllowsInternalStructuredStreamFlags covers all three flags (IncludeHookEvents, IncludePartialMessages, ReplayUserMessages) under the JSONOutput+PluginManager combination.

4. fmt.Errorf in executeStreamJSON + EffortLevel docs — Resolved. All error paths in executeStreamJSON and structured_run.go now use newWrappedClaudeError with a typed ErrorType, consistent with the rest of the package. EffortLow through EffortMax all carry one-line godoc comments and render correctly on pkg.go.dev.

New findings: None. The latest commit ("Address second review round") is narrowly scoped to the four fixes above. No regressions or new issues observed on a fresh pass.

Verdict: Approve

Rationale: Every blocking and minor finding from the previous review is addressed cleanly, with tests covering the changed behavior. The deprecation path (warn-and-ignore) is the correct semver-safe choice for a published library at this stage. Code is consistent, the typed-error model is intact, and the stream-validation logic is now correct. Ready to merge.

[lancekrogers]

Re-review — Refresh current Claude prompt surface for v1.2.0

Verdict: APPROVE (maintains prior approval).

Re-review covering the commit pushed after the 2026-04-17 20:47 approval: e6981c0 — Correct release numbering to v1.2.0.

---

Scope of the new commit

Pure docs / release-packaging change: two files touched.

• Renames docs/PROMPT_SURFACE_NOTES_0.1.1.md → docs/RELEASE_NOTES_v1.2.0.md.
• Updates the README link (README.md:509) to the new path.
• Replaces the "Planning Label 0.1.1" framing with a proper v1.2.0 - Prompt Surface Refresh header, drops the "not a Go module tag" disclaimer, swaps go test ./pkg/claude/... for just release check in the Verification section, and adds a v1.1.0...v1.2.0 compare link.

No Go source code was touched in this commit. go test ./pkg/claude/... and go test ./pkg/claude/dangerous still pass locally against the PR head SHA.

Verification against prior-review resolution

The version-mismatch concern I raised at docs/RELEASE_NOTES_v0.1.1.md:1 on the previous round is now addressed in two ways:

1. First pass (pre-approval): the file was renamed to PROMPT_SURFACE_NOTES_0.1.1.md and reframed as an internal planning label — defensible but confusing for a public-SDK doc.
2. This commit (post-approval): the doc now follows the v1.1.0 → v1.2.0 tag progression, which is the right semver step for the surface described (additive new fields, documented behavior changes, no removed exported symbols). This is the correct final state.

No other prior findings were re-opened by this commit.

Semver sanity check

v1.2.0 is the right bump, with two caveats worth calling out in the notes (not blocking):

• SubagentConfig.ToRunOptions is a silent behavioral change for callers that inspected the returned RunOptions.SystemPrompt / AllowedTools directly. Documented, but the phrasing in the new Behavior Changes section reads as a neutral redesign — see inline comment.
• PermissionModeDelegate is now a hard validation error (pkg/claude/options.go:299), while other deprecated fields got the softer one-time-warning path. The rationale is defensible (no natural ignore semantics for a permission mode) but would benefit from one explanatory sentence — see inline comment.

Neither of these block the v1.2.0 tag; both are justifiable given that the upstream Claude CLI surface changed out from under us.

What's done well

• Clean, scoped commit: docs-only, no Go diff, easy to re-review.
• Correct tag progression (v1.1.0...v1.2.0) — no accidental downgrade into v0.1.x territory.
• The new ## Full Changelog compare link is the standard GitHub-release convention and will resolve correctly once the tag is pushed.
• just release check is a real, wired-up recipe (.justfiles/releases.just:47-54) that runs lint + test + build, so the Verification claim is substantive, not aspirational.

Staff standard

The prior review already exercised the code paths; this commit is a clean packaging correction that finishes the release story. Approval stands. Inline nits (4 total this round) are phrasing / release-hygiene suggestions, none of them gating.

Release checklist reminder

After merge, to keep the compare link valid and the module properly published:

git tag v1.2.0 <merge-sha>
git push origin v1.2.0
gh release create v1.2.0 --notes-file docs/RELEASE_NOTES_v1.2.0.md

If .justfiles/releases.just prepare already encodes this, a one-line pointer in the notes would close the loop.