fix: implement P0 architecture reliability fixes

[digitarald]

Summary

• remove analyzer -> git.ts coupling for git-repo detection to avoid pulling simple-git into extension-facing analyzer usage
• fix stale state closure updates in BatchReadinessTui keyboard handlers
• fix swallowed auth/login errors from Copilot SDK session event callbacks and ensure sessions are always destroyed
• propagate real analysis and apps through PolicyContext and compiler readiness bridging
• extract per-repo readiness processing from BatchReadinessTui into src/services/batch.ts and sanitize token-bearing errors
• test the real attachDefaultPermissionHandler implementation by exporting and importing it in copilotSdk.test.ts

Validation

• npm run typecheck — passes
• npm run lint — passes
• npm run test — 529/529 tests pass

[Copilot]

Pull request overview

This PR focuses on improving reliability and architectural boundaries across the readiness/batch workflows and Copilot SDK session handling, reducing extension-facing coupling and tightening error/session lifecycle behavior.

Changes:

• Extracted per-repo batch readiness processing into src/services/batch.ts and simplified the Batch Readiness TUI to call the service.
• Propagated repo analysis and apps through the readiness → policy engine bridge to avoid stubbed readiness contexts.
• Improved Copilot SDK session error handling by capturing auth/login session errors and ensuring sessions are always destroyed; exported the default permission handler and updated tests to use the real implementation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/ui/BatchReadinessTui.tsx	Removes inline clone/readiness logic; delegates per-repo work to a service and fixes stale state updates in input handlers.
src/services/batch.ts	Adds processBatchReadinessRepo() + result types and reuses token-bearing error sanitization.
src/services/analyzer.ts	Removes dependency on services/git (and transitively simple-git) by inlining git-repo detection.
src/services/readiness.ts	Passes analysis and apps into the policy engine context for shadow runs.
src/services/policy/types.ts	Extends PolicyContext to optionally include analysis and apps.
src/services/policy/compiler.ts	Forwards real analysis/apps into the readiness bridge when available; otherwise uses safe fallbacks.
src/services/instructions.ts	Captures auth/login session errors from SDK callbacks; always destroys sessions via finally.
src/services/evalScaffold.ts	Same session-error capture pattern; throws captured auth/login errors after session teardown.
src/services/copilotSdk.ts	Exports attachDefaultPermissionHandler so it can be tested directly.
src/services/tests/copilotSdk.test.ts	Updates tests to import and validate the real permission handler implementation.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/services/instructions.ts:373

• Same issue as generateCopilotInstructions: sessionError is only thrown after a successful sendAndWait. If sendAndWait throws (e.g., due to an auth/login failure), the code won’t reach the if (sessionError) line, so callers may see an unhelpful/raw error instead of the intended friendly message. Handle this by catching sendAndWait errors and preferring sessionError when present.

    progress(`Analyzing area "${area.name}"...`);
    try {
      await session.sendAndWait({ prompt }, 180000);
    } finally {
      await session.destroy();
    }
    if (sessionError) throw sessionError;

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

[Copilot]

This changes isGitRepo semantics from “any path inside a git work tree” to “repoPath has a .git entry”. That will incorrectly report false when analyzing a subdirectory within a repo (common in monorepos / area paths), and can also diverge from previous behavior where git-worktree detection was more robust. Consider implementing a lightweight check that preserves the prior semantics without reintroducing the ./git dependency (e.g., invoking git rev-parse --is-inside-work-tree via child_process, or walking up parent directories looking for .git).

[Copilot]

When ctx.analysis is absent, the stub sets isGitRepo: true, which can produce objectively incorrect downstream behavior (e.g., git-scoped checks passing/triggering when running against a non-git directory). A safer fallback would be false (or a lightweight check), keeping the stub conservative rather than optimistic.

[Copilot]

The selection toggle still relies on cursorIndex from the render closure. If the input handler can fire faster than React/Ink commits the cursor update (e.g., rapid key presses), this can toggle the wrong item even though setCursorIndex is now functional. Consider reading the current cursor from a ref that is kept in sync with state (or otherwise ensuring the handler always references the latest cursor index) before mutating the selection set.

[Copilot]

If session.destroy() rejects, it will throw from the finally block and can mask the more actionable sessionError/sendError being captured. Consider wrapping session.destroy() in its own try/catch (or capturing destroy errors separately) so the primary failure reason is preserved. The same pattern appears in other session-based helpers (e.g., eval scaffold).

Suggested change

	try {
	await session.sendAndWait({ prompt }, 180000);
	} catch (err) {
	sendError = err;
	} finally {
	await session.destroy();
	}
	if (sessionError) throw sessionError;
	if (sendError !== undefined)
	throw sendError instanceof Error ? sendError : new Error(String(sendError));
	let destroyError: unknown;
	try {
	await session.sendAndWait({ prompt }, 180000);
	} catch (err) {
	sendError = err;
	} finally {
	try {
	await session.destroy();
	} catch (err) {
	destroyError = err;
	}
	}
	if (sessionError) throw sessionError;
	if (sendError !== undefined)
	throw sendError instanceof Error ? sendError : new Error(String(sendError));
	if (destroyError !== undefined)
	throw destroyError instanceof Error ? destroyError : new Error(String(destroyError));

[Copilot]

This removes macOS from the CI matrix, reducing platform coverage. If this is intentional (cost/flake mitigation), consider adding a brief comment in the workflow explaining why macOS was dropped (or limiting macOS to a scheduled workflow) to avoid accidental long-term regressions on that platform.