Skip to content

An orphaned provider connection attempt leaks a live runtime process until restart #448

Description

@vishaltandale00

Problem

A pending managed provider connection holds live resources in the main process,
and the only thing that reclaims them is #cancelPendingConnection, reached
through cancelConnection. An attempt that outlives its owner keeps all of them
until the application restarts.

What a pending attempt owns:

Resource Reclaimed by
A spawned child process — codex app-server --listen stdio://, or claude auth login runtime.close()
Provider runtime state on disk removeRuntimeState(candidate)
A stored credential, for secret@1 adapters credentialStore.delete(reference)
The provider label reservation deleting the pending entry

The process is the significant one. CodexCredentialAdapter spawns a
long-running app-server child and terminates it only in close(); the Claude
adapter spawns claude auth login and SIGTERMs it only in close(). An
orphaned attempt therefore leaves a live child process with no owner.

Ways an attempt outlives its owner

BRW-005 binds an attempt to the renderer that began it and cancels on
webContents destruction, which covers a closed window. Two paths remain:

  1. Renderer reload. No setApplicationMenu is installed anywhere in the
    repository, so Electron's default menu applies and View → Reload (⌘R) is
    reachable in the packaged app. A reload replaces the renderer's JavaScript
    context — the poll loop and its ownership state both die — while the same
    webContents survives, so destroyed never fires.

  2. Abandoned browser sign-in. The user starts a managed sign-in, never
    finishes in the browser, and leaves Relayer open. account() keeps returning
    disconnected, which is a legitimate "login still in progress", so nothing
    settles the attempt. The renderer polls every 750 ms for as long as the window
    lives. The BRW-002 bounded-transient rule does not apply: it bounds only
    checks that cannot reach a verdict, not a verdict of disconnected.

Cleanup can forget a live process even when it runs

Independent teardown stages are already on the #423 head and are not
sufficient. #cancelPendingConnection deletes the pending entry and the runtime
handle before teardown, then runs the stages under Promise.allSettled and
discards the results, always returning true:

const pending = this.pendingConnections.get(connectionId);
if (!pending) return false;
this.pendingConnections.delete(connectionId);
...
this.runtimes.delete(connectionId);
await Promise.allSettled([ pending.runtime.close?.(), ... ]);
return true;

So a rejecting runtime.close() leaves a live child process with no handle in
any collection and no record that cleanup failed. Runtime-state and credential
removal are lost the same way. close() at shutdown has the same shape: it
gathers pending runtimes, closes them under allSettled, discards rejections,
and clears the maps, so a failed terminate leaves the child unreferenced while
the application exits regardless.

Required behavior

Retain cleanup ownership and the runtime handle until every applicable stage has
either succeeded or been proven absent. Specifically:

  • Each stage is idempotent and independently retryable, and ownership is
    released only when the last one is discharged.
  • Failed cleanup is retained and reconciled rather than discarded — including at
    startup, for ownership that survived a previous run.
  • Failures are recorded in diagnostics rather than swallowed.
  • Shutdown discharges or preserves outstanding ownership instead of clearing the
    maps unconditionally.

Proof

Each stage must be able to fail independently, including close() failing once
and succeeding on retry. Cover every exit path an attempt can take:

Relationship to #449

#449 moves label exclusivity to the durable commit. That fixes retry admission —
a leaked attempt would no longer block a retry by name — but settles nothing
here: the orphaned runtime, its child process, and its on-disk state remain.

This issue ships before #449, or atomically with it. It need not be a separately
merged predecessor if one change proves both contracts, but #449 must not ship
alone: without the reservation, repeated reload and retry cycles accumulate
multiple UUID-backed, same-label orphan runtimes, even though SQLite still
protects durable integrity.

Direction

A wall-clock deadline covers both remaining escape paths, since it depends on
neither renderer liveness nor which teardown signal fired. It needs an explicit
maximum sign-in window recorded in the PRD, and a decision on what the renderer
shows when an attempt expires under it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingneeds-triageNeeds initial triage and routing

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions