Files
spoon/docs/superpowers/plans/2026-07-10-spoon-phase4-sync-correctness.md
T
Gabriel Brown 8aa9140191 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.
2026-07-10 15:18:51 -04:00

41 KiB
Raw Blame History

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 13.

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 8289).
  • 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:

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):

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):

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:

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 insyncForkWithUpstream). Replace with the deterministic key; when maintenanceDedupKey(...)isnull` (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):

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:

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:

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:
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):

} 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:

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:

{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.