[brennanb2025]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​52	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​52
Prod	7	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​95	$\color{#cf222e}{\Huge{\mathbf{−}}}$​42	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​53

ELI5

Antigravity chats need Google's separate Antigravity chat server. An earlier version of this stack downloaded it silently the first time a chat started: about 18 seconds of "Working for Ns" with no progress and no Cancel, and every extra chat opened started another download. Now nothing downloads until the person clicks Install, which shows the size, progress and Cancel in Settings and in the new chat.

This PR handles what that server does once installed but signed out. It doesn't answer "please sign in". It prints a sign-in link on its error output and then waits up to five minutes for someone to open it. Nobody sees that link in a chat, so the chat just hangs. With this PR an agent can declare "this line means I'm waiting for a browser sign-in". When Orca sees that line, the chat stops waiting right away and shows Orca's normal "not signed in" state.

What Changed

This revision: no code change. The differences list below was corrected: the per-agent matcher on the launch description is how the common pattern does it too, so it is no longer listed as a difference, and the long-line case is now listed.

• Before: an Agent Client Protocol (ACP) agent that waits for a browser sign-in hangs the chat start until the agent's own timeout (five minutes for Antigravity's server). If it happened during a running turn, the turn ended as "unknown" and the chat as "agent exited".
• After: an ACP launch spec can declare signInPrompt(line). Orca reads that agent's error output line by line. On a match it closes the connection with its existing typed sign-in error, AcpAuthRequiredError. A chat start fails as not signed in, and a running turn fails the same way it does when the agent itself answers "sign in required". The matched line holds the sign-in link, so it never goes into the retained error tail, error messages or diagnostics.
• Merged alone: no agent sets the field yet, so nothing visible changes. feat(acp): Antigravity in the Chat UI over its ACP server #28595 (Antigravity in Chat UI) is the first to set it.

Mechanism:

• spawnManagedProviderProcess takes an optional onStderrLine(line): boolean on the existing error-output drain. With it, output is split into complete lines and a line the callback claims is withheld from the tail. The pending partial line is capped at the tail size (8 KiB), so a long link split across chunks is still classified by its start. Without the callback, behaviour is unchanged.
• The ACP connection passes the spec's matcher in and, on a match, closes through the existing process owner.
• Mid-turn: the connection's close cause now reaches the turn tracker (AcpStructuredTurns.end(reason, cause)). When the cause is the sign-in error, the running turn takes the same failure path as a sign-in refusal from the agent (now one shared method) instead of settling "unknown". The helper for a send refused before it was written moved beside it as rejectedAcpDispatch, with no behaviour change.

Why

The common pattern for agents that sign in through a browser is to detect the waiting state and show a clear signed-out state, not to hang, and it does so the same way: a per-agent error-output handler handed to the generic agent runtime. Detecting the line through data on the launch spec keeps agent names out of shared code. Ending the turn through the existing sign-in failure path reuses what the chat already shows for "not signed in" rather than adding a new state.

Linked Issue

N/A (maintainer; part of the Antigravity Chat UI stack).

Visual Proof

This PR changes nothing visible on its own; the agent that sets the field arrives in a later PR. The result it produces was captured in a real run of the whole stack (M4Air, build d871611d303, a throwaway profile, nobody signed in, driven over the Chrome DevTools Protocol only): a message sent to a new Antigravity chat ended as "Your message was not sent. Sign in to Antigravity." instead of the chat waiting, and the sign-in link did not appear in the error text.

This screenshot was taken before this PR's last review fix (long lines no longer keep their start in the saved error output). That fix is covered by a unit test, not by a screenshot.

Testing

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

• Connection test over the existing fixture process: a pending prompt, a noise line, then the sign-in line split across two chunks, with a link longer than the 8 KiB partial-line cap. The prompt rejects with the sign-in error, the process exits, the link appears in none of the error, diagnostics or retained tail, and the noise line is kept.

• Session test: a running turn whose connection closes with the sign-in error settles as not signed in. It fails without the turn-tracker change.

• pnpm tc:node and all ACP and provider-process tests (712) passed. The change is platform-independent.

Review

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

• Accepted edge: if the agent exits before its buffered sign-in output is read, the start reports the agent exit. The matched sign-in line is still withheld from retained output.
• Security: the sign-in link is kept out of every retained or reported string.
• After a mid-turn sign-in close the session also ends, because the process is closed, so the host also reports an ended session. At chat start, QA on feat(acp): Antigravity in the Chat UI over its ACP server #28595 saw one signed-out notice and no second "agent exited" error. The mid-turn case has not been checked live.
• No SSH, mobile or wire impact: detection runs on the host that runs the agent.
• PR author: @BrennanKB5

Differences from the common pattern

• Intended: a line counts as a sign-in prompt by its prefix alone; the link itself is never parsed or used, because chats sign in through a separate flow (feat(antigravity): sign Antigravity chats in with the server's own Google sign-in #28650). The common pattern ignores a prompt line whose link fails its own check, and the server then waits up to five minutes for a browser, so that chat start hangs; here it ends signed out at once.
• Intended: a very long partial line is still recognised by its start, where the common pattern drops lines over 16 KiB and the same five-minute wait follows.
• Intended: only the error output is watched. The pinned server prints its sign-in prompt only there on purpose, because its standard output carries the protocol, and Orca runs only a copy that matches every pinned file. The common pattern also scans standard output because it can run other builds from a custom path.
• Intended: the rest of the error output is still kept for failure details; only the line holding the link is dropped. Orca's shared error-output tail feeds failure details for every agent.
• Intended: the result is the chat's existing "not signed in" failure on that send or start, not a signed-out flag for the whole agent, because each host keeps its own sign-in. The shared "not signed in" fact already drives the restart after sign-in for every agent.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[brennanb2025]

Some chat agents print a browser sign-in link on their error output and then wait, leaving chat startup or a turn hanging. This PR adds an optional detector that uses Orca's usual agent shutdown path and reports signed out; no agent enables it yet, so current chat behavior is unchanged.

Review fixed a privacy defect: a fragmented sign-in line longer than the buffer could lose its prefix and expose the link in retained output. Commit 4dea606175f8b276fb3d45875e26fc3d79551abc preserves the bounded prefix, and the existing connection test now covers that boundary. The PR remains at 70 production additions and 42 test additions.

Two review rounds, an architecture challenge, and dedicated readiness/performance review found no remaining new issue. All 712 targeted Node tests and ordinary typecheck passed. All 13 applicable final CI jobs passed, including five Node test shards, relay integration, Linux/Windows packaging, repository guards, line budget, and static analysis/typecheck. Local Bun 1.3.14 could not run the pinned CI typecheck command, which passed in CI.

Accepted edge: if the process exits before buffered sign-in output is read, startup reports the agent exit while withholding the link. Deferred to the follow-up that enables Antigravity in Chat UI (stack PR 8): verify how a mid-turn signed-out failure and the subsequent session exit appear together. Electron QA is N/A here because the detector has no enabled agent; live app flows and parser throughput were not measured. The PR remains open.

[coderabbitai]

📝 Walkthrough

The change adds configurable detection of browser sign-in prompts in ACP stderr. Matching lines are excluded from the retained stderr tail, and the connection closes with AcpAuthRequiredError. The session passes connection errors to active turns, which classify authentication refusals as notSignedIn. Dispatch rejections use a shared ACP outcome helper. Tests cover stderr prompt detection, filtering of sensitive prompt text, and classification of a pending message after an authentication error.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to c5b37

Add an accepted-turn sign-in closure test while keeping the dispatch-refusal test. No agent enables prompt matching in this PR, so the remaining risk is bounded.

Pre-merge checks: 5

Check name	Status	Explanation
Docstring Coverage		Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly summarizes the main change: ACP sign-in prompts on agent error output now end startup or turns as signed out.
Description check		The description follows the required template and clearly explains the user impact, implementation, rationale, testing, visual proof, compatibility considerations, and checklist status. The command ch…

Full details: Description check

Explanation

The description follows the required template and clearly explains the user impact, implementation, rationale, testing, visual proof, compatibility considerations, and checklist status. The command checklist is not fully checked, but the description states that targeted tests passed and CI covered the remaining validation.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 83b9a698-6575-4aaa-ba17-078de1caf612

📥 Commits

Reviewing files that changed from the base of the PR and between 88f2771 and 796e987.

📒 Files selected for processing: 9

• src/main/acp/acp-agent-connection.test.ts
• src/main/acp/acp-agent-connection.ts
• src/main/acp/acp-launch-specs.ts
• src/main/acp/acp-structured-acquire.ts
• src/main/acp/acp-structured-session-adapter.ts
• src/main/acp/acp-structured-session.ts
• src/main/acp/acp-structured-sign-in.test.ts
• src/main/acp/acp-structured-turns.ts
• src/main/provider-process/managed-provider-process.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

[coderabbitai]

🧹 Nitpick comments: 1

src/main/acp/acp-structured-sign-in.test.ts: 1

81-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Preserve preacceptance coverage and add accepted-turn close coverage.

The existing AcpAuthRequiredError test does not accept the turn, so it uniquely covers dispatch refusal. The suite covers accepted and unaccepted turns for agent.fail, but not for AcpSessionRuntime.prototype.close. Add a separate accepted-turn scenario for the connection-close failure timeline.

Suggested fix

 it('ends a running turn signed out when the connection closes for sign-in', async () => {
   const rig = await openAcpAdapterRig({ script: (agent) => agent.on('session/prompt', () => {}) })
   await rig.acquire()
   await sendHello(rig, 'first')
   await rig.frame('session/prompt')
   // The protocol close the real connection makes at a sign-in prompt line on stderr.
   AcpSessionRuntime.prototype.close.call(rig.child(), new AcpAuthRequiredError('Sign in'))
   await rig.settle()
   expect(rig.settled).toContainEqual(
     expect.objectContaining({
       clientMessageId: 'first',
       state: 'rejected',
       rejection: expect.objectContaining({ kind: 'notSignedIn' })
     })
   )
 })
 
+it('fails an accepted turn as signed out when the connection closes for sign-in', async () => {
+  const rig = await openAcpAdapterRig({
+    script: (agent) =>
+      agent.on('session/prompt', (frame) => {
+        agent.notify('session/update', {
+          sessionId: PROVIDER_SESSION,
+          _meta: promptMeta.parse(frame.params)._meta,
+          update: {
+            sessionUpdate: 'agent_message_chunk',
+            content: { type: 'text', text: 'started' }
+          }
+        })
+      })
+  })
+  await rig.acquire()
+  await sendHello(rig, 'first')
+  await rig.frame('session/prompt')
+  AcpSessionRuntime.prototype.close.call(rig.child(), new AcpAuthRequiredError('Sign in'))
+  await rig.settle()
+  const failures = (await rig.rig.rows()).flatMap(({ body }) =>
+    body.kind === 'status' && body.failure ? [body] : []
+  )
+  expect(failures).toHaveLength(1)
+  expect(failures[0]?.failure).toMatchObject({ kind: 'notSignedIn' })
+})

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 17edb4d7-142d-417b-831f-ab17187f662c

📥 Commits

Reviewing files that changed from the base of the PR and between 796e987 and c5b3745.

📒 Files selected for processing: 2

• src/main/acp/acp-agent-connection.test.ts
• src/main/provider-process/managed-provider-process.ts

🚧 Files skipped from review as they are similar to previous changes: 2

• src/main/provider-process/managed-provider-process.ts
• src/main/acp/acp-agent-connection.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.