Skip to content

refactor(acp): run the catalog as an effect service - #52471

Merged
nexxeln merged 3 commits into
v2from
acp-catalog
Oct 1, 2026
Merged

nexxeln merged 3 commits into
v2from
acp-catalog

Conversation

@nexxeln

@nexxeln nexxeln commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

#35457 | #37229 | #52284

Fixes #37229
Refs #52284

Why the change

The ACP catalog was a promise cache with hand-written reload maps and a Date.now() + Bun.sleep polling loop. This turns it into a connection-scoped Effect service that sessions can subscribe to in PR 4, and it now advertises the built-in /compact command.

Special things to note

  • ACPCatalog.promise(...) is a temporary adapter so the promise-based service.ts keeps working. PR 4 deletes it. It rejects with the typed ACPCatalog.Error, and the one promise() adapter in agent.ts classifies it. A not-ready catalog still returns -32603 "Internal service failure" and is still logged, but data.errorName is now "ACPCatalogNotReadyError" instead of "Error".
  • The default mode is now the first selectable agent from agent.list, which core orders with its resolved default first (the same selectedDefault() new sessions use), so ACP shows the agent a new session actually runs.
  • /compact comes from one built-in table in service.ts. Advertising and routing both use that table, and a built-in wins over a server command with the same name. Built-ins stay out of catalog.commands. Otherwise preparePrompt would treat /compact as a server command and set action: true, and the prompt would return before the compaction finished.

Change outline

The service is per connection. Each entry is keyed by the resolved cwd and has one SubscriptionRef, which is the only source of truth:

export interface Interface {
  readonly get: (cwd: string) => Effect.Effect<Catalog, Error>      // one shared load per cwd, failures not cached
  readonly reload: (cwd: string) => Effect.Effect<void, Error>      // resolves after a reload that started after the call
  readonly changes: (cwd: string) => Stream.Stream<Catalog, Error>  // current catalog first, then each reload
}
type Error = NotReadyError /* reason: "models" | "agents" */ | LoadError /* cause: the client error */
type Entry = { cwd; catalog: SubscriptionRef<Catalog>; lock: Semaphore(1); requested: number; loaded: number }

Every reload, whether from a caller or a server event, goes through one reload(entry) step. The reload counters let one load serve everyone who asked before it started:

reload(entry)
  target = ++entry.requested
  lock.withPermit
    if entry.loaded >= target → return              # a reload that started after this call already finished
    generation = entry.requested                    # this load covers every request made so far
    load → SubscriptionRef.set                      # a typed failure keeps the previous catalog
    entry.loaded = generation                       # skipped on defect or interrupt, so the next caller loads again

event stream (forkScoped, subscribed before the first read)
  server.connected                    → Deferred `connected` (also set if the stream ends)
  model/agent/command.updated         → forkIn(scope): reload(entry) for the matching location

The readiness poll now uses a Schedule, so it can be tested with TestClock:

- const deadline = Date.now() + 5_000
- while (Date.now() < deadline) {
-   read…; if (ready) return catalog
-   missing = defaultModel ? "No primary agents are available" : "No models are available"
-   await Bun.sleep(25)
- }
- throw new Error(missing)
+ read(client, cwd).pipe(
+   Effect.retry({
+     while: (error) => error._tag === "ACPCatalogNotReadyError",
+     schedule: Schedule.spaced("25 millis").pipe(Schedule.upTo({ duration: "5 seconds" })),
+   }),                                      // on timeout, fails with the last NotReadyError
+ )

The adapter and the request boundary:

ACPCatalog.promise({ catalog, run, changed })
  get(cwd)  → Live { cwd, get current() = runSync(catalog.get(cwd)) }   # reads the ref; never writes it
            → fork: changes(cwd) folded from the catalog `get` returned → changed(live, previous) when the ref changes
  reload(live) → catalog.reload(live.cwd)

agent.ts promise(): LoadError → its cause, then as before
  RequestError | ACPError → fail; Transport ClientError → ServerUnavailableError; anything else (incl. NotReadyError) → die, logged
 packages/cli/
 ├── src/acp/catalog.ts                # ACPCatalog.Service + make, reload step, temporary promise() adapter
 ├── src/acp/agent.ts                  # connect requires Scope and builds the catalog; promise() unwraps LoadError
 ├── src/acp/service.ts                # catalog via ACPCatalog.promise; builtinCommands table for advertising + routing
 └── test/acp/
+    ├── catalog-service.test.ts       # batched reload, failed reload keeps previous, 25ms spacing, gives up at ~5s
     ├── catalog.test.ts               # #37229 todo → test (built-in compact wins); default mode follows agent.list order
     ├── session.test.ts               # command list includes compact
     └── wire-fixture.ts               # agent runs in a Scope that dispose closes

@nexxeln
nexxeln merged commit 44336e8 into v2 Oct 1, 2026
10 checks passed
@nexxeln
nexxeln deleted the acp-catalog branch October 1, 2026 11:23
Ichinose-Kazuki pushed a commit to Ichinose-Kazuki/opencode that referenced this pull request Oct 7, 2026
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.

1 participant