Cloonar/coding-lab

Fork 0

Provider seam: adapter-owned state composition — ReadChat(runID, runtimeDir, transcriptPath) replaces ReadTranscript; PendingDialog/BlockedState become adapter-private #92

New issue

Closed

opened 2026-07-10 12:25:34 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-10 12:25:34 +02:00

Owner

Context

Decision from the 2026-07-10 grill on #62's design (full decision record on #62). Today, conversational state is composed on two floors: the adapter derives a state from its transcript (claudecode.deriveState — tail guess; codex.deriveState — rollout lifecycle events), then core's chat.Service.applyLiveSignals (internal/chat/chat.go) overlays the provider's spool signals (PendingDialog, BlockedState) with core-owned precedence rules. Core is second-guessing the adapter using the adapter's own signals.

The codex adapter (#87 / ADR-0037) shows the right altitude: it composes state from its agent's native signals (rollout task_started/task_complete/turn_aborted) with no spool at all, and core has nothing to add. The seam should make that the rule: the adapter owns "what state is my agent in", composed from whatever its agent's best signals are; core owns run lifecycle only (StateEnded override, transcript identity). "Liveness" never becomes a seam concept — #62's claude-code hook edges will then be a purely adapter-private feature, and core works identically with any adapter.

This refactor is behavior-neutral: no state semantics change, no API payload change.

Design

Seam signature. ReadTranscript(path string) (Chat, error) becomes:

ReadChat(runID, runtimeDir, transcriptPath string) (Chat, error)

transcriptPath may be "" (active run, transcript not yet located): the adapter must still consult its live signals — today core does this (chat.Service.Read consults the spool before the first LocateTranscript hit so a pre-transcript pending dialog surfaces).
runtimeDir may be "" (no runtime dir configured): signals off, transcript-only — same degradation core's liveSignals() gate applies today.
provider.Chat gains the side-channel field PendingDialog *Dialog, moved from chat.View. It stays OUT of the message stream — the #89/#90 invariant (seq numbers reparse-stable; the dialog is never a synthetic message) is preserved verbatim, just owned by the adapter now.

What leaves the seam. PendingDialog and BlockedState are removed from the LiveSignals interface and become claudecode-private functions, called inside its ReadChat with exactly the precedence core applies today:

live spool dialog → StateQuestion + Chat.PendingDialog;
else a transcript-flushed dialog stands on its own (the dormant fallback);
else a live blocked marker → StateNeedsInput;
else the transcript-derived state.

What stays on the seam. LiveSignals narrows to lifecycle plumbing lab genuinely owns: Setup (arming at spawn — lab writes the settings file and appends spawn args), SpoolSig (cheap change digest so the tailer knows when to re-read while the transcript is byte-frozen), SweepSpools (runtime-dir GC against the active keep-set). Unchanged.

Core collapse. chat.Service.Read and the tailer call ReadChat; applyLiveSignals and the liveSignals() resolver are deleted. View keeps TranscriptID and the StateEnded override for terminal runs (lifecycle, not signals) and forwards Chat.PendingDialog so the HTTP payload shape is unchanged.

Adapters.

claudecode: absorbs the composition above; deriveState and the spool readers become one private composition path.
codex: ReadChat ignores runID/runtimeDir (pure rollout fold; the exported ParseTranscript compat-test entry point is untouched).
providertest fake: migrated; keeps its LiveSignals lifecycle methods.

Conformance. Tier-1 read-path checks (internal/provider/providertest/conformance.go) drive ReadChat; both registered adapters pass via the existing registration table.

Regression guards (must not change behavior)

#89/#90: a pending dialog survives mid-window transcript writes and refetch churn; the dialog remains a side-channel field with reparse-stable seq numbers (existing tests keep passing).
The dormant fallback: a transcript-flushed dialog yields StateQuestion with no spool present.
Pre-transcript spool consult: an active run with no located transcript still surfaces a pending dialog.
An ended run's state is StateEnded regardless of any spool residue (core-owned, unchanged).

Acceptance

ReadChat(runID, runtimeDir, transcriptPath) on the seam; PendingDialog/BlockedState gone from the LiveSignals interface; no core code interprets spool signals.
applyLiveSignals deleted; state composition lives wholly in adapters; core keeps only the StateEnded override + TranscriptID.
claudecode, codex, and the providertest fake migrated; Tier-1 conformance passes for both adapters in CI.
HTTP chat payload shape unchanged (PendingDialog, TranscriptID, state vocabulary identical).
Core stays grep-clean of concrete provider references.

Out of scope

The liveness edges themselves and any state-semantics change (#62 — lands on top of this).
The runtime dead-session sweep (separate issue, filed alongside).
Any codex hook work (none needed — see #62's decision record).

Sequencing

Independently landable now. #62's implementation phase is blocked on this (its adapter needs runID+runtimeDir at read time to see its liveness markers).

## Context Decision from the 2026-07-10 grill on #62's design (full decision record on #62). Today, conversational state is composed on two floors: the adapter derives a state from its transcript (`claudecode.deriveState` — tail guess; `codex.deriveState` — rollout lifecycle events), then core's `chat.Service.applyLiveSignals` (`internal/chat/chat.go`) overlays the provider's spool signals (`PendingDialog`, `BlockedState`) with core-owned precedence rules. Core is second-guessing the adapter using the adapter's own signals. The codex adapter (#87 / ADR-0037) shows the right altitude: it composes state from its agent's native signals (rollout `task_started`/`task_complete`/`turn_aborted`) with no spool at all, and core has nothing to add. The seam should make that the rule: **the adapter owns "what state is my agent in", composed from whatever its agent's best signals are; core owns run lifecycle only** (StateEnded override, transcript identity). "Liveness" never becomes a seam concept — #62's claude-code hook edges will then be a purely adapter-private feature, and core works identically with any adapter. This refactor is **behavior-neutral**: no state semantics change, no API payload change. ## Design **Seam signature.** `ReadTranscript(path string) (Chat, error)` becomes: ```go ReadChat(runID, runtimeDir, transcriptPath string) (Chat, error) ``` - `transcriptPath` may be `""` (active run, transcript not yet located): the adapter must still consult its live signals — today core does this (`chat.Service.Read` consults the spool before the first `LocateTranscript` hit so a pre-transcript pending dialog surfaces). - `runtimeDir` may be `""` (no runtime dir configured): signals off, transcript-only — same degradation core's `liveSignals()` gate applies today. - `provider.Chat` gains the side-channel field `PendingDialog *Dialog`, moved from `chat.View`. It stays OUT of the message stream — the #89/#90 invariant (seq numbers reparse-stable; the dialog is never a synthetic message) is preserved verbatim, just owned by the adapter now. **What leaves the seam.** `PendingDialog` and `BlockedState` are removed from the `LiveSignals` interface and become claudecode-private functions, called inside its `ReadChat` with exactly the precedence core applies today: 1. live spool dialog → `StateQuestion` + `Chat.PendingDialog`; 2. else a transcript-flushed dialog stands on its own (the dormant fallback); 3. else a live blocked marker → `StateNeedsInput`; 4. else the transcript-derived state. **What stays on the seam.** `LiveSignals` narrows to lifecycle plumbing lab genuinely owns: `Setup` (arming at spawn — lab writes the settings file and appends spawn args), `SpoolSig` (cheap change digest so the tailer knows when to re-read while the transcript is byte-frozen), `SweepSpools` (runtime-dir GC against the active keep-set). Unchanged. **Core collapse.** `chat.Service.Read` and the tailer call `ReadChat`; `applyLiveSignals` and the `liveSignals()` resolver are deleted. `View` keeps `TranscriptID` and the `StateEnded` override for terminal runs (lifecycle, not signals) and forwards `Chat.PendingDialog` so the HTTP payload shape is unchanged. **Adapters.** - claudecode: absorbs the composition above; `deriveState` and the spool readers become one private composition path. - codex: `ReadChat` ignores `runID`/`runtimeDir` (pure rollout fold; the exported `ParseTranscript` compat-test entry point is untouched). - providertest fake: migrated; keeps its `LiveSignals` lifecycle methods. **Conformance.** Tier-1 read-path checks (`internal/provider/providertest/conformance.go`) drive `ReadChat`; both registered adapters pass via the existing registration table. ## Regression guards (must not change behavior) - [ ] #89/#90: a pending dialog survives mid-window transcript writes and refetch churn; the dialog remains a side-channel field with reparse-stable seq numbers (existing tests keep passing). - [ ] The dormant fallback: a transcript-flushed dialog yields `StateQuestion` with no spool present. - [ ] Pre-transcript spool consult: an active run with no located transcript still surfaces a pending dialog. - [ ] An ended run's state is `StateEnded` regardless of any spool residue (core-owned, unchanged). ## Acceptance - [ ] `ReadChat(runID, runtimeDir, transcriptPath)` on the seam; `PendingDialog`/`BlockedState` gone from the `LiveSignals` interface; no core code interprets spool signals. - [ ] `applyLiveSignals` deleted; state composition lives wholly in adapters; core keeps only the `StateEnded` override + `TranscriptID`. - [ ] claudecode, codex, and the providertest fake migrated; Tier-1 conformance passes for both adapters in CI. - [ ] HTTP chat payload shape unchanged (`PendingDialog`, `TranscriptID`, state vocabulary identical). - [ ] Core stays grep-clean of concrete provider references. ## Out of scope - The liveness edges themselves and any state-semantics change (#62 — lands on top of this). - The runtime dead-session sweep (separate issue, filed alongside). - Any codex hook work (none needed — see #62's decision record). ## Sequencing Independently landable now. #62's implementation phase is blocked on this (its adapter needs `runID`+`runtimeDir` at read time to see its liveness markers).

dominik.polakovics added the

ready-for-agent

enhancement

labels

2026-07-10 12:25:34 +02:00

dominik.polakovics referenced this issue

2026-07-10 12:26:38 +02:00

Chat: derive "working" from a real liveness signal, not the transcript-tail guess (root cause of #38 + #39) #62

dominik.polakovics referenced this issue

2026-07-10 12:26:51 +02:00

Chat: the Interrupt button does nothing #38

dominik.polakovics referenced this issue

2026-07-10 12:27:05 +02:00

Chat: a running Claude Code workflow (background subagents) reads as needs-input / idle, not working #39

dominik.polakovics referenced this issue from a commit

2026-07-10 13:00:40 +02:00

refactor(provider): adapter-owned state composition — ReadChat(runID, runtimeDir, transcriptPath) replaces ReadTranscript; PendingDialog/BlockedState become adapter-private (ADR-0038)

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-10 13:01:03 +02:00

refactor(provider): adapter-owned state composition — ReadChat replaces ReadTranscript; PendingDialog/BlockedState become adapter-private (ADR-0038) #94

dominik.polakovics commented

2026-07-10 13:17:50 +02:00

Author

Owner

This was generated by AI while landing a PR.

Validation of PR #94 (afk/92) — verdict: PASS

Relied-on verification signal: Forgejo CI ci / native — success (exit 0, 6m47s). Not re-run.

Title / AFK contract — Conventional Commits (refactor(provider):); valid Closes #92.
Core seam — Read routes runtimeDir only for active runs; ended runs read transcript-only by construction (runtimeDir "") and are force-StateEnded with nil dialog. Active-no-transcript delegates to ReadChat(id, dir, ""); transcriptID("") correctly yields "" (no identity leak).
Tailer — stages stat results and commits only after a successful ReadChat, retrying on read error; a robustness improvement, not a regression.
Adapters — claudecode preserves the exact old precedence (spool dialog → StateQuestion+side-channel → dormant transcript fallback → blocked marker → transcript state); read error precedes any overlay so ErrTranscriptGone is unchanged. codex is a pure rollout fold ignoring runID/runtimeDir.
Behavior-neutrality is pinned — all four issue-checklist regression guards asserted (#89/#90 side-channel seq-stability; dormant fallback; pre-transcript consult; ended-run StateEnded-by-construction), plus seam-arg forwarding tests and a new hermetic Tier-1 read-chat conformance obligation (empty-path idle contract + ErrTranscriptGone sentinel) driving both adapters, with negative broken-adapter coverage for all three failure shapes.

No blockers, no concerns. Clean to merge on human free-text go-ahead.

> *This was generated by AI while landing a PR.* **Validation of PR #94 (`afk/92`) — verdict: PASS** Relied-on verification signal: **Forgejo CI `ci / native` — success (exit 0, 6m47s)**. Not re-run. - **Title / AFK contract** — Conventional Commits (`refactor(provider):`); valid `Closes #92`. - **Core seam** — `Read` routes `runtimeDir` only for active runs; ended runs read transcript-only *by construction* (`runtimeDir ""`) and are force-`StateEnded` with nil dialog. Active-no-transcript delegates to `ReadChat(id, dir, "")`; `transcriptID("")` correctly yields `""` (no identity leak). - **Tailer** — stages stat results and commits only after a successful `ReadChat`, retrying on read error; a robustness improvement, not a regression. - **Adapters** — claudecode preserves the exact old precedence (spool dialog → StateQuestion+side-channel → dormant transcript fallback → blocked marker → transcript state); read error precedes any overlay so `ErrTranscriptGone` is unchanged. codex is a pure rollout fold ignoring runID/runtimeDir. - **Behavior-neutrality is pinned** — all four issue-checklist regression guards asserted (#89/#90 side-channel seq-stability; dormant fallback; pre-transcript consult; ended-run StateEnded-by-construction), plus seam-arg forwarding tests and a new hermetic Tier-1 `read-chat` conformance obligation (empty-path idle contract + `ErrTranscriptGone` sentinel) driving both adapters, with negative broken-adapter coverage for all three failure shapes. No blockers, no concerns. Clean to merge on human free-text go-ahead.

dominik.polakovics closed this issue

2026-07-10 13:21:18 +02:00