diff --git a/.agents/skills/README.md b/.agents/skills/README.md index 9dbf3737a..e80b3e107 100644 --- a/.agents/skills/README.md +++ b/.agents/skills/README.md @@ -13,9 +13,12 @@ mkdir -p .claude/skills && for d in .agents/skills/*/; do n=$(basename "$d"); ln | Skill | Source | License | |---|---|---| -| `screenshot` | this repo, wraps `scripts/screenshot.mjs` | MIT (repo) | -| `grill-me`, `codebase-design`, `improve-codebase-architecture` | github.com/mattpocock/skills | MIT | +| `screenshot` | this repo, wraps `scripts/screenshot.mjs` | AGPL-3.0 (repo) | +| `grill-me`, `codebase-design` | github.com/mattpocock/skills | MIT | -To ship another one: install it, then add `!.agents/skills//` and -`!.claude/skills/` to `.gitignore`, and a row here with its license. -Update a vendored skill by re-installing and committing the diff. +To ship another one: install it, add `!.agents/skills//` to +`.gitignore`, and a row here with its license. Only self-contained skills +qualify: one that invokes another skill by name (`/grilling`, +`/domain-modeling`) breaks in a clone that lacks it. Update a vendored skill +by re-installing and committing the diff; do not edit vendored text, raise it +upstream. diff --git a/.agents/skills/improve-codebase-architecture/HTML-REPORT.md b/.agents/skills/improve-codebase-architecture/HTML-REPORT.md deleted file mode 100644 index 17f6d2c7b..000000000 --- a/.agents/skills/improve-codebase-architecture/HTML-REPORT.md +++ /dev/null @@ -1,123 +0,0 @@ -# HTML Report Format - -The architectural review is rendered as a single self-contained HTML file in the OS temp directory. Tailwind and Mermaid both come from CDNs. Mermaid handles graph-shaped diagrams reliably; hand-built divs and inline SVG handle the more editorial visuals (mass diagrams, cross-sections). Mix the two — don't lean on Mermaid for everything, it'll start to look generic. - -## Scaffold - -```html - - - - - Architecture review — {{repo name}} - - - - - -
-
...
-
...
-
...
-
- - -``` - -## Header - -Repo name, date, and a compact legend: solid box = module, dashed line = seam, red arrow = leakage, thick dark box = deep module. No introduction paragraph — straight into the candidates. - -## Candidate card - -The diagrams carry the weight. Prose is sparse, plain, and uses the glossary terms (from the `/codebase-design` skill) without ceremony. - -Each candidate is one `
`: - -- **Title** — short, names the deepening (e.g. "Collapse the Order intake pipeline"). -- **Badge row** — recommendation strength (`Strong` = emerald, `Worth exploring` = amber, `Speculative` = slate), plus a tag for the dependency category (`in-process`, `local-substitutable`, `ports & adapters`, `mock`). -- **Files** — monospaced list, `font-mono text-sm`. -- **Before / After diagram** — the centrepiece. Two columns, side by side. See patterns below. -- **Problem** — one sentence. What hurts. -- **Solution** — one sentence. What changes. -- **Wins** — bullets, ≤6 words each. e.g. "Tests hit one interface", "Pricing logic stops leaking", "Delete 4 shallow wrappers". -- **ADR callout** (if applicable) — one line in an amber-tinted box. - -No paragraphs of explanation. If the diagram needs a paragraph to be understood, redraw the diagram. - -## Diagram patterns - -Pick the pattern that fits the candidate. Mix them. Don't make every diagram look the same — variety is part of the point. - -### Mermaid graph (the workhorse for dependencies / call flow) - -Use a Mermaid `flowchart` or `graph` when the point is "X calls Y calls Z, and look at the mess." Wrap it in a Tailwind-styled card so it doesn't feel parachuted in. Style with classDef to colour leakage edges red and the deep module dark. Sequence diagrams work well for "before: 6 round-trips; after: 1." - -```html -
- 
-    flowchart LR
-      A[OrderHandler] --> B[OrderValidator]
-      B --> C[OrderRepo]
-      C -.leak.-> D[PricingClient]
-      classDef leak stroke:#dc2626,stroke-width:2px;
-      class C,D leak
-
-
-``` - -### Hand-built boxes-and-arrows (when Mermaid's layout fights you) - -Modules as `
`s with borders and labels. Arrows as inline SVG `` or `` elements positioned absolutely over a relative container. Reach for this when you want the "after" diagram to feel like one thick-bordered deep module with greyed-out internals — Mermaid won't render that with the right weight. - -### Cross-section (good for layered shallowness) - -Stack horizontal bands (`h-12 border-l-4`) to show layers a call passes through. Before: 6 thin layers each doing nothing. After: 1 thick band labelled with the consolidated responsibility. - -### Mass diagram (good for "interface as wide as implementation") - -Two rectangles per module — one for interface surface area, one for implementation. Before: interface rectangle is nearly as tall as the implementation rectangle (shallow). After: interface rectangle is short, implementation rectangle is tall (deep). - -### Call-graph collapse - -Before: a tree of function calls rendered as nested boxes. After: the same tree collapsed into one box, with the now-internal calls shown faded inside it. - -## Style guidance - -- Lean editorial, not corporate-dashboard. Generous whitespace. Serif optional for headings (`font-serif` works well with stone/slate). -- Colour sparingly: one accent (emerald or indigo) plus red for leakage and amber for warnings. -- Keep diagrams ~320px tall so before/after sits comfortably side by side without scrolling. -- Use `text-xs uppercase tracking-wider` for module labels inside diagrams — they should read as schematic, not as UI. -- The only scripts are the Tailwind CDN and the Mermaid ESM import. The report is otherwise static — no app code, no interactivity beyond Mermaid's own rendering. - -## Top recommendation section - -One larger card. Candidate name, one sentence on why, anchor link to its card. That's it. - -## Tone - -Plain English, concise — but the architectural nouns and verbs come straight from the `/codebase-design` skill. Concision is not an excuse to drift. - -**Use exactly:** module, interface, implementation, depth, deep, shallow, seam, adapter, leverage, locality. - -**Never substitute:** component, service, unit (for module) · API, signature (for interface) · boundary (for seam) · layer, wrapper (for module, when you mean module). - -**Phrasings that fit the style:** - -- "Order intake module is shallow — interface nearly matches the implementation." -- "Pricing leaks across the seam." -- "Deepen: one interface, one place to test." -- "Two adapters justify the seam: HTTP in prod, in-memory in tests." - -**Wins bullets** name the gain in glossary terms: *"locality: bugs concentrate in one module"*, *"leverage: one interface, N call sites"*, *"interface shrinks; implementation absorbs the wrappers"*. Don't write *"easier to maintain"* or *"cleaner code"* — those terms aren't in the glossary and don't earn their place. - -No hedging, no throat-clearing, no "it's worth noting that…". If a sentence could be a bullet, make it a bullet. If a bullet could be cut, cut it. If a term isn't in the `/codebase-design` glossary, reach for one that is before inventing a new one. diff --git a/.agents/skills/improve-codebase-architecture/SKILL.md b/.agents/skills/improve-codebase-architecture/SKILL.md deleted file mode 100644 index a79b493ea..000000000 --- a/.agents/skills/improve-codebase-architecture/SKILL.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -name: improve-codebase-architecture -description: Scan a codebase for deepening opportunities, present them as a visual HTML report, then grill through whichever one you pick. -disable-model-invocation: true ---- - -# Improve Codebase Architecture - -Surface architectural friction and propose **deepening opportunities** — refactors that turn shallow modules into deep ones. The aim is testability and AI-navigability. - -This command is _informed_ by the project's domain model and built on a shared design vocabulary: - -- Run the `/codebase-design` skill for the architecture vocabulary (**module**, **interface**, **depth**, **seam**, **adapter**, **leverage**, **locality**) and its principles (the deletion test, "the interface is the test surface", "one adapter = hypothetical seam, two = real"). Use these terms exactly in every suggestion — don't drift into "component," "service," "API," or "boundary." -- The domain language in `CONTEXT.md` gives names to good seams; ADRs in `docs/adr/` record decisions this command should not re-litigate. - -## Process - -### 1. Explore - -Read the project's domain glossary (`CONTEXT.md`) and any ADRs in the area you're touching first. - -Then use the Agent tool with `subagent_type=Explore` to walk the codebase. Don't follow rigid heuristics — explore organically and note where you experience friction: - -- Where does understanding one concept require bouncing between many small modules? -- Where are modules **shallow** — interface nearly as complex as the implementation? -- Where have pure functions been extracted just for testability, but the real bugs hide in how they're called (no **locality**)? -- Where do tightly-coupled modules leak across their seams? -- Which parts of the codebase are untested, or hard to test through their current interface? - -Apply the **deletion test** to anything you suspect is shallow: would deleting it concentrate complexity, or just move it? A "yes, concentrates" is the signal you want. - -### 2. Present candidates as an HTML report - -Write a self-contained HTML file to the OS temp directory so nothing lands in the repo. Resolve the temp dir from `$TMPDIR`, falling back to `/tmp` (or `%TEMP%` on Windows), and write to `/architecture-review-.html` so each run gets a fresh file. Open it for the user — `xdg-open ` on Linux, `open ` on macOS, `start ` on Windows — and tell them the absolute path. - -The report uses **Tailwind via CDN** for layout and styling, and **Mermaid via CDN** for diagrams where a graph/flow/sequence reliably communicates the structure. Mix Mermaid with hand-crafted CSS/SVG visuals — use Mermaid when relationships are graph-shaped (call graphs, dependencies, sequences), and hand-built divs/SVG when you want something more editorial (mass diagrams, cross-sections, collapse animations). Each candidate gets a **before/after visualisation**. Be visual. - -For each candidate, render a card with: - -- **Files** — which files/modules are involved -- **Problem** — why the current architecture is causing friction -- **Solution** — plain English description of what would change -- **Benefits** — explained in terms of locality and leverage, and how tests would improve -- **Before / After diagram** — side-by-side, custom-drawn, illustrating the shallowness and the deepening -- **Recommendation strength** — one of `Strong`, `Worth exploring`, `Speculative`, rendered as a badge - -End the report with a **Top recommendation** section: which candidate you'd tackle first and why. - -**Use CONTEXT.md vocabulary for the domain, and the `/codebase-design` vocabulary for the architecture.** If `CONTEXT.md` defines "Order," talk about "the Order intake module" — not "the FooBarHandler," and not "the Order service." - -**ADR conflicts**: if a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant revisiting the ADR. Mark it clearly in the card (e.g. a warning callout: _"contradicts ADR-0007 — but worth reopening because…"_). Don't list every theoretical refactor an ADR forbids. - -See [HTML-REPORT.md](HTML-REPORT.md) for the full HTML scaffold, diagram patterns, and styling guidance. - -Do NOT propose interfaces yet. After the file is written, ask the user: "Which of these would you like to explore?" - -### 3. Grilling loop - -Once the user picks a candidate, run the `/grilling` skill to walk the design tree with them — constraints, dependencies, the shape of the deepened module, what sits behind the seam, what tests survive. - -Side effects happen inline as decisions crystallize — run the `/domain-modeling` skill to keep the domain model current as you go: - -- **Naming a deepened module after a concept not in `CONTEXT.md`?** Add the term to `CONTEXT.md`. Create the file lazily if it doesn't exist. -- **Sharpening a fuzzy term during the conversation?** Update `CONTEXT.md` right there. -- **User rejects the candidate with a load-bearing reason?** Offer an ADR, framed as: _"Want me to record this as an ADR so future architecture reviews don't re-suggest it?"_ Only offer when the reason would actually be needed by a future explorer to avoid re-suggesting the same thing — skip ephemeral reasons ("not worth it right now") and self-evident ones. -- **Want to explore alternative interfaces for the deepened module?** Run the `/codebase-design` skill and use its design-it-twice parallel sub-agent pattern. diff --git a/.agents/skills/screenshot/SKILL.md b/.agents/skills/screenshot/SKILL.md index 9fb8f6a3d..90e57a416 100644 --- a/.agents/skills/screenshot/SKILL.md +++ b/.agents/skills/screenshot/SKILL.md @@ -35,12 +35,13 @@ It prints the PNG paths. Read them with the Read tool and judge against dashboard at it with `DATABASE_URL`. 3. Auth, dashboard only. The seed (`pnpm --filter @openstatus/db seed`, through `devbox run --` because of the deno pin) creates two maintainers - with sessions that do not expire: `max@openstatus.dev` (token + with sessions that expire in 2099: `max@openstatus.dev` (token `seed-session-max`, owner of workspace 1 `love-openstatus`) and `thibault@openstatus.dev` (token `seed-session-thibault`, admin of 1). Pass `--user max@openstatus.dev` and the script finds the row, or skip the DB with `--token seed-session-max --workspace love-openstatus`. Without - `--user` it takes the newest live session of anyone. No session at all: + `--user` it takes any live session, and the two seeded ones tie, so name + the user when identity matters. No session at all: `--login --log ` submits the email form, follows the `>>> Magic Link:` line the dev server prints and saves a storage state next to the output. A database that predates the seed change has no `max@` @@ -59,7 +60,8 @@ It prints the PNG paths. Read them with the Read tool and judge against | `--full-page` | off | full-page capture when no selector | | `--base` | per app | override the dev server URL | | `--db` | `http://127.0.0.1:8080` | sqld HTTP endpoint for the session lookup | -| `--user`, `--workspace` | | narrow the session lookup | +| `--user` | | pick the session of this email | +| `--workspace` | first membership | slug for the `workspace-slug` cookie | | `--token` | | explicit session token, skips the lookup | | `--login`, `--log` | | magic-link login fallback | @@ -75,9 +77,9 @@ It prints the PNG paths. Read them with the Read tool and judge against ## Fixtures Seeded, workspace 1 `love-openstatus`: incident #1 (`/incidents/1`, declared -by Thibault, commander Max, seven events covering declare, Slack bind, linked -report #2, a Slack note, a commander change, a mitigation with a note and a -plain note), status reports #1 (resolved) and #2 (linked), maintenances, status pages +by Thibault, commander Max, eight events covering declare, Slack bind, linked +report #2, a Slack note, a commander change, a mitigation with a note, a +plain note and a Slack unbind), status reports #1 (resolved) and #2 (linked), maintenances, status pages `/status` (page 1, with components) and `/acme` (page 2). Safe to mutate; re-seed to reset. diff --git a/.gitignore b/.gitignore index c437271db..a83d9198f 100644 --- a/.gitignore +++ b/.gitignore @@ -91,7 +91,6 @@ plan.md !.agents/skills/screenshot/ !.agents/skills/grill-me/ !.agents/skills/codebase-design/ -!.agents/skills/improve-codebase-architecture/ skills-lock.json .screenshots/ diff --git a/DESIGN.md b/DESIGN.md index a09cdc4f9..f7989e5fd 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -29,8 +29,8 @@ rule, and the exception is in review. | Layer | Path | Rule | |---|---|---| -| shadcn primitives | `packages/ui/src/components/ui/` | Never customized. See `packages/ui/AGENTS.md`. | -| Public blocks | `packages/ui/src/components/blocks/` | Published registry. No app imports. | +| shadcn primitives | `packages/ui/src/components/ui/` | Stock. Edited only as a documented last resort, see `packages/ui/AGENTS.md`. | +| Public blocks | `packages/ui/src/components/blocks/` | Published registry. Imports nothing from an app; apps import them freely. | | Shared custom | `packages/ui/src/components/custom/` | Ours, used by more than one app. | | App kits | `apps/*/src/components/content/`, `common/` | Compound kits a whole app composes from. | | Feature rows | `apps/*/src/components//` | What a feature renders with the kits. | @@ -120,7 +120,7 @@ primitive (`docs/adr/0003`). ## Color - Tokens only: `background`, `foreground`, `muted`, `muted-foreground`, - `border`, `success`, `warning`, `info`, `destructive`. They are OKLCH in + `border`, `ring`, `success`, `warning`, `info`, `destructive`. They are OKLCH in `packages/ui/src/globals.css`. Never a raw palette class (`text-emerald-600`). - Dark mode comes from the tokens. No `dark:` color overrides in components. Images in content pages get a `.dark` sibling file. @@ -272,10 +272,12 @@ lines, and a last row clipped mid-glyph. ## Verify by looking An agent cannot judge rendering from a diff. For any visible change, -`scripts/screenshot.mjs` (the `screenshot` skill) shoots the element -(`[data-slot=timeline]`, not the viewport) in both themes on the local dev -server of the dashboard, the marketing site or the status page, logging in -with a seeded session where needed. Compare against the previous state. Feedback +`scripts/screenshot.mjs` (the `screenshot` skill) shoots the element, not the +viewport, in both themes on the local dev server of the dashboard, the +marketing site or the status page, logging in with a seeded session where +needed. Selectors differ per app: `[data-slot=timeline]` and the other +dashboard slots, `main` or a section on the marketing site, the block slots +from `packages/ui/src/components/blocks/` on the status page. Compare against the previous state. Feedback goes back the same way: a screenshot with the problem boxed and one sentence. Marketing and docs pages render every component on `/kitchen-sink` so drift is visible there first. diff --git a/apps/dashboard/AGENTS.md b/apps/dashboard/AGENTS.md index 181b51c63..1e0aca814 100644 --- a/apps/dashboard/AGENTS.md +++ b/apps/dashboard/AGENTS.md @@ -9,7 +9,8 @@ visible behaviour, screenshot it before reporting it done: node scripts/screenshot.mjs --app dashboard /incidents/1 --selector "[data-slot=timeline]" ``` -It runs system Chrome through Playwright against the local dev server, logs in +It runs system Chrome (or Playwright's bundled Chromium where there is none) +against the local dev server, logs in with a seeded session (`--user max@openstatus.dev` or `thibault@openstatus.dev`), shoots light and dark, and prints the PNG paths. Look at them. The `screenshot` skill has the flags, ports and fixtures; the diff --git a/packages/db/src/seed.mts b/packages/db/src/seed.mts index eadffc9e2..82e5a25cc 100644 --- a/packages/db/src/seed.mts +++ b/packages/db/src/seed.mts @@ -210,9 +210,8 @@ async function main() { .onConflictDoNothing() .run(); - // The two maintainers, with sessions that never expire locally, so the - // dashboard and apps/dashboard/scripts/screenshot.mjs log in without a - // magic link. Tokens are fixed on purpose: this seed only runs against + // The two maintainers, with sessions that outlive any local database, so + // the dashboard and scripts/screenshot.mjs log in without a magic link. Tokens are fixed on purpose: this seed only runs against // localhost. await db .insert(user) @@ -250,12 +249,12 @@ async function main() { ]) .onConflictDoNothing() .run(); - const inOneYear = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000); + const farFuture = new Date("2099-01-01T00:00:00Z"); await db .insert(session) .values([ - { sessionToken: "seed-session-max", userId: 2, expires: inOneYear }, - { sessionToken: "seed-session-thibault", userId: 3, expires: inOneYear }, + { sessionToken: "seed-session-max", userId: 2, expires: farFuture }, + { sessionToken: "seed-session-thibault", userId: 3, expires: farFuture }, ]) .onConflictDoNothing() .run(); @@ -626,6 +625,14 @@ async function main() { createdBy: 2, createdAt: minutesAfterDeclare(75), }, + // Unbound again so the timeline and the incident's (empty) Slack binding agree. + { + incidentId: 1, + type: "slack_channel_unbound", + message: "Slack channel <#C0C7Y4CG6F2> unbound", + createdBy: null, + createdAt: minutesAfterDeclare(80), + }, ]) .run(); diff --git a/scripts/screenshot.mjs b/scripts/screenshot.mjs index 59093fad4..451a97a0d 100644 --- a/scripts/screenshot.mjs +++ b/scripts/screenshot.mjs @@ -15,7 +15,8 @@ // with --user), a saved storage state, or --login, which submits the email // form and follows the `>>> Magic Link:` line the dev server prints to --log. // Playwright is borrowed from apps/screenshot-service so the version follows -// that package; the browser is the system Chrome. +// that package; the browser is the system Chrome, or Playwright's bundled +// Chromium where there is none. import { existsSync, mkdirSync, readFileSync } from "node:fs"; import { createRequire } from "node:module"; @@ -34,6 +35,7 @@ const APPS = { "status-page": { base: "http://localhost:3000", auth: false }, }; +const BOOLEAN_FLAGS = new Set(["full-page"]); const args = process.argv.slice(2); const flags = {}; const paths = []; @@ -42,7 +44,8 @@ for (let i = 0; i < args.length; i++) { if (a.startsWith("--")) { const key = a.slice(2); const next = args[i + 1]; - if (next === undefined || next.startsWith("--")) flags[key] = true; + if (BOOLEAN_FLAGS.has(key) || next === undefined || next.startsWith("--")) + flags[key] = true; else flags[key] = args[++i]; } else paths.push(a); } @@ -128,7 +131,10 @@ async function magicLinkLogin(context) { if (!flags.log) throw new Error("--login needs --log "); const page = await context.newPage(); await page.goto(`${base}/login`); + // The form opens closed; the email input appears after the mode button. const input = page.locator('input[name="email"]'); + if (!(await input.isVisible())) + await page.getByRole("button", { name: "Continue with email" }).click(); await input.fill(flags.login); const seen = readFileSync(flags.log, "utf8").length; await input.press("Enter"); @@ -179,32 +185,47 @@ else if (wantsAuth && !flags.login) auth = await sessionCookies().catch( (e) => (console.error(`auth: ${e.message}`), null), ); -if (wantsAuth && !auth && !flags.login && !existsSync(stateFile)) { +// One login for all themes: the dashboard rate-limits magic-link emails. +if (flags.login) { + const context = await browser.newContext(); + await magicLinkLogin(context); + await context.close(); +} +const useState = wantsAuth && !auth && existsSync(stateFile); +if (wantsAuth && !auth && !useState) { console.error( "auth: no session found; seed the database, pass --token, or --login --log ", ); } const written = []; +process.on("uncaughtException", async (e) => { + console.error(e.message); + await browser.close().catch(() => {}); + process.exit(1); +}); for (const theme of themes) { const context = await browser.newContext({ viewport: { width, height: 1000 }, deviceScaleFactor: 2, colorScheme: theme, - ...(wantsAuth && !auth && !flags.login && existsSync(stateFile) - ? { storageState: stateFile } - : {}), + ...(useState ? { storageState: stateFile } : {}), }); if (auth) await context.addCookies(auth); - if (flags.login) await magicLinkLogin(context); // Every app uses next-themes with the default storage key. await context.addInitScript((t) => localStorage.setItem("theme", t), theme); const page = await context.newPage(); for (const path of paths) { await page.goto(base + path, { waitUntil: "networkidle" }); - if (app.auth && page.url().includes("/login")) - throw new Error(`redirected to login for ${path}; auth failed`); + // Only the dashboard has a login page, so landing there means auth + // failed or --base points at the dashboard for another app. + if (page.url().includes("/login")) + throw new Error( + app.auth + ? `redirected to login for ${path}; auth failed` + : `${base} serves the dashboard, not ${flags.app}; pass --base`, + ); const name = path.replace(/^\/+/, "").replace(/[^a-z0-9]+/gi, "-") || "root"; if (flags.selector) { @@ -220,8 +241,10 @@ for (const theme of themes) { written.push(file); k++; } - if (k === 0) + if (k === 0) { console.error(`no visible match for ${flags.selector} on ${path}`); + process.exitCode = 1; + } } else { const file = join(out, `${name}-${theme}.png`); await page.screenshot({