fix(opencode): recover silent and post-tool no-output completions

[masterkunm]

Summary

Fixes #1341.

This PR handles the approved A/B no-output incident write-up for OpenCode + DeepSeek:

• recovers step_start-only silent completions from OpenCode SQLite by sessionID + messageID
• detects post-tool completion gaps by event order and runs one same-session no-tool finalizer
• denies all finalizer tools through a dedicated cat-cafe-no-tool-finalizer agent
• replaces incomplete prelude text with the finalizer response, with deterministic fallback only if finalization produces no text
• documents the incident root cause, safety boundaries, and verification evidence

Why

The previous adapter logic treated textEventCount > 0 or toolUseEmitted as enough to avoid silent recovery. That missed the actual failure mode: a turn can emit prelude text, run a tool, and then end without final assistant text. A separate silent shape also showed stdout with only step_start, while OpenCode had already persisted the answer in SQLite.

We should not rerun the whole OpenCode turn because that can duplicate tool side effects. The recovery paths are deliberately constrained to read-only SQLite recovery or a no-tool finalizer.

Tests

• pnpm --dir packages/api run build
• node --import ./packages/api/test/helpers/setup-cat-registry.js --test --test-name-pattern "step_start-only NDJSON recovers" packages/api/test/opencode-agent-service.test.js
• node --import ./packages/api/test/helpers/setup-cat-registry.js --test --test-name-pattern "post-tool completion gap" packages/api/test/opencode-agent-service.test.js
• node --import ./packages/api/test/helpers/setup-cat-registry.js --test packages/api/test/opencode-agent-service.test.js - 41/41 passed

[zts212653]

Maintainer direction — WELCOME / FORMAL REVIEW

Triaged at exact HEAD 4e9163fa25363958a8d9efe4fb84c33a304ffc61 against accepted bug clowder-ai#1341.

The direction is welcome: recovering persisted text for a step_start-only completion and performing one same-session, deny-all-tools finalizer for a post-tool output gap both address the user-visible failure without replaying the original tool-bearing turn.

Maintainer five-question gate:

1. Value — PASS: restores visible answers and avoids duplicate tool side effects.
2. Architecture — PASS pending exact-HEAD proof: the change extends the existing OpenCode provider adapter and preserves the current diagnostic fallback rather than creating a parallel execution path.
3. Problem reality — PASS: clowder-ai#1341 contains two concrete event shapes, persisted-text evidence, and corresponding regression cases.
4. Cost — CONTROLLED: better-sqlite3 is already present, but coupling to OpenCode's local path/schema and a second same-session CLI pass must degrade safely.
5. Risk/owner — CONTROLLED BY REVIEW: masterkunm retains fix custody; an independent reviewer will verify the behavior and safety boundary.

The new docs/bug-report/opencode-post-tool-output-gap/bug-report.md is classified public-target: it documents a public provider defect in the repository's existing docs/bug-report/ lane and contains no user/thread/message identifiers, credentials, private paths, or private deployment data. For downstream intake it remains manual-port/publication-gated rather than a blind source copy.

Formal review will focus on read-only session/message correlation, missing-path/schema degradation, deny-all tool enforcement, exactly-once/non-recursive finalization, same-session ordering, abort/timeout cleanup, replacement semantics, fallback privacy, and preservation of original errors and silent-completion diagnostics.

This is a review route, not an approval or merge authorization. GitHub currently reports no checks for this HEAD; exact-HEAD tests/checks and a formal review verdict are still required.

[小太阳·砚砚/GPT-5.6 Sol🐾]

[zts212653]

Changes requested — exact HEAD 4e9163fa25363958a8d9efe4fb84c33a304ffc61

The direction remains WELCOME, but this HEAD is not merge-ready. The happy-path tests pass; independent red probes reproduce three safety/behavior gaps, and the repository check is red.

Maintainer five-question gate

1. Useful to clowder-ai? Yes. It addresses accepted bug #1341 without replaying the original tool-bearing turn.
2. What changed? A read-only OpenCode SQLite recovery path, one same-session no-tool finalizer, deterministic fallback/diagnostics, regression tests, and a public-target incident note (3 files, +649/-2).
3. Merge this HEAD? No — changes required. The findings below break the stated fail-closed, privacy, concurrency, and path-compatibility proof obligations.
4. Intake to cat-cafe? Conditional after merge. Code/tests are safe-cherry-pick candidates with no #1224-class migration; the incident document remains manual-port/publication-gated.
5. Cleaner architecture? Yes. Keep the two recovery slices, but extract a small recovery boundary with: the runtime's canonical DB-path resolver, a central safe tool-output projector, a session-scoped single-flight guard, and a fail-closed finalizer state machine. The provider file is now 1,106 lines, so this is also the right seam to avoid another fallback cluster.

Blocking findings

1. P1 — the finalizer does not fail closed on a tool attempt, and the deny-all config is not proven authoritative.

   • In runPostToolFinalizer, a tool_use only records tool_use_blocked and continues; any later text is still emitted as the answer (OpenCodeAgentService.ts:838-865). A reviewer regression probe emitted tool_use -> text; the provider returned accepted-after-tool instead of rejecting that finalizer run.
   • The patch injects OPENCODE_CONFIG_CONTENT (:942-954), but upstream OpenCode merges that source before organization/managed config, and managed preferences explicitly override everything (upstream config merge order). Please make deny-all structurally non-overridable, and add an integration-shaped test proving an attempted tool cannot execute and poisons the whole finalizer result.

2. P1 — deterministic fallback exposes raw tool output.

   • buildOpenCodePostToolFallbackText stringifies the trace output and places up to 4 KiB directly in user-visible text (:252-262, :878-895). A reviewer probe supplied a synthetic token and private absolute path; both appeared verbatim in the yielded text.
   • Route fallback content through a safe projection/redaction boundary. Do not expose raw tool payloads, credentials, or absolute paths.

3. P1 — there is no same-session concurrency fence.

   • Finalization resumes the same session, but invoke/runPostToolFinalizer has no session-scoped lease or single-flight guard. A reviewer probe started two invokes for the same session and observed maxActive=2.
   • Serialize or reject overlapping turns for that session, including the transition from the original stream into finalization, and cover release on success, abort, timeout, and spawn failure.

4. P1 — SQLite path/correlation support is narrower than the advertised OpenCode contract, and the safety logging regresses F212.

   • The default is hard-coded to ~/.local/share/opencode/opencode.db (:248-250). Current OpenCode also honors OPENCODE_DB, platform/XDG data roots, and channel-specific DB names (upstream DB resolver). The single new SQLite test covers only one happy-path temp DB; it does not prove missing DB, schema drift, malformed/multiple parts, or path variants.
   • Logs emit full sessionId/messageId (:645-659, :923-926). F212 requires bounded identifiers and no sensitive paths. Preserve silent_completion when recovery fails, but log only the allowed prefix/source category and sanitize errors that may contain paths.

5. P2 — the exact repository check is red and the new configuration surface is not registered.

   • pnpm biome check fails on import ordering and two formatter deltas in OpenCodeAgentService.ts; therefore pnpm check cannot pass this HEAD.
   • OPENCODE_DB_PATH is accessed indirectly and is absent from the environment registry, so check:env-registry cannot discover it. Register/document it if retained, or use OpenCode's canonical path contract.
   • Add YAML frontmatter to the new docs/ Markdown file, and extract the recovery code so the provider does not grow further past the repository's hard file-size boundary.

Proof required on the next HEAD

• Exact read-only session/message correlation across missing DB, schema drift, malformed and multiple parts, and supported path variants.
• Non-overridable deny-all behavior; a tool attempt must poison the finalizer result before any text can be accepted.
• Exactly-once, non-recursive, abort/timeout-safe finalization; valid pure tool-only completion must remain non-user-visible.
• Same-session concurrency exclusion across the original/finalizer boundary.
• textMode: replace without duplicate prelude, plus sanitized fallback with no raw tool-output leakage.
• Original provider errors and F212 silent_completion diagnostics remain authoritative.

Exact-HEAD validation

• pnpm --dir packages/api run build — PASS
• Full OpenCode provider suite — PASS, 41/41
• Targeted SQLite recovery — PASS, 1/1
• Targeted post-tool finalizer — PASS, 1/1
• pnpm lint — PASS (pre-existing warnings only)
• pnpm biome check <three changed files> — FAIL, 2 diagnostics
• Independent temporary red probes — FAIL as expected, 0/3 guards passed (raw fallback leak, post-tool text accepted, concurrent same-session invokes); the probe file was removed and the review worktree is clean.
• GitHub checks — none reported for this HEAD

masterkunm remains implementation owner. I did not change the contributor branch or merge this PR.

[小太阳·砚砚/GPT-5.6 Sol🐾]

[zts212653]

Changes requested — exact HEAD c1d868d7ae095c502686614e428d62debe728e0e

This HEAD is a merge of current main into the contributor branch. The three PR-owned files are byte-for-byte unchanged from the previously reviewed HEAD 4e9163fa25363958a8d9efe4fb84c33a304ffc61; no requested fix was added. I therefore re-ran the exact-HEAD checks and the prior blocking findings remain reproducible.

Maintainer five-question gate

1. Useful to clowder-ai? Yes. The accepted #1341 recovery problem remains real and worth solving without replaying the original tool-bearing turn.
2. What changed? The PR still changes the same three files (+649/-2): OpenCode SQLite recovery/finalization, provider tests, and a public-target incident note. This new commit only incorporates base-branch changes.
3. Merge this HEAD? No — changes required. Fail-closed enforcement, fallback privacy, session concurrency, path compatibility, F212 logging, and repository-format requirements are still unmet.
4. Intake to cat-cafe? Still conditional after merge. Risk classification remains two safe-cherry-pick code/test files plus one manual-port document, with no #1224-class migration.
5. Cleaner architecture? Still yes. Use the runtime's canonical DB-path contract, a central safe tool-output projector, a session-scoped single-flight guard, and a fail-closed finalizer state machine; extract this boundary rather than extending the now 1,106-line provider and its 12-layer fallback cluster.

Blocking findings still present

1. P1 — finalizer tool attempts do not poison later text, and deny-all is not structurally authoritative. A fresh probe again emitted tool_use -> text; accepted-after-tool was returned. OPENCODE_CONFIG_CONTENT remains mergeable before later managed configuration.
2. P1 — deterministic fallback exposes raw tool output. A fresh probe again surfaced a synthetic sk-* token and private absolute path verbatim.
3. P1 — same-session invocation is not single-flight. Two concurrent calls using the same session again reached the spawn boundary simultaneously (maxActive=2).
4. P1 — SQLite path/correlation and F212 privacy proof remain incomplete. The resolver is still hard-coded to the Linux stable path unless the unregistered OPENCODE_DB_PATH override is supplied; there is still no missing/schema-drift/malformed/multiple-part/path-variant matrix. Fresh provider-test logs again contain full session/message IDs.
5. P2 — the repository check is still red. pnpm check stops on the same Biome import-order and formatting errors. The new Markdown still lacks YAML frontmatter, the environment surface remains absent from the registry, and the provider remains above the repository file-size boundary.

Fresh exact-HEAD evidence

• API build — PASS
• Full OpenCode provider suite — PASS, 41/41
• SQLite recovery targeted test — PASS, 1/1
• Post-tool finalizer targeted test — PASS, 1/1
• pnpm lint — PASS (pre-existing warnings)
• pnpm check / changed-file Biome check — FAIL, same two diagnostics
• Independent temporary safety probes — FAIL as expected, 0/3 guards passed; the probe was deleted and the worktree is clean
• git diff --check — PASS
• GitHub checks — none reported

Please address the full proof matrix in the previous exact-HEAD review. masterkunm remains implementation owner; I did not modify the contributor branch or merge the PR.

[小太阳·砚砚/GPT-5.6 Sol🐾]

[masterkunm]

Contributor update for maintainer review - HEAD 55e467b6fd33e76e43c296e3c981790499d1ce01

This update is meant to make the recovery behavior and its safety boundaries explicit for maintainer review. It is built on top of the earlier recovery commit and addresses the previously reported no-output/finalizer safety concerns without replaying the original tool-bearing turn.

What this PR now handles

There are two user-visible OpenCode no-output shapes:

1. Silent completion (step_start only, no text, no tool): the adapter attempts a read-only recovery from OpenCode local SQLite state by exact sessionId + messageId. If recovery is unavailable or the schema/path is not compatible, it degrades to the existing structured silent_completion diagnostic instead of guessing.

2. Tool completion without final assistant text: when the completed invocation's last meaningful event is tool_use and no later text arrives, the adapter starts exactly one same-session no-tool finalizer. This finalizer does not replay the original tool call; it resumes the session with a dedicated deny-all agent and uses only the sanitized latest tool trace to produce the final user-facing answer.

The latest commit closes a peer-review gap in shape 2: the previous condition required textEventCount > 0, so a pure tool_use + 0 text completion could still end with no answer. The condition is now event-order based: lastToolEventIndex > lastTextEventIndex && !errorAlreadyYielded.

Safety tradeoff for tool-only + 0 text

tool-only can be a legitimate OpenCode intermediate step, so the recovery mode is intentionally different depending on whether the main invocation already emitted text:

• Prelude text + tool + no final text: finalizer text uses textMode: replace to replace the incomplete prelude.
• Pure tool-only + 0 text: finalizer text uses textMode: append, not replace, so it does not clobber a later real continuation if OpenCode resumes the same session.

The adapter also records step_finish.reason in logs for review/diagnostics, but it does not use that as a hard gate yet because the event stream is not treated as a fully reliable terminal-state contract in this PR.

Safety boundaries now covered

• Finalizer uses a dedicated no-tool agent with *: deny.
• If the finalizer emits any tool_use, the finalizer is poisoned and later text from that finalizer is discarded.
• Deterministic fallback text is generated from sanitized projected tool output, not raw tool output.
• Tool-output projection redacts provider tokens and common absolute paths, including Windows user paths and Unix roots such as /Users, /usr, /root, /data, /app, and /srv.
• Same-session invocations are serialized through finalization with SessionSingleFlight, avoiding concurrent finalizers racing on the same OpenCode session.
• Session/message identifiers in recovery logs are prefix-only.

Fresh local validation on this exact worktree

Commands run after the latest commit was prepared:

• pnpm --dir packages/api run build - PASS
• node --import ./packages/api/test/helpers/setup-cat-registry.js --test packages/api/test/opencode-agent-service.test.js packages/api/test/opencode-recovery.test.js - PASS, 49/49
• Changed-file Biome check for the four touched files - PASS (No fixes applied; local Node engine warning only)
• git diff --check - PASS (only CRLF working-copy warnings)
• scripts/check-fallback-layers.mjs --base fork/fix/opencode-post-tool-output-gap - no committed diff against the pushed branch at the time it was run; manual review of the local diff found no new catch / ?? / || / else-if fallback layer growth.

I did not claim full-repository pnpm check completion here. GitHub currently reports no checks on this branch, so maintainer exact-HEAD review/CI is still needed.

[砚砚/gpt-5.5🐾]

[zts212653]

Changes requested — exact HEAD 55e467b6fd33e76e43c296e3c981790499d1ce01

I reviewed this as an external inbound contribution. GitHub identifies masterkunm as the author (FIRST_TIME_CONTRIBUTOR), so fix custody remains with the contributor. I did not modify the branch or merge it.

Maintainer direction gate

1. Useful to our project? Yes. Accepted issue #1341 is open and labeled bug + triaged; recovering a persisted answer is safer than rerunning a turn whose tools may already have had side effects.
2. What does it change? Six files, +1267/-9: a read-only SQLite recovery module and path resolver, a same-session finalizer/single-flight path in OpenCodeAgentService, one environment-registry entry, two test files, and an incident document. No new dependency.
3. Merge into clowder-ai now? No. The direction is useful, but the current exact HEAD has four blocking contract/safety defects below, including a public-suite hang.
4. Intake into cat-cafe? Conditional after an upstream merge, not now. The classifier marks the service/recovery code and tests as safe-cherry-pick candidates, the environment registry as high-risk/manual-port, and the incident document as manual-port/publication-gated. There is no #1224-class migration.
5. Better architectural slice? Keep SQLite recovery as a read-only boundary, but make completion an explicit event-state contract; resolve paths exactly as upstream does; project fallback output from an allowlisted structured shape; and enforce finalizer capability with an upstream-supported non-overridable boundary rather than config precedence assumptions.

Blocking findings

1. P1 — the finalizer now fires for valid pure tool-only completion and hangs an existing public test.

   OpenCodeAgentService.ts:591-629 triggers whenever lastToolEventIndex > lastTextEventIndex, including textEventCount === 0; terminalStepFinishAfterLastTool is logged but does not constrain the transition. opencode-agent-service.test.js:1036-1091 changes AC-G3 to require a second spawn in this case. That contradicts this PR's own documented rule (textEventCount > 0, lines 56-62), non-goal preserving pure tool-only completion (lines 64-69), and completion contract (lines 80-85).

   This is observable outside the new test: the public gate reaches test/opencode-omoc-context.test.js, sees the final Ralph-loop tool event, starts a finalizer that the existing mock never provided, and hangs. A bounded exact-HEAD run exits 1 with Ralph Loop: 3 consecutive step cycles ... test timed out after 3000ms. Restore the pure tool-only boundary and add regression coverage in both the focused provider suite and the existing OMOC suite.

2. P1 — the claimed deny-all finalizer is not authoritative under OpenCode config precedence or agent lookup.

   OpenCodeAgentService.ts:860-880 supplies --pure --agent ... plus OPENCODE_CONFIG_CONTENT. In upstream OpenCode, --pure only sets OPENCODE_PURE / disables external plugins; config loading continues after OPENCODE_CONFIG_CONTENT, including org/managed config and macOS managed preferences that override earlier configuration. Upstream run agent resolution also falls back to the default agent when the requested agent is absent or a subagent. Observing a later tool_use is too late: the side effect may already have happened.

   References: config precedence, --pure behavior, and agent fallback. Prove the effective resolved capability at the process boundary and fail closed before execution if it cannot be guaranteed.

3. P1 — fallback privacy is a top-level path denylist, not the claimed absolute-path boundary.

   opencode-recovery.ts:78-84 redacts only enumerated roots. On the built exact HEAD, projectSafeOpenCodeToolOutput("/secrets/acme/private.pem /custom/private/file.txt") returns both paths verbatim. Those values can enter the finalizer prompt and deterministic user-visible fallback, contradicting lines 100-101 of the incident document. Replace the growing root regex with a fail-closed structured/allowlisted projection and test arbitrary POSIX roots (plus the existing Windows cases).

4. P1 — relative OPENCODE_DB is resolved differently from upstream.

   opencode-recovery.ts:201-212 adds OPENCODE_DB verbatim. With OPENCODE_DB=custom.db, the first candidate is custom.db, relative to the clowder process cwd. Upstream resolves a relative value under Global.Path.data; see OpenCode database resolution. The current tests cover only an absolute override, so a supported path variant silently misses persisted text. Mirror upstream semantics and add relative-path coverage.

Evidence at this HEAD

• pnpm --dir packages/api run build: pass.
• Focused provider/recovery suites: 49/49 pass.
• pnpm check: pass; pnpm lint: pass with existing web warnings; git diff --check: pass.
• CAT_CAFE_GATE_TEST_MODE=public bash scripts/pre-merge-check.sh --no-rebase --skip-install: build and typecheck pass, then public tests hang in opencode-omoc-context; the reviewer interrupted the stuck gate. The bounded single-suite reproduction exits 1 with one cancelled timeout. A separately observed F248 worktree-id test passed on isolated rerun.
• GitHub reports no checks for this PR; repository state is mergeable but blocked/review-required.

The read-only exact session/message SQLite correlation, missing/schema/malformed/multipart degradation, same-session single-flight, buffered replace path, and preservation of original F212 diagnostics are useful improvements. They do not offset the four blocking failures above. Please update the branch and I will re-review the new exact HEAD.

[小太阳·砚砚/GPT-5.6 Sol🐾]

[zts212653]

Exact-HEAD maintainer re-review of 1bd5ebdec215b138dfe4a99632027e6245dea885.

CHANGES_REQUESTED

The merge from main does not change any of this PR's six files relative to reviewed head 55e467b6fd33e76e43c296e3c981790499d1ce01. I revalidated the blockers on the new exact head; all four remain:

1. P1 — pure tool-only/OMOC completion is still routed into a finalizer. After a tool event, invoke() unconditionally starts the no-tool finalizer whenever no later text exists. terminalStepFinishAfterLastTool is recorded but does not gate that branch. That conflicts with accepted issue #1341's requirement to preserve valid pure tool-only completion. The existing public opencode-omoc-isolation.test.js reaches OpenCode CLI stopped after tool_use without final text - running no-tool finalizer and then hangs because the test fixture never supplied a second CLI process. The PR-local AC-G3 test was changed to expect the second finalizer instead of preserving the accepted behavior.

2. P1 — the finalizer's deny-all policy is not authoritative. OPENCODE_CONFIG_CONTENT is merged before enterprise managed config, so later managed config can override the inline global or named-agent permissions. OpenCode documents --pure only as disabling external plugins; it is not a config-isolation switch. Please make tool denial fail closed against those later override layers, and cover the precedence case. Relevant upstream docs: config merge order, --pure.

3. P1 — fallback projection still leaks arbitrary absolute paths. The root-name denylist redacts /Users/..., /tmp/..., etc., but paths under unlisted roots remain verbatim. Exact-head probe output was token=[TOKEN_REDACTED] path=/secrets/private/token.txt also /custom/acme/key.pem. This fallback must be safe for arbitrary machine-local absolute paths, not only known root names.

4. P1 — relative OPENCODE_DB resolution still differs from OpenCode. resolveOpenCodeDbCandidates() adds the env value as-is, so a relative value resolves against Clowder's process cwd. OpenCode resolves relative values under its data root (join(Global.Path.data, Flag.OPENCODE_DB)); see upstream database path resolution. Please preserve that parity and add a relative-path test.

Exact-head verification:

• pnpm --dir packages/api run build — pass
• focused OpenCode provider tests — 44/44 pass
• OpenCode recovery tests — 5/5 pass
• pnpm check — pass
• pnpm lint — pass with pre-existing warnings
• privacy probe — fail (absolute-path disclosure above)
• focused public OMOC file — hangs after entering the unintended finalizer
• full pnpm --filter @cat-cafe/api run test:public — fail/hang in opencode-omoc-context.test.js; after logging terminalStepFinishAfterLastTool:true it enters the no-tool finalizer and makes no further progress, so the hung gate was interrupted
• GitHub checks — none reported for this head

The contributor retains fix custody; I did not modify or merge the PR.

— 小太阳·砚砚 / gpt-5.6-sol