diff --git a/.claude/agents/impl-agent.md b/.claude/agents/impl-agent.md index 881b2fb9..8f22af95 100644 --- a/.claude/agents/impl-agent.md +++ b/.claude/agents/impl-agent.md @@ -97,11 +97,14 @@ gh issue edit N --repo SGAOperations/aplio --remove-label "plan approved" --add- --title "#N " \ --body-file .temp/pr-N.md \ --assignee "" \ + --label "claude" \ --head N-ticket-name-in-kebab-case ``` `--assignee` is the **issue's assignee login recorded in pre-flight** (`@me` if the issue had none) — the PR must carry the same owner as its issue or it never shows up in that operator's `ready for review` query. **Substitute the literal login string you read in pre-flight; never `$(...)` command substitution** — it isn't allow-listed and would silently produce an empty argument. + `--label "claude"` is what **activates the approval gate**: `.github/workflows/approval-check.yml` runs only on PRs carrying `claude`, so a PR opened without it merges with **no** approval gate at all. + **Always target `dev`** (the integration branch) — never open feature PRs against `main`. Releases flow `dev → main` via the `/release` command. Note the PR number returned. diff --git a/.claude/agents/plan-agent.md b/.claude/agents/plan-agent.md index 5da2d54f..561f4c99 100644 --- a/.claude/agents/plan-agent.md +++ b/.claude/agents/plan-agent.md @@ -68,6 +68,14 @@ Construct the **full new issue body** (original ticket description preserved on gh issue edit N --repo SGAOperations/aplio --body-file .temp/plan-N.md ``` +**Session-required declaration.** Some tickets can't be handed to a dispatched agent at all. Today that means any plan whose **## Changes** touches `CLAUDE.md` or `.claude/**` — the harness blocks a subagent's `Edit` there — but the marker is deliberately about the _routing_, not the cause, so future categories reuse it. For those plans, the body's **first line, directly under the `## Implementation Plan` heading and before `## Overview`**, is the marker, with the reason after the colon: + +``` +> **SESSION REQUIRED:** touches `CLAUDE.md` / `.claude/**` — a dispatched agent can't edit those +``` + +The literal string `SESSION REQUIRED` is what the cockpit greps for — **never reword it**. The reason after the colon is free text and is the part that generalizes. Emit it in **revision mode** too, since a revised plan can change the routing, and never emit it for a plan that doesn't need a session. Full rules: `.claude/docs/PIPELINE.md` → "Session-required tickets". + **Use the fixed structure** in `.claude/docs/PIPELINE.md` → "Implementation plan" — the canonical section list, order, and writing style. Think through how the feature should actually work and look (it's a product/UX design, not just a file checklist), but write it tight: bullets, short sentences, **don't restate the ticket**, omit sections that don't apply. The plan must still _decide_ the substance — even though it's brief: - **Design each UX state** (happy + unhappy/edge), layout/hierarchy, key interactions, and the actual **copy** — in the **## UX states** section (only if there's UI). diff --git a/.claude/docs/DESIGN.md b/.claude/docs/DESIGN.md index 517da825..2fdbc5e9 100644 --- a/.claude/docs/DESIGN.md +++ b/.claude/docs/DESIGN.md @@ -45,7 +45,15 @@ Any change to a brand/status token must keep ≥4.5:1 contrast against its paire - **Radius:** `--radius: 0.75rem`. Use `rounded-md`/`rounded-lg`/`rounded-xl` (derived from the scale); don't use arbitrary radii. Inputs/buttons/cards inherit the shadcn defaults. - **Spacing:** Tailwind 4-point scale. Card padding `p-6` (compact `p-4`); stack gaps `gap-4`/`gap-6`; form field gap `gap-2`. Be consistent rather than pixel-tuning. -- **Containers:** constrain reading width (`max-w-*`), center with `mx-auto`; full-bleed only for tables/dashboards. +- **Page width tiers:** every route inside the app shell picks exactly one tier for its top-level container. `app-shell.tsx` supplies only `p-6`, so the page owns the width. + + | Tier | Container classes | Use for | + | -------------- | ------------------- | ----------------------------------------------------------------------------------------------------------------------- | + | **Full-bleed** | none (no `max-w`) | list, table and dashboard pages — `/`, `/positions`, `/users`, `/applications`, `/my-applications`, `/global-questions` | + | **Wide** | `mx-auto max-w-5xl` | two-column review/detail pages — `/applications/[id]`, `/my-applications/[id]` | + | **Narrow** | `mx-auto max-w-2xl` | single-column forms and reading views — `/profile`, `/positions/[id]/apply`, `/positions/[id]/edit` | + + There is no fourth tier: `max-w-6xl`/`4xl`/`3xl` on a page container is a bug. Inside a full-bleed page, constraining an individual prose block (`max-w-2xl` on a description) is correct and not a tier violation — see `/positions/[id]`. A route's `loading.tsx` must use the same tier as its `page.tsx`, or the skeleton shifts on resolve. Routes outside the app shell (`(legal)`, `login`) set their own width. ## 5. Components diff --git a/.claude/docs/PIPELINE.md b/.claude/docs/PIPELINE.md index 47f956aa..47d2a13a 100644 --- a/.claude/docs/PIPELINE.md +++ b/.claude/docs/PIPELINE.md @@ -11,7 +11,7 @@ claude # open a session (haiku recommended for the cockpit) Then talk to it: `work on #142` · `scope out a notifications feature` · `status` · `pause #142` · `retry #142` · `drain` · `resume` · `stop #142`. -## The two flows +## The flows ### Major feature @@ -22,6 +22,10 @@ Then talk to it: `work on #142` · `scope out a notifications feature` · `statu | 3. Approve plan | Read the summary, approve — or give feedback (it revises and comes back) | `impl-agent` builds in an isolated worktree, runs CI, opens a PR; `review-agent`/`revise-agent` loop until clean (max 5 rounds) | | 4. Merge | Click merge on GitHub | Issue closes automatically | +### Session-required ticket + +Same as above through step 3, then it diverges: some tickets can't be handed to an agent at all (today, any touching `CLAUDE.md` or `.claude/**`). Their plan carries a **`SESSION REQUIRED`** marker, and at step 3 the cockpit **announces instead of dispatching** — it hands you a launch command and you run **`/implement ` in a separate session**. Nothing moves until you do; no agent will ever pick it up. Review (step 3's second half) and merge are unchanged. Full detail: "Session-required tickets". + ### Bug fix File the issue → in the cockpit: "work on #N, auto-approve the plan" → wait → merge on GitHub. The plan gate is skipped (`auto plan`); the merge gate never is. @@ -112,16 +116,17 @@ Rule: **every stage agent's first action is swapping its trigger label for its i ### PR labels -| Label | Set by | Type | Meaning | -| ------------------ | ----------------------------- | --------- | ------------------------------------------------------------------------------------------------ | -| `ready for review` | `impl-agent` / `revise-agent` | trigger | Dispatch `review-agent` | -| `reviewing` | `review-agent` | in-flight | Review underway | -| `needs revision` | `review-agent` | trigger | Dispatch `revise-agent` (subject to cycle cap) | -| `revising` | `revise-agent` | in-flight | Fixes underway | -| `approved` | `review-agent` | terminal | Findings are at/under the current cycle's bar (escalating, below); human merges on GitHub | -| `needs human` | Cockpit / `revise-agent` | gate | 5 cycles without convergence, or an ambiguous rebase conflict needing the author; pipeline stops | -| `refresh branch` | Cockpit / human | trigger | Dispatch `revise-agent` in refresh mode — rebase onto base and force-push, no code changes | -| `refreshing` | `revise-agent` | in-flight | Branch refresh underway; the PR's other labels (e.g. `approved`) are left in place | +| Label | Set by | Type | Meaning | +| ------------------ | -------------------------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------- | +| `claude` | `impl-agent` (at `gh pr create`) | marker | Pipeline owns this PR — activates the `approved` merge gate in `approval-check.yml`; a pipeline PR missing it merges ungated | +| `ready for review` | `impl-agent` / `revise-agent` | trigger | Dispatch `review-agent` | +| `reviewing` | `review-agent` | in-flight | Review underway | +| `needs revision` | `review-agent` | trigger | Dispatch `revise-agent` (subject to cycle cap) | +| `revising` | `revise-agent` | in-flight | Fixes underway | +| `approved` | `review-agent` | terminal | Findings are at/under the current cycle's bar (escalating, below); human merges on GitHub | +| `needs human` | Cockpit / `revise-agent` | gate | 5 cycles without convergence, or an ambiguous rebase conflict needing the author; pipeline stops | +| `refresh branch` | Cockpit / human | trigger | Dispatch `revise-agent` in refresh mode — rebase onto base and force-push, no code changes | +| `refreshing` | `revise-agent` | in-flight | Branch refresh underway; the PR's other labels (e.g. `approved`) are left in place | ## Stages and models @@ -136,6 +141,8 @@ Rule: **every stage agent's first action is swapping its trigger label for its i All four workers read `.claude/docs/ENGINEERING.md` before working; the review agent treats it as a review dimension. +**Stages 2 and 4 have an operator variant.** Some tickets can't be implemented by a dispatched agent at all — today, any that touch `CLAUDE.md` or `.claude/**`, where the harness denies its `Edit`. Those two stages then run in the operator's own session via `/implement`, following these same agent files — see "Session-required tickets". + ## Permission rationale The model is **broad allow + authoritative deny**: stage agents do real dev work (install packages, read CI logs, manage git in their worktree), so the allowlist grants broad categories and the `deny` list draws the safety line. **Any permission change must update this section.** @@ -158,6 +165,56 @@ Permission mode: every stage agent runs **`permissionMode: dontAsk`** (auto-deny **Mitigation chosen instead of the hook: keep content emitters off the allowlist.** Since redirection can't be blocked, the allowlist itself is the control — so `echo`, `cat`, `head`, `tail`, `cut`, `diff` and `true` are excluded, because their whole purpose is emitting bytes to stdout and a redirect turns each into a clean file-write (`echo "…" > lib/types.ts`, `cat src > dst`). The 11 that remain emit search results, paths, or metadata. **This narrows the bypass; it does not eliminate it** — `grep -v x f > f` still strips lines, any allowed command can truncate a redirect target, and the long-standing `Bash(git *)` allow has always offered write primitives (`git apply`, `git checkout -- …`). So the "write files with the Write/Edit tools" rule above remains a **convention agents are expected to follow, not a technical guarantee** — `plan-agent`'s and `review-agent`'s read-only status rests on them following it. #326 is the real fix whenever it's worth the hook. +**CI merge gate — `approval-check.yml`.** PRs into `dev` are gated on the `approved` label **only when the PR carries `claude`** (a job-level `if:`). Every other PR — human, Dependabot — gets a `skipped` check run, which GitHub counts as satisfied, so it merges on its own merits. This is deliberately **fail-open**: an unlabelled pipeline PR is indistinguishable in CI from a human one and simply loses its gate. Nothing in the workflow can close that, so the mitigations live upstream — `impl-agent` passes `--label "claude"` at `gh pr create` (so the gate is live on the PR's first event) and the cockpit's **ungated-PR sweep** reports any tracked PR missing it. Never narrow the workflow's trigger to exclude a PR: a workflow that never runs creates no check run, leaving the required check pending forever. + +## Session-required tickets + +**Some tickets can't be handed to a dispatched agent at all, so stages 2 and 4 run in the operator's own session.** Today there is exactly one such category — anything touching `CLAUDE.md` or `.claude/**` — but the mechanism is built around the _routing_, not the cause, so a future category reuses it by supplying a different reason. + +**The harness denies `Edit`/`Write` under `.claude/` to dispatched subagents, and `settings.json` cannot grant it back.** This repo already allows `Edit(**)`/`Write(**)` with no `.claude/` deny rule and the denial persists anyway — it sits above project config, so there is nothing to fix in the permission model. An operator's **main session** is unaffected: reading `.claude/agents/impl-agent.md` and acting on it spawns no subagent, so no subagent restriction applies. That asymmetry is the whole basis of this route. + +### The marker + +One string, one rendering, both surfaces — **`SESSION REQUIRED`**, with the reason after the colon: + +``` +> **SESSION REQUIRED:** touches `CLAUDE.md` / `.claude/**` — a dispatched agent can't edit those +``` + +| Surface | Written by | Where | +| --------- | ------------ | --------------------------------------------------------------------------------- | +| **Issue** | `plan-agent` | First line of the plan body, under `## Implementation Plan`, before `## Overview` | +| **PR** | `/implement` | Directly under `Closes #N` in the PR description | + +The literal string `SESSION REQUIRED` is the contract — **never reword it**; the reason after the colon is free text and is the part that generalizes. **There is deliberately no label.** The marker lives in the body on both surfaces, and the cockpit reads it from the `body` field of the trigger query it already runs, so the check costs no extra call and there is nothing to keep in sync: + +```bash +gh issue list --repo SGAOperations/aplio --assignee "@me" --label "plan approved" --json number,title,body +gh pr list --repo SGAOperations/aplio --assignee "@me" --label "needs revision" --json number,title,body +``` + +### The route + +1. **Declare.** `plan-agent` emits the marker when the plan needs it (see "Implementation plan"). +2. **Skip dispatch.** The cockpit finds it in the trigger query's `body` and **announces the command instead of dispatching**. The item keeps its trigger label. +3. **Run.** The operator opens a **named session** and runs `/implement `. That skill resolves the stage from the item's labels, creates a worktree under `.claude/worktrees/impl-`, follows the **unmodified** `impl-agent.md` / `revise-agent.md` plus a short list of subagent-only overrides, and repeats the marker in the PR description it writes. The override list lives in the skill and nowhere else — one place to drift, one place to check. + +**What moves and what doesn't.** Only stages **2** and **4**. `plan-agent` and `review-agent` are read-only and work through `gh`, so stages 1 and 3 run unchanged — a session-required ticket is **not** out of the pipeline. `refresh branch` is still dispatched to `revise-agent` too: a rebase and force-push edit no files, and a conflict inside `CLAUDE.md` / `.claude/docs/**` is already on the never-touch list and escalates. + +**The invariant this deviates from.** Everywhere else a trigger label means something is dispatching. A session-required item **keeps** its trigger label (`plan approved` / `needs revision`) and is **never** dispatched — the cockpit announces instead. Recovery is unchanged (re-apply the trigger), but the label alone no longer implies motion, which is why `status` has to call these out explicitly: nothing else distinguishes one from an item that is genuinely mid-flight. + +**Session naming.** These sessions are long-lived and several run at once, so launch each with the issue number in its display name: + +```bash +claude -n "#503: operator config route" # then, in that session: /implement 503 +``` + +`-n/--name` sets the name shown in the prompt box, the `/resume` picker, and the terminal title. It is settable **only at launch** — a running session can neither read nor change its own name — so the cockpit's announcement hands over the command with the name pre-filled, and the skill re-states the expected string. The name always carries the **issue** number, even when the command takes a PR number. + +**Always in a worktree.** `/implement` never works in the main checkout, for two reasons: editing `.claude/` from the session that is _using_ it mutates your live configuration mid-task, and the ticket may be editing the very agent file the session is following. In a worktree the session reads its instructions by absolute path from the main checkout while every edit lands on the worktree copy, so the committed behaviour holds for the whole run. Reclaim the worktree with `/worktree-clean` once the PR merges. + +**Unverified:** whether the manual escape hatch `claude --agent impl-agent` carries the same restriction (its frontmatter forces `dontAsk` and worktree isolation). Untested — use `/implement` regardless. If a future Claude Code version lifts the harness restriction, this whole route can be deleted; the marker is the only thing to unwind. + ## Pipeline output formats Defined once here; the stage agents follow these exactly. @@ -176,6 +233,7 @@ Every plan, review, summary, and comment is written for a human scanning fast: Appended below the ticket under a `---` then `## Implementation Plan`; revision mode replaces only that block. **Do not restate the ticket** — reference it. Fixed sections in this order; the conditional ones appear **only when they apply** (omit otherwise — no stub): +- **`SESSION REQUIRED` marker** _(only when the ticket can't be dispatched to an agent — today, when it touches `CLAUDE.md` or `.claude/**`)_ — the first line, before `## Overview`. Exact format and rules: "Session-required tickets". - **## Overview** — 2–4 sentences: what, why, the approach. - **## Changes** — files to create/modify, one bullet each: `` `path` — one-line reason ``. - **## Implementation** — ordered `- [ ]` checkboxes, one line each; fold validation / states / error-model notes into the step they belong to. @@ -271,11 +329,13 @@ Closing the cockpit session also halts dispatch (it is the only dispatcher) but | Nothing dispatches for an item | It has no trigger label (paused, in-flight, or gated) | `status` shows where it is; `resume #N` re-applies the right trigger | | Nothing dispatches **and** `status` doesn't list it at all | It is unassigned, or owned by another operator (queries are assignee-filtered) | The tick's unowned sweep reports it — claim it with `work on #N`. If another operator owns it, that's their cockpit's job; `work on #N` offers an explicit take-over | | No GitHub Actions check runs at all on a new push, while `Vercel` still runs | The PR conflicts with its base, so GitHub cannot build the merge ref that `pull_request` workflows run against. Vercel deploys from the head commit, so it is unaffected | `gh pr view --json mergeable` reports `CONFLICTING`. Rebase onto the base branch and force-push; the checks return on the next push | +| `run-approval-check` shows **Skipped** on a pipeline PR | The PR is missing the `claude` label, so the approval gate is inactive | `gh pr edit --repo SGAOperations/aplio --add-label "claude"` — the `labeled` event re-evaluates the job condition and the gate activates on that run | | A stage misbehaved and you want to run it by hand | — | @-mention the subagent (`@agent-impl-agent implement #N`) or run `claude --agent impl-agent` | | Cockpit session closed | All state is in labels | Start `/pipeline` again; it resumes from the labels. `retry #N` anything parked in an in-flight label | | Labels manually changed on GitHub | Fine — labels are the source of truth | The next tick acts on whatever the labels say | | Stale worktrees / orphan `node_modules` dirs accumulating under `.claude/worktrees/` | Agents cut off mid-run; on Windows the harness leaves dirs git can't delete | Run **`/worktree-clean`** from the main checkout — it prunes registrations and force-deletes orphan dirs (`git worktree remove` alone fails with `Invalid argument` once `node_modules` exists). The cockpit reports these but never auto-deletes them. | | An agent stopped with `BLOCKED:` or hit `maxTurns` | Clean stop by design (not a crash) | Resolve the blocker (or widen scope/permissions), then `retry #N` | +| An issue sits at `plan approved`, or a PR at `needs revision`, and nothing dispatches | Its body carries the `SESSION REQUIRED` marker — the cockpit never dispatches for those | Open a named session and run it yourself: `claude -n "#N: "`, then `/implement `. See "Session-required tickets" | ## Reading current state without the cockpit diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md new file mode 100644 index 00000000..22fc4e9a --- /dev/null +++ b/.claude/skills/implement/SKILL.md @@ -0,0 +1,122 @@ +--- +name: implement +description: Run pipeline stage 2 or 4 yourself, in your own session, for a ticket marked SESSION REQUIRED — one that cannot be handed to a dispatched agent, today because it touches CLAUDE.md or .claude/**. Resolves the stage from the item's labels, works in a dedicated worktree, and follows the existing impl-agent / revise-agent definitions unchanged. Manual only. Usage: /implement +disable-model-invocation: true +allowed-tools: Read, Edit, Write, Glob, Grep, Bash, AskUserQuestion +--- + +# Implement — operator-run pipeline stage + +**Trigger:** manual, in an operator's own session. **Input:** an issue or PR number (``). **Repo:** `SGAOperations/aplio`. + +The harness denies `Edit`/`Write` under `.claude/` to **dispatched subagents**, and `settings.json` cannot grant it back — so `impl-agent` and `revise-agent` can't run for tickets that touch `CLAUDE.md` or `.claude/**`. Your session has no such restriction. Those tickets are marked **`SESSION REQUIRED`** in their body, and the cockpit announces them instead of dispatching. This skill is a **thin wrapper**: it points you at the existing agent definition and overrides only the rules that exist because that agent is a subagent. Background and rationale: `.claude/docs/PIPELINE.md` → "Session-required tickets". + +**The agent files are the workflow. Do not modify them, and do not reimplement them here.** + +## 1. Name this session (first output line) + +Derive `#: <2–5 lowercase words>` from the issue title — e.g. `#503: operator config route` — and print it as your first line: + +> Name this session: `#503: operator config route` + +If the session wasn't launched with `claude -n ""`, tell the operator to relaunch with `-n` (or rename in-client, if their client supports it). You can neither read nor set your own display name, so this is an instruction to the human — **state it and move on; never block on it.** In revise mode the name still carries the **issue** number, not the PR number. + +## 2. Record the main checkout + +```bash +git rev-parse --show-toplevel +``` + +Keep that path. **Every instruction file is read by absolute path from there** — `.claude/agents/impl-agent.md`, `.claude/agents/revise-agent.md`, `.claude/docs/ENGINEERING.md`, `.claude/docs/PIPELINE.md`, `CLAUDE.md` — and **every edit goes only to paths inside the worktree.** That split is what makes a ticket that edits `impl-agent.md` safe: you follow the committed version while changing the worktree copy. + +## 3. Resolve the mode from state + +```bash +gh pr view --repo SGAOperations/aplio --json labels,headRefName,baseRefName,title,body +``` + +- Resolves to a PR labeled `needs revision` → **revise mode** (`.claude/agents/revise-agent.md`). +- Otherwise: + + ```bash + gh issue view --repo SGAOperations/aplio --json labels,assignees,title,body + ``` + + Labeled `plan approved` → **impl mode** (`.claude/agents/impl-agent.md`). **Record the issue's assignee login** (`@me` if none) — the PR must carry it. + +- Anything else → stop, report the current labels, change nothing: `# is not awaiting an operator (labels: …). Nothing was changed.` + +**If the item carries the trigger label but its body has no `SESSION REQUIRED` marker,** ask first (AskUserQuestion): _"#412 isn't marked `SESSION REQUIRED`, so the cockpit will dispatch an agent for it too. Proceed anyway / Cancel."_ A double dispatch — cockpit and operator on the same item — is the hazard this guards. + +Then report the resolved mode, worktree path and branch before the slow steps, so the operator can see you picked the right stage. + +## 4. Create the worktree + +Never work in the main checkout: editing `.claude/` from the session using it mutates your live configuration mid-task. Never touch another worktree, and never `--force`. + +**impl mode** — branch straight off `dev` (this replaces the agent's checkout-`main`-then-rebase): + +```bash +git fetch origin +git worktree add -b -ticket-name-in-kebab-case .claude/worktrees/impl- origin/dev +``` + +**revise mode** — detached at the PR's head, rebased onto its base (per `revise-agent.md` step 2; push by refspec at the end): + +```bash +git fetch origin +git worktree add --detach .claude/worktrees/impl- origin/ +``` + +Then `cd` into the worktree and bootstrap it — this is also what activates the commit hooks there: + +```bash +npm ci +npm run prisma:generate +``` + +In revise mode, rebase onto the base branch from inside the worktree: `git rebase origin/`. + +## 5. Follow the agent file + +Read the resolved agent file from the **main checkout** and follow it end to end: label swaps, the plan checklist, the `.temp/commit-msg.txt` commit format, the three CI checks, push by refspec, PR body format, base `dev`, the issue's assignee, thread resolution, the revision note. Also read `.claude/docs/ENGINEERING.md` and its **Pre-PR self-check**, as the agent file requires. + +**Carry the marker into the PR (impl mode).** The cockpit re-reads it on the PR to decide stage 4, so the PR description must repeat it verbatim, directly under `Closes #N`: + +``` +Closes #503 + +> **SESSION REQUIRED:** touches `CLAUDE.md` / `.claude/**` — a dispatched agent can't edit those +``` + +Same literal string as the issue plan, same rendering — `.claude/docs/PIPELINE.md` → "Session-required tickets". **The routing marker is never a label.** The one label `gh pr create` does pass is `claude`, exactly as the agent file specifies — it **activates the approval gate** (`approval-check.yml` runs only on PRs carrying it), so a PR opened without it merges with no gate at all: + +```bash +gh pr create --repo SGAOperations/aplio \ + --base dev \ + --title "# " \ + --body-file .temp/pr-.md \ + --assignee "" \ + --label "claude" \ + --head -ticket-name-in-kebab-case +``` + +## 6. Overrides + +**This is the whole list. Anything not here applies unchanged** — if the agent files gain a rule that only makes sense for a subagent, it belongs here. + +1. **`permissionMode: dontAsk` and "auto-denied silently"** — not your mode. Your session prompts normally. +2. **"STOP and emit `BLOCKED:`"** — pointless with a human present. Ask the operator directly (AskUserQuestion) and wait. Never emit a `BLOCKED:` sentinel, never guess. +3. **"Never spawn subagents"** — not applicable. +4. **The shell-allowlist discipline** (no `cat`/`grep`/`sed`/`find`, bare commands only, no `cd`, quoted cwd-relative paths) — that exists for the subagent's allowlist. Use whatever is clearest. **Still preferred:** `npm run …` over `npx …` for the toolchain, and `git rm` for tracked deletions. +5. **"You are already in a worktree; never run `git worktree`"** — inverted. You create the worktree (§4) and `cd` into it. +6. **Instruction files are read from the recorded main checkout** (§2), not the cwd — the worktree copy may be the thing you are editing. + +## 7. Handoff + +Per the agent file's own Handoff step: + +- **impl** — issue `in progress` → `pr opened`; PR gets `ready for review`. +- **revise** — PR `revising` → `ready for review`. + +The cockpit picks up review on its next tick. Finish by telling the operator the PR URL, the labels applied, and that `/worktree-clean` reclaims `.claude/worktrees/impl-` once the PR merges. diff --git a/.claude/skills/pipeline/SKILL.md b/.claude/skills/pipeline/SKILL.md index 1c889f54..73b1aa26 100644 --- a/.claude/skills/pipeline/SKILL.md +++ b/.claude/skills/pipeline/SKILL.md @@ -18,6 +18,7 @@ You are the orchestrator of the agent pipeline in `.claude/docs/PIPELINE.md`. Th - Never act on issues/PRs that lack a pipeline **trigger** label — opt-in is human-initiated. - **Never act on an item assigned to another operator** — ownership transfers only through an explicit human take-over (see Ownership). - Never dispatch for an item with an **in-flight** label (`planning`, `in progress`, `reviewing`, `revising`, `refreshing`) — an agent owns it or a human paused it. +- **Never dispatch `impl-agent` or `revise-agent` for an item whose body carries the `SESSION REQUIRED` marker** — those tickets can't be handed to an agent (today: they touch `CLAUDE.md` / `.claude/**`, which the harness won't let a subagent edit). **Announce it instead, and tell the human to run `/implement` in a separate session — never this one** (see Session-required items). `review-agent` and refresh-mode dispatches are never gated by it. - Every dispatch runs in the background (`run_in_background: true`). Worktree isolation, model, tool scope, and **permission mode (`dontAsk` — auto-denies anything not allow-listed)** all come from the subagent definition in `.claude/agents/` — you do not set them at the call site. (Since CC v2.1.186 a background subagent's prompts surface to you unless it runs `dontAsk` **and** this session is in default mode — see Model & permission mode.) - **Respect the draining flag:** while draining (see Stop controls), dispatch nothing new and schedule no wakeup; only report state and relay completions. @@ -35,9 +36,9 @@ On start and on every wakeup, run one polling pass: # Trigger labels → dispatch — this operator's items only gh issue list --repo SGAOperations/aplio --assignee "@me" --label "ready" --json number,title gh issue list --repo SGAOperations/aplio --assignee "@me" --label "plan changes requested" --json number,title -gh issue list --repo SGAOperations/aplio --assignee "@me" --label "plan approved" --json number,title +gh issue list --repo SGAOperations/aplio --assignee "@me" --label "plan approved" --json number,title,body gh pr list --repo SGAOperations/aplio --assignee "@me" --label "ready for review" --json number,title -gh pr list --repo SGAOperations/aplio --assignee "@me" --label "needs revision" --json number,title +gh pr list --repo SGAOperations/aplio --assignee "@me" --label "needs revision" --json number,title,body gh pr list --repo SGAOperations/aplio --assignee "@me" --label "refresh branch" --json number,title # Gates and announcements → talk to the human — this operator's items only @@ -49,9 +50,12 @@ gh pr list --repo SGAOperations/aplio --assignee "@me" --label "needs human" --j # Unowned sweep → report only, never act (see Ownership above) gh issue list --repo SGAOperations/aplio --search "no:assignee" --limit 100 --json number,title,labels --jq '[.[] | select(.labels | map(.name) | any(. == "ready" or . == "plan changes requested" or . == "plan approved" or . == "plan review" or . == "blocked"))]' gh pr list --repo SGAOperations/aplio --search "no:assignee" --limit 100 --json number,title,labels --jq '[.[] | select(.labels | map(.name) | any(. == "ready for review" or . == "needs revision" or . == "approved" or . == "needs human"))]' + +# Ungated-PR sweep → report only, never act (see Ungated report below) +gh pr list --repo SGAOperations/aplio --assignee "@me" --json number,title,labels --jq '[.[] | select((.labels | map(.name)) as $l | ($l | any(. == "ready for review" or . == "reviewing" or . == "needs revision" or . == "revising" or . == "approved" or . == "refresh branch" or . == "refreshing" or . == "needs human")) and ($l | index("claude") | not)) | {number, title}]' ``` -Then, in order: **(1)** reconcile merged PRs (below), **(2)** handle human gates, **(3)** **unless draining,** dispatch for every actionable trigger item (all Agent calls in one message), **(4)** report the unowned sweep if its set changed, **(5)** schedule the next wakeup (**skip while draining**). +Then, in order: **(1)** reconcile merged PRs (below), **(2)** handle human gates, **(3)** **announce every session-required item** — nothing will ever dispatch for these, so surfacing them is the only thing that moves them (below), **(4)** **unless draining,** dispatch for every remaining actionable trigger item (all Agent calls in one message), **(5)** report the unowned and ungated-PR sweeps if their sets changed, **(6)** schedule the next wakeup (**skip while draining**). **Merged-PR reconciliation (each tick):** the `approved` query above is open-only, so a merged PR silently drops out of it — never trust in-session memory for "awaiting merge." Diff the set of PRs you have **announced as approved** against the live `approved` result; for each announced PR no longer present, confirm and announce it **once**: @@ -77,6 +81,10 @@ gh pr view --repo SGAOperations/aplio --json headRefOid,statusCheckRollup -- An **empty** sweep result is only meaningful if the `--jq` filter works — verify it once against a known unassigned, trigger-labeled issue rather than trusting silence. +**Ungated-PR sweep (each tick):** `approval-check.yml` gates a PR on `approved` **only if it carries `claude`** (`.claude/docs/PIPELINE.md` → "Permission rationale"), so a pipeline PR that lost that label merges with no gate at all — and CI cannot tell it from a human PR. The sweep query above lists exactly those. Report them **only when the set changes**, in one line, and **never add the label automatically**: + +> ⚠️ Pipeline PRs without the `claude` label (approval gate inactive): #501. Say "gate #501" or add the label on GitHub. + **Denial report (each tick):** stage agents auto-deny disallowed commands (`dontAsk`) instead of prompting; a `PreToolUse` hook logs each one to **`.agents/denials.log`** (gitignored, base repo). Read it each tick and track how many lines are new since the previous tick. If denials **cluster** — say **≥3 new**, or the same command repeated — report it **once**, e.g. _"⚠️ 4 commands auto-denied this tick (e.g. `npx prisma migrate …` ×2, `printf … >` ×1) — the pipeline likely needs a permission/instruction change."_ Do **not** prompt or act on it automatically; this is visibility so the human knows when to harden the pipeline. A few isolated denials are normal and need no report. ## Dispatching @@ -94,14 +102,16 @@ Agent({ Stage → trigger mapping: -| Trigger query result | subagent_type | -| -------------------------------------- | ---------------------------------------------- | -| Issue labeled `ready` | `plan-agent` (fresh plan) | -| Issue labeled `plan changes requested` | `plan-agent` (revision) | -| Issue labeled `plan approved` | `impl-agent` | -| PR labeled `ready for review` | `review-agent` | -| PR labeled `needs revision` | `revise-agent` — **after the cycle-cap check** | -| PR labeled `refresh branch` | `revise-agent` in **refresh mode** | +| Trigger query result | subagent_type | +| -------------------------------------- | ------------------------------------------------------------------------------------------------------- | +| Issue labeled `ready` | `plan-agent` (fresh plan) | +| Issue labeled `plan changes requested` | `plan-agent` (revision) | +| Issue labeled `plan approved` | `impl-agent` — **unless `SESSION REQUIRED`: announce, never dispatch** | +| PR labeled `ready for review` | `review-agent` | +| PR labeled `needs revision` | `revise-agent` — **after the cycle-cap check**; **unless `SESSION REQUIRED`: announce, never dispatch** | +| PR labeled `refresh branch` | `revise-agent` in **refresh mode** | + +**Session-required items never dispatch.** Before dispatching `impl-agent` or `revise-agent`, check that item's `body` for the literal string `SESSION REQUIRED`. It is already in the trigger query's result (both queries request `body`), so this costs no extra call. Present → **announce, don't dispatch** (see Session-required items); absent → dispatch normally. For a refresh, say so in the prompt so the agent takes its Refresh mode path: `Run your pipeline stage for PR # in refresh mode (label: refresh branch).` @@ -126,11 +136,29 @@ then notify the human. For each issue labeled `plan review`: -- **Without `auto plan`:** summarize the plan from the issue body in a few sentences, then ask (AskUserQuestion): **Approve** / **Request changes** / **Discuss**. - - Approve → `gh issue edit --repo SGAOperations/aplio --remove-label "plan review" --add-label "plan approved"` (impl dispatches this tick). +- **Without `auto plan`:** summarize the plan from the issue body in a few sentences, then ask (AskUserQuestion): **Approve** / **Request changes** / **Discuss**. If the plan carries the `SESSION REQUIRED` marker, say so in the summary — the human should learn at the gate that they'll be running this one themselves. + - Approve → `gh issue edit --repo SGAOperations/aplio --remove-label "plan review" --add-label "plan approved"` (impl dispatches this tick — **unless** the plan is session-required, in which case this tick announces instead). - Request changes → write the human's feedback to `.temp/feedback-.md`, `gh issue comment --repo SGAOperations/aplio --body-file .temp/feedback-.md`, then `--remove-label "plan review" --add-label "plan changes requested"`. - Discuss → converse; finish with one of the two transitions above. -- **With `auto plan`:** swap `plan review` → `plan approved` immediately, no interaction, and dispatch impl this tick. +- **With `auto plan`:** swap `plan review` → `plan approved` immediately, no interaction, and dispatch impl this tick (same session-required exception). + +The gate applies **no special label** for a session-required plan — the marker is already in the issue body, and the dispatch step reads it from there. + +### Session-required items + +**Surfacing these is your job, and nothing else will do it.** An item whose body carries the `SESSION REQUIRED` marker keeps its trigger label but is **never** dispatched (`.claude/docs/PIPELINE.md` → "Session-required tickets"). No agent will ever pick it up, so if you don't tell the human it sits at `plan approved` / `needs revision` indefinitely — silently, because a trigger label normally means something is already moving. Announce it **once per session per item** (the same tracked-announcement pattern as `approved` PRs), then take no other action. + +**Say "separate session", and mean it.** The human runs `/implement` in a **new session — not this one.** This cockpit has no `Edit` in its tool scope and runs on haiku, so it cannot do the work; and it has to stay free to keep ticking, since a long implementation here would stall every other item in the pipeline. Hand over the launch command with the name pre-filled, derived from the **issue** title: `#: <2–5 lowercase words>`. + +- **Issue at `plan approved` + marker:** + + > 🧰 #503 is marked **`SESSION REQUIRED`** — it touches `CLAUDE.md` / `.claude/**`, which a dispatched agent can't edit, so I won't be implementing this one. **Open a separate session and run it there** (not here — I need to keep ticking): + > `claude -n "#503: operator config route"` then `/implement 503` + > Nothing moves until you do. I'll pick it back up automatically at review. + +- **PR at `needs revision` + marker** (announce **after** the cycle-cap check, which still runs and can still escalate to `needs human`): + + > 🧰 PR #512 needs revision and is marked **`SESSION REQUIRED`** — I can't dispatch `revise-agent` for it. **In a separate session** (not here): `claude -n "#503: operator config route"` then `/implement 512`. Nothing moves until you do; I'll review again once it's back at `ready for review`. (The session name carries the **issue** number; the command takes the PR number.) ### Approved PRs @@ -166,10 +194,11 @@ Interpret intent, not literal syntax: Dispatch the plan agent the same tick. - **"scope out X" / "break down X"** — Stage 0 deserves a stronger model than haiku; suggest the human run `/scope` in their main session. -- **"status"** — re-run the tick queries **live** and build the table from them (never from session memory): each in-flight item + stage, each item waiting on the human, and each PR currently labeled `approved` (the live `gh pr list --assignee "@me" --label approved` result — a merged PR has already dropped out, so it must not appear). It **inherits the assignee filter**, so it reports only this operator's items; append the unowned sweep result as a separate **"unowned"** line so a stalled ticket is diagnosable from one command. +- **"status"** — re-run the tick queries **live** and build the table from them (never from session memory): each in-flight item + stage, each item waiting on the human, and each PR currently labeled `approved` (the live `gh pr list --assignee "@me" --label approved` result — a merged PR has already dropped out, so it must not appear). It **inherits the assignee filter**, so it reports only this operator's items; append the unowned sweep result as a separate **"unowned"** line, and the ungated-PR sweep result as an **"ungated"** line, so a stalled ticket or a PR with no approval gate is diagnosable from one command. List **session-required** items under the human-gated group with the commands to run, e.g. `#503 — plan approved · SESSION REQUIRED → claude -n "#503: operator config route" · /implement 503`. - **"pause #N"** — remove the item's current trigger label; confirm what was removed. Same ownership rule as opt-in: if the item belongs to **another operator**, say so and stop rather than touch its labels. - **"resume #N" / "retry #N"** — re-apply the trigger label for where it stalled (issue stuck in `planning` → `ready`; PR stuck in `revising` → `needs revision`; PR stuck in `refreshing` → `refresh branch`; etc.). Same ownership rule as opt-in: if the item is **unassigned**, add `--add-assignee "@me"` in the same command (re-applying a trigger to an unassigned item is a no-op for every cockpit); if it belongs to **another operator**, say so and stop rather than re-trigger. - **"refresh #N"** — apply `refresh branch` to that PR and dispatch it this tick, bypassing the per-merge cap. Use it to force a fresh preview deployment on a PR left quota-red. Same ownership rule as opt-in. +- **"gate #N"** — apply the missing `claude` marker to a PR the ungated sweep reported: `gh pr edit --repo SGAOperations/aplio --add-label "claude"`. The `labeled` event re-evaluates `approval-check.yml`'s condition, so the approval gate is live on that run. Same ownership rule as opt-in. ## Stop controls @@ -193,4 +222,4 @@ Background-agent completions wake this session automatically; the scheduled wake ## Manual / recovery -Each stage is also runnable by hand without the cockpit — @-mention the subagent (e.g. `@agent-impl-agent implement #142`) or run a whole session as it via `claude --agent impl-agent`. All durable state is in labels, so `retry #N` (or re-applying the trigger label on GitHub) recovers any stalled item. +Each stage is also runnable by hand without the cockpit — @-mention the subagent (e.g. `@agent-impl-agent implement #142`) or run a whole session as it via `claude --agent impl-agent`. All durable state is in labels, so `retry #N` (or re-applying the trigger label on GitHub) recovers any stalled item. **Exception:** an item marked `SESSION REQUIRED` is never dispatched — run `/implement ` in your own named session. diff --git a/.claude/skills/worktree-clean/SKILL.md b/.claude/skills/worktree-clean/SKILL.md index 114453f5..6c9444b6 100644 --- a/.claude/skills/worktree-clean/SKILL.md +++ b/.claude/skills/worktree-clean/SKILL.md @@ -7,7 +7,7 @@ allowed-tools: Read, Bash # Clean up pipeline worktrees -Pipeline agents run in isolated worktrees under `.claude/worktrees/agent-*`. On Windows, once an agent has run `npm ci`, the harness often **de-registers** the worktree (removes its `.git` file) but **can't delete the directory** — the populated `node_modules` (and long/`(app)`-parenthesized paths) defeat `git worktree remove` (`Invalid argument`) and `git worktree prune` (which only clears registrations whose directory is already gone). These **orphan directories** then accumulate (hundreds of MB of `node_modules` each). The cockpit deliberately does **not** force-delete them in its autonomous loop; this skill does it, interactively, run by you from the **main checkout**. +Pipeline agents run in isolated worktrees under `.claude/worktrees/agent-*`; operator-run session-required tickets (`/implement`) add `.claude/worktrees/impl-*`. Both are in scope here. On Windows, once an agent has run `npm ci`, the harness often **de-registers** the worktree (removes its `.git` file) but **can't delete the directory** — the populated `node_modules` (and long/`(app)`-parenthesized paths) defeat `git worktree remove` (`Invalid argument`) and `git worktree prune` (which only clears registrations whose directory is already gone). These **orphan directories** then accumulate (hundreds of MB of `node_modules` each). The cockpit deliberately does **not** force-delete them in its autonomous loop; this skill does it, interactively, run by you from the **main checkout**. > **Scope guard:** this skill only ever deletes directories **directly under `.claude/worktrees/`**. Never delete anything outside that directory, never the main checkout, and never a worktree of an **in-flight** pipeline item (check `gh pr list`/the cockpit first if unsure). diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..a65e0ba1 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,24 @@ +Closes # + +## Summary + + + +## Changes + + + +## Testing plan + + + +- [ ] + +## Automated checks + + + +## Notes + + diff --git a/.github/workflows/approval-check.yml b/.github/workflows/approval-check.yml index 25d8534f..158f0aa0 100644 --- a/.github/workflows/approval-check.yml +++ b/.github/workflows/approval-check.yml @@ -1,5 +1,5 @@ -# Gates PRs into `dev` on the `approved` pipeline label. `labeled`/`unlabeled` -# re-trigger so the verdict updates as the pipeline swaps labels. +# Gates `dev` PRs on `approved`, only when labeled `claude` — others skip. +# NEVER narrow the trigger or rename the job: no check run = pending forever. # `branches: [dev]` is safe only because the required-check registration is # `dev`-scoped too — NEVER add this to the `main/dev` ruleset (id 15248252); # dev → main release PRs carry no pipeline labels. @@ -18,6 +18,7 @@ permissions: jobs: run-approval-check: runs-on: ubuntu-latest + if: contains(github.event.pull_request.labels.*.name, 'claude') timeout-minutes: 5 steps: - name: Check pipeline labels @@ -58,7 +59,7 @@ jobs: EOF if ! grep -Fxq "approved" "$RUNNER_TEMP/labels.txt"; then - echo "::error::Missing the 'approved' label. A PR into dev is merge-able only after review-agent (or a human, for manual PRs) applies 'approved'." + echo "::error::Missing the 'approved' label. This PR carries 'claude', so it is a pipeline PR and merges only once review-agent applies 'approved'. If it is not a pipeline PR, remove the 'claude' label and this check will skip." FAILED=1 fi diff --git a/.github/workflows/build-check.yml b/.github/workflows/build-check.yml new file mode 100644 index 00000000..15aa254b --- /dev/null +++ b/.github/workflows/build-check.yml @@ -0,0 +1,43 @@ +name: Build Check +on: + pull_request: +concurrency: + group: build-check-${{ github.ref }} + cancel-in-progress: true +jobs: + run-build-check: + runs-on: ubuntu-latest + timeout-minutes: 15 + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: admin + POSTGRES_PASSWORD: admin + POSTGRES_DB: aplio_build + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + env: + DATABASE_URL: postgresql://admin:admin@localhost:5432/aplio_build + BETTER_AUTH_SECRET: build-check-dummy-secret + steps: + - name: Checkout code + uses: actions/checkout@v7 + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version-file: package.json + cache: npm + - name: Install dependencies + run: npm ci + - name: Generate Prisma client + run: npm run prisma:generate + - name: Apply migrations + run: npm run prisma:migrate:deploy + - name: Build + run: npm run build diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..3afef7fa --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,30 @@ +name: CodeQL + +on: + pull_request: + +concurrency: + group: codeql-${{ github.ref }} + cancel-in-progress: true + +permissions: + security-events: write + contents: read + actions: read + +jobs: + analyze: + name: Analyze (javascript-typescript) + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v4 + with: + languages: javascript-typescript + + - name: Perform CodeQL analysis + uses: github/codeql-action/analyze@v4 diff --git a/.github/workflows/commit-message-check.yml b/.github/workflows/commit-message-check.yml index a3ee3b56..b9d3388f 100644 --- a/.github/workflows/commit-message-check.yml +++ b/.github/workflows/commit-message-check.yml @@ -3,6 +3,9 @@ name: Commit Message Check on: pull_request: +concurrency: + group: commit-message-check-${{ github.ref }} + cancel-in-progress: true permissions: contents: read jobs: @@ -11,7 +14,7 @@ jobs: timeout-minutes: 5 steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v7 with: fetch-depth: 0 - name: Validate commit subjects diff --git a/.github/workflows/linting-check.yml b/.github/workflows/linting-check.yml index 5639a3b6..b51f39e0 100644 --- a/.github/workflows/linting-check.yml +++ b/.github/workflows/linting-check.yml @@ -1,16 +1,22 @@ name: Linting Check on: pull_request: +concurrency: + group: linting-check-${{ github.ref }} + cancel-in-progress: true jobs: run-linting-check: runs-on: ubuntu-latest timeout-minutes: 10 steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v7 - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v7 + with: + node-version-file: package.json + cache: npm - name: Install modules - run: npm install + run: npm ci - name: Run linting check run: npm run eslint:check diff --git a/.github/workflows/lockfile-check.yml b/.github/workflows/lockfile-check.yml new file mode 100644 index 00000000..0277abf3 --- /dev/null +++ b/.github/workflows/lockfile-check.yml @@ -0,0 +1,45 @@ +# Verifies the lockfile is internally consistent and fully pinned. +name: Lockfile Check + +on: + pull_request: + workflow_dispatch: + +concurrency: + group: lockfile-check-${{ github.ref }} + cancel-in-progress: true + +jobs: + run-lockfile-check: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout code + uses: actions/checkout@v7 + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version-file: package.json + cache: npm + - name: Install with a frozen lockfile + run: npm ci --ignore-scripts + - name: Verify lockfile version and pinning + run: | + set -euo pipefail + LOCK_VERSION=$(jq '.lockfileVersion' package-lock.json) + if [ "$LOCK_VERSION" -lt 3 ]; then + echo "::error::package-lock.json lockfileVersion is $LOCK_VERSION, expected >= 3." + exit 1 + fi + UNPINNED=$(jq -r ' + .packages + | to_entries[] + | select(.key != "" and (.value.link | not) and (.value.inBundle | not)) + | select((.value.resolved | not) or (.value.integrity | not)) + | .key + ' package-lock.json) + if [ -n "$UNPINNED" ]; then + echo "::error::Lockfile entries missing resolved/integrity: $UNPINNED" + exit 1 + fi + echo "package-lock.json is lockfileVersion $LOCK_VERSION and fully pinned." diff --git a/.github/workflows/migrate-db.yml b/.github/workflows/migrate-db.yml index de8b5aec..edef0570 100644 --- a/.github/workflows/migrate-db.yml +++ b/.github/workflows/migrate-db.yml @@ -1,18 +1,6 @@ -# Migrate DB — deploy pending migrations to long-lived environments -# -# Required secrets (GitHub → Settings → Secrets and variables → Actions): -# PRODUCTION_DATABASE_URL — direct/unpooled Postgres connection string for the -# production database; used on pushes to `main`. -# DEV_DATABASE_URL — direct/unpooled Postgres connection string for the -# dev database; used on pushes to `dev`. -# -# Both secrets MUST use a direct (unpooled) connection — `prisma migrate deploy` -# requires a direct connection; a pooled/PgBouncer endpoint will cause the -# migration step to fail. The selected secret is assigned to DATABASE_URL at -# runtime by branch. -# -# Failure model: a failed migration fails this workflow loudly (no continue-on-error). -# There is no automated rollback — manual intervention is required on failure. +# Deploys pending migrations to dev/production. Requires direct (unpooled) +# PRODUCTION_DATABASE_URL / DEV_DATABASE_URL secrets. No automated rollback — +# restore the Neon branch, then `git revert` the migration commit. name: Migrate DB @@ -21,6 +9,13 @@ on: branches: [main, dev] paths: - 'prisma/migrations/**' + workflow_dispatch: + inputs: + target: + description: 'Environment to migrate' + required: true + type: choice + options: [dev, production] concurrency: group: migrate-db-${{ github.ref }} @@ -34,13 +29,14 @@ jobs: name: Deploy migrations runs-on: ubuntu-latest timeout-minutes: 15 + environment: ${{ (inputs.target == 'production' || github.ref_name == 'main') && 'production' || 'dev' }} steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Set up Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version-file: package.json cache: npm @@ -52,9 +48,10 @@ jobs: env: PRODUCTION_DATABASE_URL: ${{ secrets.PRODUCTION_DATABASE_URL }} DEV_DATABASE_URL: ${{ secrets.DEV_DATABASE_URL }} + TARGET: ${{ inputs.target || github.ref_name }} run: | - case "${{ github.ref_name }}" in - main) + case "$TARGET" in + main|production) SELECTED_SECRET="$PRODUCTION_DATABASE_URL" SECRET_NAME="PRODUCTION_DATABASE_URL" ;; @@ -63,13 +60,13 @@ jobs: SECRET_NAME="DEV_DATABASE_URL" ;; *) - echo "::error::Unrecognized branch '${{ github.ref_name }}'. Update on.push.branches and the case statement together." + echo "::error::Unrecognized target '$TARGET'. Update on.push.branches, workflow_dispatch.inputs.target, and the case statement together." exit 1 ;; esac if [ -z "$SELECTED_SECRET" ]; then - echo "::error::Required secret $SECRET_NAME for branch ${{ github.ref_name }} is not configured. Add it under GitHub → Settings → Secrets and variables → Actions." + echo "::error::Required secret $SECRET_NAME for target $TARGET is not configured. Add it under GitHub → Settings → Secrets and variables → Actions." exit 1 fi @@ -78,5 +75,35 @@ jobs: - name: Generate Prisma client run: npm run prisma:generate + - name: Preflight — migration status + run: | + set -uo pipefail + set +e + npm run prisma:migrate:status > "$RUNNER_TEMP/status.txt" 2>&1 + STATUS_EXIT=$? + set -e + { + echo "### Migration status ($TARGET)" + echo + echo '```' + cat "$RUNNER_TEMP/status.txt" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + cat "$RUNNER_TEMP/status.txt" + # fail closed on drift or any unexpected non-zero exit + if grep -qi 'drift detected' "$RUNNER_TEMP/status.txt"; then + echo "::error::Migration drift detected against $TARGET — refusing to apply. See the step summary." + exit 1 + elif [ "$STATUS_EXIT" -ne 0 ]; then + if grep -qiE 'have not yet been applied' "$RUNNER_TEMP/status.txt"; then + echo "Pending migrations detected against $TARGET — expected pre-apply state, continuing." + else + echo "::error::prisma migrate status failed unexpectedly (exit $STATUS_EXIT) against $TARGET — refusing to apply. See the step summary." + exit 1 + fi + fi + env: + TARGET: ${{ inputs.target || github.ref_name }} + - name: Apply migrations run: npm run prisma:migrate:deploy diff --git a/.github/workflows/prettier-check.yml b/.github/workflows/prettier-check.yml index 25f5f2ed..d0cc52ec 100644 --- a/.github/workflows/prettier-check.yml +++ b/.github/workflows/prettier-check.yml @@ -1,16 +1,22 @@ name: Prettier Check on: pull_request: +concurrency: + group: prettier-check-${{ github.ref }} + cancel-in-progress: true jobs: run-prettier-check: runs-on: ubuntu-latest timeout-minutes: 10 steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v7 - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v7 + with: + node-version-file: package.json + cache: npm - name: Install modules - run: npm install + run: npm ci - name: Run prettier check run: npm run prettier:check diff --git a/.github/workflows/test-check.yml b/.github/workflows/test-check.yml index 151af52d..6a7928e6 100644 --- a/.github/workflows/test-check.yml +++ b/.github/workflows/test-check.yml @@ -1,6 +1,9 @@ name: Test Check on: pull_request: +concurrency: + group: test-check-${{ github.ref }} + cancel-in-progress: true jobs: run-test-check: runs-on: ubuntu-latest @@ -23,9 +26,9 @@ jobs: DATABASE_URL: postgresql://admin:admin@localhost:5432/aplio_test steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Set up Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version-file: package.json cache: npm diff --git a/.github/workflows/tsc-check.yml b/.github/workflows/tsc-check.yml index fc16adcd..cc375bb3 100644 --- a/.github/workflows/tsc-check.yml +++ b/.github/workflows/tsc-check.yml @@ -1,18 +1,24 @@ name: TypeScript Check on: pull_request: +concurrency: + group: tsc-check-${{ github.ref }} + cancel-in-progress: true jobs: run-tsc-check: runs-on: ubuntu-latest timeout-minutes: 15 steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v7 - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v7 + with: + node-version-file: package.json + cache: npm - name: Install dependencies - run: npm install + run: npm ci - name: Generate Prisma client - run: npx prisma generate + run: npm run prisma:generate - name: Run tsc check run: npm run tsc:check diff --git a/CLAUDE.md b/CLAUDE.md index 801317e8..4bebadda 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,7 +34,7 @@ Next.js 16 (App Router, React 19) · Prisma 7 · Tailwind CSS 4 · shadcn/ui (Ra - **Commit:** subject `#XXX message in lowercase imperative mood` (no colon after the number, **under 80 chars**, no trailing period); then — only if the _why_ isn't obvious — a blank line and a short body (wrap ~72, a few lines max; narrative belongs in the PR, not the commit); then a blank line and a `Co-Authored-By: Claude Sonnet 4.6 ` trailer. Commit each logical unit separately. **Write the message to a file and `git commit -F .temp/commit-msg.txt`** — inline multi-line `-m … -m …` collapses on Windows, dropping the subject and the co-authorship. Delete tracked files with `git rm`. **The subject-line format is enforced locally by a `commit-msg` hook** (installed automatically via `npm run prepare` / `npm ci`) **and in CI by `run-commit-message-check`**, which validates every commit in a PR by invoking that same hook — so the two cannot drift. - **Branch:** `XXX-ticket-name-in-kebab-case`, branched off `dev`. -- **PR:** title `#XXX Ticket Name In Title Case`; **base branch `dev`** (never open feature PRs against `main`); body contains `Closes #XXX`; assign the **issue's assignee** (fallback: yourself — `@me`). `dev → main` is promoted only by `/release`. +- **PR:** title `#XXX Ticket Name In Title Case`; **base branch `dev`** (never open feature PRs against `main`); body contains `Closes #XXX`; assign the **issue's assignee** (fallback: yourself — `@me`); **pipeline-authored PRs also carry the `claude` label** — it is what activates the `approved` merge gate (`approval-check.yml`), and without it the PR merges ungated. `dev → main` is promoted only by `/release`. - **Rebase conflicts in pipeline:** `revise-agent` attempts autonomous resolution for structurally unambiguous conflicts (non-overlapping sections, generated files, dual independent imports). It escalates to `needs human` only when both sides modified the same logical unit. Agents should document every resolution in the revision summary. Full protocol: `.claude/docs/PIPELINE.md` → "Rebase conflict protocol". ## Preview databases (Neon branch budget) @@ -62,6 +62,7 @@ All tests live under `tests/`, never co-located with the source they cover: `tes ## Worktrees & local dev - Pipeline agents get their own isolated worktree automatically (`isolation: worktree`) — they handle setup; see `.claude/docs/PIPELINE.md`. Do not script worktree creation for them. +- **Some tickets are implemented by you, not by an agent** — today, anything touching `CLAUDE.md` or `.claude/**`, because the harness blocks a dispatched agent's `Edit` there. The plan marks these `SESSION REQUIRED` and the cockpit announces instead of dispatching: launch a named session (`claude -n "#XXX: "`) and run `/implement XXX`, which runs stage 2/4 in its own worktree. See `.claude/docs/PIPELINE.md` → "Session-required tickets". - For manual local work in a worktree, install deps with `npm ci` (then `npm run prisma:generate`). **Do not `ln -s node_modules` — symlinks fall back to copies on Windows here.** Sync before resuming: `git fetch origin && git rebase origin/dev`. - **`npm ci` is what activates Git hooks** — it runs `prepare` (`husky && npm run hooks:check`), which regenerates the untracked `.husky/_` bootstrap dir, sets `core.hooksPath`, and fails `npm ci` itself if activation didn't take. Each worktree/clone needs its own `npm ci` for hooks to fire there. If hooks stop firing, re-run `npm ci` (or `npm run prepare`) and verify with `npm run hooks:check`. - **The same step sets `core.commentChar=';'`** — git's default `#` makes it strip the mandated `#XXX` subject as a comment every time it re-reads a message through the editor machinery (`git rebase --continue`, `git commit --amend`), silently promoting the first body line into the subject; the `commit-msg` hook does not run on that path. Side effect: git's own instructional lines in the commit editor are `;`-prefixed. This writes to the shared `.git/config`, so one worktree's `npm ci` fixes every worktree of that clone. It cannot be enforced across fresh clones or forks, which is why CI validates subjects too. diff --git a/README.md b/README.md index 9fc2abb9..574887a1 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ Aplio is an internal recruiting and application platform. Admins and managers cr ### Prerequisites -- **Node.js** 22+ LTS (see `@types/node` in `package.json`) +- **Node.js** version pinned in `package.json`'s `engines` field - **npm** (comes with Node) - **Docker** (for a local Postgres instance via `docker-compose.yml`) **or** a hosted Neon Postgres URL diff --git a/app/(main)/(auth)/applications/[id]/page.tsx b/app/(main)/(auth)/applications/[id]/page.tsx index 6b8ea5cb..434c29eb 100644 --- a/app/(main)/(auth)/applications/[id]/page.tsx +++ b/app/(main)/(auth)/applications/[id]/page.tsx @@ -4,9 +4,10 @@ import { notFound } from 'next/navigation'; import { getApplicationForReview } from '@/prisma/data/applications'; import { getCurrentUser } from '@/lib/auth/server'; +import { getRenamedTo } from '@/lib/utils'; import { ApplicationAnswersList } from '@/components/features/application-answers-list'; -import { ApplicationStatusControl } from '@/components/features/application-status-control'; +import { ApplicationStatusActions } from '@/components/features/application-status-actions'; import { ApplicationStatusBadge } from '@/components/features/status-badge'; import { PageHeader } from '@/components/layouts/page-header'; import { @@ -29,7 +30,12 @@ export async function generateMetadata({ const user = await getCurrentUser(); const application = await getApplicationForReview(id, user); if (!application) return {}; - return { title: application.user.name ?? application.user.email }; + return { + title: + application.applicantName ?? + application.user.name ?? + application.user.email, + }; } export default async function ApplicationDetailPage({ @@ -42,13 +48,17 @@ export default async function ApplicationDetailPage({ if (!application) notFound(); - const applicantName = application.user.name ?? application.user.email; + const applicantName = + application.applicantName ?? + application.user.name ?? + application.user.email; + const renamedTo = getRenamedTo(application); return (

@@ -101,9 +111,10 @@ export default async function ApplicationDetailPage({ - diff --git a/app/(main)/(auth)/applications/loading.tsx b/app/(main)/(auth)/applications/loading.tsx index ab8de85d..67d7a890 100644 --- a/app/(main)/(auth)/applications/loading.tsx +++ b/app/(main)/(auth)/applications/loading.tsx @@ -2,7 +2,7 @@ import { Skeleton } from '@/components/ui/skeleton'; export default function ApplicationsLoading() { return ( -

+
{/* Header skeleton */}
diff --git a/app/(main)/(auth)/applications/page.tsx b/app/(main)/(auth)/applications/page.tsx index 462c7692..d4b47ad2 100644 --- a/app/(main)/(auth)/applications/page.tsx +++ b/app/(main)/(auth)/applications/page.tsx @@ -1,5 +1,7 @@ import type { Metadata } from 'next'; +import { z } from 'zod/v4'; + import { getApplications, getApplicationsTotal, @@ -7,14 +9,12 @@ import { } from '@/prisma/data/applications'; import { requireManagerOrAdminOr404 } from '@/lib/auth/guards'; -import { REVIEWER_APPLICATION_STATUSES } from '@/lib/constants'; -import type { - ApplicationFilters, - ApplicationSort, - ApplicationSortDirection, - ApplicationSortField, - ReviewerStatus, -} from '@/lib/types'; +import { + APPLICATION_SORT_DIRECTIONS, + APPLICATION_SORT_FIELDS, + REVIEWER_APPLICATION_STATUSES, +} from '@/lib/constants'; +import type { ApplicationFilters } from '@/lib/types'; import { ApplicationsTable } from '@/components/features/applications-table'; import { ApplicationsToolbar } from '@/components/features/applications-toolbar'; @@ -26,8 +26,26 @@ interface ApplicationsPageProps { searchParams: Promise>; } -const VALID_SORT_FIELDS: ApplicationSortField[] = ['date', 'name', 'status']; -const VALID_SORT_DIRECTIONS: ApplicationSortDirection[] = ['asc', 'desc']; +// A single-value param is a string; a repeated one arrives as string[] — reject both +// with .catch(undefined) rather than throwing, so one bad param never sinks the rest. +const searchParamsSchema = z.object({ + positionId: z.string().trim().min(1).max(64).optional().catch(undefined), + userId: z.string().trim().min(1).max(64).optional().catch(undefined), + status: z.enum(REVIEWER_APPLICATION_STATUSES).optional().catch(undefined), + q: z.string().trim().min(1).max(200).optional().catch(undefined), + sort: z + .string() + .transform((value) => value.split(':')) + .pipe( + z.tuple([ + z.enum(APPLICATION_SORT_FIELDS), + z.enum(APPLICATION_SORT_DIRECTIONS), + ]), + ) + .transform(([field, direction]) => ({ field, direction })) + .optional() + .catch(undefined), +}); export default async function ApplicationsPage({ searchParams, @@ -36,33 +54,14 @@ export default async function ApplicationsPage({ const user = await requireManagerOrAdminOr404(); const sp = await searchParams; - - const rawStatus = typeof sp.status === 'string' ? sp.status : undefined; - const validStatus: ReviewerStatus | undefined = - rawStatus && - (REVIEWER_APPLICATION_STATUSES as readonly string[]).includes(rawStatus) - ? (rawStatus as ReviewerStatus) - : undefined; - - const rawSort = typeof sp.sort === 'string' ? sp.sort : undefined; - let validSort: ApplicationSort | undefined; - if (rawSort) { - const [rawField, rawDir] = rawSort.split(':'); - const field = rawField as ApplicationSortField; - const direction = rawDir as ApplicationSortDirection; - if ( - VALID_SORT_FIELDS.includes(field) && - VALID_SORT_DIRECTIONS.includes(direction) - ) - validSort = { field, direction }; - } + const parsed = searchParamsSchema.parse(sp); const filters: ApplicationFilters = { - positionId: typeof sp.positionId === 'string' ? sp.positionId : undefined, - status: validStatus, - userId: typeof sp.userId === 'string' ? sp.userId : undefined, - q: typeof sp.q === 'string' && sp.q.trim() ? sp.q.trim() : undefined, - sort: validSort, + positionId: parsed.positionId, + status: parsed.status, + userId: parsed.userId, + q: parsed.q, + sort: parsed.sort, }; const hasActiveFilters = !!( @@ -86,7 +85,7 @@ export default async function ApplicationsPage({ : fetchedApplications; return ( -
+
+ +
+

Account deactivated

+

+ Your account has been deactivated, so you can't access Aplio + right now. +

+

+ If you think this is a mistake, contact an administrator to have it + restored. +

+
+

+ Signed in as {deactivatedUser.email} +

+
+ + +
+
+ ); +} diff --git a/app/login/page.tsx b/app/login/page.tsx index 4a6c053b..b960d5cb 100644 --- a/app/login/page.tsx +++ b/app/login/page.tsx @@ -2,21 +2,13 @@ import type { Metadata } from 'next'; import Link from 'next/link'; import { redirect } from 'next/navigation'; -import { TriangleAlert } from 'lucide-react'; - import { safeRedirectTo, withRedirectTo } from '@/lib/auth/redirect'; -import { getOptionalUser } from '@/lib/auth/server'; -import { - ACCOUNT_DEACTIVATED_MESSAGE, - LOGIN_DEACTIVATED_REASON, - PRIVACY_HREF, - TERMS_HREF, -} from '@/lib/constants'; +import { getDeactivatedSessionUser, getOptionalUser } from '@/lib/auth/server'; +import { PRIVACY_HREF, TERMS_HREF } from '@/lib/constants'; import { isBypassAllowed } from '@/lib/utils'; import { LoginView } from '@/components/features/login-view'; import { NameField } from '@/components/features/name-field'; -import { WarningCallout } from '@/components/ui/warning-callout'; export const metadata: Metadata = { title: 'Sign In' }; @@ -30,16 +22,19 @@ export default async function SignInPage({ }: { searchParams: Promise>; }) { - const { redirectTo, reason } = await searchParams; + const { redirectTo } = await searchParams; const safeTo = safeRedirectTo(redirectTo); const applyContext = isApplyRedirect(safeTo); - const isDeactivated = reason === LOGIN_DEACTIVATED_REASON; const user = await getOptionalUser(); // Authenticated user with a name set — send them into the app. if (user?.name?.trim()) redirect(safeTo); // Authenticated user with no name — fall through to render the name form below. + // A live session for a deactivated row — routing, not denial. + if (!user && (await getDeactivatedSessionUser())) + redirect('/login/deactivated'); + // Must match isBypassAllowed, so the affordance and the action agree. const isDev = isBypassAllowed(); @@ -60,11 +55,6 @@ export default async function SignInPage({ return (
- {isDeactivated && !user && ( - - {ACCOUNT_DEACTIVATED_MESSAGE} - - )} {user ? ( ) : ( diff --git a/components/features/activity-feed.tsx b/components/features/activity-feed.tsx index a371970b..edd60ce4 100644 --- a/components/features/activity-feed.tsx +++ b/components/features/activity-feed.tsx @@ -11,6 +11,7 @@ import { STATUS_BADGE_VARIANT_TO_DOT, } from '@/lib/constants'; import { type ActivityItem, type Reviewer } from '@/lib/types'; +import { getRenamedTo } from '@/lib/utils'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { LocalTime } from '@/components/ui/local-time'; @@ -47,8 +48,7 @@ function ActivityFeedList({ items, emptyDescription }: ActivityFeedListProps) {
    {items.map((item) => { const dotClass = - STATUS_BADGE_VARIANT_TO_DOT[item.statusVariant] ?? - 'bg-muted-foreground'; + STATUS_BADGE_VARIANT_TO_DOT[item.statusVariant]; return (
  1. { - const applicantLabel = app.user.name ?? app.user.email; + const applicantLabel = app.applicantName ?? app.user.name ?? app.user.email; + const renamedTo = getRenamedTo(app); const variant = APPLICATION_STATUS_BADGE_VARIANT[app.status]; return { id: app.id, statusVariant: variant, - sentence: `${applicantLabel} applied for ${app.position.title}`, + sentence: `${applicantLabel}${renamedTo ? ` (${renamedTo})` : ''} applied for ${app.position.title}`, timestamp: app.submittedAt, }; }); diff --git a/components/features/answer-file-link.tsx b/components/features/answer-file-link.tsx index c0956bf2..234f6247 100644 --- a/components/features/answer-file-link.tsx +++ b/components/features/answer-file-link.tsx @@ -71,7 +71,7 @@ export function AnswerFileLink({ target, url }: AnswerFileLinkProps) { variant="outline" size="sm" className="min-h-11 shrink-0 sm:min-h-9" - onClick={handleDownload} + onClick={() => void handleDownload()} disabled={isPending} aria-label={`Download ${filename}`} > diff --git a/components/features/application-answers-list.tsx b/components/features/application-answers-list.tsx index 025cd2be..3a43831f 100644 --- a/components/features/application-answers-list.tsx +++ b/components/features/application-answers-list.tsx @@ -33,7 +33,7 @@ export function ApplicationAnswersList({ questionId: answer.questionId, isGlobal: answer.isGlobal, }} - url={answer.value[0]} + url={answer.value[0] ?? ''} /> ) : answer.value.length === 1 ? (

    {answer.value[0]}

    diff --git a/components/features/application-question.tsx b/components/features/application-question.tsx index df2baae8..c307b8d9 100644 --- a/components/features/application-question.tsx +++ b/components/features/application-question.tsx @@ -142,7 +142,7 @@ export function ApplicationQuestion({ onChange={(e) => field.onChange(e.target.value ? [e.target.value] : []) } - onBlur={handleBlur} + onBlur={() => void handleBlur()} maxLength={ANSWER_SHORT_MAX_LENGTH} aria-required={question.required} aria-invalid={!!error} @@ -158,7 +158,7 @@ export function ApplicationQuestion({ onChange={(e) => field.onChange(e.target.value ? [e.target.value] : []) } - onBlur={handleBlur} + onBlur={() => void handleBlur()} className="min-h-[120px]" maxLength={ANSWER_LONG_MAX_LENGTH} aria-required={question.required} @@ -206,7 +206,7 @@ export function ApplicationQuestion({ setOtherSelected(false); setOtherText(''); field.onChange([option]); - save([option]); + void save([option]); }} className="accent-primary size-4" /> @@ -226,7 +226,7 @@ export function ApplicationQuestion({ setOtherSelected(true); const next = otherText ? [otherText] : []; field.onChange(next); - save(next); + void save(next); }} className="accent-primary size-4" /> @@ -252,7 +252,7 @@ export function ApplicationQuestion({ setOtherText(e.target.value); field.onChange(e.target.value ? [e.target.value] : []); }} - onBlur={handleBlur} + onBlur={() => void handleBlur()} aria-labelledby={`${labelId} ${question.id}-other-label`} maxLength={ANSWER_OTHER_MAX_LENGTH} /> @@ -282,7 +282,7 @@ export function ApplicationQuestion({ ? [...fitted, option] : fitted.filter((v) => v !== option); field.onChange(next); - save(next); + void save(next); }} /> {option} @@ -304,12 +304,12 @@ export function ApplicationQuestion({ ? [...checkedOptions, otherText] : checkedOptions; field.onChange(next); - save(next); + void save(next); } else { // Drops the typed text immediately, so it isn't resubmitted. setOtherText(''); field.onChange(checkedOptions); - save(checkedOptions); + void save(checkedOptions); } }} /> @@ -342,7 +342,7 @@ export function ApplicationQuestion({ : checkedOptions, ); }} - onBlur={handleBlur} + onBlur={() => void handleBlur()} aria-labelledby={`${labelId} ${question.id}-other-label`} maxLength={ANSWER_OTHER_MAX_LENGTH} /> diff --git a/components/features/application-status-actions.tsx b/components/features/application-status-actions.tsx new file mode 100644 index 00000000..780924b8 --- /dev/null +++ b/components/features/application-status-actions.tsx @@ -0,0 +1,266 @@ +'use client'; + +import { useState, useTransition } from 'react'; + +import { Loader2, MoreHorizontal } from 'lucide-react'; +import { toast } from 'sonner'; + +import { updateApplicationStatus } from '@/prisma/actions/applications'; +import type { $Enums } from '@/prisma/client'; + +import { + APPLICATION_STATUS_ACTION_LABELS, + APPLICATION_STATUS_LABELS, + APPLICATION_STATUS_TRANSITIONS, + NON_REVIEWABLE_APPLICATION_STATUS_NOTES, + REJECTABLE_APPLICATION_STATUSES, + TERMINAL_DECISION_STATUS_NOTES, + isNonReviewableApplicationStatus, +} from '@/lib/constants'; + +import { Button } from '@/components/ui/button'; +import { ConfirmDialog } from '@/components/ui/confirm-dialog'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/components/ui/dropdown-menu'; + +interface ApplicationStatusActionsProps { + applicationId: string; + currentStatus: $Enums.ApplicationStatus; + applicantName?: string; + compact?: boolean; +} + +const CONFIRM_COPY = { + accepted: { + title: (name: string) => `Accept ${name}?`, + description: + "They'll see Accepted on their application and can no longer withdraw it. You can move this back later.", + confirmLabel: 'Accept', + pendingLabel: 'Accepting…', + }, + rejected: { + title: (name: string) => `Reject ${name}?`, + description: + "They'll see Rejected on their application and can no longer withdraw it. You can move this back later.", + confirmLabel: 'Reject', + pendingLabel: 'Rejecting…', + }, +} as const; + +export function ApplicationStatusActions({ + applicationId, + currentStatus, + applicantName, + compact, +}: ApplicationStatusActionsProps) { + const [isPending, startTransition] = useTransition(); + const [pendingTarget, setPendingTarget] = + useState<$Enums.ApplicationStatus | null>(null); + const [menuOpen, setMenuOpen] = useState(false); + const [confirmTarget, setConfirmTarget] = useState<'accepted' | 'rejected'>( + 'accepted', + ); + const [confirmOpen, setConfirmOpen] = useState(false); + + const displayName = applicantName ?? 'this application'; + + function performMove( + target: $Enums.ApplicationStatus, + onSettled?: () => void, + ) { + setPendingTarget(target); + startTransition(async () => { + try { + const result = await updateApplicationStatus({ + applicationId, + status: target, + }); + if (result && 'error' in result) { + toast.error(result.error); + return; + } + toast.success(`Moved to ${APPLICATION_STATUS_LABELS[target]}`); + } catch { + toast.error('Something went wrong. Please try again.'); + } finally { + setPendingTarget(null); + onSettled?.(); + } + }); + } + + function openConfirm(target: 'accepted' | 'rejected') { + setConfirmTarget(target); + setConfirmOpen(true); + } + + function handleForwardSelect(target: $Enums.ApplicationStatus) { + if (target === 'accepted') openConfirm('accepted'); + else performMove(target); + } + + if (isNonReviewableApplicationStatus(currentStatus)) { + if (compact) return null; + return ( +

    + {NON_REVIEWABLE_APPLICATION_STATUS_NOTES[currentStatus]} +

    + ); + } + + const isTerminalDecision = + currentStatus === 'accepted' || currentStatus === 'rejected'; + const { forward, back } = APPLICATION_STATUS_TRANSITIONS[currentStatus]; + const isRejectable = ( + REJECTABLE_APPLICATION_STATUSES as readonly $Enums.ApplicationStatus[] + ).includes(currentStatus); + + const confirmCopy = CONFIRM_COPY[confirmTarget]; + + const confirmDialog = ( + performMove(confirmTarget, () => setConfirmOpen(false))} + /> + ); + + if (compact) { + return ( + <> + + + + + + {forward.map((target) => ( + { + e.preventDefault(); + setMenuOpen(false); + handleForwardSelect(target); + }} + > + {APPLICATION_STATUS_ACTION_LABELS[target]} + + ))} + {isRejectable && ( + <> + + { + e.preventDefault(); + setMenuOpen(false); + openConfirm('rejected'); + }} + > + {APPLICATION_STATUS_ACTION_LABELS.rejected} + + + )} + {back.length > 0 && ( + <> + + Move back + {back.map((target) => ( + { + e.preventDefault(); + setMenuOpen(false); + performMove(target); + }} + > + Move back to {APPLICATION_STATUS_LABELS[target]} + + ))} + + )} + + + {confirmDialog} + + ); + } + + return ( +
    + {isTerminalDecision && ( +

    + {TERMINAL_DECISION_STATUS_NOTES[currentStatus]} +

    + )} + {!isTerminalDecision && ( +
    + {forward.map((target, i) => ( + + ))} + {isRejectable && ( + + )} +
    + )} + {back.length > 0 && ( +
    + {back.map((target) => ( + + ))} +
    + )} + {confirmDialog} +
    + ); +} diff --git a/components/features/application-status-control.tsx b/components/features/application-status-control.tsx deleted file mode 100644 index 368aadc3..00000000 --- a/components/features/application-status-control.tsx +++ /dev/null @@ -1,117 +0,0 @@ -'use client'; - -import { useId, useTransition } from 'react'; - -import { Loader2 } from 'lucide-react'; -import { toast } from 'sonner'; - -import { updateApplicationStatus } from '@/prisma/actions/applications'; -import { type $Enums } from '@/prisma/client'; - -import { - APPLICATION_STATUS_LABELS, - NON_REVIEWABLE_APPLICATION_STATUS_NOTES, - REVIEWER_APPLICATION_STATUS_OPTIONS, - isNonReviewableApplicationStatus, -} from '@/lib/constants'; -import { cn } from '@/lib/utils'; - -import { Label } from '@/components/ui/label'; -import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from '@/components/ui/select'; - -interface ApplicationStatusControlProps { - applicationId: string; - currentStatus: $Enums.ApplicationStatus; - labelText?: string; - // Visually hides the label — kept in the a11y tree and still focuses the select on click. - hideLabel?: boolean; -} - -export function ApplicationStatusControl({ - applicationId, - currentStatus, - labelText = 'Status', - hideLabel, -}: ApplicationStatusControlProps) { - const [isPending, startTransition] = useTransition(); - const fieldId = useId(); - const noteId = useId(); - - // Reviewer-selectable options — 'draft' is already excluded from this constant. - const options = REVIEWER_APPLICATION_STATUS_OPTIONS; - - // Derived from the shared constant (not a hand-written check) so this can - // never drift from the action's own non-reviewable guard. - const isReadOnly = isNonReviewableApplicationStatus(currentStatus); - - function handleValueChange(value: string) { - startTransition(async () => { - try { - const result = await updateApplicationStatus({ - applicationId, - status: value, - }); - if (result && 'error' in result) { - toast.error(result.error); - } else { - toast.success('Status updated'); - } - } catch { - toast.error('Something went wrong. Please try again.'); - } - }); - } - - return ( -
    - -
    - {isPending && ( - - )} - -
    - {isReadOnly && ( -

    - {NON_REVIEWABLE_APPLICATION_STATUS_NOTES[currentStatus]} -

    - )} -
    - ); -} diff --git a/components/features/application-stepper.tsx b/components/features/application-stepper.tsx index bd496e49..2fd4bb88 100644 --- a/components/features/application-stepper.tsx +++ b/components/features/application-stepper.tsx @@ -99,7 +99,7 @@ function ReadOnlyQuestionCard({ // Read-only shows the profile's own answer, so the target is profile-scoped. ) : question.type === 'multiple_choice' ? (
    @@ -501,7 +501,7 @@ export function ApplicationStepper({ variant={isCustomizing ? 'default' : 'outline'} size="sm" className="mt-0.5 shrink-0" - onClick={handleToggleCustomize} + onClick={() => void handleToggleCustomize()} disabled={isReverting} > {isCustomizing @@ -603,7 +603,10 @@ export function ApplicationStepper({ > Back - + ); +} diff --git a/components/features/edit-name-dialog.tsx b/components/features/edit-name-dialog.tsx new file mode 100644 index 00000000..de0a8fc7 --- /dev/null +++ b/components/features/edit-name-dialog.tsx @@ -0,0 +1,75 @@ +'use client'; + +import type { ReactNode } from 'react'; + +import { TriangleAlert } from 'lucide-react'; +import { toast } from 'sonner'; +import type { z } from 'zod/v4'; + +import { setUserName } from '@/prisma/actions/profile'; + +import { nameSchema } from '@/lib/constants'; + +import { + FormControl, + FormField, + FormItem, + FormLabel, + FormMessage, +} from '@/components/ui/form'; +import { FormDialog } from '@/components/ui/form-dialog'; +import { Input } from '@/components/ui/input'; +import { WarningCallout } from '@/components/ui/warning-callout'; + +type NameFormValues = z.infer; + +interface EditNameDialogProps { + currentName: string | null; + trigger: ReactNode; +} + +export function EditNameDialog({ currentName, trigger }: EditNameDialogProps) { + async function onSubmit(data: NameFormValues): Promise { + const result = await setUserName(data); + if (result?.error) { + toast.error(result.error); + return false; + } + toast.success('Name updated.'); + return true; + } + + return ( + + ( + + Full name + + + + + + )} + /> + + This updates your name going forward. Applications you've already + submitted keep the name you used at the time. + + + ); +} diff --git a/components/features/global-question-dialog.tsx b/components/features/global-question-dialog.tsx index 73ffc703..2ccf2969 100644 --- a/components/features/global-question-dialog.tsx +++ b/components/features/global-question-dialog.tsx @@ -94,7 +94,7 @@ function RequiredField() { @@ -148,7 +148,7 @@ function AllowOtherField() { diff --git a/components/features/global-questions-table.tsx b/components/features/global-questions-table.tsx index 61e5535f..9ee7f01f 100644 --- a/components/features/global-questions-table.tsx +++ b/components/features/global-questions-table.tsx @@ -277,7 +277,7 @@ export function GlobalQuestionsTable({ questions }: GlobalQuestionsTableProps) {