diff --git a/apps/integration-gateway/src/server.test.ts b/apps/integration-gateway/src/server.test.ts index 0eb2e11..326d8b2 100644 --- a/apps/integration-gateway/src/server.test.ts +++ b/apps/integration-gateway/src/server.test.ts @@ -489,10 +489,11 @@ describe("GatewayServer session pages", () => { await server.close(); }); - it("posts nothing upfront for a labeled-triage turn when no Remote Control URL ever arrives -- just the final result", async () => { - // Default `invoke` mock from beforeEach never calls onRemoteControlUrl. - // The old "Starting work... {session page link}" comment is gone for - // good -- silence is the correct fallback, not a placeholder link. + it("posts a deterministic 'starting work' ack as soon as the turn is running, even when no Remote Control URL ever arrives", async () => { + // Default `invoke` mock from beforeEach never calls onRemoteControlUrl, + // only onRunning -- the ack must not depend on the delegated agent + // successfully producing a Remote Control session; it fires purely off + // this gateway's own poll of the turn's status. await postWebhook(port, "issues", { action: "labeled", repository: { owner: { login: "acme" }, name: "widgets" }, @@ -502,8 +503,9 @@ describe("GatewayServer session pages", () => { }); await flush(); - expect(postIssueComment).toHaveBeenCalledTimes(1); - expect(postIssueComment).toHaveBeenNthCalledWith(1, "acme", "widgets", 7, "Working on it."); + expect(postIssueComment).toHaveBeenCalledTimes(2); + expect(postIssueComment).toHaveBeenNthCalledWith(1, "acme", "widgets", 7, "🤖 Starting work on this now."); + expect(postIssueComment).toHaveBeenNthCalledWith(2, "acme", "widgets", 7, "Working on it."); const entry = await sessionPageStore.getOrCreate(sessionIdFor("acme", "widgets", 7), { owner: "acme", @@ -527,7 +529,10 @@ describe("GatewayServer session pages", () => { expect(postIssueComment).not.toHaveBeenCalled(); }); - it("posts a single comment linking the Remote Control URL as soon as it arrives, before the final result", async () => { + it("posts a separate follow-up comment linking the Remote Control URL as soon as it arrives, before the final result", async () => { + // This override skips onRunning entirely (unlike the default mock), so + // the deterministic ack from the previous test does NOT fire here -- + // isolates the onRemoteControlUrl follow-up comment on its own. invoke.mockImplementation(async (..._args: unknown[]) => { const onRemoteControlUrl = _args[5] as ((url: string) => Promise | void) | undefined; await onRemoteControlUrl?.("https://claude.ai/code/session_abc123"); @@ -549,7 +554,7 @@ describe("GatewayServer session pages", () => { "acme", "widgets", 7, - "🤖 Starting work on this now. Watch live or take over the session here: https://claude.ai/code/session_abc123", + "🤖 Watch live or take over the session here: https://claude.ai/code/session_abc123", ); expect(postIssueComment).toHaveBeenNthCalledWith(2, "acme", "widgets", 7, "Working on it."); }); @@ -684,12 +689,14 @@ describe("GatewayServer unauthenticated triage: deferred ack + auto-resume", () }); await flush(); - // The very first comment is the auth prompt. No "starting work" - // placeholder comment exists anymore -- the second comment is the real - // result, posted once the link lands and the turn resumes. + // The very first comment is the auth prompt. The resumed invoke fires + // onRunning (it's genuinely running now, past the link pre-flight), so + // the deterministic "starting work" ack posts next, then the real result + // once the turn completes. expect(postIssueComment.mock.calls[0]?.[3]).toMatch(/link your Claude account/i); - expect(postIssueComment.mock.calls[1]?.[3]).toBe("Opened PR #42."); - expect(postIssueComment).toHaveBeenCalledTimes(2); + expect(postIssueComment.mock.calls[1]?.[3]).toBe("🤖 Starting work on this now."); + expect(postIssueComment.mock.calls[2]?.[3]).toBe("Opened PR #42."); + expect(postIssueComment).toHaveBeenCalledTimes(3); // Waited on the gateway-owned claude token store for the gateway subject, // and re-invoked the SAME triage request once it landed. The window is the @@ -767,8 +774,9 @@ describe("GatewayServer unauthenticated triage: deferred ack + auto-resume", () await flush(); expect(postIssueComment.mock.calls[0]?.[3]).toMatch(/link your Claude account/i); - expect(postIssueComment.mock.calls[1]?.[3]).toBe("Opened PR #42."); - expect(postIssueComment).toHaveBeenCalledTimes(2); + expect(postIssueComment.mock.calls[1]?.[3]).toBe("🤖 Starting work on this now."); + expect(postIssueComment.mock.calls[2]?.[3]).toBe("Opened PR #42."); + expect(postIssueComment).toHaveBeenCalledTimes(3); expect(waitForCompletion).toHaveBeenCalledWith("client-integration-gateway", 10 * 60 * 1000, "login"); expect(invoke).toHaveBeenCalledTimes(2); }); diff --git a/apps/integration-gateway/src/server.ts b/apps/integration-gateway/src/server.ts index b68a498..184332a 100644 --- a/apps/integration-gateway/src/server.ts +++ b/apps/integration-gateway/src/server.ts @@ -403,35 +403,53 @@ export class GatewayServer { // Ordinary conversational opened/comment replies are meant to feel like // near-instant chat, so an extra comment there would just be noise. // - // The gateway's own session page (ADR 0025/0026) is still created and - // tracked here for debugging, but its URL is deliberately NEVER posted - // to the issue -- the only thing posted up front is a real Claude Code - // Remote Control link (`onRemoteControlUrl`, fired by a - // `remote-control-url` progress event from the delegated agent), and - // only once one genuinely exists. If Remote Control never activates for - // this run (not configured for the Agent, or the CLI never hands back a - // session), no upfront comment is posted at all -- silence, not a - // placeholder link nobody asked to see, is the fallback. This was a - // deliberate, explicit product decision after the old "Starting work... - // {session page link}" comment kept appearing instead of the intended - // Remote Control link. + // TWO separate signals post TWO separate comments here, deliberately not + // collapsed into one: + // + // `announce` (wired to `runTurn`'s `onRunning`) is posted by THIS + // gateway the moment `orchestratorClient.invoke`'s own poll loop first + // sees the turn genuinely running (past any identity-link pre-flight) -- + // it depends on nothing the delegated agent does, so it fires whether or + // not Remote Control ever comes up for this run. This used to be the + // ONLY upfront comment (issue #81), then got removed entirely in favor + // of `onRemoteControlUrl` below because it linked the session page, + // which was never meant to be user-facing (c49a533) -- but removing it + // also removed the one deterministic "your triage request was received + // and is running" acknowledgement, so a run whose delegated agent never + // gets around to emitting `remote-control-url` (not configured for the + // Agent, the CLI never hands back a session, or the agent fails before + // that point) posts no comment at all even though it visibly launched a + // Job. Restored here with no link -- just the ack -- so it no longer + // depends on anything downstream succeeding. + // + // `onRemoteControlUrl`, fired by a `remote-control-url` progress event + // from the delegated agent IF one ever arrives, posts a second, separate + // follow-up comment with the live session link. Independent of + // `announce` -- may land before, after, or never. + let announce: (() => Promise) | undefined; let onRemoteControlUrl: ((url: string) => Promise) | undefined; if (event && this.options.sessionPageStore && this.options.publicBaseUrl) { await this.options.sessionPageStore.getOrCreate(sessionId, { owner, repo, issueNumber }); - let posted = false; + let announced = false; + announce = async () => { + if (announced) return; + announced = true; + await this.options.githubReplyClient.postIssueComment(owner, repo, issueNumber, "🤖 Starting work on this now."); + }; + let postedRemoteControlUrl = false; onRemoteControlUrl = async (url) => { - if (posted) return; - posted = true; + if (postedRemoteControlUrl) return; + postedRemoteControlUrl = true; await this.options.githubReplyClient.postIssueComment( owner, repo, issueNumber, - `🤖 Starting work on this now. Watch live or take over the session here: ${url}`, + `🤖 Watch live or take over the session here: ${url}`, ); }; } try { - await this.runTurn(owner, repo, issueNumber, sessionId, request, event, undefined, onRemoteControlUrl); + await this.runTurn(owner, repo, issueNumber, sessionId, request, event, announce, onRemoteControlUrl); } finally { // `finally`, not the happy path only: a failed or blocked run is // exactly when someone wants to re-trigger, so the label must come off @@ -487,13 +505,14 @@ export class GatewayServer { // This relay has no browser -- always force device flow explicitly rather // than relying on agent-orchestrator's own default ("authcode", intended // for browser-based callers). `announce` fires once the turn is genuinely - // running past any auth pre-flight -- `relayAndReply` no longer uses it - // (it posts nothing until a real Remote Control URL exists), but - // `orchestratorClient.invoke` still accepts it for any other caller that - // wants an "actually running now" signal. `onRemoteControlUrl` (when set) - // is only ever passed on the triage path (alongside `event`), see - // `relayAndReply`. Only pass an `event` when there is one -- keeps the - // call shape identical for the ordinary opened/comment paths. + // running past any auth pre-flight -- `relayAndReply` passes it on the + // triage path (alongside `event`) as the deterministic "starting work" + // ack; unset for the ordinary conversational paths, which want no extra + // comment at all. `onRemoteControlUrl` (when set) is likewise only ever + // passed on the triage path, and posts an independent follow-up comment + // if a live session link shows up. Only pass an `event` when there is + // one -- keeps the call shape identical for the ordinary opened/comment + // paths. const invokeOnce = (): Promise => event ? this.options.orchestratorClient.invoke(request, sessionId, "device", event, announce, onRemoteControlUrl)