Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions plugins/canopy/skills/update-plan/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ Once confirmed, make **exactly one** call:

## Hard rules (invariants)

- **Server-gated to `ADMIN_LOGINS`.** `update_plan` is only registered for admin principals — a
non-admin bearer doesn't have the tool at all (absent from `tools/list`, tool-not-found if called).
This skill's own instructions are a second layer, not the enforcement boundary.
- **Explicit only.** Never fire without a direct admin ask.
- **Read before write, every time** — step 1 is not optional, even for a small edit.
- **Confirm the diff before writing** — no silent writes.
Expand Down
10 changes: 10 additions & 0 deletions src/auth/principal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ export interface Principal {

export type AppEnv = { Bindings: Env; Variables: { principal: Principal } };

/**
* Is this login an admin? ADMIN_LOGINS is a comma-separated allowlist of GitHub
* logins permitted to run admin actions (e.g. the server-side backfill). An
* absent/empty var means nobody is an admin — fails closed.
*/
export function isAdmin(env: Env, login: string): boolean {
const allow = (env.ADMIN_LOGINS ?? "").split(",").map((s) => s.trim()).filter(Boolean);
return allow.includes(login);
}

// The only routes reachable without a session. Everything else is gated.
const PUBLIC_PATHS = new Set(["/auth/login", "/auth/callback"]);

Expand Down
3 changes: 2 additions & 1 deletion src/auth/routes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Hono } from "hono";
import { setCookie, getCookie, deleteCookie } from "hono/cookie";
import type { AppEnv } from "./principal";
import { isAdmin } from "./principal";
import { pkce, randomToken, hmacSeal, hmacUnseal } from "./crypto";
import { buildAuthorizeUrl, exchangeCode, getUser, isActiveOrgMember } from "./github";
import { createSession, setSessionCookie, readSessionCookie, deleteSession, clearSessionCookie } from "./session";
Expand Down Expand Up @@ -73,7 +74,7 @@ authApp.get("/callback", async (c) => {
authApp.get("/me", async (c) => {
const login = c.get("principal").login;
const row = await first<{ name: string | null; avatar_url: string | null }>(c.env.DB, `SELECT name, avatar_url FROM users WHERE github_login = ?`, login);
return c.json({ login, name: row?.name ?? null, avatar_url: row?.avatar_url ?? null, org: SAPLING_ORG });
return c.json({ login, name: row?.name ?? null, avatar_url: row?.avatar_url ?? null, org: SAPLING_ORG, admin: isAdmin(c.env, login) });
});

// GATED (by sessionGate in src/routes.ts): revoke this session.
Expand Down
1 change: 1 addition & 0 deletions src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ export interface Env {
DEV_LOGIN?: string; // LOCAL DEV ONLY (set in .dev.vars): bypass OAuth, act as this seeded user. Never set in prod.
AI?: Ai; // Workers AI binding: capture-time completed-PR summaries only, never at render.
GITHUB_SERVICE_TOKEN?: string; // app-level token for the scheduled progress-cache recompute backstop; absent → scheduled() no-ops
ADMIN_LOGINS?: string; // comma-separated GitHub logins allowed to run admin actions (e.g. the server-side backfill)
}
41 changes: 24 additions & 17 deletions src/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createMcpHandler } from "agents/mcp";
import { z } from "zod";
import type { Env } from "./env";
import type { Principal } from "./auth/principal";
import { isAdmin } from "./auth/principal";
import { get_doc, list_docs, get_feed, query } from "./tools/reads";
import { getMyWork, list_events } from "./tools/mywork";
import { ingestFeedEntry, ingestDocProposal, consume } from "./consumer";
Expand Down Expand Up @@ -145,23 +146,29 @@ export function buildCanopyMcpServer(env: Env, principal: Principal): McpServer
async (payload) => runTool(() => consume(env.DB, IngestPayload.parse(payload), principal)),
);

server.tool(
"update_plan",
"ADMIN plan write: replace the roadmap narrative and create/update milestones (including status 'done') in one direct, non-destructively versioned write — same authored-write class as promote, NOT the ingestion gate. Milestones not listed are untouched. Use via the update-plan skill.",
{
narrative: z.string(),
milestones: z.array(z.object({
id: z.number().int().optional(),
title: z.string(),
description: z.string().nullable().optional(),
phase: z.string().nullable().optional(),
target_date: z.string(),
status: z.enum(["upcoming", "in_progress", "done"]),
github_ref: z.union([z.number(), z.array(z.number())]).nullable().optional(),
})).default([]),
},
async (input) => runTool(() => write_plan(env.DB, input as PlanWrite, principal.login))
);
// ADMIN-only: the plan write surface — non-admin principals don't even see the tool
// (conditional registration means it's absent from tools/list and calling it by
// name errors tool-not-found, since a fresh server is built per request with the
// principal already in scope).
if (isAdmin(env, principal.login)) {
server.tool(
"update_plan",
"ADMIN plan write: replace the roadmap narrative and create/update milestones (including status 'done') in one direct, non-destructively versioned write — same authored-write class as promote, NOT the ingestion gate. Milestones not listed are untouched. Use via the update-plan skill.",
{
narrative: z.string(),
milestones: z.array(z.object({
id: z.number().int().optional(),
title: z.string(),
description: z.string().nullable().optional(),
phase: z.string().nullable().optional(),
target_date: z.string(),
status: z.enum(["upcoming", "in_progress", "done"]),
github_ref: z.union([z.number(), z.array(z.number())]).nullable().optional(),
})).default([]),
},
async (input) => runTool(() => write_plan(env.DB, input as PlanWrite, principal.login))
);
}

return server;
}
Expand Down
15 changes: 14 additions & 1 deletion src/routes.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Hono } from "hono";
import { IngestPayload } from "@shared/contract";
import type { AppEnv } from "./auth/principal";
import { sessionGate } from "./auth/principal";
import { sessionGate, isAdmin } from "./auth/principal";
import { authApp } from "./auth/routes";
import { consume } from "./consumer";
import { runBackfill } from "./tools/backfill";
import { get_doc, list_docs, get_feed, query, list_needs_triage, list_adrs, list_milestone_proposals, list_proposals } from "./tools/reads";
import { promote_doc, ratify_adr, promote_milestone_proposal, reject_milestone_proposal, complete_milestone, reject_doc_version, reject_adr, resolve_triage, assign_triage, type AssignType } from "./tools/writes";
import { get_plan } from "./tools/plan";
Expand Down Expand Up @@ -219,6 +220,18 @@ app.post("/milestone-proposals/:id/reject", async (c) => {
}
});

// ADMIN action (session-gated + admin-gated): server-side GitHub backfill.
// A computed/authored direct writer in the promote class — humans (admins)
// trigger it — but every captured event still funnels through the ingestEvent
// gate fn. Non-admins get 403; a missing service token/repo → 503 with the error.
app.post("/admin/backfill", async (c) => {
const login = c.get("principal").login;
if (!isAdmin(c.env, login)) return c.json({ error: "admin only" }, 403);
const res = await runBackfill(c.env, login);
if (!res.ok) return c.json({ error: res.error }, 503);
return c.json(res);
});
Comment on lines +227 to +233

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

No error boundary around runBackfill.

runBackfill can throw (see linked review comment in src/tools/backfill.ts on unguarded ingestEvent/storePrSummary calls). Since there's no try/catch here, an unexpected exception will bubble up as an unhandled rejection rather than the documented { error } JSON envelope used for the config-missing case, breaking the route's error contract for admins.

🛡️ Proposed fix
 app.post("/admin/backfill", async (c) => {
   const login = c.get("principal").login;
   if (!isAdmin(c.env, login)) return c.json({ error: "admin only" }, 403);
-  const res = await runBackfill(c.env, login);
-  if (!res.ok) return c.json({ error: res.error }, 503);
-  return c.json(res);
+  try {
+    const res = await runBackfill(c.env, login);
+    if (!res.ok) return c.json({ error: res.error }, 503);
+    return c.json(res);
+  } catch (err) {
+    return c.json({ error: "backfill failed" }, 502);
+  }
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app.post("/admin/backfill", async (c) => {
const login = c.get("principal").login;
if (!isAdmin(c.env, login)) return c.json({ error: "admin only" }, 403);
const res = await runBackfill(c.env, login);
if (!res.ok) return c.json({ error: res.error }, 503);
return c.json(res);
});
app.post("/admin/backfill", async (c) => {
const login = c.get("principal").login;
if (!isAdmin(c.env, login)) return c.json({ error: "admin only" }, 403);
try {
const res = await runBackfill(c.env, login);
if (!res.ok) return c.json({ error: res.error }, 503);
return c.json(res);
} catch (err) {
return c.json({ error: "backfill failed" }, 502);
}
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes.ts` around lines 227 - 233, The /admin/backfill handler currently
calls runBackfill without an error boundary, so unexpected exceptions can escape
instead of returning the route’s JSON error shape. Wrap the runBackfill(c.env,
login) call in try/catch inside the app.post("/admin/backfill", ...) handler,
and convert thrown errors into a c.json({ error: ... }, 503) response consistent
with the existing res.ok failure path. Use the existing
app.post("/admin/backfill") route and runBackfill symbol to locate the change.


// Human confirmation (session-gated): flip a live milestone to 'done'.
app.post("/milestones/:id/complete", async (c) => {
const id = Number(c.req.param("id"));
Expand Down
220 changes: 220 additions & 0 deletions src/tools/backfill.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
import type { Env } from "../env";
import { nowIso } from "../db";
import { ingestEvent } from "../consumer";
import { eventsFromDelivery } from "../webhook";
import { type Summarizer, workersAiSummarizer, storePrSummary } from "./summarize";
import { applyEventProgress } from "./progress";

// Admin-triggered server-side GitHub backfill. Unlike scripts/backfill-events.mjs
// (which signs synthetic webhook deliveries with the webhook secret), this runs
// INSIDE the Worker with GITHUB_SERVICE_TOKEN — the same token the scheduled()
// progress recompute uses — so it fetches GitHub REST directly, no webhook secret.
//
// It reconstructs the SAME deliveries the webhook would have received, reuses the
// PURE eventsFromDelivery() derivation (never duplicated here), post-maps each
// event's provenance to "backfill", and writes through the ONE gate fn ingestEvent
// — but with the ADMIN principal as the writer (an authenticated identity), not
// the fixed "github-webhook" string. Downstream projections (PR summaries, issue
// progress) mirror handleGithubWebhook, hung off newly-written events only.

const GH_API = "application/vnd.github+json";
const USER_AGENT = "canopy";
const DAYS_BACK = 14;

export interface BackfillResult {
ok: boolean;
error?: string;
captured: number;
unchanged: number;
prs: number;
issues: number;
}

// Minimal typed views over the GitHub REST list items — only the fields the
// delivery synthesizers below read are modeled; everything else is ignored.
interface GhUserLite {
login: string;
}
interface GhMilestoneLite {
number: number;
open_issues?: number;
closed_issues?: number;
}
interface GhPrListItem {
number: number;
title: string;
body: string | null;
html_url: string;
merged_at: string | null;
closed_at: string | null;
updated_at: string;
user: GhUserLite;
milestone?: GhMilestoneLite | null;
}
interface GhIssueListItem {
number: number;
title: string;
html_url: string;
state: string;
updated_at: string;
user: GhUserLite;
assignees?: GhUserLite[];
assignee?: GhUserLite | null;
labels?: (string | { name: string })[];
milestone?: GhMilestoneLite | null;
pull_request?: unknown; // present only when the "issue" is really a PR
}

// The `rel="next"` URL from a GitHub Link header, or null when there is no next page.
function nextLink(res: Response): string | null {
const link = res.headers.get("link");
const next = link?.split(",").find((part) => part.includes('rel="next"'));
return next ? (next.match(/<([^>]+)>/)?.[1] ?? null) : null;
}

// Synthesize the delivery bodies eventsFromDelivery reads — SAME raw slice shapes
// as scripts/backfill-events.mjs (test/fixtures/*.json). PR list items carry no
// `merged` boolean (that's single-PR-fetch only), so derive it from merged_at.
function prClosedDelivery(pr: GhPrListItem) {
return {
action: "closed",
number: pr.number,
pull_request: {
number: pr.number,
title: pr.title,
body: pr.body,
html_url: pr.html_url,
merged: pr.merged_at != null,
merged_at: pr.merged_at,
closed_at: pr.closed_at,
user: { login: pr.user.login },
milestone: pr.milestone
? { number: pr.milestone.number, open_issues: pr.milestone.open_issues, closed_issues: pr.milestone.closed_issues }
: null,
},
};
}

function issueDelivery(issue: GhIssueListItem) {
const assignee = issue.assignees?.[0] ?? issue.assignee ?? null;
const action = assignee ? "assigned" : "opened";
return {
action,
...(assignee ? { assignee: { login: assignee.login } } : {}),
issue: {
number: issue.number,
title: issue.title,
html_url: issue.html_url,
state: issue.state,
updated_at: issue.updated_at,
user: { login: issue.user.login },
assignees: (issue.assignees ?? []).map((a) => ({ login: a.login })),
labels: issue.labels ?? [],
milestone: issue.milestone
? { number: issue.milestone.number, open_issues: issue.milestone.open_issues, closed_issues: issue.milestone.closed_issues }
: null,
},
};
}

export async function runBackfill(
env: Env,
principalLogin: string,
opts?: { fetchImpl?: typeof fetch; summarizer?: Summarizer | null; now?: string }
): Promise<BackfillResult> {
const token = env.GITHUB_SERVICE_TOKEN;
const repo = env.GITHUB_REPO;
if (!token || !repo) {
return { ok: false, error: "service token or repo not configured", captured: 0, unchanged: 0, prs: 0, issues: 0 };
}

const doFetch = opts?.fetchImpl ?? fetch;
const summarizer = opts?.summarizer ?? (env.AI ? workersAiSummarizer(env.AI) : null);
const headers = {
authorization: `Bearer ${token}`,
accept: GH_API,
"user-agent": USER_AGENT,
"x-github-api-version": "2022-11-28",
};
const cutoffMs = new Date(opts?.now ?? nowIso()).getTime() - DAYS_BACK * 24 * 60 * 60 * 1000;

// (a) Closed PRs updated in the last 14 days. Sorted updated-desc, so we stop
// paginating at the first item whose updated_at predates the cutoff.
const prList: GhPrListItem[] = [];
{
let url: string | null = `https://api.github.com/repos/${repo}/pulls?state=closed&sort=updated&direction=desc&per_page=100`;
let done = false;
while (url && !done) {
const res: Response = await doFetch(url, { headers });
if (!res.ok) break;
const page = (await res.json()) as GhPrListItem[];
for (const pr of page) {
if (pr.updated_at && new Date(pr.updated_at).getTime() < cutoffMs) {
done = true;
break;
}
prList.push(pr);
}
url = done ? null : nextLink(res);
}
}
Comment on lines +147 to +160

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Non-2xx GitHub responses are silently swallowed and reported as success.

When !res.ok mid-pagination, the loop simply breaks and the function still returns ok: true with whatever partial list was collected so far. A rate-limit (403/429), transient 5xx, or auth failure from GitHub during pagination is indistinguishable from "reached the end of results" — the admin sees a normal success toast with an artificially low prs/issues/captured count instead of an error, and there's no signal to retry.

🛡️ Proposed fix: surface fetch failures instead of silently truncating
     while (url && !done) {
       const res: Response = await doFetch(url, { headers });
-      if (!res.ok) break;
+      if (!res.ok) {
+        return { ok: false, error: `GitHub pulls fetch failed: ${res.status}`, captured, unchanged, prs: prList.length, issues: 0 };
+      }
       const page = (await res.json()) as GhPrListItem[];

(similarly for the issues loop)

Also applies to: 166-177

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tools/backfill.ts` around lines 147 - 160, The pagination loop in
backfill.ts is swallowing non-2xx GitHub responses by breaking on !res.ok and
still returning a successful result with partial data. Update the pagination
logic in the PR/issue fetch paths (the loop using doFetch, nextLink, and the
surrounding backfill function) to surface fetch failures as errors instead of
treating them like end-of-results, so rate limits, auth failures, and 5xx
responses fail the job and do not report ok: true with truncated counts.


// (b) All open issues, paginated. The issues endpoint also returns PRs — those
// carry a `pull_request` field and are not our surface, so skip them.
const issueList: GhIssueListItem[] = [];
{
let url: string | null = `https://api.github.com/repos/${repo}/issues?state=open&per_page=100`;
while (url) {
const res: Response = await doFetch(url, { headers });
if (!res.ok) break;
const page = (await res.json()) as GhIssueListItem[];
for (const issue of page) {
if (issue.pull_request) continue;
issueList.push(issue);
}
url = nextLink(res);
}
}
Comment on lines +164 to +177

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What are the current Cloudflare Workers subrequest limits and CPU time limits for paid plans?

💡 Result:

As of July 3, 2026, the Cloudflare Workers Paid (Standard) plan features the following limits for subrequests and CPU time: Subrequest Limits The default limit for subrequests per invocation is 10,000 [1][2]. This limit can be increased up to 10 million per invocation by configuring the limit in your Worker's Wrangler configuration file [1][3]. CPU Time Limits The maximum CPU time allowed per HTTP request invocation is 5 minutes (300,000 ms) [1][4]. By default, this is set to 30 seconds, but you can adjust this limit up to the 5-minute maximum via your Wrangler configuration or the Cloudflare dashboard [1][5]. For Cron Triggers or Queue Consumer invocations, the maximum CPU time is 15 minutes [4][5]. (Note that for Cron Triggers specifically, the available CPU time may depend on the interval, with 30 seconds for intervals less than 1 hour and 15 minutes for intervals of 1 hour or more [1][6].) You can manage these settings to prevent excessive usage or to accommodate CPU-intensive tasks through your Worker's settings in the Cloudflare dashboard or within your Wrangler configuration file [4][5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/tools/backfill.ts --view expanded || true
printf '\n---\n'
sed -n '1,260p' src/tools/backfill.ts | cat -n

Repository: SaplingLearn/canopy

Length of output: 1924


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files:\n'
git ls-files 'src/tools/backfill.ts' 'src/tools/*' | sed -n '1,50p'
printf '\n--- src/tools/backfill.ts ---\n'
sed -n '1,260p' src/tools/backfill.ts | cat -n
printf '\n--- search for issue pagination helpers ---\n'
rg -n "nextLink|pull_request|issues\\?state=open|updated_at" src/tools -S

Repository: SaplingLearn/canopy

Length of output: 1924


Cap open-issue pagination in backfill
The open-issues pass can page through the entire repo with no explicit limit, so a large backlog can burn through the Worker’s subrequest/CPU budget in /admin/backfill. Add a page/time cap here or move this work off the request path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tools/backfill.ts` around lines 164 - 177, The open-issues pagination in
backfill can run indefinitely through very large repos, so the loop around the
GhIssueListItem collection in backfill.ts needs a hard stop. Update the
fetch/pagination flow in the issueList section to enforce a page or time cap
while following nextLink(res), and exit early once the limit is reached so
/admin/backfill cannot exhaust Worker subrequest/CPU budget. Keep the fix
localized to the backfill pagination logic and the issueList population path.


let captured = 0;
let unchanged = 0;

for (const pr of prList) {
const payload = prClosedDelivery(pr);
for (const base of eventsFromDelivery("pull_request", payload)) {
const ev = { ...base, provenance: "backfill" as const };
const res = await ingestEvent(env.DB, ev, principalLogin);
if (res.outcome === "written") {
captured++;
// Mirror handleGithubWebhook's summary seam: parse THIS PR's own raw and
// store a capture-time summary (storePrSummary never throws).
const parsed = JSON.parse(ev.raw) as { pr: { number: number; title: string; body: string | null } };
await storePrSummary(env.DB, summarizer, {
semantic_key: ev.semantic_key,
pr_number: parsed.pr.number,
title: parsed.pr.title,
body: parsed.pr.body ?? "",
});
} else {
unchanged++;
}
}
}

for (const issue of issueList) {
const payload = issueDelivery(issue);
for (const base of eventsFromDelivery("issues", payload)) {
const ev = { ...base, provenance: "backfill" as const };
const res = await ingestEvent(env.DB, ev, principalLogin);
if (res.outcome === "written") {
captured++;
// Mirror handleGithubWebhook's progress seam for newly-written issues.
await applyEventProgress(env.DB, payload);
} else {
unchanged++;
}
}
}

return { ok: true, captured, unchanged, prs: prList.length, issues: issueList.length };
}
Loading