docs: multi-phase implementation plan (63 tasks across 5 phases)
Phase 1 Stabilize (13), Phase 2 Runtime unification (12), Phase 3 Dev-box surface (11), Phase 4 Sync correctness (12), Phase 5 Notifications & polish (15), plus index.
This commit is contained in:
Gabriel Brown
@@ -0,0 +1,95 @@
|
|||||||
|
# Spoon: Stabilize → Unify → Build — Implementation Plan Index
|
||||||
|
|
||||||
|
> **For agentic workers:** Execute the phase plans **in order**. Each phase is a
|
||||||
|
> standalone plan that ships working, tested software on its own. Use
|
||||||
|
> `superpowers:subagent-driven-development` (fresh subagent per task, review
|
||||||
|
> between tasks) or `superpowers:executing-plans` (batched with checkpoints).
|
||||||
|
|
||||||
|
**Spec:** [`../specs/2026-07-10-spoon-stabilize-unify-build-design.md`](../specs/2026-07-10-spoon-stabilize-unify-build-design.md)
|
||||||
|
**Design review:** Gib, 2026-07-10 (tenancy = owner + trusted friends; runtimes =
|
||||||
|
Codex + OpenCode + Claude Code, all in the per-user box; all four feature tracks;
|
||||||
|
ordering = stabilize → unify → build).
|
||||||
|
|
||||||
|
## Phases
|
||||||
|
|
||||||
|
| # | Plan | Tasks | Scope |
|
||||||
|
| - | ---- | ----- | ----- |
|
||||||
|
| 1 | [phase1-stabilize](2026-07-10-spoon-phase1-stabilize.md) | 13 | Critical/High bug fixes on the current architecture: home-deletion data loss, box lifecycle races/leaks, symlink escape, job-lifecycle recovery (heartbeat/timeout/cancel), auto-sync gating, terminal sizing, workspace shell keying, not-found handling. |
|
||||||
|
| 2 | [phase2-runtime-unification](2026-07-10-spoon-phase2-runtime-unification.md) | 12 | `AgentRuntime` adapter interface; Codex/OpenCode/Claude Code all exec into the box; delete the legacy per-job OpenCode container; runtime picker + queue-time validation; Fedora image + smoke; secrets hygiene. |
|
||||||
|
| 3 | [phase3-dev-box-surface](2026-07-10-spoon-phase3-dev-box-surface.md) | 11 | `/machine` page (box status/lifecycle + full-page terminal + home file browser); user-scoped `/box/*` worker routes + HMAC tokens; persistent workspace terminal (survives tab switch); terminal QoL. |
|
||||||
|
| 4 | [phase4-sync-correctness](2026-07-10-spoon-phase4-sync-correctness.md) | 12 | Correct head-SHA (branch ref, not truncated compare); stable maintenance-thread dedup; GitHub webhooks (push + installation lifecycle); Octokit retry/throttle + `rate_limited` status; bound unbounded queries; surface `needs_reauth`. |
|
||||||
|
| 5 | [phase5-notifications-polish](2026-07-10-spoon-phase5-notifications-polish.md) | 15 | `notifications` + `notificationPreferences` tables, in-app bell, preference-gated UseSend email; polish sweep (skeletons, error handling, form re-sync, auth copy, workspace UX, a11y, dead-code removal); docs rewrite. |
|
||||||
|
|
||||||
|
**Total: 63 tasks**, each ending in an independently testable, committed deliverable.
|
||||||
|
|
||||||
|
## Cross-phase dependencies
|
||||||
|
|
||||||
|
- **Phase 1 → 2, 3.** Phases 2 and 3 consume the Phase-1 box-lifecycle API:
|
||||||
|
`export type BoxHandle = { boxName: string; release: () => void }` and
|
||||||
|
`acquireUserBox(args: { username; workdir; containerHome }) => Promise<BoxHandle>`
|
||||||
|
(idempotent `release`, per-username mutex, `--init`, startup reconcile of
|
||||||
|
`spoon-box-*`). Both later plans carry a fallback note for the pre-Phase-1
|
||||||
|
`Promise<string>` + `releaseUserBox(username)` signature — **build Phase 1
|
||||||
|
first and the `BoxHandle` form is what lands.**
|
||||||
|
- **Phase 1 → 5.** Phase 5's `agent_turn_finished` / `agent_needs_input`
|
||||||
|
notifications emit at the job-status transitions that Phase 1 hardens
|
||||||
|
(`updateStatus`, `applyMaintenanceDecision`).
|
||||||
|
- **Phase 4 → 5.** Phase 5's `connection_needs_reauth` email emits where the
|
||||||
|
Phase-4 webhook sets `gitConnections.status = needs_reauth`; Phase 5 Task 3
|
||||||
|
marks that emit point defensively if Phase 4 hasn't landed.
|
||||||
|
- **Phase 4 depends on Phase 1's auto-sync gating** (`autoSyncEnabled` etc.
|
||||||
|
land in Phase 1 Task 8) and layers head-SHA/dedup/webhooks on top.
|
||||||
|
|
||||||
|
Phases 3 and 4 are otherwise independent and could be reordered or parallelized
|
||||||
|
after Phases 1–2. The recommended sequential order matches the spec.
|
||||||
|
|
||||||
|
## Deliberate deferrals (documented by the plan authors)
|
||||||
|
|
||||||
|
These are called out inline in the plans, not gaps:
|
||||||
|
|
||||||
|
- **Phase 1:** `updateStatus` implements the terminal-state guard (the
|
||||||
|
cancel-revert fix) rather than a full forward-only transition matrix — no
|
||||||
|
illegal transition is otherwise exercised. Stale-job recovery **times out**
|
||||||
|
rather than requeues (requeue-with-attempt-limit needs a new `claimAttempts`
|
||||||
|
field — deferred). `requireAiLowRiskForSync` is scoped to Phase 4 (the
|
||||||
|
AI-review→merge path it guards doesn't exist yet).
|
||||||
|
- **Phase 2:** exact `opencode run` / `claude -p` flags and the
|
||||||
|
`@anthropic-ai/claude-code` version pin are reconciled at implementation time
|
||||||
|
via the extended `scripts/smoke-agent-container` (`--help` probes + `npm view`),
|
||||||
|
since the live CLIs aren't available while planning.
|
||||||
|
- **Phase 3:** dotfiles re-materialization for the `/box` surface is out of
|
||||||
|
scope (the persistent home already carries dotfiles from prior job runs);
|
||||||
|
box start only ensures a login-shell `.bash_profile`. Docker-touching code is
|
||||||
|
verified by documented manual steps; only pure logic is unit-tested.
|
||||||
|
- **Phase 4:** the webhook route test cannot assert the scheduled `'use node'`
|
||||||
|
refresh ran in-process (convex-test VM limitation) — covered by the manual
|
||||||
|
checklist. Octokit plugin versions may need a bump on `bun install`.
|
||||||
|
- **Phase 5:** `(app)/error.tsx` + not-found handling lives in **Phase 1**
|
||||||
|
(finding #14), not here.
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
Every task is test-first where a test is meaningful. Test surfaces:
|
||||||
|
|
||||||
|
- **Worker** (`apps/agent-worker/tests/unit/`, vitest): box-registry
|
||||||
|
mutex/handle semantics, path containment incl. symlinks, terminal
|
||||||
|
resize-message parsing, runtime-adapter fixture-stream parsing.
|
||||||
|
- **Backend** (`packages/backend/tests/unit/`, `convex-test`): claim
|
||||||
|
validation/rollback, heartbeat-timeout cron, transition guard incl.
|
||||||
|
cancel-vs-worker race, auto-sync gating matrix, maintenance-thread dedup,
|
||||||
|
webhook signature verification, notification emit/preferences.
|
||||||
|
- **Next** (`apps/next/tests/component`, jsdom + testing-library): terminal
|
||||||
|
fit-on-fonts-ready + resize, workspace shell remount on job switch, form
|
||||||
|
re-sync, not-found states, notification bell.
|
||||||
|
- **Docker-touching worker code** (box `--init`, dockerode TTY exec, `/box`
|
||||||
|
lifecycle) uses documented manual verification — needs a live daemon.
|
||||||
|
|
||||||
|
## How to run each phase
|
||||||
|
|
||||||
|
```
|
||||||
|
Subagent-driven (recommended): one fresh subagent per task, two-stage review between tasks.
|
||||||
|
Inline: superpowers:executing-plans, batched with checkpoints.
|
||||||
|
```
|
||||||
|
|
||||||
|
Start with Phase 1, Task 1 (`cleanupOrphanedWorkspaces` layout-awareness) — it
|
||||||
|
closes the highest-severity bug (a cleanup call deletes every user's home).
|
||||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,545 @@
|
|||||||
|
# Spoon Phase 2 — Runtime Unification: Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Collapse the three agent runtimes (Codex, OpenCode, Claude Code) behind one `AgentRuntime` adapter interface in the worker, run all three *inside* the per-user `spoon-box-{username}` container (no per-job containers), and let users pick the runtime per thread / per spoon with queue-time validation against their AI provider profile.
|
||||||
|
|
||||||
|
**Architecture:** The worker (`apps/agent-worker`) polls Convex, claims a job, acquires the user's persistent box (Phase 1), clones `~/Code/{spoon}/{branch}` into the mounted home, then runs agent turns by `docker exec`-ing a CLI into the box. Today dispatch is implicit (Codex if the profile is a ChatGPT-login profile, otherwise a *separate* `opencode serve` container). Phase 2 makes `job.runtime` (`codex|opencode|claude`) authoritative: a registry maps it to an adapter; every adapter execs into the box via `streamExecInContainer`, tags its process group with a unique marker, and kills that group in-box on timeout/abort. The OpenCode server-in-a-side-container path is deleted. The Codex `auth.json` encrypted-snapshot pattern is generalized to Claude OAuth credentials.
|
||||||
|
|
||||||
|
**Tech Stack:** TypeScript, Bun, execa (docker/podman CLI), Convex (`packages/backend/convex`), Vitest (worker unit tests + convex-test backend tests), Next.js 16 (`apps/next`), Fedora 41 job image (`docker/agent-job.Dockerfile`) shipping `opencode`, `@openai/codex`, and (new) `@anthropic-ai/claude-code`.
|
||||||
|
|
||||||
|
**Depends on:** Phase 1 (box lifecycle: per-username mutex, idempotent acquire/release handles, `--init` on the box, heartbeat + `cancelRequested` teardown, startup reconcile of `spoon-box-*`). This plan assumes `acquireUserBox`/`releaseUserBox` are already concurrency-safe and the box runs with `--init` so orphaned in-box children are reaped. Where Phase-1 handle semantics are referenced, use whatever `acquireUserBox` returns after Phase 1; if Phase 1 kept the `string` box-name return used in `worker.ts:1330`, keep using it.
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- **Worker unit tests** live in `apps/agent-worker/tests/unit/*.test.ts`; run `cd apps/agent-worker && bun run test:unit` (`vitest run --project unit`). **Worker typecheck:** `cd apps/agent-worker && bun run typecheck`.
|
||||||
|
- **Backend tests** live in `packages/backend/tests/unit/*.test.ts` using `convex-test` (see `packages/backend/tests/unit/harness.test.ts` for the exact fixture style: `convexTest(schema, modules)`, `authed(t, userId)`, `spoonInput`/`githubSpoonInput`). Run `cd packages/backend && bun run test:unit`. **Always `cd packages/backend && bun run codegen` before `bun run typecheck`** (codegen regenerates `_generated/` after schema/function changes).
|
||||||
|
- Match the existing fixture/test style: `expect(normalizeX(...)).toContainEqual({ kind: ... })` for event normalizers (see `apps/agent-worker/tests/unit/agent-events.test.ts`); `mkdtemp` + `afterEach` cleanup for filesystem tests (see `apps/agent-worker/tests/unit/codex-runtime.test.ts`).
|
||||||
|
- **Conventional commits; one commit per task.** Run the relevant `test:unit` + `typecheck` (and `codegen` for backend) before committing. Do not commit with a failing suite.
|
||||||
|
- **Three runtimes: `codex`, `opencode`, `claude` — all `docker exec` into `spoon-box-{username}`. No per-job containers.** Never leave a `spoon-agent-job-*` code path behind.
|
||||||
|
- A real turn against a live CLI cannot run in unit tests. Adapters therefore take an **injectable exec function** so `runTurn` can be driven with fixture stream lines; live behavior is covered by the extended `scripts/smoke-agent-container` manual step. Event parsing is covered by pure normalizer fixtures.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: Define the `AgentRuntime` adapter interface, shared types, and registry
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create `apps/agent-worker/src/runtime/agent-runtime.ts` (new).
|
||||||
|
- Create `apps/agent-worker/src/runtime/provider.ts` (new — pure helpers moved out of `worker.ts` so adapters don't import `worker.ts` at runtime).
|
||||||
|
- Edit `apps/agent-worker/src/worker.ts` (extract the `Claim` type and pure helpers; make `ActiveWorkspace` extend the shared workspace shape; add `runtime`/`turnMarker` fields).
|
||||||
|
- Create `apps/agent-worker/tests/unit/agent-runtime.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
*Produces* (`runtime/agent-runtime.ts`):
|
||||||
|
```ts
|
||||||
|
import type { NormalizedAgentEvent } from '../agent-events';
|
||||||
|
import type { streamExecInContainer } from './docker';
|
||||||
|
import type { AdapterWorkspace } from './provider';
|
||||||
|
|
||||||
|
export type AgentRuntimeName = 'codex' | 'opencode' | 'claude';
|
||||||
|
|
||||||
|
export type ExecStreamFn = typeof streamExecInContainer;
|
||||||
|
|
||||||
|
export type TurnResult = {
|
||||||
|
// Assistant text the adapter captured out-of-band (Codex --output-last-message
|
||||||
|
// file, Claude `result` event). Empty/undefined when everything streamed via onEvent.
|
||||||
|
finalMessage?: string;
|
||||||
|
// Runtime session id to persist for continuity (resume/--session/--resume).
|
||||||
|
sessionId?: string;
|
||||||
|
// Non-empty when the runtime reported a hard failure the caller must surface.
|
||||||
|
error?: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type AgentRuntime = {
|
||||||
|
readonly name: AgentRuntimeName;
|
||||||
|
prepareAuth(workspace: AdapterWorkspace): Promise<void>;
|
||||||
|
runTurn(
|
||||||
|
workspace: AdapterWorkspace,
|
||||||
|
prompt: string,
|
||||||
|
onEvent: (event: NormalizedAgentEvent) => Promise<void>,
|
||||||
|
): Promise<TurnResult>;
|
||||||
|
abort(workspace: AdapterWorkspace): Promise<void>;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type AdapterFactory = (deps?: { execStream?: ExecStreamFn }) => AgentRuntime;
|
||||||
|
```
|
||||||
|
|
||||||
|
*Produces* (`runtime/provider.ts`) — `AdapterWorkspace` is the subset of `ActiveWorkspace` adapters read, plus the pure helpers currently in `worker.ts:47-95, 350-446`:
|
||||||
|
```ts
|
||||||
|
export type Claim = { /* moved verbatim from worker.ts:47-95, but runtime widened */ };
|
||||||
|
// job.runtime union becomes: runtime?: 'codex' | 'opencode' | 'claude';
|
||||||
|
|
||||||
|
export type AdapterWorkspace = {
|
||||||
|
claim: Claim;
|
||||||
|
workdir: string;
|
||||||
|
homeDir: string;
|
||||||
|
username: string;
|
||||||
|
containerHome: string; // e.g. /home/{username}
|
||||||
|
containerRepo: string; // e.g. /home/{username}/Code/{spoon}/{branch}
|
||||||
|
repoDir: string; // host path to the checkout
|
||||||
|
boxName: string; // spoon-box-{username}
|
||||||
|
redact: (value: string) => string;
|
||||||
|
runtime: AgentRuntimeName;
|
||||||
|
codexSessionId?: string;
|
||||||
|
opencodeSessionId?: string;
|
||||||
|
claudeSessionId?: string;
|
||||||
|
turnMarker?: string; // set per turn; used for in-box process-group kill
|
||||||
|
};
|
||||||
|
|
||||||
|
export const isCodexLoginProfile: (claim: Claim) => boolean; // moved from worker.ts:350
|
||||||
|
export const collectJsonStringValues: (value?: string) => string[]; // moved from worker.ts:354
|
||||||
|
export const providerEnvironment: (claim: Claim, workspaceRoot?: string) => Record<string, string>; // moved from worker.ts:379
|
||||||
|
export const opencodeModel: (claim: Claim) => string; // moved from worker.ts:425
|
||||||
|
export const codexModel: (claim: Claim) => string; // moved from worker.ts:440
|
||||||
|
export const codexModelArgs: (claim: Claim) => string[]; // moved from worker.ts:445
|
||||||
|
```
|
||||||
|
|
||||||
|
*Consumes:* `NormalizedAgentEvent` from `agent-events.ts`; `streamExecInContainer` from `runtime/docker.ts`.
|
||||||
|
|
||||||
|
**Registry** (`runtime/agent-runtime.ts`): a lazy map. Adapters register in later tasks; until then `getAdapter` throws for unknown names.
|
||||||
|
```ts
|
||||||
|
const factories = new Map<AgentRuntimeName, AdapterFactory>();
|
||||||
|
export const registerAdapter = (name: AgentRuntimeName, factory: AdapterFactory): void => { factories.set(name, factory); };
|
||||||
|
export const getAdapter = (name: AgentRuntimeName): AgentRuntime => {
|
||||||
|
const factory = factories.get(name);
|
||||||
|
if (!factory) throw new Error(`No agent runtime adapter registered for "${name}".`);
|
||||||
|
return factory();
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write a failing test `apps/agent-worker/tests/unit/agent-runtime.test.ts`: `registerAdapter('codex', () => fake)` then `expect(getAdapter('codex').name).toBe('codex')`, and `expect(() => getAdapter('opencode')).toThrow(/No agent runtime adapter/)`. Also assert `isCodexLoginProfile`/`opencodeModel` re-exported from `runtime/provider.ts` behave as before (copy one existing expectation, e.g. `opencodeModel({ aiProviderProfile: { provider: 'anthropic', model: 'claude-x', ... } } as Claim)` → `'anthropic/claude-x'`).
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run test:unit` → FAIL (module/exports missing).
|
||||||
|
- [ ] Create `runtime/provider.ts`: move `Claim` (worker.ts:47-95), `isCodexLoginProfile`, `collectJsonStringValues`, `providerEnvironment`, `opencodeModel`, `codexModel`, `codexModelArgs` (worker.ts:350-446) verbatim; widen `Claim['job']['runtime']` to `'codex' | 'opencode' | 'claude'`; add and export `AdapterWorkspace`.
|
||||||
|
- [ ] Create `runtime/agent-runtime.ts` with the interface, `TurnResult`, `ExecStreamFn`, `AdapterFactory`, and the registry above.
|
||||||
|
- [ ] Edit `worker.ts`: import `Claim`, the helpers, and `AdapterWorkspace` from `runtime/provider.ts` (delete the now-moved definitions); change `ActiveWorkspace` (worker.ts:97) to `type ActiveWorkspace = AdapterWorkspace & { githubToken: string; runtimeMode?: ...; agentTurnActive?: boolean; resolveTurn?: () => void; lastRecordedDiffSignature?: string; codexTurnError?: string; ... }`; set `runtime` when the workspace is built in `runClaim` (worker.ts:1348 — temporarily `runtime: 'codex'` placeholder; real resolution lands in Task 3).
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run test:unit` → PASS. Run `bun run typecheck` → PASS.
|
||||||
|
- [ ] Commit: `refactor(worker): extract AgentRuntime interface, shared provider helpers, and adapter registry`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: In-box process-group kill + marked-command helpers
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `apps/agent-worker/src/runtime/docker.ts` (add two exported helpers near `streamExecInContainer` at line 385).
|
||||||
|
- Create `apps/agent-worker/tests/unit/box-process-kill.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
*Produces* (`runtime/docker.ts`):
|
||||||
|
```ts
|
||||||
|
// Wraps a CLI argv so the process runs as a new session/process-group leader whose
|
||||||
|
// bash parent carries the marker in its argv (matchable by `pgrep -f`). We do NOT
|
||||||
|
// `exec` (bash must survive so the marker stays visible); stdout/stderr fds are
|
||||||
|
// inherited by the CLI so streaming still works.
|
||||||
|
export const buildMarkedCommand = (marker: string, command: string[]): string[] => {
|
||||||
|
const script = `# ${marker}\nexec 0</dev/null\n${command.map(shellQuote).join(' ')}`;
|
||||||
|
return ['setsid', 'bash', '-lc', script];
|
||||||
|
};
|
||||||
|
|
||||||
|
// Kills every process group whose bash parent matches the marker, TERM then KILL.
|
||||||
|
export const killBoxProcessesByMarker = async (args: {
|
||||||
|
containerName: string;
|
||||||
|
marker: string;
|
||||||
|
}): Promise<void> => {
|
||||||
|
const script = [
|
||||||
|
`pids=$(pgrep -f ${shellQuote(args.marker)} || true)`,
|
||||||
|
`for pid in $pids; do kill -TERM -"$pid" 2>/dev/null || true; done`,
|
||||||
|
`sleep 2`,
|
||||||
|
`for pid in $pids; do kill -KILL -"$pid" 2>/dev/null || true; done`,
|
||||||
|
].join('\n');
|
||||||
|
await execa(containerRuntime(), ['exec', args.containerName, 'bash', '-lc', script], {
|
||||||
|
reject: false, stdin: 'ignore',
|
||||||
|
});
|
||||||
|
};
|
||||||
|
```
|
||||||
|
Add a local `const shellQuote = (v: string) => `'${v.replaceAll("'", "'\\''")}'`;` (mirror `worker.ts:1049`).
|
||||||
|
|
||||||
|
*Consumes:* `execa`, `containerRuntime()` (already in file).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `apps/agent-worker/tests/unit/box-process-kill.test.ts`: import `buildMarkedCommand`. Assert `buildMarkedCommand('spoon-turn-abc', ['codex', 'exec', '--json', 'hi'])` returns `['setsid', 'bash', '-lc', expect.stringContaining('# spoon-turn-abc')]` and the script's last line contains `codex exec --json 'hi'`. (Do not unit-test `killBoxProcessesByMarker` end-to-end — it shells out; assert only the argv via a spy if you refactor it to a pure `buildKillScript(marker)` helper. **Recommended:** extract `export const buildKillScript = (marker: string): string` and test it contains `pgrep -f 'spoon-turn-abc'`, `kill -TERM -"$pid"`, and `kill -KILL -"$pid"`.)
|
||||||
|
- [ ] Run `bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `buildMarkedCommand`, `buildKillScript`, and `killBoxProcessesByMarker` in `docker.ts`.
|
||||||
|
- [ ] Run `bun run test:unit` → PASS. `bun run typecheck` → PASS.
|
||||||
|
- [ ] Commit: `feat(worker): in-box process-group marker + kill helpers for agent turns`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: CodexAdapter — refactor `runCodexTurn` behind the interface, dispatch codex through the registry
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create `apps/agent-worker/src/runtime/codex-adapter.ts` (new).
|
||||||
|
- Edit `apps/agent-worker/src/worker.ts` (replace `runCodexTurn` call path in `sendWorkspaceMessage`; resolve `workspace.runtime`; route abort).
|
||||||
|
- Create `apps/agent-worker/tests/unit/codex-adapter.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
*Produces* (`runtime/codex-adapter.ts`):
|
||||||
|
```ts
|
||||||
|
export const createCodexAdapter: AdapterFactory = (deps) => {
|
||||||
|
const execStream = deps?.execStream ?? streamExecInContainer;
|
||||||
|
return {
|
||||||
|
name: 'codex',
|
||||||
|
prepareAuth, // moves prepareCodexAuth (worker.ts:459) + prepareCodexWorkspaceFiles + writeJsonFile here
|
||||||
|
runTurn, // moves runCodexTurn body (worker.ts:712-853), using buildMarkedCommand + execStream
|
||||||
|
abort, // killBoxProcessesByMarker({ containerName: ws.boxName, marker: ws.turnMarker })
|
||||||
|
};
|
||||||
|
};
|
||||||
|
```
|
||||||
|
`runTurn(ws, prompt, onEvent)`:
|
||||||
|
1. `ws.runtime = 'codex'`; `ws.turnMarker = 'spoon-turn-' + randomUUID()`; `ws.codexSessionId` continuity preserved.
|
||||||
|
2. Build the same codex argv as `worker.ts:737-761` (`codex exec [resume <id>] --json ...codexModelArgs --dangerously-bypass-approvals-and-sandbox --output-last-message <containerPath> [--cd <repo>] <prompt>`), then `const command = buildMarkedCommand(ws.turnMarker, codexArgv)`.
|
||||||
|
3. `await execStream({ containerName: ws.boxName, containerCwd: ws.containerRepo, command, environment: { ...providerEnvironment(ws.claim, ws.containerHome), ...secretEnv }, redact: ws.redact, timeoutMs: env.jobTimeoutMs, onStdoutLine: (line) => forEach normalizeCodexJsonLine(line) → onEvent(event), onStderrLine: (line) => onEvent({ kind: 'status', status: line }) if not noise })`.
|
||||||
|
4. On timeout: register `setTimeout(() => void killBoxProcessesByMarker(...), env.jobTimeoutMs)` and clear it when execStream resolves (execa's own `timeout` only kills the local client — the marker kill is what stops the in-box process).
|
||||||
|
5. Read `--output-last-message` file (worker.ts:811-843) → `finalMessage`. Capture `error` events into a local var → `TurnResult.error`. Return `{ finalMessage, sessionId: capturedSessionId, error }`.
|
||||||
|
|
||||||
|
*Consumes:* `normalizeCodexJsonLine` (`agent-events.ts`), `providerEnvironment`/`codexModelArgs`/`isCodexLoginProfile` (`runtime/provider.ts`), `buildMarkedCommand`/`killBoxProcessesByMarker`/`streamExecInContainer` (`runtime/docker.ts`), `prepareCodexWorkspaceFiles` (`codex-runtime.ts`), `env`.
|
||||||
|
|
||||||
|
*Worker dispatch change* (`sendWorkspaceMessage`, worker.ts:1624): replace the `isCodexLoginProfile(claim) ? runCodexTurn : env.runtime==='docker' ? runOpenCodeTurn : localRun` branching with:
|
||||||
|
```ts
|
||||||
|
const adapter = getAdapter(workspace.runtime);
|
||||||
|
const onEvent = (event: NormalizedAgentEvent) => handleAgentEvent({ workspace, event, assistantMessageId, assistantContent });
|
||||||
|
const result = await adapter.runTurn(workspace, prompt, onEvent);
|
||||||
|
if (result.sessionId) await persistRuntimeSession(workspace, result.sessionId); // generic setter, replaces setCodexSessionId call site
|
||||||
|
if (!assistantContent.value.trim() && result.finalMessage) { assistantContent.value = truncate(workspace.redact(result.finalMessage), 40_000); await updateMessage(...); }
|
||||||
|
if (!assistantContent.value.trim() && result.error) throw new Error(`${workspace.runtime} failed:\n${result.error}`);
|
||||||
|
```
|
||||||
|
Resolve `workspace.runtime` in `runClaim` (worker.ts:1348): for now set `runtime: isCodexLoginProfile(claim) ? 'codex' : 'opencode'` so behavior is byte-identical to today (job.runtime becomes authoritative in Task 8). Route `abortWorkspaceAgent` (worker.ts:1562) through `getAdapter(workspace.runtime).abort(workspace)`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `apps/agent-worker/tests/unit/codex-adapter.test.ts`: build the adapter with a fake `execStream` that calls `onStdoutLine` with fixture lines (reuse the exact JSON shapes from `agent-events.test.ts:110-172`, e.g. `{"type":"item.completed","item":{"id":"item-1","type":"agent_message","text":"done"}}` and `{"type":"turn.completed"}`) then resolves `{ exitCode: 0, output: '' }`. Assert `onEvent` received `{ kind: 'assistant_delta', content: 'done\n\n', externalMessageId: 'item-1' }` and `{ kind: 'assistant_completed' }`, and that `runTurn` returns a `TurnResult`. Add a second case: fake execStream that emits `{"type":"turn.failed","error":{"message":"boom"}}` and returns exitCode 0 → `result.error` contains `boom`. Assert `buildMarkedCommand` was used (spy the fake to capture `args.command[0] === 'setsid'`).
|
||||||
|
- [ ] Run `bun run test:unit` → FAIL.
|
||||||
|
- [ ] Create `runtime/codex-adapter.ts`; move Codex logic out of `worker.ts` (`prepareCodexAuth`, `runCodexTurn`, `writeJsonFile`, `readCodexTurnError`). Register it: add `registerAdapter('codex', createCodexAdapter)` in a new `runtime/register.ts` imported once from `worker.ts` top-level.
|
||||||
|
- [ ] Rewire `sendWorkspaceMessage` and `abortWorkspaceAgent` per above; add `persistRuntimeSession` (writes `setCodexSessionId`/`opencodeSessionId`/`claudeSessionId` based on `workspace.runtime`). Keep the maintenance-decision parse, diff artifact, `recordChangedFiles` tail (worker.ts:1731-1758) unchanged.
|
||||||
|
- [ ] Run `bun run test:unit` (all existing + new) → PASS. `bun run typecheck` → PASS.
|
||||||
|
- [ ] Commit: `refactor(worker): move Codex turn logic into CodexAdapter behind AgentRuntime`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: OpenCodeAdapter — run `opencode run --format json` inside the box
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create `apps/agent-worker/src/runtime/opencode-adapter.ts` (new).
|
||||||
|
- Edit `apps/agent-worker/src/agent-events.ts` (add `normalizeOpenCodeRunLine` for `opencode run --format json` line output; may delegate to `normalizeOpenCodeEvent`).
|
||||||
|
- Edit `apps/agent-worker/src/runtime/register.ts` (register `opencode`).
|
||||||
|
- Create `apps/agent-worker/tests/unit/opencode-adapter.test.ts` (new).
|
||||||
|
- Edit `apps/agent-worker/tests/unit/agent-events.test.ts` (add `normalizeOpenCodeRunLine` cases).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
*Produces* (`runtime/opencode-adapter.ts`):
|
||||||
|
```ts
|
||||||
|
export const createOpenCodeAdapter: AdapterFactory = (deps) => {
|
||||||
|
const execStream = deps?.execStream ?? streamExecInContainer;
|
||||||
|
return {
|
||||||
|
name: 'opencode',
|
||||||
|
prepareAuth: async (ws) => { /* opencode reads ANTHROPIC_API_KEY/OPENAI_API_KEY from env; for opencode_auth_json profiles, write ~/.local/share/opencode/auth.json 0600 (reuse logic from worker.ts:472-480) */ },
|
||||||
|
runTurn,
|
||||||
|
abort: (ws) => killBoxProcessesByMarker({ containerName: ws.boxName, marker: ws.turnMarker ?? '' }),
|
||||||
|
};
|
||||||
|
};
|
||||||
|
```
|
||||||
|
`runTurn(ws, prompt, onEvent)`:
|
||||||
|
1. `ws.runtime = 'opencode'`; `ws.turnMarker = 'spoon-turn-' + randomUUID()`.
|
||||||
|
2. argv: `['opencode', 'run', '--format', 'json', '--model', opencodeModel(ws.claim), ...(ws.opencodeSessionId ? ['--session', ws.opencodeSessionId] : []), '--', prompt]`. **Verify the continuity flag** against `opencode run --help` during the smoke step (Task 11); if `--session` is unsupported, drop it and note that OpenCode turns are stateless (acceptable — Codex/Claude carry continuity).
|
||||||
|
3. `command = buildMarkedCommand(ws.turnMarker, argv)`; `execStream({ containerName: ws.boxName, containerCwd: ws.containerRepo, command, environment: { ...providerEnvironment(ws.claim), ...secretEnv }, redact, timeoutMs, onStdoutLine: (line) => normalizeOpenCodeRunLine(line).forEach(onEvent), onStderrLine })`.
|
||||||
|
4. Return `{ finalMessage, sessionId: capturedSessionId, error }` (session id parsed from the run output if present).
|
||||||
|
|
||||||
|
*Produces* (`agent-events.ts`): `export const normalizeOpenCodeRunLine = (line: string): NormalizedAgentEvent[]` — `JSON.parse` each line; if it matches the SSE-style `{ type, properties }` shape, delegate to `normalizeOpenCodeEvent(parsed)`; otherwise map `opencode run`'s result shape (capture `sessionID`, final text). Unparseable lines → `[{ kind: 'status', status: line }]`.
|
||||||
|
|
||||||
|
*Consumes:* `opencodeModel`/`providerEnvironment` (`runtime/provider.ts`), `buildMarkedCommand`/`killBoxProcessesByMarker`/`streamExecInContainer` (`runtime/docker.ts`).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `agent-events.test.ts` case: `normalizeOpenCodeRunLine(JSON.stringify({ type: 'message.part.delta', properties: { part: { text: 'hi' }, messageID: 'm1' } }))` → `toContainEqual({ kind: 'assistant_delta', content: 'hi', externalMessageId: 'm1' })`; and a non-JSON line → `{ kind: 'status', status: '<line>' }`.
|
||||||
|
- [ ] Write failing `opencode-adapter.test.ts`: fake `execStream` emits the delta line + a final line, returns `{ exitCode: 0, output: '' }`; assert `onEvent` got the delta, `runTurn` resolves, and the captured `args.command[0] === 'setsid'` and `args.command[3]` (script) contains `opencode run --format json`.
|
||||||
|
- [ ] Run `bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `normalizeOpenCodeRunLine`, `createOpenCodeAdapter`, and register it.
|
||||||
|
- [ ] Run `bun run test:unit` → PASS. `bun run typecheck` → PASS.
|
||||||
|
- [ ] Commit: `feat(worker): OpenCodeAdapter runs opencode in the per-user box (no side container)`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 5: Delete the per-job OpenCode container path and dead docker helpers
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `apps/agent-worker/src/worker.ts` (remove `ensureOpenCodeSession`, `runOpenCodeTurn`, `workspaceContainerName`, `startWorkspaceContainer` import + usage, `stopWorkspaceContainer` calls, `containerName`/`containerId`/`opencodePassword`/`opencodeSession` workspace fields, the OpenCode-server branches of `abortWorkspaceAgent`/`replyToInteraction`/`getWorkspaceAgentStatus`; retarget health/cleanup to `spoon-box-`).
|
||||||
|
- Delete `apps/agent-worker/src/opencode-session.ts`.
|
||||||
|
- Edit `apps/agent-worker/src/runtime/docker.ts` (delete `runInJobContainer`, `streamInJobContainer`, `execInWorkspaceContainer`, `inspectWorkspaceContainer`, `startWorkspaceContainer`, `getPublishedPort`; keep `ensureUserContainer`, `streamExecInContainer`, `runExecInContainer`, `stopWorkspaceContainer`, `listWorkspaceContainerNames`, `jobWorkspaceVolumeSpec`, `normalizeRunResult`, `ensureJobImagePulled`).
|
||||||
|
- Edit `apps/agent-worker/src/env.ts` (remove `containerAccess`; remove now-dead `terminalImage`/`terminalIdleMs`/`maxConcurrentJobs` only if not read anywhere else — grep first).
|
||||||
|
- Edit `packages/backend/convex/*` — none here.
|
||||||
|
- Edit `apps/agent-worker/tests/unit/*` as needed (no test imports the deleted symbols; if any do, update).
|
||||||
|
|
||||||
|
**Interfaces:** *Consumes:* now only box-scoped docker helpers. `getWorkerHealth` (worker.ts:1880) enumerates `listWorkspaceContainerNames('spoon-box-')`; `cleanupOrphanedWorkspaces` (worker.ts:1911) enumerates `spoon-box-` for the container list (do **not** touch the `homes/` workdir removal here — that is Phase 1's layout-aware rewrite; leave whatever Phase 1 produced).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] `grep -rn "spoon-agent-job\|startWorkspaceContainer\|ensureOpenCodeSession\|opencode-session\|containerAccess\|host_port\|runInJobContainer\|streamInJobContainer\|execInWorkspaceContainer\|inspectWorkspaceContainer" apps/agent-worker/src` → build the deletion checklist from the hits.
|
||||||
|
- [ ] `grep -rn "env.terminalImage\|env.terminalIdleMs\|env.maxConcurrentJobs" apps/agent-worker/src` → only remove the env var if zero hits remain after Phase 1 (`maxConcurrentJobs` is surfaced in `getWorkerHealth` at worker.ts:1903 — drop that line too, or keep the field; note it in the commit).
|
||||||
|
- [ ] Remove the symbols above. In `abortWorkspaceAgent`/`replyToInteraction`/`getWorkspaceAgentStatus`, drop the `opencodeSession` branches: `abortWorkspaceAgent` → `await getAdapter(workspace.runtime).abort(workspace)`; `replyToInteraction` → `throw new Error('Interactive replies are not supported by exec-based runtimes.')` (no runtime generates interaction requests after this refactor); `getWorkspaceAgentStatus` → return `{ runtimeMode: workspace.runtime, codexSessionId, opencodeSessionId, claudeSessionId, active }`.
|
||||||
|
- [ ] In `openWorkspacePullRequest` (worker.ts:1852-1855) and `stopWorkspace` (worker.ts:1869-1872), delete the `workspace.opencodeSession?.close()` and `stopWorkspaceContainer(workspace.containerName)` lines (box lifecycle is now purely `releaseUserBox`).
|
||||||
|
- [ ] Delete `apps/agent-worker/src/opencode-session.ts` and its imports (worker.ts:31-35).
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run typecheck` → PASS (fix any dangling refs). Run `bun run test:unit` → PASS.
|
||||||
|
- [ ] Commit: `refactor(worker): delete per-job OpenCode container path, host_port split, and dead docker helpers`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 6: ClaudeCodeAdapter — `claude -p --output-format stream-json` in the box
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create `apps/agent-worker/src/runtime/claude-adapter.ts` (new).
|
||||||
|
- Edit `apps/agent-worker/src/agent-events.ts` (add `normalizeClaudeJsonLine`).
|
||||||
|
- Edit `apps/agent-worker/src/runtime/register.ts` (register `claude`).
|
||||||
|
- Create `apps/agent-worker/tests/unit/claude-adapter.test.ts` (new).
|
||||||
|
- Edit `apps/agent-worker/tests/unit/agent-events.test.ts` (add Claude normalizer cases).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
*Produces* (`agent-events.ts`): `export const normalizeClaudeJsonLine = (line: string): NormalizedAgentEvent[]`. Claude Code `stream-json` emits one JSON object per line:
|
||||||
|
- `{"type":"system","subtype":"init","session_id":"abc",...}` → `{ kind: 'session', sessionId: 'abc' }`.
|
||||||
|
- `{"type":"assistant","message":{"content":[{"type":"text","text":"..."}]}}` → `{ kind: 'assistant_delta', content: '...' }` (concatenate text blocks); `{"type":"content_block_delta",...}` if present → `assistant_delta`.
|
||||||
|
- `{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Bash","input":{...}}]}}` → `{ kind: 'tool_started', name, input: JSON.stringify(input) }`.
|
||||||
|
- `{"type":"user","message":{"content":[{"type":"tool_result",...}]}}` → `{ kind: 'tool_completed', name: 'tool', output }`.
|
||||||
|
- `{"type":"result","subtype":"success","result":"final text","session_id":"abc"}` → `{ kind: 'assistant_completed', content: 'final text' }` (+ a `session` event if the id is new).
|
||||||
|
- `{"type":"result","subtype":"error_max_turns"|"error_during_execution",...}` → `{ kind: 'error', message }`.
|
||||||
|
- Unparseable → `[{ kind: 'status', status: line }]`.
|
||||||
|
|
||||||
|
*Produces* (`runtime/claude-adapter.ts`):
|
||||||
|
```ts
|
||||||
|
export const createClaudeAdapter: AdapterFactory = (deps) => {
|
||||||
|
const execStream = deps?.execStream ?? streamExecInContainer;
|
||||||
|
return {
|
||||||
|
name: 'claude',
|
||||||
|
prepareAuth, // API-key kind: nothing on disk (ANTHROPIC_API_KEY comes from providerEnvironment). OAuth-json kind: write <homeDir>/.claude/.credentials.json 0600 from claim.aiProviderProfile.secret (reuse writeJsonFile).
|
||||||
|
runTurn,
|
||||||
|
abort: (ws) => killBoxProcessesByMarker({ containerName: ws.boxName, marker: ws.turnMarker ?? '' }),
|
||||||
|
};
|
||||||
|
};
|
||||||
|
```
|
||||||
|
`runTurn(ws, prompt, onEvent)`:
|
||||||
|
1. `ws.runtime = 'claude'`; `ws.turnMarker = 'spoon-turn-' + randomUUID()`.
|
||||||
|
2. argv: `['claude', '-p', prompt, '--output-format', 'stream-json', '--verbose', '--dangerously-skip-permissions', '--model', claudeModel(ws.claim), ...(ws.claudeSessionId ? ['--resume', ws.claudeSessionId] : [])]` (cwd = `containerRepo`). `--verbose` is required for `stream-json`. **Confirm exact flags** in the smoke step against the pinned CLI (`claude --help`).
|
||||||
|
3. `command = buildMarkedCommand(ws.turnMarker, argv)`; `execStream({ containerName: ws.boxName, containerCwd: ws.containerRepo, command, environment: { ...claudeEnv(ws.claim, ws.containerHome), ...secretEnv }, redact, timeoutMs, onStdoutLine: (line) => normalizeClaudeJsonLine(line).forEach(onEvent), onStderrLine })`. `claudeEnv`: for API-key kind `{ ANTHROPIC_API_KEY: secret, HOME: ws.containerHome }`; for OAuth-json kind `{ HOME: ws.containerHome }` (CLI reads `~/.claude/.credentials.json`).
|
||||||
|
4. Return `{ finalMessage, sessionId, error }` (`finalMessage` from the `result` event; `sessionId` from `system:init`/`result`).
|
||||||
|
|
||||||
|
*Consumes:* `normalizeClaudeJsonLine` (`agent-events.ts`), `providerEnvironment` (`runtime/provider.ts`), box helpers (`runtime/docker.ts`), `writeJsonFile` (move to `runtime/provider.ts` or a small `runtime/auth.ts` shared by Codex + Claude adapters).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `agent-events.test.ts` Claude cases: `system:init` → session; `assistant` text block → `assistant_delta`; `result:success` → `assistant_completed` with content; `result:error_max_turns` → `error`.
|
||||||
|
- [ ] Write failing `claude-adapter.test.ts`: fake `execStream` emits `system:init` + `assistant` text + `result:success` lines, returns `{ exitCode: 0, output: '' }`; assert `onEvent` received the mapped events, `runTurn` returns `{ finalMessage: 'final text', sessionId: 'abc' }`, and `args.command[0] === 'setsid'` with the script containing `claude -p`.
|
||||||
|
- [ ] Run `bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `normalizeClaudeJsonLine`, `createClaudeAdapter`, register it. Move `writeJsonFile` to a shared `runtime/auth.ts`.
|
||||||
|
- [ ] Run `bun run test:unit` → PASS. `bun run typecheck` → PASS.
|
||||||
|
- [ ] Commit: `feat(worker): ClaudeCodeAdapter runs claude -p stream-json in the per-user box`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 7: Backend schema — add `claude` runtime, Anthropic OAuth auth kind, and profile→runtimes derivation
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `packages/backend/convex/schema.ts` (runtime unions + authType union).
|
||||||
|
- Create `packages/backend/convex/runtimeSupport.ts` (new — pure derivation helper, no `'use node'`).
|
||||||
|
- Edit `packages/backend/convex/agentJobs.ts` and `packages/backend/convex/spoonAgentSettings.ts` and `packages/backend/convex/aiProviderProfiles.ts`/`aiProviderProfilesNode.ts` (union constants).
|
||||||
|
- Create `packages/backend/tests/unit/runtime-support.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
*Schema changes:*
|
||||||
|
- `agentJobs.runtime` (schema.ts:536): `v.optional(v.union(v.literal('openai_direct'), v.literal('opencode'), v.literal('codex'), v.literal('claude')))` — keep `openai_direct` for legacy reads (deprecated, never written).
|
||||||
|
- `spoonAgentSettings.runtime` (schema.ts:472): same widened union.
|
||||||
|
- `aiProviderProfiles.authType` (schema.ts:389): add `v.literal('anthropic_oauth_json')`.
|
||||||
|
|
||||||
|
*Produces* (`runtimeSupport.ts`):
|
||||||
|
```ts
|
||||||
|
import type { Doc } from './_generated/dataModel';
|
||||||
|
export type AgentRuntimeName = 'codex' | 'opencode' | 'claude';
|
||||||
|
|
||||||
|
// Which runtimes a provider profile can drive.
|
||||||
|
export const runtimesForProfile = (
|
||||||
|
profile: Pick<Doc<'aiProviderProfiles'>, 'provider' | 'authType'>,
|
||||||
|
): AgentRuntimeName[] => {
|
||||||
|
if (profile.provider === 'opencode_openai_login' || profile.authType === 'opencode_auth_json') {
|
||||||
|
return ['codex']; // ChatGPT-login snapshot → Codex CLI only
|
||||||
|
}
|
||||||
|
if (profile.provider === 'anthropic') {
|
||||||
|
return profile.authType === 'anthropic_oauth_json' ? ['claude'] : ['claude', 'opencode'];
|
||||||
|
}
|
||||||
|
if (profile.provider === 'openai') return ['opencode', 'codex'];
|
||||||
|
// Every remaining API-key provider is OpenAI-compatible → OpenCode.
|
||||||
|
return ['opencode'];
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `packages/backend/tests/unit/runtime-support.test.ts`: `runtimesForProfile({ provider: 'anthropic', authType: 'api_key' })` → `['claude','opencode']`; `{ provider: 'opencode_openai_login', authType: 'opencode_auth_json' }` → `['codex']`; `{ provider: 'openrouter', authType: 'api_key' }` → `['opencode']`; `{ provider: 'openai', authType: 'api_key' }` → `['opencode','codex']`.
|
||||||
|
- [ ] Run `cd packages/backend && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Edit `schema.ts` unions; create `runtimeSupport.ts`; widen the `authType` union constant in `aiProviderProfiles.ts:24`, `aiProviderProfilesNode.ts:24`.
|
||||||
|
- [ ] Run `cd packages/backend && bun run codegen` then `bun run typecheck` → PASS. `bun run test:unit` → PASS.
|
||||||
|
- [ ] Commit: `feat(backend): add claude runtime, anthropic OAuth auth kind, and profile→runtime derivation`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 8: Backend queue-time runtime validation + claim payload carries runtime
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `packages/backend/convex/agentJobs.ts` (`insertJob` at line 379, the three `runtime` union constants at line 21, `createFromRequest`/`createForThread`/`createForThreadInternal` args, `claimNextInternal` return).
|
||||||
|
- Edit `packages/backend/convex/agentJobsNode.ts` (map `job.runtime` into the `WorkerClaim`).
|
||||||
|
- Edit `packages/backend/convex/threads.ts` (`createUserThread` passes `runtime`).
|
||||||
|
- Edit `packages/backend/convex/aiProviderProfiles.ts` (`publicProfile` exposes `supportedRuntimes`).
|
||||||
|
- Edit `apps/agent-worker/src/runtime/provider.ts` (`Claim.job.runtime` already widened in Task 1 — confirm) and `worker.ts` `runClaim` (use `claim.job.runtime` as authoritative).
|
||||||
|
- Edit `packages/backend/tests/unit/harness.test.ts` or add `packages/backend/tests/unit/runtime-validation.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
- `const runtime = v.union(v.literal('codex'), v.literal('opencode'), v.literal('claude'))` (agentJobs.ts:21). The three create mutations gain `runtime: v.optional(runtime)`.
|
||||||
|
- `insertJob` resolves and validates:
|
||||||
|
```ts
|
||||||
|
const profile = await getJobProfile(ctx, ownerId, aiProviderProfileId);
|
||||||
|
const supported = runtimesForProfile(profile);
|
||||||
|
const resolvedRuntime =
|
||||||
|
requestedRuntime ?? (settings.runtime as AgentRuntimeName | undefined) ?? supported[0];
|
||||||
|
if (!supported.includes(resolvedRuntime)) {
|
||||||
|
throw new ConvexError(
|
||||||
|
`Provider "${profile.name}" cannot run the "${resolvedRuntime}" runtime. Supported: ${supported.join(', ')}.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
// insert agentJobs with runtime: resolvedRuntime
|
||||||
|
```
|
||||||
|
- `claimNextInternal` already returns the full `job` doc (schema.ts row includes `runtime`), so no change there; `agentJobsNode.claimNextForWorker` must forward `job.runtime` — it already returns `claimed.job` verbatim in `WorkerClaim.job`, so `job.runtime` flows through. Confirm the worker's `Claim.job.runtime` type (widened in Task 1) matches.
|
||||||
|
- `publicProfile` (aiProviderProfiles.ts:45): add `supportedRuntimes: runtimesForProfile(profile)` so the UI can filter.
|
||||||
|
|
||||||
|
*Worker change:* `runClaim` (worker.ts:1301) currently throws for non-`opencode`. Replace with: resolve `const runtime = (claim.job.runtime ?? 'opencode') as AgentRuntimeName;` set `workspace.runtime = runtime`; delete the legacy `openai_direct` throw (queue-time guard now prevents bad runtimes). `prepareAuth` for the resolved adapter runs in place of the `isCodexLoginProfile` special-case (worker.ts:1375): `await getAdapter(runtime).prepareAuth(workspace)`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `packages/backend/tests/unit/runtime-validation.test.ts` (convex-test style from harness.test.ts): create a user + GitHub spoon + an `anthropic`/`api_key` profile set default; call `createForThread`/`createUserThread` equivalent with `runtime: 'codex'` → expect throw `/cannot run the "codex" runtime/`; with `runtime: 'claude'` → resolves; with no runtime and an `opencode_openai_login` profile → job.runtime === 'codex'.
|
||||||
|
- [ ] Run `cd packages/backend && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement the union widening, `insertJob` validation, thread/request arg plumbing, and `publicProfile.supportedRuntimes`.
|
||||||
|
- [ ] Update `worker.ts` `runClaim` to set `workspace.runtime` from `claim.job.runtime` and call `getAdapter(runtime).prepareAuth(workspace)`; delete the legacy throw.
|
||||||
|
- [ ] Run `cd packages/backend && bun run codegen && bun run typecheck && bun run test:unit` → PASS. Run `cd apps/agent-worker && bun run typecheck && bun run test:unit` → PASS.
|
||||||
|
- [ ] Commit: `feat(backend): queue-time runtime/profile validation; job.runtime authoritative in worker`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 9: Migrate/deprecate `openai_direct`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create `packages/backend/convex/migrations.ts` (new internalMutation) OR add to an existing maintenance module — grep for an existing pattern first (`grep -rn "internalMutation" packages/backend/convex | grep -i migrat`; there is none, so create `migrations.ts`).
|
||||||
|
- Edit `packages/backend/tests/unit/` (new test or extend).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
```ts
|
||||||
|
export const migrateOpenAiDirectRuntime = internalMutation({
|
||||||
|
args: {},
|
||||||
|
handler: async (ctx) => {
|
||||||
|
let patched = 0;
|
||||||
|
for (const job of await ctx.db.query('agentJobs').collect()) {
|
||||||
|
if (job.runtime === 'openai_direct') { await ctx.db.patch(job._id, { runtime: 'opencode', updatedAt: Date.now() }); patched++; }
|
||||||
|
}
|
||||||
|
for (const s of await ctx.db.query('spoonAgentSettings').collect()) {
|
||||||
|
if (s.runtime === 'openai_direct') { await ctx.db.patch(s._id, { runtime: 'opencode', updatedAt: Date.now() }); patched++; }
|
||||||
|
}
|
||||||
|
return { patched };
|
||||||
|
},
|
||||||
|
});
|
||||||
|
```
|
||||||
|
Run once post-deploy via `npx convex run migrations:migrateOpenAiDirectRuntime` (document in the smoke/deploy notes). `openai_direct` stays in the schema unions as a legacy-read literal; nothing writes it.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing test: insert a `spoonAgentSettings` row with `runtime: 'openai_direct'` (via `t.run`/raw ctx), run the migration, assert it becomes `'opencode'` and `patched >= 1`.
|
||||||
|
- [ ] Run `cd packages/backend && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `migrations.ts`.
|
||||||
|
- [ ] Run `cd packages/backend && bun run codegen && bun run typecheck && bun run test:unit` → PASS.
|
||||||
|
- [ ] Commit: `feat(backend): migration to retire openai_direct runtime`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 10: Next UI — runtime picker (thread create + spoon agent settings) and Anthropic OAuth profile kind
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `apps/next/src/components/threads/thread-workspace-form.tsx` (replace the disabled `Input value='OpenCode workspace'` at line 142-144 with a runtime `Select`; pass `runtime` to `createUserThread`).
|
||||||
|
- Edit `apps/next/src/components/spoons/spoon-agent-settings-form.tsx` (replace the disabled Runtime input at line 187-190 with a `Select`; pass `runtime` to `spoonAgentSettings.update`).
|
||||||
|
- Edit `apps/next/src/components/integrations/ai-provider-profiles-panel.tsx` (add an Anthropic OAuth JSON option to `providerOptions` / auth-type handling at line 71-97).
|
||||||
|
- Edit `packages/backend/convex/threads.ts` `createUserThread` args + `spoonAgentSettings.ts` `update` args + `agentJobs.ts` create mutations to accept `runtime` (done in Task 8 for agentJobs; add to `createUserThread`/`update`).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- `createUserThread` gains `runtime: v.optional(v.union(v.literal('codex'), v.literal('opencode'), v.literal('claude')))`, forwarded to `createForThreadInternal` → `insertJob`.
|
||||||
|
- `spoonAgentSettings.update` `runtime` arg union widens to `codex|opencode|claude` and `patch.runtime = args.runtime` (drop the `= 'opencode'` hardcode at spoonAgentSettings.ts:126).
|
||||||
|
- UI: both forms read `selectedProfile.supportedRuntimes` (new field from Task 8) and render only those `SelectItem`s; default the value to the settings runtime or `supportedRuntimes[0]`; disable submit if the chosen runtime ∉ supportedRuntimes.
|
||||||
|
- Profiles panel: add `{ value: 'anthropic', label: 'Anthropic OAuth (paste credentials)', authType: 'anthropic_oauth_json' }` alongside the existing `anthropic` API-key option; when `anthropic_oauth_json`, render a `Textarea` for the pasted `~/.claude/.credentials.json` (same UX as the Codex `opencode_auth_json` textarea).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Add `runtime` to `createUserThread` and `spoonAgentSettings.update` args + handlers; `cd packages/backend && bun run codegen && bun run typecheck`.
|
||||||
|
- [ ] Edit the three Next components: runtime `Select`s wired to `supportedRuntimes`; Anthropic OAuth option in the profiles panel.
|
||||||
|
- [ ] Typecheck Next: `cd apps/next && bun run typecheck` (or the repo's Next typecheck script — confirm via `apps/next/package.json`). If component tests exist for these forms, extend them; otherwise cover behavior in the Task 11 manual checklist (note: no jsdom test is required by the spec for the picker, but add one if a sibling form test already exists).
|
||||||
|
- [ ] Commit: `feat(web): per-thread/per-spoon runtime picker and Anthropic OAuth provider profile`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 11: Add Claude Code CLI to the job image + extend the smoke test
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `docker/agent-job.Dockerfile` (line 50 npm install line).
|
||||||
|
- Edit `scripts/smoke-agent-container` (version checks + one turn per runtime).
|
||||||
|
- Edit `README.md` (the "Production agent runtime images" `<details>` at line 189-260: mention Claude Code CLI + the `anthropic_oauth_json` credential snapshot pattern, mirroring the Codex `auth.json` paragraph at line 255-258).
|
||||||
|
|
||||||
|
**Interfaces / concrete commands:**
|
||||||
|
- Dockerfile line 50 becomes:
|
||||||
|
```dockerfile
|
||||||
|
RUN npm install -g pnpm yarn bun@1.3.10 opencode-ai@1.17.9 @openai/codex@0.142.0 @anthropic-ai/claude-code@<PIN> \
|
||||||
|
&& npm cache clean --force
|
||||||
|
```
|
||||||
|
Pin `<PIN>` to the latest published `@anthropic-ai/claude-code` version at implementation time (`npm view @anthropic-ai/claude-code version`); record the exact version in the commit message.
|
||||||
|
- `scripts/smoke-agent-container`: add to the in-container check block (after `codex --version` at line 30): `claude --version`. Then add a documented manual per-runtime turn section (commented, runnable by hand) — build the image, start a throwaway box, exec one turn per runtime:
|
||||||
|
```sh
|
||||||
|
# Manual per-runtime smoke (requires creds in your shell env):
|
||||||
|
# docker build -f docker/agent-job.Dockerfile -t spoon-agent-job:latest .
|
||||||
|
# docker run -d --name spoon-box-smoke --init spoon-agent-job:latest sleep infinity
|
||||||
|
# docker exec -e OPENAI_API_KEY -w /workspace spoon-box-smoke \
|
||||||
|
# opencode run --format json --model openai/gpt-5.5 -- 'print hello'
|
||||||
|
# docker exec -e ANTHROPIC_API_KEY -w /workspace spoon-box-smoke \
|
||||||
|
# claude -p 'print hello' --output-format stream-json --verbose --dangerously-skip-permissions
|
||||||
|
# docker exec -e CODEX_HOME=/root/.codex -w /workspace spoon-box-smoke \
|
||||||
|
# codex exec --json --dangerously-bypass-approvals-and-sandbox 'print hello'
|
||||||
|
# docker rm -f spoon-box-smoke
|
||||||
|
```
|
||||||
|
Use these commands to also **confirm the real flag names** assumed in Tasks 4/6 (`opencode run --help`, `claude --help`) and fix the adapters if they differ.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] `npm view @anthropic-ai/claude-code version` → choose the pin. Edit the Dockerfile.
|
||||||
|
- [ ] Build the image and run `SPOON_AGENT_JOB_IMAGE=spoon-agent-job:latest ./scripts/smoke-agent-container` → all `--version` checks pass (`claude --version` included).
|
||||||
|
- [ ] Run the manual per-runtime turns; reconcile adapter flags with reality (patch Task 4/6 code if needed, keeping unit tests green).
|
||||||
|
- [ ] Update `README.md`.
|
||||||
|
- [ ] Commit: `build(agent-job): pin Claude Code CLI; smoke one turn per runtime`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 12: Secrets hygiene — 0600 env files, deleted on stop, re-materialized per run
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Edit `apps/agent-worker/src/worker.ts` (`materializeEnvFile` at line 1167; `stopWorkspace`/`openWorkspacePullRequest` teardown; call `materializeEnvFile` per run).
|
||||||
|
- Create `apps/agent-worker/tests/unit/materialize-env.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- `materializeEnvFile` (worker.ts:1167): write with `{ mode: 0o600 }` (currently no mode). Extract the file path resolution + write into a pure-ish helper `writeMaterializedEnv(repoDir, envFilePath, secrets): Promise<string>` returning the absolute path, so it is unit-testable with `mkdtemp`.
|
||||||
|
- Track the materialized path on the workspace (`workspace.materializedEnvPath?: string`).
|
||||||
|
- On `stopWorkspace` (worker.ts:1866) and `openWorkspacePullRequest` (worker.ts:1851) and the `runClaim` catch (worker.ts:1410) teardown: if `workspace.materializedEnvPath` exists, `await rm(workspace.materializedEnvPath, { force: true })`.
|
||||||
|
- Re-materialize per run: call `materializeEnvFile(workspace)` at the top of `sendWorkspaceMessage` (worker.ts:1624) when `claim.job.materializeEnvFile` is set, before the adapter turn — so the env file is fresh each turn and can be deleted between turns. Keep the existing `ensureNoEnvFilesStaged` staging guard (worker.ts:1269) unchanged.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `materialize-env.test.ts`: `mkdtemp` a repo dir, call `writeMaterializedEnv(dir, '.env.local', [{ name: 'A', value: 'x' }])`, assert the file exists, `(await stat(path)).mode & 0o777 === 0o600`, and content is `A="x"\n` (matches worker.ts:1172-1174 formatting). Mirror the `codex-runtime.test.ts` `mode()` helper + `afterEach` cleanup.
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `writeMaterializedEnv` (0600), wire deletion on stop/PR/failure, re-materialize per run.
|
||||||
|
- [ ] Run `bun run test:unit` → PASS. `bun run typecheck` → PASS.
|
||||||
|
- [ ] Commit: `fix(worker): materialize env files 0600, re-materialize per run, delete on stop`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-review against the Phase 2 spec
|
||||||
|
|
||||||
|
- **Adapter interface, all exec into the box, process-group kill** → Tasks 1–2 (`buildMarkedCommand` = `setsid` + marker; `killBoxProcessesByMarker` = in-box `pgrep`+`kill -TERM/-KILL` of the group; never the local execa client). ✅
|
||||||
|
- **Codex refactored behind the interface** → Task 3. ✅
|
||||||
|
- **OpenCode inside the box, per-job path deleted** → Tasks 4–5 (`opencode run --format json` in the box; `startWorkspaceContainer`/`spoon-agent-job-*`/`containerAccess host_port|network`/dead helpers all removed). ✅
|
||||||
|
- **Claude Code adapter + new provider kind + pinned CLI + prepareAuth** → Tasks 6, 7, 10, 11 (`anthropic_oauth_json` snapshot mirrors Codex `auth.json`; API-key path uses `ANTHROPIC_API_KEY`). ✅
|
||||||
|
- **Runtime picker + profiles declare supported runtimes + queue-time validation replacing run-time `openai_direct` discovery + migrate/deprecate** → Tasks 7–10. ✅
|
||||||
|
- **Secrets hygiene (0600, delete on stop, re-materialize; staging guard stays)** → Task 12. ✅
|
||||||
|
- **Testing approach** → adapter-level fixture stream tests (injectable `execStream`), normalizer fixtures matching the existing style, convex-test backend tests, extended manual smoke. ✅
|
||||||
@@ -0,0 +1,409 @@
|
|||||||
|
# Spoon Phase 3 — Dev Box as a First-Class Surface: Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Give every user a standalone "My Machine" page at `/machine` that exposes their persistent per-user Fedora box directly: live status + start/stop/restart controls, a full-page terminal rooted at `~`, and a home-scoped file browser (reuse `FileTree`/`CodeEditor`). Make the workspace terminal survive tab switches, and add terminal QoL (web-links, copy-on-select). The box is USER-scoped (there is no `jobId`): the browser reaches it only through Next routes that mint short-lived HMAC tokens **after** Convex auth resolves the caller's own box username server-side — the browser never sees a worker secret and can only ever reach its OWN box.
|
||||||
|
|
||||||
|
**Architecture:** Same ownership-proxy pattern already used for `/jobs/:id/*`, replicated for a user-scoped `/box/*` surface:
|
||||||
|
- **Worker (Bun/dockerode/ws):** new user-scoped HMAC terminal token (`terminal-token.ts`), a box helper module (`box.ts`) for home-path derivation + home-scoped file access + status, HTTP routes `GET /box/status`, `POST /box/lifecycle`, `GET /box/tree`, `GET|PUT /box/file`, and a WS route `GET /box/terminal`. All authed by the internal bearer token (HTTP) or the signed user token (WS), which embeds the username.
|
||||||
|
- **Next (App Router):** `agent-worker-proxy.ts` gains `mintBoxTerminalToken(username)`, `resolveBoxUsername()`, and `proxyBox(...)`; `/api/box/*` route handlers derive the caller's username from Convex auth and proxy to the worker; a client `/machine` page reuses an extracted `XtermSession` component plus `FileTree` + `CodeEditor`.
|
||||||
|
|
||||||
|
**Tech Stack:** Next.js 16 (React 19), Convex, agent-worker (Bun, dockerode, ws, xterm).
|
||||||
|
|
||||||
|
**Depends on:** Phase 1 (terminal sizing fix + box handle/mutex lifecycle). Phase 3 assumes the Phase-1 `acquireUserBox` handle API (see "Phase-1 assumptions" below) and the Phase-1 realpath-based path containment. It benefits from Phase 2 (one-box-three-agents) but does **not** require it: the `/box` surface only needs the persistent box to exist, not any particular agent runtime.
|
||||||
|
|
||||||
|
## Phase-1 assumptions (read before starting)
|
||||||
|
|
||||||
|
Phase 1 reworks `apps/agent-worker/src/user-container.ts` to a per-username mutex + handle API. This plan is written against:
|
||||||
|
|
||||||
|
```ts
|
||||||
|
// apps/agent-worker/src/user-container.ts (Phase-1 shape)
|
||||||
|
export type BoxHandle = { boxName: string; release: () => void }; // release is idempotent
|
||||||
|
export const acquireUserBox: (args: {
|
||||||
|
username: string; workdir: string; containerHome: string;
|
||||||
|
}) => Promise<BoxHandle>;
|
||||||
|
```
|
||||||
|
|
||||||
|
**If Phase 1 kept the pre-Phase-1 signature** (`acquireUserBox(...) => Promise<string>` + `releaseUserBox(username)`, as in commit `b092955`), adapt every `handle.release()` in this plan to `releaseUserBox(username)` and treat the returned string as `boxName`. Check the real signature in `user-container.ts` before writing Task 5, and follow it.
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- **Next tests:** jsdom + @testing-library/react. Component tests live in `apps/next/tests/component/*.test.tsx`, unit tests in `apps/next/tests/unit/*.test.ts`. Run `cd apps/next && bun run test:component` and `bun run test:unit`. Mock `convex/react`, `next/navigation`, and `sonner` exactly as `apps/next/tests/component/render.test.tsx:10-38` does. Monaco (`CodeEditor`) and xterm do not render under jsdom — mock those child components in page tests (see Task 9).
|
||||||
|
- **Worker tests:** `apps/agent-worker/tests/unit/*.test.ts` (vitest). Run `cd apps/agent-worker && bun run test:unit`. Match `apps/agent-worker/tests/unit/terminal-token.test.ts` for token tests. Docker-dependent code (status/lifecycle/exec) is **not** unit-testable without a daemon — document manual verification instead; unit-test only the pure logic (path containment, token parsing, tree building against a real tmpdir).
|
||||||
|
- **Conventional commits, one per task** (e.g. `feat(worker): user-scoped box terminal token`). Commit only after the task's tests pass.
|
||||||
|
- **Browser NEVER receives a worker token.** Tokens are minted server-side in Next route handlers after Convex auth. The username is derived server-side from the authed user (`api.userEnvironment.getMine`), never taken from the request body/query — so a user can only ever reach their own box.
|
||||||
|
- **Reuse, don't duplicate.** Extract the xterm wiring from `workspace-terminal.tsx` into a shared `XtermSession` (Task 8) before building the box terminal; reuse `FileTree` and `CodeEditor` verbatim on `/machine`. Reuse `safeHomeJoin`-style realpath containment for `/box/file`.
|
||||||
|
|
||||||
|
## Reference facts (verified against the codebase)
|
||||||
|
|
||||||
|
- **Job terminal token** (`apps/agent-worker/src/terminal-token.ts`): format `${expiresAtMs}.${jobId}.${hmacHex}`, HMAC over `${expiresAtMs}.${jobId}`. Minted in `apps/next/src/lib/agent-worker-proxy.ts:20` (`mintTerminalToken`). Secret precedence (both sides): `SPOON_AGENT_TERMINAL_SECRET` → `SPOON_AGENT_WORKER_INTERNAL_TOKEN` → `SPOON_WORKER_TOKEN`. Worker reads it as `env.terminalSecret` (`apps/agent-worker/src/env.ts:43-47`); Next builds it in `terminalSecret()` (`agent-worker-proxy.ts:12-15`).
|
||||||
|
- **Worker HTTP auth**: `requireAuth` (`apps/agent-worker/src/server.ts:45-51`) compares the `Authorization: Bearer` header to `env.internalToken`. Routes are dispatched in `startWorkerServer` (`server.ts:59-186`); only `/health`, `/cleanup`, and `/jobs/:id/*` exist today. The `jobRoute` regex (`server.ts:53-57`) is the pattern to mirror for `/box/*`.
|
||||||
|
- **Worker WS**: `attachTerminalServer` (`terminal.ts:157-181`) handles `server.on('upgrade')`, matches `/^\/jobs\/([^/]+)\/terminal$/`, verifies the token, then calls `bridge(ws, jobId)`. Only enabled when `env.runtime === 'docker'`.
|
||||||
|
- **`bridge()`** (`terminal.ts:21-150`): resolves the workspace via `getTerminalWorkspace(jobId)`, acquires the box via `acquireUserBox`, then spawns `<runtime> exec -i -e TERM=... -e HOME=... -w <containerRepo> <box> /bin/bash -lc 'exec script -qfc <launcher> /dev/null'`. The launcher does `stty rows.. cols..` then `exec tmux new-session -A -s spoon` (or `bash -il`). Input/resize buffered until the exec is ready; `cleanup()` kills the proc and releases the box.
|
||||||
|
- **Box home layout** (`apps/agent-worker/src/worker.ts:1315-1334`): `username = userEnv?.username ?? 'user'`; `homeDir = path.resolve(env.workdir, 'homes', username)`; `containerHome = path.posix.join('/home', username)`. The box is acquired with `{ username, workdir: homeDir, containerHome }`.
|
||||||
|
- **Box container** (`apps/agent-worker/src/runtime/docker.ts`): `userContainerName(username)` = `spoon-box-<sanitized>` (`docker.ts:338-339`); `ensureUserContainer({username,workdir,containerHome})` starts `spoon-box-*` with `--memory 4g --cpus 2` mounting the home (`docker.ts:341-383`); `stopWorkspaceContainer(name)` = `rm -f` (`docker.ts:438-442`); `inspectWorkspaceContainer(name)` runs `inspect` (`docker.ts:444-453`). Container runtime binary = `containerRuntime()`.
|
||||||
|
- **Username resolution (server)**: `api.userEnvironment.getMine` (`packages/backend/convex/userEnvironment.ts:15-34`) returns `{ enabled, username, firstName, dotfilesRepoUrl, dotfilesRepoRef, setupCommand }` where `username = settings?.homeUsername ?? deriveHomeUsername(user?.name)` — the exact value the worker uses. This is the authoritative source for the caller's box username on the Next side.
|
||||||
|
- **Existing file/tree worker fns** (`worker.ts`): `listWorkspaceTree` (1440), `readWorkspaceFile` (1477), `writeWorkspaceFile` (1483) — model the box variants on these but root at `homeDir` and use realpath containment.
|
||||||
|
- **Reusable UI**: `FileTree` (`apps/next/src/components/agent-workspace/file-tree.tsx`) props `{ tree: FileTreeNode|null; selectedPath?; expandedPaths: string[]; onSelect; onToggleDirectory }`. `CodeEditor` (`code-editor.tsx:33-51`) props `{ path?; content; savedContent; readOnly; vimEnabled; onSave; onChange; onVimEnabledChange }`. `FileTreeNode`, `FileResponse`, `DiffResponse` live in `agent-workspace/types.ts`. Nav items are declared in `apps/next/src/components/layout/header/index.tsx:23-45` (`NavItem` = `{ href; icon; label; external? }`).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: Worker — user-scoped box terminal token (mint helper is Next-side; verify is worker-side)
|
||||||
|
|
||||||
|
Add a user-scoped token alongside the existing job-scoped one. It must be unambiguous from the 3-part job token, so it embeds a literal `box` segment.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/agent-worker/src/terminal-token.ts` — add `verifyBoxTerminalToken`; keep `verifyTerminalToken` unchanged.
|
||||||
|
- `apps/agent-worker/tests/unit/box-terminal-token.test.ts` — new test.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Token format (Produces/Consumes):** `${expiresAtMs}.box.${username}.${hmacSha256Hex}`, HMAC over the string `${expiresAtMs}.box.${username}`. Four dot-separated parts (job token has three), so the two can never collide. Username is `[a-z0-9_-]+` (already sanitized by `deriveHomeUsername`) and therefore never contains `.`.
|
||||||
|
- **Produces:**
|
||||||
|
```ts
|
||||||
|
export const verifyBoxTerminalToken = (
|
||||||
|
token: string,
|
||||||
|
username: string,
|
||||||
|
secret: string,
|
||||||
|
): boolean;
|
||||||
|
```
|
||||||
|
Returns `true` only when: token has exactly 4 parts, part[1] === `'box'`, part[2] === `username`, expiry not passed, and the HMAC (over `${parts[0]}.box.${parts[2]}`) matches via `timingSafeEqual` on equal-length buffers. Empty token or empty secret → `false`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write `apps/agent-worker/tests/unit/box-terminal-token.test.ts` mirroring `terminal-token.test.ts`: a local `mintBox(username, expiresAt, secret)` helper producing the 4-part token; cases: accepts valid unexpired username-matched token; rejects expired; rejects token minted for another username; rejects wrong secret; rejects malformed/empty; **rejects a 3-part job token passed to `verifyBoxTerminalToken`** (cross-scheme confusion guard).
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run test:unit` → FAIL (function missing).
|
||||||
|
- [ ] Implement `verifyBoxTerminalToken` in `terminal-token.ts` reusing the existing `signature()` helper. Split on `.`, require `parts.length === 4 && parts[1] === 'box'`, check `parts[2] === username`, parse+check expiry, `timingSafeEqual`.
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] Commit: `feat(worker): user-scoped box terminal token verification`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: Worker — box helper module (home paths, status, lifecycle by username)
|
||||||
|
|
||||||
|
Centralize per-user box path derivation, status inspection, and lifecycle so both HTTP routes and the WS bridge share one source of truth.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/agent-worker/src/box.ts` — new module.
|
||||||
|
- `apps/agent-worker/src/runtime/docker.ts` — add `inspectUserBoxStatus`.
|
||||||
|
- `apps/agent-worker/tests/unit/box-paths.test.ts` — new test (pure path logic only).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces (`box.ts`):**
|
||||||
|
```ts
|
||||||
|
export const boxHomePaths = (username: string) => ({
|
||||||
|
homeDir: string, // path.resolve(env.workdir, 'homes', username)
|
||||||
|
containerHome: string, // path.posix.join('/home', username)
|
||||||
|
});
|
||||||
|
|
||||||
|
export type BoxStatus = {
|
||||||
|
running: boolean;
|
||||||
|
image: string | null;
|
||||||
|
startedAt: string | null; // ISO from container State.StartedAt, null if stopped/absent
|
||||||
|
memoryLimitBytes: number | null;
|
||||||
|
containerName: string; // userContainerName(username)
|
||||||
|
};
|
||||||
|
export const getBoxStatus = (username: string) => Promise<BoxStatus>;
|
||||||
|
|
||||||
|
export const startBox = (username: string) => Promise<void>; // ensureUserContainer(boxHomePaths)
|
||||||
|
export const stopBox = (username: string) => Promise<void>; // stop container + drop registry entry
|
||||||
|
export const restartBox = (username: string) => Promise<void>; // stopBox then startBox
|
||||||
|
```
|
||||||
|
- **Produces (`docker.ts`):**
|
||||||
|
```ts
|
||||||
|
export const inspectUserBoxStatus = (username: string) => Promise<{
|
||||||
|
running: boolean; image: string | null; startedAt: string | null; memoryLimitBytes: number | null;
|
||||||
|
}>;
|
||||||
|
```
|
||||||
|
Implementation: `execa(containerRuntime(), ['inspect','--format','{{.State.Running}}|{{.Config.Image}}|{{.State.StartedAt}}|{{.HostConfig.Memory}}', userContainerName(username)], { reject:false, stdin:'ignore' })`. On non-zero exit (no such container) return `{ running:false, image:null, startedAt:null, memoryLimitBytes:null }`. Parse the pipe-split line; `running = field==='true'`; `memoryLimitBytes = Number(field) || null` (0 means unlimited → null); `startedAt` null when not running.
|
||||||
|
- **Consumes:** `ensureUserContainer`, `stopWorkspaceContainer`, `userContainerName` (docker.ts); `env.workdir` (env.ts). `stopBox` must also clear the in-memory registry so a stopped box isn't considered "held" — add `export const resetBox = (username: string) => void` to `user-container.ts` (clears the idle timer and deletes the map entry) and call it from `stopBox`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Add `resetBox(username)` to `user-container.ts` (clear `box.idleTimer`, `boxes.delete(username)`).
|
||||||
|
- [ ] Write `apps/agent-worker/tests/unit/box-paths.test.ts`: assert `boxHomePaths('gib')` returns `homeDir` ending in `homes/gib` and `containerHome === '/home/gib'`; assert two usernames produce distinct homeDirs. (Do NOT test docker-touching fns here.)
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `inspectUserBoxStatus` in `docker.ts`, then `box.ts` (`boxHomePaths`, `getBoxStatus` wrapping `inspectUserBoxStatus` + `userContainerName`, `startBox`/`stopBox`/`restartBox`).
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] **Manual verification (docker required), record in the commit body:** with the worker running and a box up, `curl -H "Authorization: Bearer $TOKEN" localhost:3921/box/status?user=<you>` (after Task 4) shows `running:true` + image + startedAt; after `POST /box/lifecycle {"action":"stop"}` status flips to `running:false`; `start` brings it back.
|
||||||
|
- [ ] Commit: `feat(worker): per-user box status and lifecycle helpers`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: Worker — home-scoped file access (tree/read/write with realpath containment)
|
||||||
|
|
||||||
|
Home-scoped equivalents of `listWorkspaceTree`/`readWorkspaceFile`/`writeWorkspaceFile`, rooted at the user's `homeDir`, hardened against symlink escape (reuse the Phase-1 containment approach: resolve `fs.realpath` of the parent and re-check containment; reject symlinked write targets).
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/agent-worker/src/box.ts` — add tree/read/write.
|
||||||
|
- `apps/agent-worker/tests/unit/box-files.test.ts` — new test against a real tmpdir.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces:**
|
||||||
|
```ts
|
||||||
|
export const listBoxTree = (username: string) => Promise<FileTreeNode>;
|
||||||
|
export const readBoxFile = (username: string, relPath: string) => Promise<string>;
|
||||||
|
export const writeBoxFile = (username: string, relPath: string, content: string) => Promise<{ success: true }>;
|
||||||
|
// internal: safeHomePath(homeDir, relPath) => Promise<string> (realpath-checked absolute path)
|
||||||
|
```
|
||||||
|
- `listBoxTree`: walk `homeDir` (root node `name: '~'`, `path: ''`), skipping a small ignore set: `.git`, `node_modules`, `.cache`, `.local/share/Trash`, `.npm`, `.bun`, `.cargo`, `dist`, `build`, `.next`. Cap traversal (e.g. skip directories once total node count exceeds ~5000) so a huge home can't hang the request. Node shape matches `FileTreeNode` from `agent-workspace/types.ts` (`{ name; path; type: 'file'|'directory'; children? }`).
|
||||||
|
- `readBoxFile`/`writeBoxFile`: resolve via `safeHomePath`. `writeBoxFile` `mkdir -p` the parent, then write. **No job/diff/event recording** (there is no job) — this is the key difference from `writeWorkspaceFile`.
|
||||||
|
- `safeHomePath`: lexical resolve, then `realpath` the existing ancestor and re-check `startsWith(homeDir + sep)`; throw `Refusing to access path outside home: <relPath>` on escape. For writes, if the target itself exists and is a symlink, reject.
|
||||||
|
- **Consumes:** `boxHomePaths`, `node:fs/promises` (`stat`, `readdir`, `readFile`, `writeFile`, `mkdir`, `realpath`), `FileTreeNode` type (import from where the worker declares it — check `worker.ts` for the local `FileTreeNode` type; if not exported, define a matching local type in `box.ts`).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write `apps/agent-worker/tests/unit/box-files.test.ts`: create a tmp home dir (`os.tmpdir()` + `mkdtemp`), stub `env.workdir` so `boxHomePaths('t').homeDir` points at it (either set `SPOON_AGENT_WORKDIR` before importing, or test `safeHomePath` directly by exporting it). Cases: `writeBoxFile` then `readBoxFile` round-trips; `readBoxFile('../../etc/passwd')` rejects; a symlink inside home pointing outside is rejected on read AND write; `listBoxTree` includes a created file and excludes `node_modules`.
|
||||||
|
- [ ] Run `cd apps/agent-worker && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement `safeHomePath`, `listBoxTree`, `readBoxFile`, `writeBoxFile` in `box.ts`.
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] Commit: `feat(worker): home-scoped box file tree/read/write with realpath containment`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: Worker — `/box/*` HTTP routes
|
||||||
|
|
||||||
|
Wire the box helpers into the HTTP server, authed by the internal bearer token like every other route.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/agent-worker/src/server.ts` — add box route dispatch (import from `./box`).
|
||||||
|
|
||||||
|
**Interfaces (route → response shape):**
|
||||||
|
- `GET /box/status?user=<username>` → `200 { status: BoxStatus }`.
|
||||||
|
- `POST /box/lifecycle?user=<username>` body `{ action: 'start' | 'stop' | 'restart' }` → `200 { status: BoxStatus }` (return the post-action status; unknown action → `400 { error }`).
|
||||||
|
- `GET /box/tree?user=<username>` → `200 { tree: FileTreeNode }`.
|
||||||
|
- `GET /box/file?user=<username>&path=<rel>` → `200 { path, content }`.
|
||||||
|
- `PUT /box/file?user=<username>` body `{ path, content }` → `200 { success: true }`.
|
||||||
|
- Missing/empty `user` → `400 { error: 'Missing user' }`.
|
||||||
|
|
||||||
|
**Notes:** the `user` query param is trusted here because this route is only reachable with the internal bearer token, and the only caller (Next, Task 7) derives it from Convex auth. Add a `boxRoute(pathname)` matcher (mirror `jobRoute`, `server.ts:53-57`) matching `/^\/box\/(status|lifecycle|tree|file)$/`, and dispatch inside the existing try/catch in `startWorkerServer` **before** the `jobRoute` block. Read `user` via `url.searchParams.get('user')`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] (No new unit test — this is HTTP glue over already-tested helpers; verify manually.) Add the `boxRoute` matcher and the five handlers in `server.ts`, importing `getBoxStatus, startBox, stopBox, restartBox, listBoxTree, readBoxFile, writeBoxFile` from `./box`. Reuse `sendJson`/`parseJson`.
|
||||||
|
- [ ] `cd apps/agent-worker && bun run build` (or `bun run typecheck`) → passes.
|
||||||
|
- [ ] **Manual verification (record in commit body):** with a box for your username, `curl -H "Authorization: Bearer $SPOON_WORKER_TOKEN" 'localhost:3921/box/status?user=<you>'` returns status JSON; `curl -X PUT ... 'localhost:3921/box/file?user=<you>' -d '{"path":"spoon-test.txt","content":"hi"}'` then `GET .../box/file?...&path=spoon-test.txt` round-trips; `GET .../box/tree?user=<you>` lists the home.
|
||||||
|
- [ ] Commit: `feat(worker): /box status, lifecycle, tree, and file HTTP routes`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 5: Worker — `/box/terminal` WebSocket bridge
|
||||||
|
|
||||||
|
Add a user-scoped terminal WS that acquires the box and opens a login shell rooted at `~`. Extract the shared PTY-bridging logic so the job and box bridges don't duplicate the exec/buffer/cleanup dance.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/agent-worker/src/terminal.ts` — add `bridgeBox`, refactor shared exec bridging into a helper, add the `/box/terminal` upgrade match.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **WS route:** `GET /box/terminal?token=<boxToken>`. Verify with `verifyBoxTerminalToken(token, username, env.terminalSecret)` — but the **username comes from the token itself** (parse part[2] after a successful format check), since the token is what authorizes the connection. Concretely: parse the token, extract the candidate username, then `if (!verifyBoxTerminalToken(token, candidateUsername, env.terminalSecret)) reject`. On success call `bridgeBox(ws, candidateUsername)`.
|
||||||
|
- **`bridgeBox(ws, username)`:** derive `{ homeDir, containerHome } = boxHomePaths(username)`; `const handle = await acquireUserBox({ username, workdir: homeDir, containerHome })`; ensure a login shell works even for a brand-new home by writing a minimal `.bash_profile` if absent (reuse the snippet from `user-environment.ts:62-68`, or extract that into a shared `ensureBashProfile(homeDir)` and call it here); spawn the same `exec -i` shell as `bridge()` but with `-w <containerHome>` (root at `~`, not a repo) and env `TERM` + `HOME=<containerHome>` only (**no per-job secrets** — the box terminal is not job-scoped). Reuse the buffered-input + resize + cleanup logic; `cleanup()` calls `handle.release()` (or `releaseUserBox(username)` per the Phase-1 assumption).
|
||||||
|
|
||||||
|
**Refactor detail:** extract from the current `bridge()` (`terminal.ts:21-150`) a helper like:
|
||||||
|
```ts
|
||||||
|
const runShellBridge = (ws, boxName, opts: { cwd: string; envFlags: string[]; getSize: () => {cols:number;rows:number} }) => { /* buffered input, spawn, forward, exit */ };
|
||||||
|
```
|
||||||
|
so both `bridge(ws, jobId)` and `bridgeBox(ws, username)` call it. Keep behavior identical for the job path (regression-guard by leaving its manual smoke test intact).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] In `attachTerminalServer` (`terminal.ts:157-181`), add a second upgrade match `/^\/box\/terminal$/`: read `token`, parse username, verify with `verifyBoxTerminalToken`, on failure `401` + destroy, on success `wss.handleUpgrade(... => bridgeBox(ws, username))`. Keep the existing `/jobs/:id/terminal` branch working.
|
||||||
|
- [ ] Implement `bridgeBox` and the shared `runShellBridge` (refactor `bridge` to use it). Add `ensureBashProfile` (shared with `user-environment.ts` or duplicated minimal).
|
||||||
|
- [ ] `cd apps/agent-worker && bun run test:unit` → existing tests still PASS; `bun run build`/typecheck passes.
|
||||||
|
- [ ] **Manual verification (record in commit body):** after Task 8/9 the browser flow covers this; for now, mint a box token by hand and connect a `websocat`/wscat client to `ws://localhost:3921/box/terminal?token=...`, confirm you land in `~` (`pwd` shows `/home/<you>`), typing works, resize works (`stty size` reflects the client), and disconnect releases the box (status idle-reaps after `boxIdleMs`).
|
||||||
|
- [ ] Commit: `feat(worker): user-scoped /box/terminal websocket bridge`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 6: Next — box proxy helpers (mint token, resolve username, proxy)
|
||||||
|
|
||||||
|
Server-only helpers that mint the box token and proxy `/box/*` to the worker, deriving the username from Convex auth so the caller can only reach their own box.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/lib/agent-worker-proxy.ts` — add `mintBoxTerminalToken`, `resolveBoxUsername`, `proxyBox`, `withBox`.
|
||||||
|
- `apps/next/tests/unit/box-terminal-token.test.ts` — new unit test for the mint format.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces:**
|
||||||
|
```ts
|
||||||
|
// Mirrors mintTerminalToken (agent-worker-proxy.ts:20). Returns null if secret/WS base unset.
|
||||||
|
export const mintBoxTerminalToken = (username: string): { url: string; expiresAt: number } | null;
|
||||||
|
// payload = `${expiresAt}.box.${username}`; token = `${payload}.${hmacHex}`;
|
||||||
|
// url = `${wsBase}/box/terminal?token=${encodeURIComponent(token)}` (NOTE: /box/terminal, no jobId)
|
||||||
|
|
||||||
|
// Resolves the authed caller's own box username, or an unauthorized/again response.
|
||||||
|
export const resolveBoxUsername = (): Promise<
|
||||||
|
| { ok: true; username: string }
|
||||||
|
| { ok: false; response: NextResponse }
|
||||||
|
>;
|
||||||
|
// uses convexAuthNextjsToken() then fetchQuery(api.userEnvironment.getMine, {}, { token }) -> .username
|
||||||
|
|
||||||
|
// Proxies to the worker with the internal token, always appending user=<derived username>.
|
||||||
|
export const proxyBox = (
|
||||||
|
username: string, action: 'status'|'lifecycle'|'tree'|'file',
|
||||||
|
init?: RequestInit, search?: URLSearchParams,
|
||||||
|
): Promise<NextResponse>;
|
||||||
|
|
||||||
|
// Convenience wrapper: resolve username -> run handler, 401/500 on failure (mirror withOwnedJob).
|
||||||
|
export const withBox = (
|
||||||
|
handler: (username: string) => Promise<Response>,
|
||||||
|
): Promise<Response>;
|
||||||
|
```
|
||||||
|
- **Consumes:** `terminalSecret()` and `workerToken()` (existing, `agent-worker-proxy.ts:12-15,41-42`), `env.NEXT_PUBLIC_SPOON_AGENT_WORKER_WS_URL`, `env.SPOON_AGENT_WORKER_URL`, `convexAuthNextjsToken`, `fetchQuery`, `api.userEnvironment.getMine`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write `apps/next/tests/unit/box-terminal-token.test.ts`: import `createHmac`, replicate the expected token string for a fixed secret+username+expiry, and assert `mintBoxTerminalToken`'s `url` contains `/box/terminal?token=` and the token has 4 dot-parts with `parts[1]==='box'`. To make the secret deterministic, set `process.env.SPOON_AGENT_TERMINAL_SECRET` and `NEXT_PUBLIC_SPOON_AGENT_WORKER_WS_URL` before import (match how `apps/next/tests/unit/environment.test.ts` handles env). If importing `agent-worker-proxy.ts` pulls in `server-only`/Convex and fails under vitest, extract the pure token-building into a tiny `buildBoxToken(username, secret, wsBase, now)` and unit-test that instead (keep `mintBoxTerminalToken` a thin wrapper). Prefer the extraction — it keeps the test hermetic.
|
||||||
|
- [ ] Run `cd apps/next && bun run test:unit` → FAIL.
|
||||||
|
- [ ] Implement the four helpers (+ `buildBoxToken` if extracted). `resolveBoxUsername` returns `{ ok:false, response: 401 }` when no auth token; `proxyBox` mirrors `proxyWorker` (`agent-worker-proxy.ts:95-131`) but targets `/box/${action}` and force-sets `user` in the search params from the derived username.
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] Commit: `feat(web): box terminal token mint + ownership proxy helpers`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 7: Next — `/api/box/*` route handlers
|
||||||
|
|
||||||
|
App-Router handlers that derive the caller's username and proxy to the worker.
|
||||||
|
|
||||||
|
**Files (all new):**
|
||||||
|
- `apps/next/src/app/api/box/status/route.ts` — `GET`.
|
||||||
|
- `apps/next/src/app/api/box/lifecycle/route.ts` — `POST`.
|
||||||
|
- `apps/next/src/app/api/box/tree/route.ts` — `GET`.
|
||||||
|
- `apps/next/src/app/api/box/file/route.ts` — `GET` + `PUT`.
|
||||||
|
- `apps/next/src/app/api/box/terminal-token/route.ts` — `GET`.
|
||||||
|
|
||||||
|
**Interfaces (client contract):**
|
||||||
|
- `GET /api/box/status` → `{ status: BoxStatus }`.
|
||||||
|
- `POST /api/box/lifecycle` body `{ action:'start'|'stop'|'restart' }` → `{ status: BoxStatus }`.
|
||||||
|
- `GET /api/box/tree` → `{ tree: FileTreeNode }`.
|
||||||
|
- `GET /api/box/file?path=<rel>` → `{ path, content }`; `PUT /api/box/file` body `{ path, content }` → `{ success:true }`.
|
||||||
|
- `GET /api/box/terminal-token` → `{ url, expiresAt }` or `503 { error }` when unconfigured (mirror `agent-jobs/[jobId]/terminal-token/route.ts`).
|
||||||
|
- All return `401` when unauthenticated (via `resolveBoxUsername`/`withBox`).
|
||||||
|
|
||||||
|
**Implementation:** each handler uses `withBox`/`resolveBoxUsername`. Example (`tree`):
|
||||||
|
```ts
|
||||||
|
export const GET = async () => await withBox(async (username) => await proxyBox(username, 'tree', { method: 'GET' }));
|
||||||
|
```
|
||||||
|
`terminal-token` uses `resolveBoxUsername` then `mintBoxTerminalToken(username)` (503 if null). `file` forwards `path` and request body like `agent-jobs/[jobId]/file/route.ts:1-28`. `lifecycle` forwards the JSON body.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Create the five route files following the `agent-jobs/[jobId]/*` handlers as templates but with `withBox`/`resolveBoxUsername` instead of `withOwnedJob`.
|
||||||
|
- [ ] `cd apps/next && bun run typecheck` (or the repo's lint/build) → passes.
|
||||||
|
- [ ] **Manual verification (record in commit body):** signed in, hit `/api/box/status` in the browser/devtools → `200` with your box status; signed out → `401`. `PUT`+`GET /api/box/file` round-trips a file into your home.
|
||||||
|
- [ ] Commit: `feat(web): /api/box status, lifecycle, tree, file, terminal-token routes`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 8: Next — extract a reusable `XtermSession` component
|
||||||
|
|
||||||
|
Factor the xterm wiring out of `workspace-terminal.tsx` so `/machine` and the workspace both use one implementation (DRY the WS/fit/fonts/theme logic). This also sets up Task 10's persistence work.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/components/agent-workspace/xterm-session.tsx` — new shared component (move the themes + effect from `workspace-terminal.tsx:16-209`).
|
||||||
|
- `apps/next/src/components/agent-workspace/workspace-terminal.tsx` — becomes a thin wrapper over `XtermSession`.
|
||||||
|
- `apps/next/tests/component/xterm-session.test.tsx` — new (behavioral, jsdom-limited).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces:**
|
||||||
|
```ts
|
||||||
|
type XtermStatus = 'idle' | 'connecting' | 'connected' | 'closed' | 'error' | 'unconfigured';
|
||||||
|
export const XtermSession = (props: {
|
||||||
|
active: boolean; // mount/connect gate
|
||||||
|
tokenUrl: string; // e.g. `/api/box/terminal-token` or `/api/agent-jobs/${jobId}/terminal-token`
|
||||||
|
sessionKey: string; // identity for the connection effect deps (jobId or username)
|
||||||
|
waitingLabel?: string; // shown when active is false-but-expected (see Task 10)
|
||||||
|
copyOnSelect?: boolean; // Task 11
|
||||||
|
}) => JSX.Element;
|
||||||
|
```
|
||||||
|
Preserve current behavior: lazy-import `@xterm/xterm`, `@xterm/addon-fit`, `@xterm/addon-web-links`; fetch `tokenUrl` for `{ url }`; open WS; `fit()` after `term.open()`; refit on `document.fonts.ready` / Nerd-Font load (Phase-1 sizing fix — keep it); theme swap without teardown; reconnect button. Move the two `ITheme` objects into this file.
|
||||||
|
- **`WorkspaceTerminal`** keeps its `{ jobId, active }` signature and renders `<XtermSession active={active} tokenUrl={`/api/agent-jobs/${jobId}/terminal-token`} sessionKey={jobId} />`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write `apps/next/tests/component/xterm-session.test.tsx`. Since xterm can't render in jsdom, mock the three `@xterm/*` dynamic imports (`vi.mock('@xterm/xterm', ...)` returning a fake `Terminal` class with `open`/`loadAddon`/`onData`/`onResize`/`dispose`/`refresh` spies, etc.) and stub `global.WebSocket`. Assert: with `active:false` it renders the waiting label and does NOT construct a WebSocket; with `active:true` it fetches `tokenUrl` and opens a WebSocket to the returned `url`. (Mock `fetch` to return `{ url: 'ws://x' }`.) Keep assertions coarse — this is a smoke/behavior guard, not pixel-level.
|
||||||
|
- [ ] Run `cd apps/next && bun run test:component` → FAIL.
|
||||||
|
- [ ] Extract `XtermSession`, rewrite `WorkspaceTerminal` as the wrapper. Ensure the existing terminal tab still works (`agent-workspace-shell.tsx:664-667` passes `active`).
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] Commit: `refactor(web): extract reusable XtermSession from WorkspaceTerminal`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 9: Next — `/machine` page + nav entry ("My Machine")
|
||||||
|
|
||||||
|
The standalone dev-box surface: status card + lifecycle controls, home file browser (`FileTree` + `CodeEditor`), and a full-page `~`-rooted terminal.
|
||||||
|
|
||||||
|
**Files (new unless noted):**
|
||||||
|
- `apps/next/src/app/(app)/machine/page.tsx` — the page.
|
||||||
|
- `apps/next/src/components/machine/machine-shell.tsx` — client shell (status card, actions, split file-browser/editor + terminal). Mirrors the load/save/tree logic from `agent-workspace-shell.tsx` but against `/api/box/*` and with no job concepts.
|
||||||
|
- `apps/next/src/components/machine/box-status-card.tsx` — status + start/stop/restart buttons.
|
||||||
|
- `apps/next/src/components/layout/header/index.tsx` — add the `/machine` nav item (edit, not new).
|
||||||
|
- `apps/next/tests/component/machine-shell.test.tsx` — new component test.
|
||||||
|
|
||||||
|
**Interfaces / behavior:**
|
||||||
|
- **Nav:** add `{ href: '/machine', icon: <lucide icon, e.g. `Server` or `HardDrive`>, label: 'Machine' }` to the authenticated `navItems` array (`index.tsx:23-45`), placed after Threads.
|
||||||
|
- **Status card:** fetch `GET /api/box/status` on mount + after each lifecycle action; poll every ~10s. Show running/stopped badge, image, uptime (derive from `startedAt`), memory cap (format `memoryLimitBytes` → GiB, or "unlimited" when null). Buttons call `POST /api/box/lifecycle` with `start`/`stop`/`restart`; disable while a request is in flight; `toast.error` on failure and only `toast.success` after the awaited response resolves (per the repo's error-handling principle). Refresh status from the response.
|
||||||
|
- **File browser:** reuse `FileTree` (props exactly as `agent-workspace-shell.tsx:574-580`) fed by `GET /api/box/tree`; open files via `GET /api/box/file?path=`; edit in `CodeEditor`; save via `PUT /api/box/file`. Reuse the `OpenFileState`/`openFile`/`loadFile`/`writeFileContent` shapes from `agent-workspace-shell.tsx` (copy the minimal subset — no diff/UI-state persistence needed). `readOnly={false}`, `vimEnabled` local state.
|
||||||
|
- **Terminal:** `<XtermSession active tokenUrl="/api/box/terminal-token" sessionKey={username} waitingLabel="Starting your machine…" />`, laid out full-page (own tab or a resizable pane — a Radix `Tabs` with "Files" and "Terminal", or a two-pane layout). The page is `~`-rooted by construction (worker `bridgeBox` uses `containerHome`).
|
||||||
|
- **Username:** the page reads `api.userEnvironment.getMine` via `useQuery` for display (firstName/username), but all box access is server-derived — the client never sends the username.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write `apps/next/tests/component/machine-shell.test.tsx`: mock `convex/react` (`useQuery` → `{ username:'gib', firstName:'Gib', enabled:true }`), `next/navigation`, `sonner`, and `@/components/agent-workspace/xterm-session` (render `<div>terminal</div>`) and `@/components/agent-workspace/code-editor` (Monaco won't run in jsdom). Mock `global.fetch` so `/api/box/status` returns a stopped box and `/api/box/tree` returns a small tree. Assert: the status card renders "Stopped" and a "Start" button; clicking "Start" POSTs `/api/box/lifecycle` with `{action:'start'}` (assert on the `fetch` mock); the file tree renders a known file; the terminal placeholder renders.
|
||||||
|
- [ ] Run `cd apps/next && bun run test:component` → FAIL.
|
||||||
|
- [ ] Build `box-status-card.tsx`, `machine-shell.tsx`, `machine/page.tsx`, and add the nav item. The page is a server component that renders the client `MachineShell` (like other `(app)` pages); auth-gating follows the existing `(app)` layout — no special guard needed beyond what the layout provides.
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] **Manual verification (record in commit body):** sign in, open `/machine`. (1) Status card shows correct running/stopped + image + uptime + mem cap; Start/Stop/Restart each change the status within a few seconds. (2) Terminal drops you into `~` (`pwd` = `/home/<you>`), typing + resize work, output survives idle. (3) File browser lists your home; open+edit+save a file, confirm it persists (re-open shows the change; `cat` in the terminal confirms).
|
||||||
|
- [ ] Commit: `feat(web): /machine dev-box page with status, terminal, and home file browser`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 10: Next — persistent workspace terminal (survive tab switches)
|
||||||
|
|
||||||
|
Make the workspace Terminal tab keep its PTY/scrollback when switching Radix tabs, refit + resize-send when it becomes visible, and distinguish "waiting for workspace" from "connecting".
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/components/agent-workspace/agent-workspace-shell.tsx` — the Terminal `TabsContent` (currently `660-668`).
|
||||||
|
- `apps/next/src/components/agent-workspace/xterm-session.tsx` — add refit-on-visible + `waiting` handling.
|
||||||
|
- `apps/next/tests/component/xterm-session.test.tsx` — extend from Task 8.
|
||||||
|
|
||||||
|
**Behavior / interfaces:**
|
||||||
|
- **Persistence:** give the Terminal `TabsContent` `forceMount` and hide it with CSS when inactive instead of unmounting (Radix `TabsContent` unmounts by default — that destroys the PTY, audit item #15). Pattern: `<TabsContent value='terminal' forceMount className={cn('m-0 min-h-0 flex-1 overflow-hidden', activeWorkspaceTab !== 'terminal' && 'hidden')}>`. The other tabs stay as-is. Keep `<XtermSession active={workspaceReady} ... />` **mounted regardless of tab** so the WS/PTY persists; use a separate `visible` prop for fit behavior.
|
||||||
|
- **Refit on visible:** add a `visible: boolean` prop to `XtermSession`. When it transitions to `true`, call `fitAddon.fit()` inside `requestAnimationFrame` and send a resize frame (the Phase-1 pattern). Keep `active` as the connect gate (`workspaceReady`), `visible` as the CSS-visibility gate (`activeWorkspaceTab === 'terminal'`). In the shell, pass `active={workspaceReady}` and `visible={activeWorkspaceTab === 'terminal'}`.
|
||||||
|
- **Waiting vs connecting:** `XtermSession` status starts `'idle'`; when `active` is false but the workspace is expected (job pending), show `waitingLabel` ("Waiting for workspace…") — distinct from `'connecting'` (token fetched, WS opening). The shell already computes `workspacePending`/`workspaceReady` (`agent-workspace-shell.tsx:109-122`); pass an appropriate `waitingLabel`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Extend `xterm-session.test.tsx`: assert that toggling `visible` false→true triggers a `fit()` call on the mocked fit addon and sends a resize frame over the mocked WebSocket; assert that with `active:false` + a `waitingLabel`, the waiting label renders and no WebSocket is constructed; assert the component stays mounted (WS not closed) when `visible` goes true→false while `active` stays true.
|
||||||
|
- [ ] Run `cd apps/next && bun run test:component` → FAIL.
|
||||||
|
- [ ] Add the `visible` prop + refit effect to `XtermSession`; update `WorkspaceTerminal` to forward `visible`; change the Terminal `TabsContent` in `agent-workspace-shell.tsx` to `forceMount` + CSS-hide, and mount `<WorkspaceTerminal active={workspaceReady} visible={activeWorkspaceTab==='terminal'} .../>` (update `WorkspaceTerminal` props to accept `visible`).
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] **Manual verification (record in commit body):** open a running thread workspace, go to Terminal, run `top` (or type a long-running output), switch to Editor then back — the session and scrollback are intact (not a fresh shell), and the terminal is correctly sized (not quarter-size) on return. Before a worker claims the job, the Terminal tab shows "Waiting for workspace…", not "Connecting…".
|
||||||
|
- [ ] Commit: `feat(web): persistent workspace terminal across tab switches`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 11: Next — terminal QoL (copy-on-select; confirm web-links)
|
||||||
|
|
||||||
|
Web-links is already loaded (`workspace-terminal.tsx:144` / carried into `XtermSession`). Add a copy-on-select toggle available in both terminals.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/components/agent-workspace/xterm-session.tsx` — copy-on-select via `term.onSelectionChange`.
|
||||||
|
- `apps/next/tests/component/xterm-session.test.tsx` — extend.
|
||||||
|
|
||||||
|
**Interfaces / behavior:**
|
||||||
|
- Add a `copyOnSelect?: boolean` prop (already declared in Task 8) with a small toggle control in the terminal header (a button/checkbox). When enabled, on `term.onSelectionChange` copy `term.getSelection()` to the clipboard (`navigator.clipboard.writeText`, guarded — clipboard may be unavailable/denied; swallow errors). Persist the toggle in `localStorage` (`spoon.terminal.copyOnSelect`) so it sticks across sessions. Confirm `WebLinksAddon` is loaded in `XtermSession` (it is — keep it; it makes URLs clickable).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Extend `xterm-session.test.tsx`: mock `navigator.clipboard.writeText`; with `copyOnSelect` on, simulate the mocked terminal's selection-change callback with a non-empty selection and assert `writeText` was called with it; with it off, assert not called. Assert the header toggle renders and flipping it updates behavior.
|
||||||
|
- [ ] Run `cd apps/next && bun run test:component` → FAIL.
|
||||||
|
- [ ] Implement the toggle + `onSelectionChange` handler + `localStorage` persistence in `XtermSession`; ensure `WorkspaceTerminal` and the `/machine` terminal both expose it.
|
||||||
|
- [ ] Run tests → PASS.
|
||||||
|
- [ ] **Manual verification (record in commit body):** in `/machine` and a workspace terminal, enable copy-on-select, select text → it's on the clipboard; a printed URL is clickable (web-links) and opens in a new tab.
|
||||||
|
- [ ] Commit: `feat(web): terminal copy-on-select toggle and web-links polish`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Final verification (run before declaring the phase done)
|
||||||
|
|
||||||
|
- [ ] `cd apps/agent-worker && bun run test:unit` — all green (token, box-paths, box-files).
|
||||||
|
- [ ] `cd apps/next && bun run test:unit && bun run test:component` — all green.
|
||||||
|
- [ ] Typecheck/lint per repo scripts for both apps.
|
||||||
|
- [ ] End-to-end manual smoke on a docker-enabled deployment: `/machine` status+lifecycle+terminal+file-edit all work; workspace terminal persists across tab switches and is correctly sized; a user signed in as A cannot reach user B's box (all `/api/box/*` derive the username server-side — confirm by inspecting that no request carries a username).
|
||||||
|
|
||||||
|
## Self-review against the Phase 3 spec
|
||||||
|
|
||||||
|
- ✅ `/machine` page: status (running/stopped, image, uptime, mem cap), start/stop/restart, full-page `~`-rooted terminal, home file browser reusing FileTree/CodeEditor — Tasks 9 (+2,3,4 backing).
|
||||||
|
- ✅ Worker user-scoped HMAC tokens + `/box/tree|file|terminal` + status/lifecycle endpoints — Tasks 1–5.
|
||||||
|
- ✅ Same ownership-proxy pattern (Next mints after Convex auth, browser never sees worker secret) — Tasks 6–7; username derived server-side (a user can only reach their own box).
|
||||||
|
- ✅ Persistent workspace terminal (forceMount + CSS hide, refit-on-visible, waiting-vs-connecting) — Task 10.
|
||||||
|
- ✅ Terminal QoL (web-links, copy-on-select) — Task 11.
|
||||||
|
- ✅ Secret precedence + payload around username+expiry — Task 1/6.
|
||||||
@@ -0,0 +1,621 @@
|
|||||||
|
# Spoon Phase 4 — Upstream-Sync Correctness: Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Make Spoon's GitHub upstream-sync correct and event-driven: resolve head SHAs from the branch ref (never inferred from a truncated compare list), paginate compares where counts matter, give maintenance threads a stable dedup key (merge-base + head digest, never `Date.now()`), add a signature-verified GitHub webhook that does targeted refreshes on `push` and flips `gitConnections.status` on installation lifecycle events, add Octokit retry/throttling with a distinct "rate-limited" sync status, and fix unbounded `.collect()`s on hot paths. Surface `needs_reauth`/`revoked` in Settings → Integrations.
|
||||||
|
|
||||||
|
**Architecture:** All work is in `packages/backend/convex` (Convex functions, one `httpAction`, one internalAction, schema indexes) plus a small `apps/next` settings UI change. The webhook `httpAction` runs in Convex's default (non-node) runtime and verifies the HMAC with Web Crypto (`crypto.subtle`), then `ctx.scheduler.runAfter(0, …)` schedules the node-runtime refresh. All Octokit calls stay in the existing `'use node'` modules (`githubClient.ts`, `githubSync.ts`).
|
||||||
|
|
||||||
|
**Tech Stack:** Convex (functions, httpActions, crons), Octokit (`@octokit/rest` + `@octokit/plugin-retry` + `@octokit/plugin-throttling`), Next.js settings UI.
|
||||||
|
|
||||||
|
**Depends on:** Phase 1 (auto-sync gating on `autoSyncEnabled` and the `require*` flags lands there; this phase does NOT touch the auto-sync decision at `githubSync.ts:205`). This phase adds head-SHA correctness, dedup, webhooks, and rate-limit handling on top. It is otherwise independent of Phases 1–3.
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- **Backend tests:** live in `packages/backend/tests/unit/*.test.ts`, use `convex-test`. Run from `packages/backend`: `bun run test:unit` (this is `vitest run --project unit`). The test harness pattern is in `packages/backend/tests/unit/harness.test.ts` — copy its `convexTest(schema)`, `import.meta.glob('../../convex/**/*.*s')`, `createUser`, and `authed` helpers.
|
||||||
|
- **convex-test HTTP:** `const t = convexTest(schema, modules); const res = await t.fetch('/webhooks/github', { method: 'POST', headers, body })` returns a `Response`. Use this to test the webhook `httpAction`.
|
||||||
|
- **convex-test with node actions:** functions in `'use node'` files (`githubSync.ts`, `githubClient.ts`, `githubNode.ts`) cannot be unit-tested with convex-test directly (no node runtime in the test VM). For those, extract pure logic into a **non-`'use node'`** helper module and unit-test that; mock Octokit as a plain object literal shaped like the calls used (see Task 1/2 for the mock shape). Do NOT try to `t.action(...)` a node action in unit tests.
|
||||||
|
- Run `bun codegen:convex` from the repo root before `typecheck` whenever you add/rename a Convex function (regenerates `convex/_generated/api.d.ts`). Typecheck with `cd packages/backend && bun run typecheck`.
|
||||||
|
- Octokit plugin deps are NOT yet installed. Task 9 adds `@octokit/plugin-retry` and `@octokit/plugin-throttling` to `packages/backend/package.json` and runs `bun install` from the repo root.
|
||||||
|
- Conventional commits, one commit per task (e.g. `fix(sync): resolve head SHA from branch ref`).
|
||||||
|
- **Webhook signature:** HMAC-SHA256 over the **raw request body** (never a re-serialized JSON), header `X-Hub-Signature-256` formatted `sha256=<hex>`, compared with a constant-time comparison. Secret is `process.env.GITHUB_APP_WEBHOOK_SECRET` (already declared in `convex/globals.d.ts:14`).
|
||||||
|
- Ownership/security: the webhook is unauthenticated (GitHub calls it) — it MUST reject on bad/missing signature with HTTP 401 and never trust any user id from the payload; it only maps `installation.id` / repo full-name to existing owned records.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: `resolveBranchHead` — real head SHA from the branch ref
|
||||||
|
|
||||||
|
**Problem:** `compareAcrossForkNetwork` sets `headSha = commits[commits.length - 1]?.sha` (`githubClient.ts:132`). GitHub's compare endpoint truncates `commits` to the last page (max ~250 across pages, 100/page here with no pagination), so for a divergence >100 commits the "head" is a middle commit, not the branch tip. Fetch the tip from the branch ref instead.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/githubClient.ts` (add `resolveBranchHead` near `getRepository` ~line 82–89).
|
||||||
|
- `packages/backend/tests/unit/github-head.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Produces: `export const resolveBranchHead = async (octokit: Octokit, owner: string, repo: string, branch: string): Promise<{ sha: string }>`
|
||||||
|
- Implementation: `const result = await octokit.rest.repos.getBranch({ owner, repo, branch }); return { sha: result.data.commit.sha };`
|
||||||
|
- Consumes: an `Octokit` whose `rest.repos.getBranch` resolves `{ data: { commit: { sha } } }`.
|
||||||
|
|
||||||
|
**Note on testing node modules:** `githubClient.ts` is NOT a `'use node'` file (it has no `'use node'` pragma — verify: it starts with imports, no pragma). It can therefore be imported directly in a vitest unit test and called with a hand-rolled mock Octokit. The mock is a plain object: `{ rest: { repos: { getBranch: async () => ({ data: { commit: { sha: 'abc123' } } }) } } } as unknown as Octokit`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing test `tests/unit/github-head.test.ts`: import `resolveBranchHead` from `../../convex/githubClient`; call it with a mock Octokit whose `getBranch` returns `{ data: { commit: { sha: 'deadbeef' } } }` and asserts the returned `sha === 'deadbeef'` and that `getBranch` was called with `{ owner: 'o', repo: 'r', branch: 'main' }` (use a `vi.fn()`).
|
||||||
|
- [ ] Run `cd packages/backend && bun run test:unit` — confirm it FAILS (export does not exist).
|
||||||
|
- [ ] Add `resolveBranchHead` to `githubClient.ts` exactly as in Interfaces.
|
||||||
|
- [ ] Run `bun run test:unit` — confirm PASS.
|
||||||
|
- [ ] `bun codegen:convex` (repo root) + `cd packages/backend && bun run typecheck` — confirm clean.
|
||||||
|
- [ ] Commit: `feat(sync): add resolveBranchHead to fetch branch tip SHA`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: Paginate compare + use resolved head; fix `getLastCommitAt`
|
||||||
|
|
||||||
|
**Problem:** `compareAcrossForkNetwork` (`githubClient.ts:110-137`) requests `per_page: 100` with no pagination and derives `headSha` from the truncated list; `getLastCommitAt` (`githubSync.ts:34-35`) has the same last-element bug. Paginate the commit list (bounded), take `ahead_by`/`merge_base_commit`/`base_commit` from the first page, and set `headSha` from `resolveBranchHead`.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/githubClient.ts` (rewrite `compareAcrossForkNetwork` ~110-137; add `headRepo` to its args).
|
||||||
|
- `packages/backend/convex/githubSync.ts` (fix `getLastCommitAt` ~34-35; pass `headRepo` at both `compareAcrossForkNetwork` call sites ~107-122).
|
||||||
|
- `packages/backend/tests/unit/github-head.test.ts` (extend).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Changed: `compareAcrossForkNetwork(octokit, args: { owner; repo; baseOwner; baseBranch; headOwner; headRepo; headBranch })` — **new required `headRepo: string`** (the repo the head branch lives in; upstream repo for the upstream compare, fork repo for the fork compare).
|
||||||
|
- The function now internally calls `resolveBranchHead(octokit, args.headOwner, args.headRepo, args.headBranch)` and sets `headSha` from it.
|
||||||
|
- `GitHubCompareSummary` shape (`githubClient.ts:37-44`) is unchanged.
|
||||||
|
|
||||||
|
**Implementation for `compareAcrossForkNetwork`:**
|
||||||
|
```ts
|
||||||
|
const COMPARE_PER_PAGE = 100;
|
||||||
|
const COMPARE_MAX_PAGES = 30; // bound: up to 3000 commits collected
|
||||||
|
|
||||||
|
export const compareAcrossForkNetwork = async (
|
||||||
|
octokit: Octokit,
|
||||||
|
args: {
|
||||||
|
owner: string;
|
||||||
|
repo: string;
|
||||||
|
baseOwner: string;
|
||||||
|
baseBranch: string;
|
||||||
|
headOwner: string;
|
||||||
|
headRepo: string;
|
||||||
|
headBranch: string;
|
||||||
|
},
|
||||||
|
): Promise<GitHubCompareSummary> => {
|
||||||
|
const basehead = `${args.baseOwner}:${args.baseBranch}...${args.headOwner}:${args.headBranch}`;
|
||||||
|
const first = await octokit.rest.repos.compareCommitsWithBasehead({
|
||||||
|
owner: args.owner,
|
||||||
|
repo: args.repo,
|
||||||
|
basehead,
|
||||||
|
per_page: COMPARE_PER_PAGE,
|
||||||
|
page: 1,
|
||||||
|
});
|
||||||
|
const rawCommits = [...first.data.commits];
|
||||||
|
const totalCommits = first.data.total_commits;
|
||||||
|
let page = 2;
|
||||||
|
while (rawCommits.length < totalCommits && page <= COMPARE_MAX_PAGES) {
|
||||||
|
const next = await octokit.rest.repos.compareCommitsWithBasehead({
|
||||||
|
owner: args.owner,
|
||||||
|
repo: args.repo,
|
||||||
|
basehead,
|
||||||
|
per_page: COMPARE_PER_PAGE,
|
||||||
|
page,
|
||||||
|
});
|
||||||
|
if (next.data.commits.length === 0) break;
|
||||||
|
rawCommits.push(...next.data.commits);
|
||||||
|
page += 1;
|
||||||
|
}
|
||||||
|
const head = await resolveBranchHead(
|
||||||
|
octokit,
|
||||||
|
args.headOwner,
|
||||||
|
args.headRepo,
|
||||||
|
args.headBranch,
|
||||||
|
);
|
||||||
|
return {
|
||||||
|
aheadBy: first.data.ahead_by,
|
||||||
|
mergeBaseSha: first.data.merge_base_commit.sha,
|
||||||
|
headSha: head.sha,
|
||||||
|
baseSha: first.data.base_commit.sha,
|
||||||
|
htmlUrl: first.data.html_url,
|
||||||
|
commits: rawCommits.map(normalizeCompareCommit),
|
||||||
|
};
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
**Implementation for `getLastCommitAt` (`githubSync.ts`):**
|
||||||
|
```ts
|
||||||
|
const getLastCommitAt = (compare: GitHubCompareSummary) => {
|
||||||
|
const head = compare.commits.find((c) => c.sha === compare.headSha);
|
||||||
|
return (
|
||||||
|
head?.committedAt ??
|
||||||
|
compare.commits[compare.commits.length - 1]?.committedAt
|
||||||
|
);
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
**Call-site edits in `githubSync.ts` (~107-122):** add `headRepo: spoon.upstreamRepo` to the upstream compare call and `headRepo: forkRepo` to the fork compare call.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Extend `tests/unit/github-head.test.ts` with a test for `compareAcrossForkNetwork`: mock Octokit with `compareCommitsWithBasehead` returning page 1 of 100 commits with `total_commits: 150`, page 2 of 50 commits, and `getBranch` returning `{ data: { commit: { sha: 'TIP' } } }`. Assert `result.headSha === 'TIP'` (NOT the last commit in the list), `result.commits.length === 150`, `result.aheadBy` equals the mocked `ahead_by`. Use `merge_base_commit`/`base_commit` objects with `.sha` in the mock so mapping doesn't throw.
|
||||||
|
- [ ] Run `bun run test:unit` — confirm FAIL.
|
||||||
|
- [ ] Rewrite `compareAcrossForkNetwork` and `getLastCommitAt`; add `headRepo` to both call sites.
|
||||||
|
- [ ] Run `bun run test:unit` — confirm PASS.
|
||||||
|
- [ ] `bun codegen:convex` + `bun run typecheck` — confirm clean (the new required `headRepo` arg must be satisfied at both call sites).
|
||||||
|
- [ ] Commit: `fix(sync): paginate compare and resolve head SHA from branch ref`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: Stable maintenance-thread dedup key (schema + pure helper)
|
||||||
|
|
||||||
|
**Problem:** Threads are deduped by `upstreamTo` string with a `Date.now()` fallback (`githubSync.ts:232,271`, `threads.ts:426-440`). When head is unresolved the fallback makes a unique key every run → a brand-new maintenance thread + agent job every scheduled check. Replace with a deterministic key of `mergeBase:head`, and treat "no head" as "cannot dedup → skip".
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/maintenanceDedup.ts` (new, NOT `'use node'` — pure, unit-testable).
|
||||||
|
- `packages/backend/convex/schema.ts` (add `dedupKey` field + index to `threads` table).
|
||||||
|
- `packages/backend/tests/unit/maintenance-dedup.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Produces: `export const maintenanceDedupKey = (input: { mergeBaseSha?: string; headSha?: string }): string | null` — returns `null` when `headSha` is missing/empty; otherwise `` `${input.mergeBaseSha ?? 'nobase'}:${input.headSha}` ``.
|
||||||
|
|
||||||
|
**Schema change (`threads` table):** add `dedupKey: v.optional(v.string())` to the field list and a new index at the bottom of the table def: `.index('by_spoon_dedup', ['spoonId', 'dedupKey'])`. (Optional so existing rows validate; new maintenance threads always set it.)
|
||||||
|
|
||||||
|
**Implementation (`maintenanceDedup.ts`):**
|
||||||
|
```ts
|
||||||
|
export const maintenanceDedupKey = (input: {
|
||||||
|
mergeBaseSha?: string;
|
||||||
|
headSha?: string;
|
||||||
|
}): string | null => {
|
||||||
|
const head = input.headSha?.trim();
|
||||||
|
if (!head) return null;
|
||||||
|
const base = input.mergeBaseSha?.trim() || 'nobase';
|
||||||
|
return `${base}:${head}`;
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `tests/unit/maintenance-dedup.test.ts`: `maintenanceDedupKey({ mergeBaseSha: 'a', headSha: 'b' }) === 'a:b'`; `maintenanceDedupKey({ headSha: 'b' }) === 'nobase:b'`; `maintenanceDedupKey({ mergeBaseSha: 'a' }) === null`; `maintenanceDedupKey({ mergeBaseSha: 'a', headSha: '' }) === null`.
|
||||||
|
- [ ] Run `bun run test:unit` — confirm FAIL (module missing).
|
||||||
|
- [ ] Create `maintenanceDedup.ts`; add `dedupKey` field + `by_spoon_dedup` index to `threads` in `schema.ts`.
|
||||||
|
- [ ] Run `bun run test:unit` — PASS. `bun codegen:convex` + `bun run typecheck` — clean.
|
||||||
|
- [ ] Commit: `feat(sync): add deterministic maintenance-thread dedup key`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: `createMaintenanceThread` / `findOpenMaintenanceThread` dedup by key (indexed)
|
||||||
|
|
||||||
|
**Problem:** Both dedup helpers `.collect()` every thread for the spoon then `.find(...)` in JS (`threads.ts:389-393` and `426-440`) — unbounded, and keyed on `upstreamTo`. Switch to `dedupKey` via the `by_spoon_dedup` index and stop collecting.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/threads.ts` (`findOpenMaintenanceThread` ~382-405; `createMaintenanceThread` ~407-497).
|
||||||
|
- `packages/backend/tests/unit/maintenance-thread.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Changed `createMaintenanceThread` args: **replace `upstreamTo: v.string()` with `dedupKey: v.string()`** (required; caller guarantees non-null — see Task 5). Keep `upstreamTo` but make it `v.optional(v.string())` (still stored for display). Persist `dedupKey` on the inserted thread row.
|
||||||
|
- Changed `findOpenMaintenanceThread` args: `{ spoonId, ownerId, dedupKey: v.string() }`.
|
||||||
|
- Both now query `.withIndex('by_spoon_dedup', (q) => q.eq('spoonId', args.spoonId).eq('dedupKey', args.dedupKey))`, then filter by `ownerId` and non-terminal status in JS over the (now tiny) result set.
|
||||||
|
|
||||||
|
**Non-terminal status set (unchanged):** exclude `['resolved', 'ignored', 'failed', 'cancelled']`.
|
||||||
|
|
||||||
|
**Existing-thread lookup in `createMaintenanceThread`:**
|
||||||
|
```ts
|
||||||
|
const existing = await ctx.db
|
||||||
|
.query('threads')
|
||||||
|
.withIndex('by_spoon_dedup', (q) =>
|
||||||
|
q.eq('spoonId', args.spoonId).eq('dedupKey', args.dedupKey),
|
||||||
|
)
|
||||||
|
.order('desc')
|
||||||
|
.collect()
|
||||||
|
.then((threads) =>
|
||||||
|
threads.find(
|
||||||
|
(thread) =>
|
||||||
|
thread.ownerId === args.ownerId &&
|
||||||
|
!['resolved', 'ignored', 'failed', 'cancelled'].includes(
|
||||||
|
thread.status,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
```
|
||||||
|
Insert path sets `dedupKey: args.dedupKey` and `upstreamTo: args.upstreamTo` on the new `threads` row.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `tests/unit/maintenance-thread.test.ts` using convex-test: create a user + github spoon (copy `githubSpoonInput` + insert pattern from `harness.test.ts`; insert the spoon row directly via `t.mutation(async ctx => ctx.db.insert('spoons', {...}))`). Call `internal.threads.createMaintenanceThread` twice with the SAME `dedupKey` → assert the second returns the same `threadId` and that only one `threads` row exists for that spoon; then call with a DIFFERENT `dedupKey` → asserts a new thread id. (Invoke internal fns via `t.mutation(internal.threads.createMaintenanceThread, args)`.)
|
||||||
|
- [ ] Run `bun run test:unit` — FAIL (arg `dedupKey` not accepted yet).
|
||||||
|
- [ ] Update `createMaintenanceThread` + `findOpenMaintenanceThread` signatures and bodies; ensure the `threads.insert` writes `dedupKey`.
|
||||||
|
- [ ] Run `bun run test:unit` — PASS. `bun codegen:convex` + `bun run typecheck` — clean (call sites in `githubSync.ts` will still be broken until Task 5 — that's expected; if typecheck must be green per-commit, do Task 5 in the same commit; otherwise note it).
|
||||||
|
- [ ] Commit: `refactor(threads): dedup maintenance threads by stable key via index`.
|
||||||
|
|
||||||
|
> **Sequencing note:** Tasks 4 and 5 both touch the `createMaintenanceThread` contract. If your workflow requires each commit to typecheck green, combine Tasks 4 and 5 into one commit. Otherwise keep them separate and run typecheck at the end of Task 5.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 5: `githubSync` call sites — pass `dedupKey`, never `Date.now()`, skip when unresolvable
|
||||||
|
|
||||||
|
**Problem:** `githubSync.ts` builds `upstreamTo: upstreamCompare.headSha ?? \`${Date.now()}\`` at lines 232 and 271 (and a similar `${Date.now()}` fallback at 398 in `syncForkWithUpstream`). Replace with the deterministic key; when `maintenanceDedupKey(...)` is `null` (no head resolvable), **skip thread creation entirely** and record the skip on the sync run instead.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/githubSync.ts` (merge-conflict branch ~223-258; diverged branch ~261-284; `syncForkWithUpstream` conflict branch ~387-411).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Consumes: `maintenanceDedupKey` from `./maintenanceDedup`, `internal.threads.createMaintenanceThread` (now takes `dedupKey`).
|
||||||
|
- Import at top of `githubSync.ts`: `import { maintenanceDedupKey } from './maintenanceDedup';`
|
||||||
|
|
||||||
|
**Pattern (apply at each of the 3 thread-creation sites):**
|
||||||
|
```ts
|
||||||
|
const dedupKey = maintenanceDedupKey({
|
||||||
|
mergeBaseSha: upstreamCompare.mergeBaseSha ?? forkCompare.mergeBaseSha,
|
||||||
|
headSha: upstreamCompare.headSha,
|
||||||
|
});
|
||||||
|
if (!dedupKey) {
|
||||||
|
await ctx.runMutation(internal.syncRuns.patchInternal, {
|
||||||
|
syncRunId,
|
||||||
|
status: 'failed',
|
||||||
|
error: 'Could not resolve upstream head SHA; skipping maintenance thread.',
|
||||||
|
});
|
||||||
|
// return the appropriate summary object for this branch (do NOT create a thread)
|
||||||
|
} else {
|
||||||
|
const threadId = await ctx.runMutation(
|
||||||
|
internal.threads.createMaintenanceThread,
|
||||||
|
{
|
||||||
|
/* existing args, but: */
|
||||||
|
dedupKey,
|
||||||
|
upstreamTo: upstreamCompare.headSha, // now optional, display only
|
||||||
|
upstreamFrom: upstreamCompare.mergeBaseSha,
|
||||||
|
// ...title, summary, forkHeadAtCreation, mergeBaseAtCreation, relatedSyncRunId, jobType
|
||||||
|
},
|
||||||
|
);
|
||||||
|
// existing syncRuns/spoons patches
|
||||||
|
}
|
||||||
|
```
|
||||||
|
For the `syncForkWithUpstream` conflict branch (~387), build the key from `state.mergeBaseSha ?? spoon.lastMergeBaseCommit` and `state.upstreamHeadSha ?? spoon.lastUpstreamCommit`; if `null`, skip thread creation and just patch the sync run/ spoon error.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Grep-confirm there are no remaining `Date.now()}` fallbacks feeding `upstreamTo` in `githubSync.ts` after edits: `grep -n "Date.now()}" packages/backend/convex/githubSync.ts` returns nothing.
|
||||||
|
- [ ] Apply the pattern to all 3 sites; remove the old `upstreamTo: … ?? \`${Date.now()}\`` fallbacks.
|
||||||
|
- [ ] `bun codegen:convex` + `bun run typecheck` — clean. Re-run `bun run test:unit` (Task 4 test still passes).
|
||||||
|
- [ ] Commit: `fix(sync): use stable dedup key and skip threads when head unresolved`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 6: Webhook signature verifier (pure, Web Crypto) + known-vector unit test
|
||||||
|
|
||||||
|
**Problem:** No webhook verification exists. Implement HMAC-SHA256 over the raw body, constant-time compared against `X-Hub-Signature-256`. Must run in Convex's default (non-node) httpAction runtime, so use Web Crypto (`crypto.subtle`), not `node:crypto`.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/githubWebhookVerify.ts` (new, NOT `'use node'`).
|
||||||
|
- `packages/backend/tests/unit/github-webhook-verify.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Produces: `export const verifyGithubSignature = async (secret: string, rawBody: string, signatureHeader: string | null): Promise<boolean>`
|
||||||
|
- Produces (helper, exported for reuse/testing): `export const timingSafeEqualHex = (a: string, b: string): boolean` — length-checked, constant-time char compare.
|
||||||
|
|
||||||
|
**Implementation:**
|
||||||
|
```ts
|
||||||
|
const toHex = (buffer: ArrayBuffer): string =>
|
||||||
|
Array.from(new Uint8Array(buffer))
|
||||||
|
.map((b) => b.toString(16).padStart(2, '0'))
|
||||||
|
.join('');
|
||||||
|
|
||||||
|
export const timingSafeEqualHex = (a: string, b: string): boolean => {
|
||||||
|
if (a.length !== b.length) return false;
|
||||||
|
let mismatch = 0;
|
||||||
|
for (let i = 0; i < a.length; i += 1) {
|
||||||
|
mismatch |= a.charCodeAt(i) ^ b.charCodeAt(i);
|
||||||
|
}
|
||||||
|
return mismatch === 0;
|
||||||
|
};
|
||||||
|
|
||||||
|
export const verifyGithubSignature = async (
|
||||||
|
secret: string,
|
||||||
|
rawBody: string,
|
||||||
|
signatureHeader: string | null,
|
||||||
|
): Promise<boolean> => {
|
||||||
|
if (!signatureHeader || !signatureHeader.startsWith('sha256=')) return false;
|
||||||
|
const provided = signatureHeader.slice('sha256='.length);
|
||||||
|
const key = await crypto.subtle.importKey(
|
||||||
|
'raw',
|
||||||
|
new TextEncoder().encode(secret),
|
||||||
|
{ name: 'HMAC', hash: 'SHA-256' },
|
||||||
|
false,
|
||||||
|
['sign'],
|
||||||
|
);
|
||||||
|
const mac = await crypto.subtle.sign(
|
||||||
|
'HMAC',
|
||||||
|
key,
|
||||||
|
new TextEncoder().encode(rawBody),
|
||||||
|
);
|
||||||
|
return timingSafeEqualHex(toHex(mac), provided);
|
||||||
|
};
|
||||||
|
```
|
||||||
|
|
||||||
|
**Known test vector (from GitHub's webhook docs):** secret `It's a Secret to Everybody`, body `Hello, World!` → `sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `tests/unit/github-webhook-verify.test.ts`:
|
||||||
|
- `await verifyGithubSignature("It's a Secret to Everybody", 'Hello, World!', 'sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17')` → `true`.
|
||||||
|
- Wrong signature → `false`; missing `sha256=` prefix → `false`; `null` header → `false`; correct hex but wrong secret → `false`.
|
||||||
|
- `timingSafeEqualHex('abcd','abcd') === true`, `timingSafeEqualHex('abcd','abce') === false`, different lengths → `false`.
|
||||||
|
- [ ] Run `bun run test:unit` — FAIL (module missing).
|
||||||
|
- [ ] Create `githubWebhookVerify.ts`.
|
||||||
|
- [ ] Run `bun run test:unit` — PASS (the known vector proves the HMAC is correct). `bun run typecheck` — clean.
|
||||||
|
- [ ] Commit: `feat(webhooks): add GitHub webhook signature verifier`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 7: `gitConnections` status-by-installation index + mutation
|
||||||
|
|
||||||
|
**Problem:** `gitConnections.status` is only ever written `'active'` (`github.ts:61,123`); nothing sets `needs_reauth`/`revoked`. The webhook needs to flip status by GitHub `installation.id`, but there is no index on `installationId`. Add one and an internal mutation.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/schema.ts` (`gitConnections` table ~32-56: add `.index('by_installation', ['installationId'])`).
|
||||||
|
- `packages/backend/convex/github.ts` (add internal mutation).
|
||||||
|
- `packages/backend/tests/unit/git-connection-status.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Produces: `export const setConnectionStatusByInstallation = internalMutation({ args: { installationId: v.string(), status: v.union(v.literal('active'), v.literal('needs_reauth'), v.literal('revoked')) }, handler })`
|
||||||
|
- Body: query `by_installation` for the given `installationId`, patch every matching row with `{ status, updatedAt: Date.now() }`. Return `{ updated: <count> }`. (Use `.collect()` here — installations map to at most a handful of connection rows; this is not a hot path.)
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `tests/unit/git-connection-status.test.ts` (convex-test): create a user, insert a `gitConnections` row with `installationId: '42'`, `status: 'active'`; call `internal.github.setConnectionStatusByInstallation` with `{ installationId: '42', status: 'needs_reauth' }`; re-read the row and assert `status === 'needs_reauth'`. Add a case where `installationId` matches nothing → `{ updated: 0 }`.
|
||||||
|
- [ ] Run `bun run test:unit` — FAIL.
|
||||||
|
- [ ] Add `by_installation` index + `setConnectionStatusByInstallation` mutation.
|
||||||
|
- [ ] `bun codegen:convex` + `bun run test:unit` PASS + `bun run typecheck` clean.
|
||||||
|
- [ ] Commit: `feat(github): index connections by installation + status mutation`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 8: Find owned spoons for a pushed repo (index + internal query)
|
||||||
|
|
||||||
|
**Problem:** A `push` webhook carries a repo full-name (`owner/repo`) and `installation.id`. To do a targeted refresh we must map that to owned `spoons`. Pushes to the **fork** change `forkHeadSha`; pushes to the **upstream** (if the app is installed there) change `upstreamHeadSha`. The `spoons` table already has `by_upstream` (`['provider','upstreamOwner','upstreamRepo']`) but no fork index.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/schema.ts` (`spoons` table: add `.index('by_fork', ['provider', 'forkOwner', 'forkRepo'])`).
|
||||||
|
- `packages/backend/convex/spoons.ts` (add internal query).
|
||||||
|
- `packages/backend/tests/unit/spoons-for-repo.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Produces: `export const findForRepo = internalQuery({ args: { owner: v.string(), repo: v.string() }, handler }): Promise<{ spoonId: Id<'spoons'>; ownerId: Id<'users'> }[]>`
|
||||||
|
- Query `by_upstream` with `provider:'github', upstreamOwner: owner, upstreamRepo: repo` → collect. Query `by_fork` with `provider:'github', forkOwner: owner, forkRepo: repo` → collect. Union, dedup by `spoonId`, exclude `status === 'archived'`, return `{ spoonId, ownerId }[]`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `tests/unit/spoons-for-repo.test.ts` (convex-test): insert two github spoons — one where `upstreamOwner/upstreamRepo = 'up/x'`, another where `forkOwner/forkRepo = 'me/x-fork'`. Call `internal.spoons.findForRepo` with `{ owner: 'up', repo: 'x' }` → returns the first spoon. Call with `{ owner: 'me', repo: 'x-fork' }` → returns the second. Call with an unknown repo → `[]`. Add a case where the same spoon matches both (upstream == fork owner/repo edge) → returned once.
|
||||||
|
- [ ] Run `bun run test:unit` — FAIL.
|
||||||
|
- [ ] Add `by_fork` index + `findForRepo` internal query.
|
||||||
|
- [ ] `bun codegen:convex` + `bun run test:unit` PASS + `bun run typecheck` clean.
|
||||||
|
- [ ] Commit: `feat(spoons): find owned spoons by pushed repo (indexed)`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 9: Targeted-refresh internalAction + webhook httpAction route
|
||||||
|
|
||||||
|
**Problem:** No webhook endpoint. Add a Convex `httpAction` at `/webhooks/github` that verifies the signature, then: on `push` → schedule a targeted refresh for each matching spoon; on `installation`/`installation_repositories` → flip `gitConnections.status`. The hourly cron (`crons.ts`) stays as fallback (unchanged).
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/githubSync.ts` (add `refreshSpoonById` internalAction wrapping the existing module-local `refreshOwnedSpoon`).
|
||||||
|
- `packages/backend/convex/githubWebhooks.ts` (new — the `httpAction` handler; NOT `'use node'`, uses Web Crypto + scheduler + runQuery/runMutation only).
|
||||||
|
- `packages/backend/convex/http.ts` (register the route).
|
||||||
|
- `packages/backend/tests/unit/github-webhook-route.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Produces: `export const refreshSpoonById = internalAction({ args: { spoonId: v.id('spoons'), ownerId: v.id('users') }, handler })` — calls `await refreshOwnedSpoon(ctx, ownerId, spoonId, 'scheduled_check')`; catches and swallows errors into a returned `{ success: boolean; error?: string }` so webhook fan-out never throws unhandled.
|
||||||
|
- Produces: `export const handleGithubWebhook = httpAction(async (ctx, request) => Response)` in `githubWebhooks.ts`.
|
||||||
|
- Route (in `http.ts`): `http.route({ path: '/webhooks/github', method: 'POST', handler: handleGithubWebhook });`
|
||||||
|
- **Webhook route path:** `POST /webhooks/github` (full URL is `<CONVEX_SITE_URL>/webhooks/github` — this is the URL to configure in the GitHub App settings; note it in the final report for deploy docs).
|
||||||
|
|
||||||
|
**`handleGithubWebhook` behavior:**
|
||||||
|
```ts
|
||||||
|
export const handleGithubWebhook = httpAction(async (ctx, request) => {
|
||||||
|
const secret = process.env.GITHUB_APP_WEBHOOK_SECRET;
|
||||||
|
if (!secret) return new Response('Webhook not configured', { status: 503 });
|
||||||
|
const rawBody = await request.text();
|
||||||
|
const signature = request.headers.get('x-hub-signature-256');
|
||||||
|
const ok = await verifyGithubSignature(secret, rawBody, signature);
|
||||||
|
if (!ok) return new Response('Invalid signature', { status: 401 });
|
||||||
|
|
||||||
|
const event = request.headers.get('x-github-event');
|
||||||
|
const payload = JSON.parse(rawBody) as GithubWebhookPayload; // narrow type below
|
||||||
|
|
||||||
|
if (event === 'push') {
|
||||||
|
const fullName: string | undefined = payload.repository?.full_name;
|
||||||
|
if (fullName) {
|
||||||
|
const [owner, repo] = fullName.split('/');
|
||||||
|
const targets = await ctx.runQuery(internal.spoons.findForRepo, {
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
});
|
||||||
|
for (const t of targets) {
|
||||||
|
await ctx.scheduler.runAfter(0, internal.githubSync.refreshSpoonById, {
|
||||||
|
spoonId: t.spoonId,
|
||||||
|
ownerId: t.ownerId,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else if (event === 'installation' || event === 'installation_repositories') {
|
||||||
|
const installationId = String(payload.installation?.id ?? '');
|
||||||
|
const action = payload.action; // 'deleted' | 'suspend' | 'unsuspend' | 'created' | ...
|
||||||
|
if (installationId) {
|
||||||
|
const status =
|
||||||
|
action === 'deleted' || action === 'suspend'
|
||||||
|
? ('revoked' as const)
|
||||||
|
: action === 'unsuspend' || action === 'created'
|
||||||
|
? ('active' as const)
|
||||||
|
: ('needs_reauth' as const); // 'new_permissions_accepted', 'removed', etc.
|
||||||
|
await ctx.runMutation(
|
||||||
|
internal.github.setConnectionStatusByInstallation,
|
||||||
|
{ installationId, status },
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Always 200 for verified events we don't act on, so GitHub doesn't retry.
|
||||||
|
return new Response('ok', { status: 200 });
|
||||||
|
});
|
||||||
|
```
|
||||||
|
Define a minimal payload type (`repository?: { full_name?: string }; installation?: { id?: number }; action?: string`) rather than importing Octokit webhook types.
|
||||||
|
|
||||||
|
**Runtime note:** `httpAction` cannot call node actions synchronously, but `ctx.scheduler.runAfter(0, internal.githubSync.refreshSpoonById, …)` schedules the node internalAction fine. `refreshSpoonById` lives in the `'use node'` `githubSync.ts` — that is allowed as a scheduled target.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Add `refreshSpoonById` internalAction to `githubSync.ts` (wrap `refreshOwnedSpoon` in try/catch, return `{ success, error? }`).
|
||||||
|
- [ ] Create `githubWebhooks.ts` with `handleGithubWebhook`; register the route in `http.ts` (keep `auth.addHttpRoutes(http)`).
|
||||||
|
- [ ] Write `tests/unit/github-webhook-route.test.ts` (convex-test `t.fetch`):
|
||||||
|
- Set the secret for the test: `t.mutation` can't set env; instead the verifier reads `process.env.GITHUB_APP_WEBHOOK_SECRET`. In the test, set `process.env.GITHUB_APP_WEBHOOK_SECRET = 'testsecret'` in a `beforeAll` (vitest allows mutating `process.env`), and compute the expected signature with the same Web Crypto HMAC (import `verifyGithubSignature`'s sibling or recompute via `node:crypto` `createHmac('sha256', secret).update(body).digest('hex')` in the test file — node crypto is available in the vitest process even though the function under test uses Web Crypto).
|
||||||
|
- **Bad signature:** `t.fetch('/webhooks/github', { method:'POST', headers:{'x-github-event':'push','x-hub-signature-256':'sha256=deadbeef'}, body })` → assert `res.status === 401`.
|
||||||
|
- **installation deleted:** seed a `gitConnections` row (installationId `'99'`, status `active`); POST an `installation` event `{ action:'deleted', installation:{ id:99 } }` with a VALID signature → assert `res.status === 200` and the connection row is now `revoked`.
|
||||||
|
- **push:** seed a github spoon whose fork is `me/x-fork`; POST a `push` with `{ repository:{ full_name:'me/x-fork' } }` and valid signature → assert `res.status === 200`. (You cannot assert the node action ran under convex-test; assert the 200 + that no throw occurred. Optionally assert `findForRepo` is exercised by checking a scheduled function was enqueued via `await t.finishInProgressScheduledFunctions()` doesn't throw — but since `refreshSpoonById` is a node action it will no-op/throw in the test VM; prefer NOT to finish scheduled functions here, just assert the 200.)
|
||||||
|
- [ ] Run `bun run test:unit` — iterate to PASS.
|
||||||
|
- [ ] `bun codegen:convex` + `bun run typecheck` — clean.
|
||||||
|
- [ ] Commit: `feat(webhooks): add signature-verified GitHub webhook route`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 10: Octokit retry + throttling plugins; distinguish rate-limited from hard error
|
||||||
|
|
||||||
|
**Problem:** `getInstallationOctokit` (`githubClient.ts:54-68`) uses a bare `Octokit` with no retry/backoff. On rate limits, refreshes hard-fail and the spoon shows `syncStatus: 'error'` with no distinction. Add the retry + throttling plugins and a distinct `rate_limited` status.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/package.json` (add deps).
|
||||||
|
- `packages/backend/convex/githubClient.ts` (compose plugins; add `isRateLimitError` helper).
|
||||||
|
- `packages/backend/convex/schema.ts` (`spoons.syncStatus` union: add `v.literal('rate_limited')`).
|
||||||
|
- `packages/backend/convex/githubSync.ts` (catch block ~291-308: branch on `isRateLimitError`).
|
||||||
|
- `packages/backend/tests/unit/rate-limit.test.ts` (new).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Deps: `"@octokit/plugin-retry": "^7.1.0"`, `"@octokit/plugin-throttling": "^9.3.0"` (match the `@octokit/rest@^22` core version line; verify with `bun install` that peer ranges resolve — bump if bun errors).
|
||||||
|
- `getInstallationOctokit` composition:
|
||||||
|
```ts
|
||||||
|
import { retry } from '@octokit/plugin-retry';
|
||||||
|
import { throttling } from '@octokit/plugin-throttling';
|
||||||
|
const SpoonOctokit = Octokit.plugin(retry, throttling);
|
||||||
|
export const getInstallationOctokit = (installationId: string) =>
|
||||||
|
new SpoonOctokit({
|
||||||
|
authStrategy: createAppAuth,
|
||||||
|
auth: { appId: getEnv('GITHUB_APP_ID'), privateKey: normalizePrivateKey(getEnv('GITHUB_APP_PRIVATE_KEY')), installationId },
|
||||||
|
userAgent: 'Spoon',
|
||||||
|
request: { headers: { 'X-GitHub-Api-Version': '2022-11-28' } },
|
||||||
|
throttle: {
|
||||||
|
onRateLimit: (retryAfter, options, _octokit, retryCount) => retryCount < 2,
|
||||||
|
onSecondaryRateLimit: (retryAfter, options, _octokit, retryCount) => retryCount < 2,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
```
|
||||||
|
- Produces (pure, exported): `export const isRateLimitError = (error: unknown): boolean` — returns true when `error` looks like an Octokit `RequestError` with `status === 403 || status === 429` and a rate-limit signal (message includes `rate limit`, or `response.headers['x-ratelimit-remaining'] === '0'`, or `x-ratelimit-reset` present). Guard with `typeof error === 'object' && error !== null`.
|
||||||
|
|
||||||
|
**Catch-block change in `refreshOwnedSpoon` (`githubSync.ts:291`):**
|
||||||
|
```ts
|
||||||
|
} catch (error) {
|
||||||
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
|
const rateLimited = isRateLimitError(error);
|
||||||
|
await Promise.all([
|
||||||
|
ctx.runMutation(internal.spoons.patchSyncFields, {
|
||||||
|
spoonId,
|
||||||
|
syncStatus: rateLimited ? 'rate_limited' : 'error',
|
||||||
|
lastGithubRefreshAt: Date.now(),
|
||||||
|
lastCheckedAt: Date.now(),
|
||||||
|
lastError: rateLimited ? `Rate limited by GitHub; will retry. ${message}` : message,
|
||||||
|
}),
|
||||||
|
ctx.runMutation(internal.syncRuns.patchInternal, {
|
||||||
|
syncRunId,
|
||||||
|
status: 'failed',
|
||||||
|
error: message,
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
throw new ConvexError(message);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
(The hourly cron / webhook already re-drive the refresh, so `rate_limited` is the "will retry" state; no new scheduling needed. `patchSyncFields` already accepts `syncStatus` — extend its arg validator if it enumerates the union; check `spoons.patchSyncFields` ~316 and add `'rate_limited'` there too if the union is inlined.)
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Add the two deps to `packages/backend/package.json`; run `bun install` from repo root; confirm it resolves (bump versions if bun reports peer conflicts against `@octokit/core` used by `@octokit/rest@22`).
|
||||||
|
- [ ] Write failing `tests/unit/rate-limit.test.ts`: `isRateLimitError({ status: 403, response: { headers: { 'x-ratelimit-remaining': '0' } } })` → true; `isRateLimitError({ status: 429 })` → true; `isRateLimitError({ status: 404 })` → false; `isRateLimitError(new Error('boom'))` → false; `isRateLimitError({ status: 403, message: 'secondary rate limit' })` → true.
|
||||||
|
- [ ] Run `bun run test:unit` — FAIL.
|
||||||
|
- [ ] Add `isRateLimitError` + plugin composition to `githubClient.ts`; add `'rate_limited'` to `spoons.syncStatus` in `schema.ts` (and to `patchSyncFields`'s inline union if present); update the catch block.
|
||||||
|
- [ ] `bun codegen:convex` + `bun run test:unit` PASS + `bun run typecheck` clean.
|
||||||
|
- [ ] Commit: `feat(sync): add Octokit retry/throttle and rate-limited status`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 11: Fix unbounded `.collect()`s on hot paths
|
||||||
|
|
||||||
|
**Problem:** Several hot queries `.collect()` owner-wide then filter/slice in JS:
|
||||||
|
- `spoonCommits.listForSpoon` no-side branch (`spoonCommits.ts:26-33`) collects ALL of an owner's commits.
|
||||||
|
- `agentJobs.countOldWorkspaces` (`agentJobs.ts:1009-1012`) and `deleteOldWorkspaces` (`agentJobs.ts:1031-1034`) collect ALL of an owner's jobs.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/spoonCommits.ts` (~26-33).
|
||||||
|
- `packages/backend/convex/agentJobs.ts` (~1001-1044).
|
||||||
|
- `packages/backend/tests/unit/hot-paths.test.ts` (new).
|
||||||
|
|
||||||
|
**Fix — `spoonCommits.listForSpoon` no-side branch:** the table already has `by_spoon_side`. Replace the owner-wide collect with two indexed `.take()`s (one per side) merged and sorted:
|
||||||
|
```ts
|
||||||
|
const [upstream, fork] = await Promise.all([
|
||||||
|
ctx.db.query('spoonCommits').withIndex('by_spoon_side', (q) => q.eq('spoonId', spoonId).eq('side', 'upstream')).order('desc').take(limit ?? 100),
|
||||||
|
ctx.db.query('spoonCommits').withIndex('by_spoon_side', (q) => q.eq('spoonId', spoonId).eq('side', 'fork')).order('desc').take(limit ?? 100),
|
||||||
|
]);
|
||||||
|
return [...upstream, ...fork].sort((a, b) => (b.committedAt ?? 0) - (a.committedAt ?? 0)).slice(0, limit ?? 100);
|
||||||
|
```
|
||||||
|
|
||||||
|
**Fix — `agentJobs.countOldWorkspaces` / `deleteOldWorkspaces`:** cap the scan with `.take()` on the existing `by_owner` index instead of unbounded `.collect()`. For count, take a bounded window (e.g. `.take(500)`) — document that count is "up to N"; for delete, `deleteOldWorkspaces` already slices to `max` (≤100), so change its collect to `.take(500)` before filtering, keeping the `max` slice. (These are maintenance/settings actions, not per-request hot loops, but the owner-wide `.collect()` is still the audited unbounded read; bounding it removes the growth risk.)
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing `tests/unit/hot-paths.test.ts` (convex-test): seed a spoon with, say, 3 upstream + 3 fork `spoonCommits` rows (insert directly); call `api.spoonCommits.listForSpoon` (authed) with no `side`, `limit: 4` → assert exactly 4 rows returned, newest `committedAt` first, and both sides represented. (This asserts behavior; the index change must keep the same observable ordering.)
|
||||||
|
- [ ] Run `bun run test:unit` — should PASS against current code IF ordering matches; adjust the test to pin ordering so the refactor is covered (make committedAt values interleave upstream/fork so a per-side take + merge is required to get the right top-4). Confirm the test FAILS if you naively `.take(4)` one side — i.e. it genuinely exercises the merge.
|
||||||
|
- [ ] Apply the `spoonCommits.listForSpoon` fix; apply the `.take(500)` bound to both agentJobs functions.
|
||||||
|
- [ ] Run `bun run test:unit` — PASS. `bun run typecheck` — clean.
|
||||||
|
- [ ] Commit: `perf(convex): bound unbounded owner-wide collects on hot paths`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 12: Surface `needs_reauth`/`revoked` in Settings → Integrations
|
||||||
|
|
||||||
|
**Problem:** `GithubIntegrationPanel` (`apps/next/src/components/integrations/github-integration-panel.tsx`) shows the connection but never surfaces `connection.status`. Users can't see when GitHub access is broken. `api.github.getConnection` already returns the full row (with `status`).
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/components/integrations/github-integration-panel.tsx`.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Consumes: `connection.status: 'active' | 'needs_reauth' | 'revoked'` (already on the query result).
|
||||||
|
- Add a status badge/alert block in the connected branch (~line 51-61): when `status !== 'active'`, render a prominent warning with a "Reconnect GitHub App" CTA linking to `installUrl` (the existing `installUrl` query). Copy:
|
||||||
|
- `needs_reauth`: "GitHub access needs re-authorization. Reconnect the app to resume syncing." (amber)
|
||||||
|
- `revoked`: "The GitHub App installation was removed. Reinstall to resume syncing." (red)
|
||||||
|
|
||||||
|
**Implementation sketch:**
|
||||||
|
```tsx
|
||||||
|
{connection && connection.status !== 'active' ? (
|
||||||
|
<div
|
||||||
|
className={
|
||||||
|
connection.status === 'revoked'
|
||||||
|
? 'rounded-md border border-red-500/40 bg-red-500/10 p-3 text-sm text-red-600'
|
||||||
|
: 'rounded-md border border-amber-500/40 bg-amber-500/10 p-3 text-sm text-amber-600'
|
||||||
|
}
|
||||||
|
>
|
||||||
|
<p className='font-medium'>
|
||||||
|
{connection.status === 'revoked'
|
||||||
|
? 'GitHub App installation removed'
|
||||||
|
: 'GitHub access needs re-authorization'}
|
||||||
|
</p>
|
||||||
|
<p className='mt-1'>
|
||||||
|
{connection.status === 'revoked'
|
||||||
|
? 'Reinstall the app to resume syncing.'
|
||||||
|
: 'Reconnect the app to resume syncing.'}
|
||||||
|
</p>
|
||||||
|
{installUrl ? (
|
||||||
|
<a className='mt-2 inline-block underline' href={installUrl} target='_blank' rel='noreferrer'>
|
||||||
|
Reconnect GitHub App
|
||||||
|
</a>
|
||||||
|
) : null}
|
||||||
|
</div>
|
||||||
|
) : null}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Add the status warning block to `github-integration-panel.tsx` (guard on `connection` being present; place above or within the connected grid).
|
||||||
|
- [ ] Verify types: `bun run typecheck` in `apps/next` (or repo-root typecheck task) — the `status` field is already typed via the generated api.
|
||||||
|
- [ ] Manual check (no automated Next component test required for this phase): run the app, temporarily set a connection row's `status` to `needs_reauth` (via Convex dashboard or a test mutation) and confirm the banner + reconnect CTA render on Settings → Integrations.
|
||||||
|
- [ ] Commit: `feat(settings): surface GitHub connection needs_reauth/revoked`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Manual verification checklist (end of phase)
|
||||||
|
|
||||||
|
- [ ] `cd packages/backend && bun run test:unit` — all green.
|
||||||
|
- [ ] `bun codegen:convex` (root) then `cd packages/backend && bun run typecheck` — clean.
|
||||||
|
- [ ] Deploy notes for the operator: configure the GitHub App webhook URL to `<CONVEX_SITE_URL>/webhooks/github`, subscribe to `push`, `installation`, and `installation_repositories` events, and set `GITHUB_APP_WEBHOOK_SECRET` in the Convex deployment env (matching the App's webhook secret).
|
||||||
|
- [ ] Trigger a real `push` to a watched fork → confirm a targeted refresh runs (spoon `lastGithubRefreshAt` updates within seconds, not the hourly cron window).
|
||||||
|
- [ ] Uninstall/reinstall the App → confirm `gitConnections.status` flips to `revoked`/`active` and the Settings → Integrations banner reflects it.
|
||||||
|
- [ ] Force a divergence >100 commits (or a fork far behind) → confirm `upstreamHeadSha` equals the real branch tip and only ONE maintenance thread is created across repeated scheduled checks.
|
||||||
@@ -0,0 +1,405 @@
|
|||||||
|
# Spoon Phase 5 — Notifications & Polish: Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Ship the one net-new subsystem for Phase 5 — a `notifications` table with an in-app header bell and preference-gated email via the existing UseSend integration — then sweep the audit's polish findings (#14–#20: loading skeletons, fire-and-forget error handling, form re-sync, auth copy, workspace UX, accessibility, dead code) and rewrite the docs for the box-first model.
|
||||||
|
|
||||||
|
**Architecture:** Notifications are written by a single shared TS helper `emitNotification(ctx, {...})` invoked at the existing event points (maintenance-thread creation, job status transitions, sync failure, connection re-auth). The helper inserts an in-app row and, when the user's per-kind email preference allows, schedules a `'use node'` internal action that sends through UseSend (`usesend-js`, already a dependency). The header bell subscribes to a reactive inbox query; mark-read/mark-all-read are mutations. Preferences live in a per-user `notificationPreferences` row edited from a new Settings → Notifications page. Everything else in the phase is independent, grouped, small fixes to `apps/next` UI and `apps/agent-worker` cleanup.
|
||||||
|
|
||||||
|
**Tech Stack:** Convex (notifications table + queries/mutations, UseSend email), Next.js 16 (React 19) UI, agent-worker minor cleanup.
|
||||||
|
|
||||||
|
**Depends on:** Phases 1–4 (notification emit points reference their statuses/events — job status transitions from Phase 1, connection `needs_reauth` from the Phase 4 webhook); polish tasks are independent and can run in any order after the notifications subsystem.
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- **Backend tests:** `packages/backend/tests/unit` with `convex-test`. Run `cd packages/backend && bun run test:unit`. The harness pattern is in `packages/backend/tests/unit/harness.test.ts`: `convexTest(schema, import.meta.glob('../../convex/**/*.*s'))`, `createUser`/`authed(t, userId)` helpers, `t.withIdentity({ subject: '<userId>|session', issuer: 'https://convex.test' })`. Add new tests as `packages/backend/tests/unit/<name>.test.ts`.
|
||||||
|
- **Next component tests:** jsdom + `@testing-library` in `apps/next/tests/component` (see `render.test.tsx`). Run `cd apps/next && bun run test:component`.
|
||||||
|
- **Codegen before typecheck:** after any `convex/schema.ts` or function-signature change run `cd packages/backend && bunx convex codegen` so `_generated` is current, then typecheck.
|
||||||
|
- **Conventional commits, one per task** (e.g. `feat(notifications): add notifications + notificationPreferences tables`).
|
||||||
|
- **Theme:** use existing semantic tokens only (`text-foreground`, `bg-card`, `text-muted-foreground`, `bg-primary/10`, etc.). No hard-coded hex colors. The codebase is already clean here.
|
||||||
|
- **Auth in Convex functions:** queries/mutations get the caller via `getRequiredUserId(ctx)` from `convex/model.ts`. Actions use `getAuthUserId(ctx)` (see `githubSync.ts`).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: `notifications` + `notificationPreferences` schema
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/schema.ts` (add two tables to `applicationTables`, ~after `threadMessages` / before `ignoredUpstreamChanges`, line ~820)
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces** table `notifications`:
|
||||||
|
```ts
|
||||||
|
notifications: defineTable({
|
||||||
|
userId: v.id('users'),
|
||||||
|
kind: v.union(
|
||||||
|
v.literal('maintenance_thread'),
|
||||||
|
v.literal('agent_turn_finished'),
|
||||||
|
v.literal('agent_needs_input'),
|
||||||
|
v.literal('sync_failed'),
|
||||||
|
v.literal('connection_needs_reauth'),
|
||||||
|
),
|
||||||
|
title: v.string(),
|
||||||
|
body: v.string(),
|
||||||
|
link: v.optional(v.string()), // in-app relative path, e.g. `/threads/<id>`
|
||||||
|
spoonId: v.optional(v.id('spoons')),
|
||||||
|
threadId: v.optional(v.id('threads')),
|
||||||
|
readAt: v.optional(v.number()),
|
||||||
|
emailedAt: v.optional(v.number()),
|
||||||
|
createdAt: v.number(),
|
||||||
|
})
|
||||||
|
.index('by_user', ['userId'])
|
||||||
|
.index('by_user_unread', ['userId', 'readAt'])
|
||||||
|
.index('by_user_created', ['userId', 'createdAt']),
|
||||||
|
```
|
||||||
|
- **Produces** table `notificationPreferences` (one row per user; unset field = default enabled):
|
||||||
|
```ts
|
||||||
|
notificationPreferences: defineTable({
|
||||||
|
userId: v.id('users'),
|
||||||
|
emailEnabled: v.optional(v.boolean()), // master email switch, default true
|
||||||
|
emailMaintenance: v.optional(v.boolean()),
|
||||||
|
emailAgentTurnFinished: v.optional(v.boolean()),
|
||||||
|
emailAgentNeedsInput: v.optional(v.boolean()),
|
||||||
|
emailSyncFailed: v.optional(v.boolean()),
|
||||||
|
emailConnectionReauth: v.optional(v.boolean()),
|
||||||
|
updatedAt: v.number(),
|
||||||
|
}).index('by_user', ['userId']),
|
||||||
|
```
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Write failing test `packages/backend/tests/unit/notifications.test.ts` that inserts a `notifications` row via `t.run(async (ctx) => ctx.db.insert('notifications', {...}))` and reads it back, plus one `notificationPreferences` row. FAIL: tables don't exist (schema validation error).
|
||||||
|
- [ ] Add both `defineTable` blocks to `applicationTables` in `schema.ts`.
|
||||||
|
- [ ] `cd packages/backend && bunx convex codegen`.
|
||||||
|
- [ ] Run `bun run test:unit` — PASS.
|
||||||
|
- [ ] Commit: `feat(notifications): add notifications + notificationPreferences tables`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: emit helper, `emit` mutation wrapper, and UseSend email dispatch
|
||||||
|
|
||||||
|
The single choke point all emit points call. A plain TS helper is callable directly inside mutations; an `internalMutation` wrapper lets Convex **actions** (e.g. `githubSync`) call it via `ctx.runMutation`.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/notifications.ts` (new — helper + emit wrapper + preference read)
|
||||||
|
- `packages/backend/convex/notificationsNode.ts` (new, `'use node'` — email send action reusing `usesend-js`)
|
||||||
|
- Reference for UseSend usage: `packages/backend/convex/custom/auth/providers/usesend.ts` (env `USESEND_API_KEY`, `USESEND_URL`, `USESEND_FROM_EMAIL`; `new UseSend(apiKey, useSendUrl)`, `useSend.emails.send({from,to,subject,text,html})`).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces** exported type + helper (import `MutationCtx` from `./_generated/server`, `Id` from `./_generated/dataModel`):
|
||||||
|
```ts
|
||||||
|
export type NotificationKind =
|
||||||
|
| 'maintenance_thread' | 'agent_turn_finished' | 'agent_needs_input'
|
||||||
|
| 'sync_failed' | 'connection_needs_reauth';
|
||||||
|
|
||||||
|
// Maps a kind to the notificationPreferences field that gates its email.
|
||||||
|
const EMAIL_PREF_FIELD: Record<NotificationKind, keyof Doc<'notificationPreferences'>> = {
|
||||||
|
maintenance_thread: 'emailMaintenance',
|
||||||
|
agent_turn_finished: 'emailAgentTurnFinished',
|
||||||
|
agent_needs_input: 'emailAgentNeedsInput',
|
||||||
|
sync_failed: 'emailSyncFailed',
|
||||||
|
connection_needs_reauth: 'emailConnectionReauth',
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function emitNotification(
|
||||||
|
ctx: MutationCtx,
|
||||||
|
args: { userId: Id<'users'>; kind: NotificationKind; title: string; body: string;
|
||||||
|
link?: string; spoonId?: Id<'spoons'>; threadId?: Id<'threads'> },
|
||||||
|
): Promise<Id<'notifications'>>;
|
||||||
|
```
|
||||||
|
Behavior: insert the `notifications` row (`readAt` unset, `createdAt: Date.now()`). Then load the user's `notificationPreferences` (index `by_user`); email is allowed when `emailEnabled !== false` **and** `prefs[EMAIL_PREF_FIELD[kind]] !== false` (unset = true). If allowed and the user has an `email`, `ctx.scheduler.runAfter(0, internal.notificationsNode.sendNotificationEmail, { notificationId })`. Return the id.
|
||||||
|
- **Produces** `internalMutation emit` (thin wrapper for actions) with the same args as the helper, body `return await emitNotification(ctx, args);`.
|
||||||
|
- **Produces** `internalMutation markEmailed({ notificationId })` → patches `emailedAt: Date.now()` (called by the email action).
|
||||||
|
- **Produces** `internalQuery getForEmail({ notificationId })` → returns `{ to: string | null; title; body; link }` by loading the notification + its user's email.
|
||||||
|
- **Produces** in `notificationsNode.ts`: `internalAction sendNotificationEmail({ notificationId })` — runs `getForEmail`; if no `to`, return; else send via UseSend (subject = title, text = body + optional link, minimal HTML mirroring `usesend.ts`); on success `ctx.runMutation(internal.notifications.markEmailed, { notificationId })`. Missing `USESEND_*` env → log and return (do **not** throw; email is best-effort).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] In `notifications.test.ts` add failing tests: (a) `emitNotification` inserts an unread row for the user; (b) with `emailEnabled: false` no email is scheduled (assert via a `t.run` count of scheduled functions is impractical — instead assert `emailedAt` stays undefined after `t.finishAllScheduledFunctions()` with UseSend env unset, i.e. row exists and is not emailed); (c) with prefs default (no row) and `USESEND_*` unset, `sendNotificationEmail` runs without throwing and leaves `emailedAt` undefined. FAIL: module doesn't exist.
|
||||||
|
- [ ] Implement `notifications.ts` (helper + `emit` + `markEmailed` + `getForEmail`) and `notificationsNode.ts` (`sendNotificationEmail`).
|
||||||
|
- [ ] `cd packages/backend && bunx convex codegen`.
|
||||||
|
- [ ] `bun run test:unit` — PASS.
|
||||||
|
- [ ] Commit: `feat(notifications): add emit helper + UseSend email dispatch`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: wire emit points
|
||||||
|
|
||||||
|
Call the helper/mutation at each event the spec lists. All are existing functions.
|
||||||
|
|
||||||
|
**Files & exact emit points:**
|
||||||
|
- `packages/backend/convex/threads.ts` — `createMaintenanceThread` (internalMutation, ~line 407). After the **new-thread** insert (~line 475, not the dedup/existing branch), call `await emitNotification(ctx, { userId: args.ownerId, kind: 'maintenance_thread', title: args.title, body: args.summary, link: \`/threads/${threadId}\`, spoonId: args.spoonId, threadId })`. (Import `emitNotification` from `./notifications`.)
|
||||||
|
- `packages/backend/convex/agentJobs.ts` — `updateStatus` (mutation, ~line 1108). Inside the `if (job.threadId)` block, after the thread patch: when `args.status === 'changes_ready'` **or** `'draft_pr_opened'`, emit `kind: 'agent_turn_finished'` (title `Agent turn finished`, body = `args.summary ?? job.summary ?? ''`, link `/threads/${job.threadId}`, threadId, spoonId). Emit only on the transition into that status (guard `job.status !== args.status`).
|
||||||
|
- `packages/backend/convex/agentJobs.ts` — `applyMaintenanceDecision` (mutation, ~line 1423). When the computed `status === 'waiting_for_user'`, emit `kind: 'agent_needs_input'` (title `Agent needs your input`, body `args.summary`, link `/threads/${job.threadId}`).
|
||||||
|
- `packages/backend/convex/githubSync.ts` — this is a `'use node'` action, so use `await ctx.runMutation(internal.notifications.emit, {...})`. Two sites:
|
||||||
|
- Terminal `catch` of `refreshOwnedSpoon` (~line 291, the outer catch that sets `syncStatus: 'error'`): emit `kind: 'sync_failed'` (title `Sync failed for ${spoon.name}`, body `message`, link `/spoons/${spoonId}`, spoonId). `spoon.name` is in scope.
|
||||||
|
- The maintenance-thread branches already call `createMaintenanceThread` (which now emits) — do **not** double-emit there.
|
||||||
|
- **Connection needs re-auth** — the Phase 4 webhook sets `gitConnections.status = 'needs_reauth' | 'revoked'`. Emit from that mutation (created in Phase 4). If that mutation does not yet exist in this branch, add a `// TODO(phase4): emit connection_needs_reauth here` marker at `packages/backend/convex/github.ts` near the connection status writes (~line 181) and emit from the first mutation that patches a connection to `needs_reauth`. Kind `connection_needs_reauth`, title `GitHub connection needs re-authorization`, body `Reconnect your GitHub account to keep syncing.`, link `/settings/integrations`.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Consumes** `emitNotification` (mutations) / `internal.notifications.emit` (actions) from Task 2.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Add failing tests in `notifications.test.ts`: (a) calling `createMaintenanceThread` (via `t.run`/internal) for a fresh upstream produces one `maintenance_thread` notification for the owner; a second dedup call does **not** add another. (b) `updateStatus` to `changes_ready` on a claimed job with a thread produces one `agent_turn_finished` notification; a repeat call at the same status does not add a second. Use the harness helpers to seed a spoon/thread/job like existing agentJobs tests. FAIL: no notifications produced.
|
||||||
|
- [ ] Add the emit calls at the five points above (imports as noted).
|
||||||
|
- [ ] `cd packages/backend && bunx convex codegen`; `bun run test:unit` — PASS.
|
||||||
|
- [ ] Commit: `feat(notifications): emit at maintenance/turn/sync/reauth events`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: inbox query + mark-read mutations
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/notifications.ts` (add public query/mutations)
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces** `query listMine({ limit?: number })` → `Doc<'notifications'>[]` for `getRequiredUserId(ctx)`, index `by_user_created` desc, `take(args.limit ?? 30)`.
|
||||||
|
- **Produces** `query unreadCount({})` → `number`. Use index `by_user_unread` filtered to `readAt === undefined` (`.withIndex('by_user_unread', q => q.eq('userId', uid).eq('readAt', undefined))`), `.take(100)` then `.length` capped display (return raw count via `.collect().length` bounded by take(100)).
|
||||||
|
- **Produces** `mutation markRead({ notificationId })` → ensures the row's `userId === getRequiredUserId(ctx)` (else `ConvexError('Notification not found.')`), patch `readAt: Date.now()` if unset.
|
||||||
|
- **Produces** `mutation markAllRead({})` → iterate `by_user_unread` unread rows for the caller, patch each `readAt`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Failing tests in `notifications.test.ts`: seed 3 notifications for user A + 1 for user B via `t.run`; `authed(t, A).query(api.notifications.listMine, {})` returns exactly A's 3 newest-first; `unreadCount` returns 3; `markRead` on one drops `unreadCount` to 2 and rejects B's row for A; `markAllRead` zeroes it. FAIL: functions missing.
|
||||||
|
- [ ] Implement the query + mutations.
|
||||||
|
- [ ] `bunx convex codegen`; `bun run test:unit` — PASS.
|
||||||
|
- [ ] Commit: `feat(notifications): inbox query + mark-read mutations`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 5: header notification bell
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/components/layout/header/controls/notification-bell.tsx` (new)
|
||||||
|
- `apps/next/src/components/layout/header/controls/index.tsx` (or wherever `Controls` is composed — confirm by reading `apps/next/src/components/layout/header/controls/`) — mount `<NotificationBell />` before `<AvatarDropdown />`, only when authenticated (use `useConvexAuth().isAuthenticated`, matching `header/index.tsx`).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Consumes** `api.notifications.unreadCount`, `api.notifications.listMine`, `api.notifications.markRead`, `api.notifications.markAllRead`.
|
||||||
|
- Client component (`'use client'`). Uses `useQuery`/`useMutation` from `convex/react`. Bell icon from `lucide-react` (`Bell`). Wrap in the existing `@spoon/ui` `DropdownMenu` primitives (same set `AvatarDropdown.tsx` imports). Unread badge: small `bg-primary text-primary-foreground` count pill when `unreadCount > 0` (show `9+` above 9). Each item is a `Link` to `notification.link ?? '#'` that calls `markRead({ notificationId })` on click; a "Mark all read" action calls `markAllRead`. Empty state: `No notifications yet.` Relative time via existing date util if present (grep `formatDistanceToNow`/`date-fns` under `apps/next/src`); otherwise a minimal inline formatter.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Failing component test `apps/next/tests/component/notification-bell.test.tsx`: render `<NotificationBell />` with a mocked Convex client (follow `render.test.tsx` for the provider/mock pattern) returning `unreadCount: 2` and two notifications; assert the badge shows `2` and both titles render; clicking one invokes the `markRead` mock. FAIL: component missing.
|
||||||
|
- [ ] Implement `notification-bell.tsx`; mount it in the header controls.
|
||||||
|
- [ ] `cd apps/next && bun run test:component` — PASS.
|
||||||
|
- [ ] Manual verify: `bun run dev`, sign in, confirm the bell renders and the badge updates reactively after seeding a notification. Document the check.
|
||||||
|
- [ ] Commit: `feat(notifications): header notification bell`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 6: Settings → Notifications preferences
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `packages/backend/convex/notifications.ts` (add `getPreferences` query + `updatePreferences` mutation)
|
||||||
|
- `apps/next/src/app/(app)/settings/notifications/page.tsx` (new)
|
||||||
|
- `apps/next/src/components/settings/notification-preferences-panel.tsx` (new)
|
||||||
|
- `apps/next/src/app/(app)/settings/layout.tsx` (add nav item `{ href: '/settings/notifications', label: 'Notifications', icon: Bell }` — import `Bell` from `lucide-react`; insert after Dotfiles)
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Produces** `query getPreferences({})` → the caller's `notificationPreferences` row or a defaults object `{ emailEnabled: true, emailMaintenance: true, emailAgentTurnFinished: true, emailAgentNeedsInput: true, emailSyncFailed: true, emailConnectionReauth: true }` when no row (unset = enabled).
|
||||||
|
- **Produces** `mutation updatePreferences({ emailEnabled?, emailMaintenance?, emailAgentTurnFinished?, emailAgentNeedsInput?, emailSyncFailed?, emailConnectionReauth? })` → upsert the caller's row (patch existing or insert), set `updatedAt`.
|
||||||
|
- Panel: `'use client'`, `useQuery(api.notifications.getPreferences)` + `useMutation(api.notifications.updatePreferences)`. **Seed local state from the query using the hydrated-flag pattern** (Task 9 reference) so late-resolving prefs don't clobber. Each toggle is a `Switch`/`Checkbox` from `@spoon/ui`; on change call the mutation in a `try/catch` with `toast.success('Preferences saved.')` on resolve and `toast.error(...)` on failure (see Task 8 pattern). Master `emailEnabled` disables the per-kind toggles when off.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Failing backend test in `notifications.test.ts`: `getPreferences` returns all-true defaults with no row; `updatePreferences({ emailSyncFailed: false })` persists and a subsequent `getPreferences` reflects it; the gating in Task 2 respects it (emit `sync_failed` with pref false → no email scheduled / `emailedAt` stays unset). FAIL.
|
||||||
|
- [ ] Implement `getPreferences` + `updatePreferences`; `bunx convex codegen`; `bun run test:unit` — PASS.
|
||||||
|
- [ ] Build the page + panel; add the settings nav item.
|
||||||
|
- [ ] Manual verify: toggle a preference, reload, confirm it persists. Document the check.
|
||||||
|
- [ ] Commit: `feat(notifications): settings notification preferences`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 7: loading skeletons for dashboard / spoons / threads
|
||||||
|
|
||||||
|
Kills the zero-state flash where `useQuery(...) ?? []` renders an empty state before data resolves.
|
||||||
|
|
||||||
|
**Files (read each first for exact `useQuery` lines):**
|
||||||
|
- `apps/next/src/app/(app)/dashboard/page.tsx` (~lines 14–16: `?? []` on `listMineWithState`, `syncRuns.listRecent`, `threads.listMine`)
|
||||||
|
- `apps/next/src/app/(app)/spoons/page.tsx` (~line 35 `?? []`)
|
||||||
|
- `apps/next/src/app/(app)/threads/page.tsx` (`?? []` on list)
|
||||||
|
- New: `apps/next/src/components/ui/list-skeleton.tsx` (or reuse an existing `Skeleton` from `@spoon/ui` if present — grep `Skeleton` under `packages/ui`/`apps/next` first).
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- **Consumes** existing Convex list queries. Change pattern: keep the raw query result **without** `?? []`, so `undefined` (loading) is distinguishable from `[]` (empty). Render a skeleton block while `data === undefined`; render the real empty-state only when `data.length === 0`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Component test `apps/next/tests/component/dashboard-loading.test.tsx` (or extend `render.test.tsx`): with the query mock returning `undefined`, assert a skeleton (`data-testid="list-skeleton"` / `role="status"`) is shown and the empty-state copy is **not**; with `[]`, assert the empty state shows and no skeleton. FAIL.
|
||||||
|
- [ ] Add the skeleton component; update the three pages to branch on `undefined` vs `[]`. Give the skeleton `role="status"` + `aria-label="Loading"`.
|
||||||
|
- [ ] `bun run test:component` — PASS.
|
||||||
|
- [ ] Commit: `fix(ui): loading skeletons for dashboard/spoons/threads`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 8: fire-and-forget mutation error handling across panels
|
||||||
|
|
||||||
|
Wrap unhandled mutation calls in `try/catch`; only `toast.success` after resolve; `toast.error` on failure. Model on the already-correct `importAll`/`newFile` in `dotfiles-manager.tsx` (lines 172–181, 221–230).
|
||||||
|
|
||||||
|
**Files & exact sites:**
|
||||||
|
- `apps/next/src/components/settings/dotfiles/dotfiles-manager.tsx` — `saveSelected` (lines 161–170: wrap `await putFile(...)` in try/catch; `toast.success('Saved.')` inside try after resolve, `toast.error(...)` in catch) and `deleteSelected` (lines 232–239: same around `await removeFile(...)`).
|
||||||
|
- `apps/next/src/components/integrations/ai-provider-profiles-panel.tsx` (~lines 235–240 and 282–285: the save + delete mutations).
|
||||||
|
- `apps/next/src/components/spoons/spoon-clone-panel.tsx` (~lines 225–228).
|
||||||
|
- `apps/next/src/components/spoons/spoon-secrets-form.tsx` (~lines 312–315; **and** the sequential import at ~134–141 — wrap each iteration so one failure surfaces via `toast.error` and doesn't silently swallow the rest; report count of successes).
|
||||||
|
|
||||||
|
**Interfaces:** no signature changes; behavior only. Error message: `error instanceof Error ? error.message : '<action> failed.'`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Read each file at the cited lines to capture exact `await <mutation>(...)` calls.
|
||||||
|
- [ ] Component test `apps/next/tests/component/dotfiles-error.test.tsx`: render the dotfiles manager with a `putFile` mock that rejects; trigger save; assert `toast.error` called and `toast.success` **not** called. (Mock `sonner`'s `toast`.) FAIL (currently unconditional success).
|
||||||
|
- [ ] Apply try/catch + resolve-gated success to all sites above.
|
||||||
|
- [ ] `bun run test:component` — PASS. Manual grep to confirm no remaining unconditional `toast.success(` sits before its `await` in these files.
|
||||||
|
- [ ] Commit: `fix(ui): error-handle fire-and-forget mutations in settings panels`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 9: settings/thread forms re-sync on loaded record id
|
||||||
|
|
||||||
|
`useState` seeded once from a late-resolving Convex prop can save defaults over real config. Fix with the **hydrated-flag** pattern already in `dotfiles-manager.tsx` RepoPanel (lines 383–394): a `hydrated` boolean, an effect that seeds from the loaded record then sets `hydrated`, keyed on the record's `_id`.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/components/spoons/spoon-agent-settings-form.tsx` (~lines 61–119: `useState` seeded from the settings query)
|
||||||
|
- `apps/next/src/components/threads/thread-workspace-form.tsx` (~lines 57–74)
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Pattern (per form): `const [hydrated, setHydrated] = useState(false);` + `useEffect(() => { if (!record || hydrated) return; /* seed each useState from record fields */ setHydrated(true); }, [record, hydrated]);`. If the mounting parent can swap records, prefer remounting via `key={record._id}` on the form in the parent **or** reset `hydrated` when `record._id` changes (`useEffect(() => setHydrated(false), [record?._id])`). Choose keyed remount if the parent already has the id; otherwise the id-change effect.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Component test `apps/next/tests/component/form-resync.test.tsx`: render `spoon-agent-settings-form` first with the settings prop `undefined`, then rerender with a resolved record having non-default values; assert the inputs reflect the loaded values (not the initial defaults). FAIL (state seeded once from undefined).
|
||||||
|
- [ ] Apply the hydrated-flag (or keyed-remount) pattern to both forms.
|
||||||
|
- [ ] `bun run test:component` — PASS.
|
||||||
|
- [ ] Commit: `fix(ui): re-sync settings/thread forms on loaded record`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 10: sign-in + forgot-password copy & input-retention fixes
|
||||||
|
|
||||||
|
**Files & exact fixes:**
|
||||||
|
- `apps/next/src/app/(auth)/sign-in/page.tsx`
|
||||||
|
- Remove `signInForm.reset()` from the `finally` on failed sign-in (line 140) — keep the email/password on error. Only reset on success (move a `.reset()` into the success branch if desired, or drop it). Same for `signUpForm.reset()` (line 163) and `verifyEmailForm.reset()` (line 182): do not wipe input in `finally`; reset only after a successful transition.
|
||||||
|
- `pendingText='Signing Up...'` on the **Verify Email** button (line 235) → `pendingText='Verifying...'`. Leave the actual sign-up button (line 443) as `Signing Up...`.
|
||||||
|
- Typo `Confirm Passsword` (line 426) → `Confirm Password`.
|
||||||
|
- `apps/next/src/app/(auth)/forgot-password/page.tsx`
|
||||||
|
- `Please enter the one-time password sent to your phone.` (~line 232–233) → `...sent to your email.`
|
||||||
|
- `Confirm Passsword` typo (~line 268) → `Confirm Password`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Component test `apps/next/tests/component/sign-in-retention.test.tsx`: render the sign-in form, type an email + password, make the `signIn` mock reject, submit; assert the email/password inputs still hold the typed values after the rejection settles. FAIL (reset in finally wipes them).
|
||||||
|
- [ ] Apply the input-retention + copy fixes above.
|
||||||
|
- [ ] `bun run test:component` — PASS. Manual grep: `grep -rn "Passsword\|sent to your phone\|Signing Up" apps/next/src/app/(auth)` returns only the intentional sign-up button.
|
||||||
|
- [ ] Commit: `fix(auth): retain input on failure + copy fixes`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 11: profile page `'use server'` removal + AvatarDropdown fixes
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/app/(app)/settings/profile/page.tsx` (line 1: remove the stray `'use server';` — this is an async server component using `preloadQuery`; `'use server'` marks it as a Server Actions module and is wrong here).
|
||||||
|
- `apps/next/src/components/layout/header/controls/AvatarDropdown.tsx`
|
||||||
|
- Line 73: `Link href='/profile'` → `href='/settings/profile'` (the `/profile` route is being deleted in Task 14).
|
||||||
|
- Lines 64 & 67: empty-string name falls through `??`. Change `user?.name ?? user?.email` and `user.name?.trim() ?? user.email?.trim()` to use `||` so an empty/whitespace name falls back to email: `(user?.name?.trim() || user?.email)` and label `{user.name?.trim() || user.email?.trim()}`.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Manual verification (test impractical — server component + dropdown wiring):
|
||||||
|
- Before: `head -1 apps/next/src/app/(app)/settings/profile/page.tsx` shows `'use server';`. After: first line is `import ...`; `grep -n "use server" apps/next/src/app/(app)/settings/profile/page.tsx` returns nothing.
|
||||||
|
- Before: `grep -n "href='/profile'" .../AvatarDropdown.tsx` matches. After: it matches `href='/settings/profile'` and no bare `/profile`.
|
||||||
|
- Before: `grep -n "?? user" .../AvatarDropdown.tsx` matches lines 64/67. After: those use `|| user`.
|
||||||
|
- [ ] Apply the three edits.
|
||||||
|
- [ ] `cd apps/next && bun run typecheck` (or `bun run build` if no dedicated typecheck script) — passes.
|
||||||
|
- [ ] Commit: `fix(ui): profile page directive + avatar dropdown links/fallbacks`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 12: accessibility — remove `role='link'` with nested interactive elements
|
||||||
|
|
||||||
|
A `role='link'` container wrapping real `<a>`/`<Link>` children is an invalid nested-interactive pattern.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `apps/next/src/app/(app)/spoons/page.tsx` (~lines 112–124: `TableRow role='link'` containing links)
|
||||||
|
- `apps/next/src/app/(app)/threads/page.tsx` (~lines 317–329: `Card role='link'` containing links)
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Pattern: drop the `role='link'` (and any `tabIndex`/`onKeyDown` that simulated link behavior on the container). Make the row/card navigable via a single primary `Link` — either a full-row `Link` wrapping the content, or a "stretched link" overlay (`<Link className="absolute inset-0" ...><span className="sr-only">…</span></Link>` on a `relative` container) so nested action links remain individually clickable. Preserve existing hover styles.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Read both cited blocks to capture current structure.
|
||||||
|
- [ ] Component test `apps/next/tests/component/list-a11y.test.tsx`: render the spoons table (mock data) and assert no element has `role="link"` and the row exposes an accessible link to the spoon detail. FAIL (role='link' present).
|
||||||
|
- [ ] Refactor both to the stretched-link (or single wrapping Link) pattern; remove `role='link'` and simulated key handlers.
|
||||||
|
- [ ] `bun run test:component` — PASS. Manual grep: `grep -rn "role='link'" apps/next/src/app` returns nothing.
|
||||||
|
- [ ] Commit: `fix(a11y): remove nested-interactive role='link' on spoon/thread lists`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 13: agent workspace UX polish
|
||||||
|
|
||||||
|
Group the Phase-5 workspace items. Read each cited range first.
|
||||||
|
|
||||||
|
**Files & fixes:**
|
||||||
|
- `apps/next/src/components/agent-workspace/agent-workspace-shell.tsx`
|
||||||
|
- **Diff/tree load race** (~124–138): guard out-of-order responses. Track a per-request sequence number (or `AbortController`); ignore a response whose request id is stale so a slower earlier fetch can't overwrite newer data.
|
||||||
|
- **Editor buffer not refreshed when agent edits the open file** (~252–265): when a workspace change (`agentWorkspaceChanges`) arrives for the currently open file: if the buffer is **not dirty**, reload the file content into the editor; if dirty, set a conflict flag (badge/toast "This file changed on disk") rather than silently overwriting.
|
||||||
|
- **Recovery panel over-triggers** (~503–557): require **N consecutive** status/health failures (e.g. `const RECOVERY_THRESHOLD = 3`) before showing the recovery panel; reset the counter on any success. One transient error must not trip it.
|
||||||
|
- **Mobile layout** (~476 `min-h-[720px]`, ~562 `grid-cols-1`): replace the fixed `min-h-[720px]` with a viewport-relative min-height (e.g. `min-h-[70vh]`/`min-h-[calc(100dvh-…)]`) and ensure the single-column mobile layout collapses the file tree (collapsible/`Sheet` or a toggle) instead of stacking full-height panels.
|
||||||
|
- `apps/next/src/components/agent-workspace/agent-thread.tsx`
|
||||||
|
- **Auto-scroll fights the user** (~114–132): only auto-scroll to newest when the user is already near the bottom (compute `scrollHeight - scrollTop - clientHeight < threshold`, e.g. 80px). If scrolled up, don't yank; optionally show a "jump to latest" affordance.
|
||||||
|
- `apps/next/src/components/agent-workspace/code-editor.tsx`
|
||||||
|
- **editorRef not cleared on unmount** (~53, ~157): in the mount effect's cleanup, dispose the editor and set `editorRef.current = null` to avoid a stale ref / leak across remounts.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Read each cited range.
|
||||||
|
- [ ] Component test `apps/next/tests/component/agent-thread-scroll.test.tsx`: simulate the messages list scrolled up (mock `scrollTop`/`scrollHeight`/`clientHeight`), append a new message, assert `scrollTo`/`scrollIntoView` is **not** called; then with near-bottom, assert it **is**. FAIL.
|
||||||
|
- [ ] Add a unit-style test for the recovery threshold: extract the consecutive-failure decision into a small pure helper (`shouldShowRecovery(consecutiveFailures, threshold)`) and test it, or assert via a component test that 1–2 failures don't render the recovery panel and the 3rd does. FAIL.
|
||||||
|
- [ ] Implement all fixes above (sequencing guard, dirty-aware buffer refresh, consecutive-failure threshold, mobile min-height + collapsible tree, near-bottom auto-scroll, editorRef cleanup).
|
||||||
|
- [ ] `bun run test:component` — PASS.
|
||||||
|
- [ ] Manual verify in `bun run dev`: open a workspace, scroll the thread up while the agent streams (no yank), trigger an agent edit of an open clean file (buffer refreshes), resize to mobile width (tree collapses). Document the checks.
|
||||||
|
- [ ] Commit: `fix(workspace): scroll/diff-race/recovery/editor/mobile polish`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 14: dead code removal + worker `redact.ts` / `env.ts` cleanup
|
||||||
|
|
||||||
|
Group all removals + the two worker cleanups. **Note:** `redact.ts` and `env.ts` items are Phase-5-scoped per the audit; some `env.ts` vars may already be gone after Phase 2 — only remove ones that are still present **and** unreferenced.
|
||||||
|
|
||||||
|
**Files & actions:**
|
||||||
|
- Delete route dirs: `apps/next/src/app/(app)/updates/`, `apps/next/src/app/(app)/agents/`, `apps/next/src/app/(app)/settings/ai/`, `apps/next/src/app/(auth)/profile/`.
|
||||||
|
- Delete component: `apps/next/src/components/landing/tech-stack.tsx` (grep first for imports; remove any import/usage — likely in a landing page).
|
||||||
|
- Delete empty dirs: `apps/next/src/components/agents/`, `apps/next/src/components/updates/` (only if empty after the above).
|
||||||
|
- `apps/next/src/proxy.ts` (lines 11–12): remove `'/updates(.*)'`, `'/agents(.*)'`, and `'/profile(.*)'` from `isProtectedRoute` (those routes no longer exist).
|
||||||
|
- `apps/agent-worker/src/redact.ts` (lines 1–17): the first three patterns (`ghs_…`, `github_pat_…`, `sk-…`) have **no** capture group, so the shared `'$1=[redacted]'` replacement emits a literal `$1`. Fix: give each pattern its own replacement — group-less patterns replace with `'[redacted]'`; the `(client_secret|…)=(…)` pattern keeps `'$1=[redacted]'`. Restructure `secretPatterns` to `{ pattern, replacement }` objects and map accordingly.
|
||||||
|
- `apps/agent-worker/src/env.ts`: remove `terminalImage` and `terminalIdleMs` (and any other audit-listed unused vars — `containerAccess`, `maxConcurrentJobs`) **only if** a repo-wide grep shows no remaining references. Grep before deleting each.
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] Grep for references to each route/component before deleting: `grep -rn "tech-stack\|/updates\|/agents\|/settings/ai\|(auth)/profile\|href='/profile'" apps/next/src`. Confirm nothing (except the AvatarDropdown link already fixed in Task 11) still points at them.
|
||||||
|
- [ ] Failing worker test `apps/agent-worker/src/redact.test.ts` (vitest): `createRedactor([])('token ghs_ABC123 sk-xyz')` must contain `[redacted]` and **not** contain the literal `$1`; the `api_key=secret` case still yields `api_key=[redacted]`. FAIL (current output has `$1`).
|
||||||
|
- [ ] Fix `redact.ts` per-pattern replacement; run the worker test (`cd apps/agent-worker && bunx vitest run src/redact.test.ts` or the repo's worker test command) — PASS.
|
||||||
|
- [ ] Delete the routes/components/empty dirs; trim `proxy.ts`; remove verified-unused `env.ts` vars (grep each first).
|
||||||
|
- [ ] `cd apps/next && bun run build` (or typecheck) passes with no missing-import errors; `cd apps/agent-worker && <typecheck>` passes.
|
||||||
|
- [ ] Manual grep confirms removals: `find apps/next/src/app -type d \( -name updates -o -name agents -o -name ai -o -name profile \)` returns nothing under the deleted paths.
|
||||||
|
- [ ] Commit: `chore: remove dead routes/components + fix redact/env cleanup`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 15: docs rewrite for the box-first model
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `README.md`
|
||||||
|
- `docs/*.md` (enumerate: `ls docs/*.md` and any relevant subdirs; the spec calls out server-deploy notes)
|
||||||
|
|
||||||
|
**Interfaces:** documentation only. Rewrite the per-job-container narrative to the Phase-2 **one long-running per-user box** model:
|
||||||
|
- Each user owns a persistent Fedora container (`spoon-box-*`) with a persistent home (`homes/<username>/`, `~/Code`), dotfiles, terminal, and agent CLIs; the legacy per-job OpenCode container path is gone.
|
||||||
|
- Three agent runtimes exec into the box: Codex, OpenCode, **and Claude Code** (note the new Anthropic provider-profile kind).
|
||||||
|
- **GitHub webhooks:** document the webhook URL (Convex `httpAction`) and `GITHUB_APP_WEBHOOK_SECRET`; hourly cron remains a fallback.
|
||||||
|
- **New env vars:** list the box/terminal/notification/webhook env (`SPOON_AGENT_BOX_IDLE_MS`, `USESEND_API_KEY`/`USESEND_URL`/`USESEND_FROM_EMAIL`, `GITHUB_APP_WEBHOOK_SECRET`, Claude Code auth) and drop removed ones (`SPOON_AGENT_TERMINAL_IMAGE`, `SPOON_AGENT_TERMINAL_IDLE_MS`, and other Task-14-removed vars).
|
||||||
|
- Add a short **Notifications** section (in-app bell + email via UseSend, per-user preferences, web push out of scope).
|
||||||
|
|
||||||
|
**Steps:**
|
||||||
|
- [ ] `ls docs/*.md docs/**/*.md README.md`; read each to find per-job-container language (`grep -rn "per-job\|spoon-agent-job\|container per\|host_port" README.md docs`).
|
||||||
|
- [ ] Rewrite the affected sections per the bullets above. No stale references to deleted routes/env.
|
||||||
|
- [ ] Manual verification: `grep -rn "spoon-agent-job\|per-job container" README.md docs` returns nothing; the deploy doc lists the webhook URL + `GITHUB_APP_WEBHOOK_SECRET` + `USESEND_*` + Claude Code; a Notifications section exists.
|
||||||
|
- [ ] Commit: `docs: rewrite for box-first model + webhooks + notifications`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Manual verification checklist (end of phase)
|
||||||
|
|
||||||
|
- [ ] Trigger a maintenance thread (diverged spoon) → in-app bell increments and (with email pref on + `USESEND_*` set) an email arrives.
|
||||||
|
- [ ] Finish an agent turn (`changes_ready`) → `agent_turn_finished` notification; a maintenance decision needing approval → `agent_needs_input`.
|
||||||
|
- [ ] Force a sync failure → `sync_failed` notification.
|
||||||
|
- [ ] Toggle each preference off → the corresponding email stops while the in-app row still appears.
|
||||||
|
- [ ] Mark-read / mark-all-read update the badge reactively.
|
||||||
|
- [ ] Dashboard/spoons/threads show skeletons (no empty-state flash) on cold load.
|
||||||
|
- [ ] Sign-in keeps input on failure; auth copy fixed; forms re-sync from loaded records.
|
||||||
|
- [ ] Deleted routes 404; app builds; worker `redact.ts` no longer emits `$1`.
|
||||||
Reference in New Issue
Block a user