feat(runtime): start retained runs in existing environments - #827
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 2cd9c660
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-13T20:52:26Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.1s |
| Interrogation | 131.0s (2 bridge agents) |
| Total | 131.1s |
💰 Value — sound-with-nits
Adds a crash-safe 'fresh run in an existing environment' start operation and fixes a false-positive binding rejection of canonical session.updated events — both coherent, in-grain extensions of the retained-run module; ship.
- What it does: Two deltas. (1) New exported operation startRetainedRunInEnvironment (src/runtime/retained-run-start.ts:133): reconstructs an environment via provider.get(id), verifies id/provider match and dispatch+session capability, then runs the full durability protocol — phase 'environment' admission, detached dispatch with fresh sessionId/executionId minted from (environment.idempotencyKey, turn.turnId), ph
- Goals it achieves: Lets a caller start a new detached, replayable run that reuses an existing workspace/machine while explicitly not claiming prior-chat continuity — the docblock (retained-run-start.ts:124-131) directs same-chat turns to RetainedRunHandle.continueNative, keeping the two identity namespaces separate that commit 2cd9c66 named. Before this, the only options were startRetainedRun (always creates a new e
- Assessment: Good on its merits. The capability gap is real and the operation reuses the module's existing machinery rather than reinventing it: the ~70-line dispatch/admission/binding sequence is factored into dispatchRetainedRun, so the new function adds only reconstruction and validation. The ownership distinction is carefully honored and tested (retained-run.test.ts:248-398: zero create calls, zero destroy
- Better / existing approach: None found — searched for it. (a) Extending startRetainedRun with a union-typed environment param (create-input OR existing-id) would merge two operations with materially different failure semantics: startRetainedRun destroys an environment it created that turns out unusable (retained-run-start.ts:97-110), while startRetainedRunInEnvironment must never destroy an environment it does not own — the
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
A coherent new primitive that starts a fresh harness chat inside an existing retained environment, complementing continueNative (same-chat) without competing with it, plus a correctly-scoped binding fix for session.updated payloads.
- Integration: Fully wired. Exported through the public package surface at src/runtime/index.ts:366-369 alongside the sibling retained-run primitives. Like startRetainedRun/reconnectRetainedRun/recoverRetainedRun, it has no in-repo production caller because this is a published library consumed downstream; that matches the established pattern, not a gap. It is reachable by any downstream orchestrator that holds a
- Fit with existing patterns: Fits the grain. It reuses the exact admission/durability hook model and shares the dispatch core via the extracted dispatchRetainedRun helper (retained-run-start.ts:178-262), eliminating duplication with startRetainedRun. The capability is orthogonal to the existing continueNative (verified same-chat continuation): this is new-chat/same-workspace, which no existing primitive covers. Keeping it a s
- Real-world viability: Holds up off the happy path. Every pre-dispatch failure (provider.get returns null, environment lacks dispatch/session, provider reconstructs a foreign-named environment) throws before any admission record is persisted, leaving no orphaned durable state — verified by the test at retained-run.test.ts:350-393. The session.updated binding relaxation is narrow: runId and executionId checks remain acti
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 ~20-line validation preamble duplicated across the two start operations [maintenance] ``
startRetainedRun (retained-run-start.ts:68-83) and startRetainedRunInEnvironment (retained-run-start.ts:136-152) repeat the same assertStableText/identity/onAdmission/capabilities sequence with different field labels. A small shared preamble helper would remove it, but the field sets differ (environment.id vs create-input fields) and the codebase style favors explicit inline validation with per-operation error text; the heavy logic is already shared via dispatchRetainedRun. Note for a future tid
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
What changed
session.updatedevents.startRetainedRunInEnvironmentfor a fresh harness session in an exact retained environment.Proof
pnpm run typecheck: passed.pnpm run lint: 585 files passed.pnpm run verify:package: passed.pnpm run docs:check: passed.origin/mainmerge tree: clean.