fix(worker): continue research when expansion names only attached repositories (AIW-284) (#315)

Author: outof-place

apps/worker/src/repository-discovery/runner.test.ts

@@ -179,17 +179,20 @@ describe("repository expansion validation", () => {
       completedRounds: 0,
     },
     {
-      name: "already attached repository",
+      name: "repository requested twice in one round",
       requests: [
         {
           provider: "gitlab" as const,
           repoPath: "acme/shared/contracts",
           rationale: "imports",
         },
+        {
+          provider: "gitlab" as const,
+          repoPath: "acme/shared/contracts",
+          rationale: "imports again",
+        },
       ],
-      attached: [
-        { provider: "gitlab" as const, repoPath: "acme/shared/contracts" },
-      ],
+      attached: [],
       completedRounds: 0,
     },
   ])("returns targeted clarification for $name", ({ requests, attached, completedRounds }) => {
@@ -203,6 +206,49 @@ describe("repository expansion validation", () => {
     ).toBe("clarification_needed");
   });
 
+  it("reports already_attached instead of asking a human when every request is attached", () => {
+    // A clarification here would park the run on an unanswerable question: the
+    // workspace already holds everything research named, so there is nothing a
+    // human could add (AIW-284).
+    expect(
+      validateRepositoryExpansionRequests({
+        requests: [
+          {
+            provider: "gitlab",
+            repoPath: "acme/shared/contracts",
+            rationale: "imports",
+          },
+        ],
+        catalog,
+        attached: [
+          { provider: "gitlab", repoPath: "acme/shared/contracts" },
+        ],
+        completedRounds: 0,
+      }),
+    ).toEqual({ kind: "already_attached" });
+  });
+
+  it("keeps the round limit ahead of the already-attached no-op", () => {
+    // The two-round limit is checked first, so an all-attached request on the
+    // third round is still the human question it was, not a silent continue.
+    expect(
+      validateRepositoryExpansionRequests({
+        requests: [
+          {
+            provider: "gitlab",
+            repoPath: "acme/shared/contracts",
+            rationale: "imports",
+          },
+        ],
+        catalog,
+        attached: [
+          { provider: "gitlab", repoPath: "acme/shared/contracts" },
+        ],
+        completedRounds: 2,
+      }).kind,
+    ).toBe("clarification_needed");
+  });
+
   it("returns clarification for more than three fresh repositories in one round, distinct from the total cap", () => {
     // The per-round cap (>3 in a single round) is enforced before the catalog is
     // even consulted, and is separate from both the two-round limit and the

apps/worker/src/repository-discovery/runner.ts

@@ -88,6 +88,10 @@ export function assembleRepositoryDiscoveryPrompt(input: {
 
 export type RepositoryExpansionDecision =
   | { kind: "attach"; repositories: SelectedRepository[] }
+  // Every requested repository is already in the workspace. Not an error and not
+  // a question: nothing is left to clone, so the caller continues research with
+  // what is attached.
+  | { kind: "already_attached" }
   | { kind: "clarification_needed"; questions: string[] };
 
 // Total repositories one research workspace may ever hold. This is a hard cap:
@@ -157,7 +161,8 @@ export function validateRepositoryExpansionRequests(input: {
     }
     requested.add(key);
     // Already-attached repositories are filtered out and the fresh ones proceed;
-    // only an all-attached request (nothing fresh remains) becomes a clarification.
+    // an all-attached request (nothing fresh remains) is reported as
+    // already_attached below, never as a clarification.
     if (attached.has(key)) {
       continue;
     }
@@ -175,9 +180,10 @@ export function validateRepositoryExpansionRequests(input: {
     });
   }
   if (repositories.length === 0) {
-    return clarification(
-      "Research requested only repositories that are already attached. Which additional repository is required?",
-    );
+    // Asking a human "which additional repository is required?" here has no
+    // useful answer: research already holds everything it named. Report the
+    // no-op so the caller keeps going instead of parking the run (AIW-284).
+    return { kind: "already_attached" };
   }
   if (input.attached.length + repositories.length > MAX_WORKSPACE_REPOSITORIES) {
     return clarification(

apps/worker/src/workflows/agent.ts

@@ -1025,9 +1025,11 @@ export async function applyHumanRepositoryExpansion(
   if (decision.kind === "clarification_needed") {
     return { kind: "clarification", questions: decision.questions };
   }
-  if (decision.repositories.length === 0) {
+  if (decision.kind === "already_attached" || decision.repositories.length === 0) {
     // Every named repository is already attached: nothing new to clone, so let
-    // the caller run research instead of re-raising the clarification.
+    // the caller run research instead of re-raising the clarification. The human
+    // validator reports this as an empty attach rather than already_attached, so
+    // both shapes land here.
     return { kind: "noop" };
   }
   const attached = await deps.attach(decision.repositories);
@@ -4078,6 +4080,33 @@ async function agentWorkflowBody(
         if (decision.kind === "clarification_needed") {
           return planningClarificationResult(decision.questions);
         }
+        if (decision.kind === "already_attached") {
+          // Research asked only for repositories the workspace already holds:
+          // nothing to clone, and no question a human could usefully answer, so
+          // continue with what is attached instead of parking the run (AIW-284).
+          // The round still counts and the requests are still recorded. That is
+          // deliberate: it bounds a model that keeps re-requesting the same
+          // repositories (the third round trips the expansion limit, which IS a
+          // legitimate human question), and it puts the requests into the
+          // "Repository expansion history" note the next research prompt carries,
+          // which tells the model those repositories are attached and that it
+          // should continue the same research.
+          ctx.repositoryExpansion = {
+            rounds: ctx.repositoryExpansion.rounds + 1,
+            priorRequests: [
+              ...ctx.repositoryExpansion.priorRequests,
+              ...requests,
+            ],
+          };
+          await emitRepositoryWorkflowObservation(execution?.observations, {
+            event: "expansion",
+            round: ctx.repositoryExpansion.rounds,
+            attachedCount: 0,
+            totalCount: ctx.selectedRepositories.length,
+            cloneDurationMs: 0,
+          });
+          return null;
+        }
         const attached = await attachResearchRepositoriesStep(
           ctx.sandboxId,
           ctx.workspaceManifest,

apps/worker/src/workflows/multi-repo-research.test.ts

@@ -705,4 +705,84 @@ describe("expansion round counter survives a clarification round-trip", () => {
     });
     expect(second.kind).toBe("attach");
   });
+
+  // AIW-284: research asking only for repositories the workspace already holds
+  // used to park the whole run on "Which additional repository is required?",
+  // which no human can answer, because everything named was already there. Same
+  // limitation as the test above: expandResearchWorkspace is an inline closure in
+  // agent.ts, so this drives the exported validator and mirrors the call site's
+  // state transitions rather than executing the loop itself.
+  it("continues without a clarification and still burns a round when every requested repository is attached", () => {
+    const ctx = makeCtx({
+      sandboxId: "sbx-research",
+      workspaceManifest: { version: 2, repositories: [] },
+      selectedRepositories: [
+        {
+          provider: "github",
+          repoPath: "acme/service",
+          defaultBranch: "main",
+          selectedRationale: "symptom",
+        },
+        {
+          provider: "gitlab",
+          repoPath: "acme/shared/contracts",
+          defaultBranch: "main",
+          selectedRationale: "imports",
+        },
+      ],
+    });
+
+    const requests = [
+      {
+        provider: "gitlab" as const,
+        repoPath: "acme/shared/contracts",
+        rationale: "need the schema",
+      },
+      {
+        provider: "github" as const,
+        repoPath: "acme/service",
+        rationale: "need the caller",
+      },
+    ];
+    const decision = validateRepositoryExpansionRequests({
+      requests,
+      catalog,
+      attached: ctx.selectedRepositories,
+      completedRounds: ctx.repositoryExpansion.rounds,
+    });
+
+    // The run keeps going: no clarification, and nothing to clone.
+    expect(decision).toEqual({ kind: "already_attached" });
+
+    // The call site advances the durable counter exactly as the attach path
+    // does, so a model that keeps re-asking is bounded instead of looping.
+    ctx.repositoryExpansion = {
+      rounds: ctx.repositoryExpansion.rounds + 1,
+      priorRequests: [...ctx.repositoryExpansion.priorRequests, ...requests],
+    };
+    expect(ctx.repositoryExpansion.rounds).toBe(1);
+    expect(ctx.repositoryExpansion.priorRequests).toEqual(requests);
+
+    // Round two repeats the no-op, and round three hits the expansion limit,
+    // which IS a question a human can act on.
+    expect(
+      validateRepositoryExpansionRequests({
+        requests,
+        catalog,
+        attached: ctx.selectedRepositories,
+        completedRounds: 1,
+      }),
+    ).toEqual({ kind: "already_attached" });
+    expect(
+      validateRepositoryExpansionRequests({
+        requests,
+        catalog,
+        attached: ctx.selectedRepositories,
+        completedRounds: 2,
+      }),
+    ).toMatchObject({
+      kind: "clarification_needed",
+      questions: [expect.stringContaining("maximum of 2")],
+    });
+  });
 });