Skip to content

refactor(runtime): move headless permission setup into the agent adapter - #55

Merged
FreshlyBrewedCode merged 6 commits into
35-move-chunk-interpretation-into-adapterfrom
37-move-headless-permissions
Oct 2, 2026
Merged

FreshlyBrewedCode merged 6 commits into
35-move-chunk-interpretation-into-adapterfrom
37-move-headless-permissions

Conversation

@FreshlyBrewedCode

@FreshlyBrewedCode FreshlyBrewedCode commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Part of #32 · Closes #37

allocateWorkspace and resetClone write opencode.json (permission: {"*": "allow"}) into every run tree — the fix for #24, a real production deadlock where a headless opencode serve parks a turn on a permission ask nobody can answer. That policy is entirely opencode's business; a git-shaped workspace allocator has no way to know it exists and no business knowing it. ADR 0012 §3 draws the line: the agent runtime prepares its own workspace, and lib/workspace.ts goes back to being about git.

What changed

  • AgentAdapter gains prepareWorkspace(dir): Promise<void>; opencodeAdapter implements it by calling writeHeadlessPermissions (unchanged: still writes opencode.json and registers it in .git/info/exclude so writeBack never stages it). The replay/slow-fake test adapters no-op.
  • startRun calls adapter.prepareWorkspace(dir) once, before workflow.run, gated by a new prepareWorkspace?: boolean option — so it runs before the first agent step regardless of which adapter is wired in.
  • lib/workspace.ts and lib/clone.ts no longer import sandbox-config; allocateWorkspace's existsSync guard (only needed because workspace.test.ts's injected fake exec spawns nothing and leaves dir absent) is gone with the responsibility it was guarding.
  • The flag is threaded to cover both clone paths: the daemon sets it when startTrackedRun itself ran allocateWorkspace (workspaceAllocated && kind === "clone"); the CLI and the legacy HTTP body.dir + body.clone path set it explicitly after calling resetClone. Scratch workspaces never set it, matching current behaviour (the pre-refactor code only ever wrote the config past the scratch early-return).
  • sandbox-config.test.ts now drives opencodeAdapter.prepareWorkspace directly instead of going through resetClone; workspace.test.ts's now-obsolete opencode.json assertion is removed since that's no longer allocateWorkspace's job; run.test.ts adds coverage for both the true and absent prepareWorkspace cases.

Notes for reviewers

Verification

  • Tests added/updated: sandbox-config.test.ts (adapter-level), run.test.ts (prepareWorkspace true/absent), inline adapter fixtures across agent-step.test.ts/agent-step.usage.test.ts/replay/adapter.ts updated for the new interface member.
  • CI (gh pr checks 55): check (format + lint + typecheck + bun test) passes, lint-title passes. I did not re-run bun run check locally; relying on CI's green result.

Stack

  1. refactor(cli): replace hand-rolled parsers with effect/unstable/cli #52 — Parse CLI with Effect CLI (issue Parse CLI arguments with effect/unstable/cli #33)
  2. refactor(errors): represent domain failures as Schema.TaggedError #53 — Domain errors as tagged errors (issue Represent domain failures as Schema.TaggedError #34)
  3. refactor(runtime): move chunk interpretation into the agent adapter #54 — Move chunk interpretation into adapter (issue Move chunk interpretation into the agent adapter #35)
  4. refactor(runtime): move headless permission setup into the agent adapter #55 — Move headless permission setup into the agent adapter (issue Move headless permission setup out of the workspace allocator #37) ← this PR
  5. feat(runtime): add Effect composition root and agent runtime service #56 — Agent runtime service (issue Add an Effect composition root and make the agent runtime a service #36)
  6. refactor(daemon): move singletons into per-daemon layers #57 — Move singletons into layers (issue Move the daemon's remaining singletons into layers #38)

Stack created with GitHub Stacks CLI • Give Feedback 💬

@FreshlyBrewedCode
FreshlyBrewedCode added this pull request to stack #58 September 20, 2026 13:00
@FreshlyBrewedCode FreshlyBrewedCode changed the title 37 move headless permissions refactor(runtime): move headless permission setup into the agent adapter Sep 20, 2026
@FreshlyBrewedCode
FreshlyBrewedCode force-pushed the 37-move-headless-permissions branch from 5afc474 to b64a9be Compare September 20, 2026 13:15
FreshlyBrewedCode added a commit that referenced this pull request Sep 23, 2026
A 0-byte .tanstack-projected-* file under a stray data/src/factory/...
path leaked into 2f97451 from a sandboxed run writing through an
absolute path that mirrored the checkout. .gitignore already covers
.tanstack-projected-* and data/ (added in PR #55), which is why
nothing flagged it since; git rm drops it and its now-empty parent
dirs. No other stray data/ or .factory/workspaces/ paths are tracked
on this branch.

Part of #32
@FreshlyBrewedCode
FreshlyBrewedCode force-pushed the 37-move-headless-permissions branch from b64a9be to cdf760e Compare September 23, 2026 07:08
FreshlyBrewedCode added a commit that referenced this pull request Sep 23, 2026
A 0-byte .tanstack-projected-* file under a stray data/src/factory/...
path leaked into 2f97451 from a sandboxed run writing through an
absolute path that mirrored the checkout. .gitignore already covers
.tanstack-projected-* and data/ (added in PR #55), which is why
nothing flagged it since; git rm drops it and its now-empty parent
dirs. No other stray data/ or .factory/workspaces/ paths are tracked
on this branch.

Part of #32
FreshlyBrewedCode and others added 6 commits October 2, 2026 09:04
ADR 0012 §3: the adapter prepares its own workspace. The opencode adapter
writes opencode.json with the never-ask permission policy (#24). Test
adapters (corpus replay, slow fake) no-op.

Co-Authored-By: opencode-go/qwen3.7-plus <noreply@opencode.ai>
ADR 0012 §3: workspace preparation is the agent runtime's responsibility.
Called for clone workspaces only — scratch has no git tree to prepare.
Covers both the per-run allocateWorkspace path and the legacy resetClone
path, since both reach startRun.

Co-Authored-By: opencode-go/qwen3.7-plus <noreply@opencode.ai>
…e allocator (#37)

lib/workspace.ts and lib/clone.ts no longer import or call
writeHeadlessPermissions. The existsSync guard in allocateWorkspace
(needed only for the injected-fake-exec test) is gone along with the
responsibility.

The opencode adapter now owns workspace preparation via prepareWorkspace
(ADR 0012 §3), called by startRun when the caller signals a clone
workspace was provisioned. The daemon sets the flag when allocateWorkspace
ran; the CLI/HTTP legacy paths set it when resetClone ran.

sandbox-config.test.ts now tests opencodeAdapter.prepareWorkspace
directly instead of going through resetClone.

Co-Authored-By: opencode-go/qwen3.7-plus <noreply@opencode.ai>
…k artifacts (#37)

The signalAdapter helper in agent-step.test.ts was missing the new
prepareWorkspace method required by the AgentAdapter interface. Also
gitignore .tanstack-projected-* and data/ created by sandbox tooling.

Co-Authored-By: opencode-go/qwen3.7-plus <noreply@opencode.ai>
`.tanstack-projected-*` already matches at any depth; `data/` only helped
because workspaces live under /data on this host. The no-op adapters are
replay and fake, not scratch (a workspace kind).

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

Guards the #24 permission-deadlock regression: an allocated clone and a
legacy dir+clone POST must reach adapter.prepareWorkspace; scratch and
bare-dir runs must not.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FreshlyBrewedCode added a commit that referenced this pull request Oct 2, 2026
A 0-byte .tanstack-projected-* file under a stray data/src/factory/...
path leaked into 2f97451 from a sandboxed run writing through an
absolute path that mirrored the checkout. .gitignore already covers
.tanstack-projected-* and data/ (added in PR #55), which is why
nothing flagged it since; git rm drops it and its now-empty parent
dirs. No other stray data/ or .factory/workspaces/ paths are tracked
on this branch.

Part of #32
@FreshlyBrewedCode
FreshlyBrewedCode force-pushed the 37-move-headless-permissions branch from cdf760e to 183dc23 Compare October 2, 2026 09:41
@FreshlyBrewedCode
FreshlyBrewedCode merged commit 8e0a056 into main Oct 2, 2026
4 checks passed
FreshlyBrewedCode added a commit that referenced this pull request Oct 2, 2026
* refactor(dedupe): remove module-level singleton export

The dedupe registry is now always created via createDedupeRegistry()
and passed to consumers. This is the first step in moving the four
module-level singletons (dedupe, pubsub, active registry, refresh
gates) into per-daemon instances so two daemons can coexist in one
process.

Part of #38

* refactor(pubsub): convert module singleton to instance-based createPubSub()

Subscribers are now per-instance, so two daemons in one process do not
share event fan-out state. Consumers receive a PubSub instance rather
than calling module-level publish/subscribe.

Part of #38

* refactor(workspace): convert module-level refreshGates to instance-based

Refresh gates are now per-instance via createRefreshGates() and passed
to allocateWorkspace. This allows two daemons to maintain independent
mirror-refresh queues.

Part of #38

* refactor(runs): convert active registry to instance-based, remove test seams

The active-run registry is now per-daemon via createRunRegistry().
startTrackedRun and dispatchChildRun accept DaemonServices (registry,
pubsub, dedupeRegistry, refreshGates) as required parameters.

Removed optional test-seam fields:
- dedupeRegistry? from StartTrackedRunOptions and DispatchEnv
- beforeStart? from StartTrackedRunOptions

The slot reservation and dedupe-key claim remain synchronous
check-then-set with no await between, preserving concurrency safety.

Part of #38

* refactor(scheduler): remove registry?/now? test seams, use Effect Clock

SchedulerDeps now requires dedupeRegistry and clock as mandatory fields.
The clock comes from Effect's Clock service (currentTimeMillisUnsafe),
replacing the bespoke now?: () => number test seam. Tests provide a
Clock instance instead of injecting a now function.

Part of #38

* refactor(daemon,http): wire per-daemon services through composition root

startDaemon creates DaemonServices (registry, pubsub, dedupe, refresh
gates) and passes them to serve() and the scheduler. http.ts receives
services as a required ServerOptions field and uses them throughout.

This completes the core refactor - two daemons can now coexist in one
process with independent state. Tests still need updating to provide
services instead of using test seams.

Part of #38

* test: update all tests to use per-daemon services

Tests now create their own DaemonServices instances instead of using
module-level singletons or test seams. The beforeStart test seam is
gone - tests use slow workspace allocation or direct registry
manipulation to test the reserved-but-not-started window.

Scheduler tests use Effect's Clock interface instead of the bespoke
now?: () => number test seam.

All 250 tests pass.

Part of #38

* test: add two-daemon coexistence test (#38 acceptance criterion)

Proves that two daemons can run in one process without sharing run
state. Each daemon has its own registry, pubsub, dedupe registry, and
refresh gates. Tests verify:
- Independent run registries (each daemon sees only its own runs)
- Independent pubsub (events don't cross between daemons)
- Independent dedupe state (same key can be claimed in both daemons)

Part of #38

* fix: add port guard in two-daemon test for typecheck

* fix(stack): preserve usage accounting, domain-error messages and lint cleanups

* chore: remove committed sandbox artifact

A 0-byte .tanstack-projected-* file under a stray data/src/factory/...
path leaked into 2f97451 from a sandboxed run writing through an
absolute path that mirrored the checkout. .gitignore already covers
.tanstack-projected-* and data/ (added in PR #55), which is why
nothing flagged it since; git rm drops it and its now-empty parent
dirs. No other stray data/ or .factory/workspaces/ paths are tracked
on this branch.

Part of #32

* test(scheduler): drive scheduler tests with Effect's real TestClock

Replace the hand-rolled fake Clock.Clock with effect/testing's
TestClock, per issue #38's "tests use TestClock rather than an
injected now". tickOnce only ever reads the clock synchronously
(currentTimeMillisUnsafe()) and never suspends on Effect.sleep, so
none of TestClock's fiber-coordination machinery is exercised here —
but building it via Effect.runPromise(Effect.scoped(TestClock.make()))
and bridging setTime back into the plain-async fixture (ADR 0009 §5)
composes cleanly, so there was no need to keep the fake.

Part of #32

* test(scheduler): await the async fixture in the unknown-tag case

The #34 exhaustiveness test landed while `fixture` was still synchronous;
#38 made it async for `TestClock`. Rebasing the stack put the two together,
which typecheck caught.

* fix(runtime): use the constructed DedupeKeyError message in DispatchCollision

The domainErrorMessage helper was removed on the #34 layer; DedupeKeyError
now carries its message as a constructed field.

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

* test(server): pass daemon services to the prepareWorkspace tests

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

* fix(server): carry the agent-runtime fixes through the per-daemon services

Rebasing onto the reworked agent-runtime layer: startRun is synchronous
again, the daemon handle disposes its managed runtime via `stop()`, the
opencode default lives only in AgentRuntimeLayer, and the doc comments
restored below are kept on the per-daemon run registry and its options.

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

* fix(server): restore the lower-layer wiring and rationale the rebase dropped

- restore `prepareWorkspace` on StartTrackedRunOptions and the startRun call
  (allocated clone workspaces and the legacy `{dir, clone}` POST are prepared
  again, #37)
- restore the deleted rationale comments in http.ts, scheduler.ts, daemon.ts
  and runs.ts (SSE keepalive vs Bun's idleTimeout, the `development: false`
  router crash, D29 admission, DaemonOptions.config, runOnStart consumption)
- drop the unrelated `error` field added to DispatchCollision
- use Effect's default Clock instead of a hand-rolled live clock

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

* test(server): restore the invariant assertions the per-daemon rewrite weakened

The reserved-but-not-started window is now held deterministically by
pre-seeding the daemon's refresh gate for the workspace mirror, replacing the
removed `beforeStart` seam rather than relying on git clone timing. Restores
the strong M1/L1 assertions, the #34 TaggedError block, the DedupeKeyError
`_tag` check and the deleted test comments; adds a synchronous second-start
refusal check; drops the stale `adapter` DispatchEnv cast; and makes the
two-daemon test check dedupe holding and pubsub isolation while runs are live.

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

* fix(runs): decide admission before claiming the dedupe key

A start refused by the concurrency limit threw after claiming its dedupe key
and outside the try that releases it, so the key stayed held by a run that
never started. A scheduled fire with `overlap: "skip"` at the limit then
reported `skipped-overlap` forever. Admission is now checked first; claim and
slot reservation still happen with no await in between.

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

* refactor(daemon): compose per-daemon state as Effect layers

The run registry, run-event pubsub, dedupe registry and mirror refresh
gates become Context.Service classes with layers that build fresh state
per layer build. DaemonLayer composes them with AgentRuntimeLayer into
the daemon's ManagedRuntime (server/daemon-runtime.ts), replacing the
hand-threaded DaemonServices bag.

The plain-async boundaries (HTTP handlers, startTrackedRun, ctx.dispatch,
the scheduler's fire) take the daemon runtime and resolve services with
serviceOf, a synchronous runSync lookup over all-synchronous layers, so
startTrackedRun still reserves its slot and claims its dedupe key with no
await between check and set. The scheduler loop is forked on the daemon
runtime and resolves its dedupe registry and Clock from context; the
unused DaemonOptions.clock seam is gone. startRun accepts any runtime that
can run AgentRuntime effects (AgentRunner), since ManagedRuntime is
invariant in its services.

Tests build a fresh daemon runtime per case (createTestDaemon); the
two-daemon test also asserts no service instance is shared.

Refs #38

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

* feat(daemon): cancel active runs on shutdown

DaemonHandle.stop() (and so factory serve's SIGINT/SIGTERM handler) now
shuts the run registry down before the runtime goes away: it refuses new
starts (DaemonShuttingDownError, 503 over HTTP), cancels every running
handle, marks every reserved slot for cancel-on-start, and waits up to
shutdownTimeoutMs (default 5s) for the runs to settle. Runs therefore
persist RunCancelled and their ctx.exec children are killed, instead of
being orphaned and read back as interrupted after a restart.

Order: interrupt the scheduler, shut the registry down while the daemon
runtime is still live, stop the HTTP server (so SSE tails see the
cancellations), dispose the runtime. stop() is idempotent. The registry
layer's finalizer runs the same shutdown as a safety net when a runtime is
disposed without stop().

Every start now reserves its registry slot, not only admission-limited
ones, so a run still allocating is visible to shutdown either way.

Refs #38

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

* fix(runtime): end an abandoned agent stream at once

Stream.fromAsyncIterable closes the adapter iterable with a scope
finalizer that awaits iter.return(), and an async generator's return()
queues behind its pending next(). Interrupting a step that was waiting on
the model therefore blocked until opencode's next chunk, and the
onInterrupt abort that would have hurried it along only ran after that
finalizer: a POST /cancel took ~20s to record RunCancelled, and shutdown
blew its budget.

abortableIterable wraps the adapter stream: next() races the abort
signal, and return() aborts, starts the adapter's own return() without
awaiting it, and resolves immediately. The adapter teardown (killing the
opencode process) still runs; AgentStepHandle.teardown exposes it,
RunHandle.settled waits for every abandoned step's teardown after the
result, and registry shutdown now waits for settled rather than only for
the registry to empty, so stop() does not exit under a live agent.

Refs #38

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

* fix(daemon): let cancelled runs record their end after a timed-out shutdown

When the shutdown wait times out, stop() used to resolve as soon as the
runtime was disposed, and factory serve's process.exit could land between
AgentStepFinished{cancelled} and RunCancelled, so the run read back as
interrupted. After disposal (which interrupts whatever still held the
runs), stop() now waits a further bounded 2s for the cancelled runs to
settle via RunRegistry.awaitCancelled.

Refs #38

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

* fix(runs): refuse a start whose run id is still reserving

Since every start reserves its slot, a reserved entry for the same run id
can only be a concurrent start with the same explicit runId, and that path
skipped admission and started a duplicate run. Any existing entry, running
or reserved, now throws "already active".

Refs #38

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

* fix(daemon): build the daemon runtime's layers at startup

serviceOf resolves with runSync, which only works over an already-built
or synchronously buildable context. startDaemon now awaits
runtime.context() before serving, so a failing (or future asynchronous)
layer surfaces at startup rather than on the first request, and every
later lookup reads the built context. createTestDaemon already builds
eagerly by resolving its services at construction; documented.

Refs #38

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

* fix(runtime): bound how long a run waits for adapter teardown

Measured live against opencode: after an abort, @tanstack/ai's chat
engine can sit on the aborted stream indefinitely, so the adapter's
generator teardown never settles, even though the abort has already
killed the opencode process (the local-process sandbox kills the process
group on the spawn's abort signal). Waiting on that teardown made every
shutdown with an agent run use its whole budget. RunHandle.settled now
waits at most AGENT_TEARDOWN_GRACE_MS (1s) per abandoned step.

Refs #38

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

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

Move headless permission setup out of the workspace allocator

1 participant