refactor(sdk,workflows): migrate Copilot sendAndWait to send

[lavaman131]

Summary

Migrates all Copilot workflow stages from sendAndWait() to send(), eliminating per-call timeout constants and simplifying workflow code. Updates documentation and skill references to establish send() as the recommended default API.

Key Changes

• API migration: Replaced s.session.sendAndWait({ prompt }, TIMEOUT_MS) with s.session.send({ prompt }) across all Copilot workflow implementations
• Builtin workflows simplified: ralph and deep-research-codebase Copilot implementations remove per-agent timeout constants (AGENT_SEND_TIMEOUT_MS, SCOUT_TIMEOUT_MS, EXPLORER_TIMEOUT_MS, etc.)
• Example workflows updated: hello-world and parallel-hello-world Copilot examples use the new send() API
• Documentation overhauled: All workflow-creator skill references (agent-sessions.md, control-flow.md, failure-modes.md, getting-started.md, session-config.md, user-input.md, SKILL.md) updated to recommend send() by default
• Failure mode F10 updated: Now recommends send to avoid the timeout problem entirely; sendAndWait is preserved for explicit timeout-gated use cases

Migration Notes

Replace all Copilot sendAndWait calls with send:

// Before
const SEND_TIMEOUT_MS = 30 * 60 * 1000;
await s.session.sendAndWait({ prompt }, SEND_TIMEOUT_MS);

// After
await s.session.send({ prompt });

sendAndWait remains available when explicit timeout-based waiting is required. If used, always pass an explicit timeout (default to 5 minutes if unsure) — the 60-second SDK default throws on any real agent work.

[claude]

Review

Thanks for tackling the sendAndWait 60-second-timeout pitfall — it's a real footgun. However, I think this refactor replaces it with a worse bug that silently breaks every Copilot workflow it touches.

🔴 Critical: send() is fire-and-forget; pairing it with an immediate getMessages() drops the agent's response

Per docs/copilot-cli/sdk.md:216-218 (reflecting the SDK itself):

send(options: MessageOptions): Promise<string> — Send a message to the session. Returns immediately after the message is queued; use event handlers or sendAndWait() to wait for completion.

The new workflow pattern is:

await s.session.send({ prompt });            // returns as soon as queued
s.save(await s.session.getMessages());       // returns whatever's there now — likely empty
return getAssistantText(messages);           // empty string

Then run(ctx) resolves (src/sdk/runtime/executor.ts:882) and cleanupProvider calls session.disconnect() (src/sdk/runtime/executor.ts:621) before the agent has had a chance to respond. There is no runtime-level auto-await on session.idle, and disconnect() does not drain pending work — per the docs, "Session data on disk is preserved for later resumption."

Concrete breakage I'd expect:

• ralph/copilot/index.ts:85-93 — planner.result becomes \"\", so buildOrchestratorPrompt(userPromptText, { plannerNotes: planner.result }) on line 106 sends the orchestrator an empty plan. Same for the reviewer/debugger handoffs that run through getAssistantText.
• deep-research-codebase/copilot/index.ts — scout overview, history overview, explorer reports, and aggregator all save empty transcripts; downstream s.transcript(scout), s.transcript(history), and explorerHandles.map(h => h.result) receive no content.
• hello-world / parallel-hello-world — user-facing examples will appear to run (session spawns) but produce empty outputs.

Even the PR's own updated docs describe this: agent-sessions.md says send "dispatches the prompt and returns the messageId immediately", which is accurate — but the surrounding examples (e.g. runAgent in the refactored Critical pitfall section) then call getMessages() one line later, which now returns too early.

The reference SDK example in docs/copilot-cli/sdk.md:381-400 shows the required pattern:

const done = new Promise<void>((resolve) => {
  session.on(\"session.idle\", () => resolve());
});
await session.send({ prompt });
await done;

The PR drops the await done half.

Suggested fixes (any one resolves it)

1. Revert to sendAndWait with an explicit generous timeout. The old pattern was correct; the problem is just the 60s default. The documentation changes (naming the default timeout, suggesting 30-minute budgets) can stand on their own.
2. Use send + an explicit session.idle wait in every stage:

   const idle = new Promise<void>((resolve) => s.session.once(\"session.idle\", resolve));
   await s.session.send({ prompt });
   await idle;
   s.save(await s.session.getMessages());

   This is noisier than sendAndWait, so it's only worth it if we want to avoid a timeout entirely.
3. Teach the runtime to auto-await session.idle before cleanupProvider so send becomes implicitly safe inside stage callbacks. Cleanest user-facing API, most intrusive to the runtime — would live in src/sdk/runtime/executor.ts between lines 882 and 894.

I'd lean toward (1) as the lowest-risk fix and pair it with a named constant like SEND_TIMEOUT_MS = 30 * 60 * 1000 at module scope, which several of the original workflows already had.

Other observations

• No test coverage for the change. There's nothing in src/sdk/runtime/executor.test.ts or elsewhere that would have caught this — the behavioral contract ("workflow stage captures the full assistant response") isn't asserted. A small integration-style smoke test that stubs a Copilot session, sends one prompt, and asserts the saved transcript is non-empty would protect against regressions either way.
• Doc inconsistency introduced by the refactor. failure-modes.md F10 still exists and still warns about the 60s timeout (good), but the "Fix" now says "use send by default" — which, given the bug above, is not actually a fix. Worth re-aligning once the approach is settled.
• Style nit: .atomic/workflows/parallel-hello-world/copilot/index.ts used to name the prompt variable userPromptText with a justifying comment about collision with the prompt: object key. The PR inlines most of those and in getting-started.md actively renames userPromptText → prompt. That's fine, but the comment on line 68-72 of ralph/copilot/index.ts still references the old naming rationale and now contradicts sibling workflows. Worth a sweep.
• Good simplification potential in deep-research-codebase/copilot/index.ts — dropping the four timeout constants is a nice readability win, if the underlying mechanism is sound.

Summary

The motivation is right but the mechanism is broken. As-is, I would not merge this — every Copilot workflow touched here will silently save empty transcripts and pass empty strings between stages. Happy to help prototype any of the three fix paths above.