This skill should be used when executing work plans efficiently while maintaining quality and finishing features.
Scanned 5/27/2026
Install via CLI
openskills install jikig-ai/soleur---
name: work
description: "This skill should be used when executing work plans efficiently while maintaining quality and finishing features."
---
# Work Plan Execution Command
Execute a work plan efficiently while maintaining quality and finishing features.
## Introduction
This command takes a work document (plan, specification, or todo file) and executes it systematically. The focus is on **shipping complete features** by understanding requirements quickly, following existing patterns, and maintaining quality throughout.
## Headless Mode Detection
If `$ARGUMENTS` contains `--headless`, set `HEADLESS_MODE=true`. Strip `--headless` from `$ARGUMENTS` before processing the remainder as a plan path. Pipeline mode (file path detection) already covers all prompt bypasses for work's own prompts — `--headless` is only needed for forwarding to child skills in Phase 4.
## Input Document
<input_document> #$ARGUMENTS </input_document>
<decision_gate>
**API budget.** This skill executes a work plan iteratively across many phases. Tier A (Agent Teams) carries ~7x per-task token cost; Tier B (Subagent Fan-Out) is moderate; Tier C is single-agent. Total cost scales with plan length, chosen tier, and per-task RED/GREEN/REFACTOR cycles. Soleur does not bill or proxy these calls — Anthropic does, against the key in your session. The Soleur LICENSE (BSL 1.1) disclaims warranty for runtime cost; you operate this loop against your own budget.
The tier offer fires inline at the right phase. Decline if running an unfamiliar plan against a tight budget.
</decision_gate>
## Execution Workflow
### Phase 0: Load Knowledge Base Context (if exists)
**Load project conventions:**
```bash
# Load project conventions
if [[ -f "CLAUDE.md" ]]; then
cat CLAUDE.md
fi
```
**Clean up merged worktrees (silent, runs in background):**
Navigate to the repository root, then run `bash ./plugins/soleur/skills/git-worktree/scripts/worktree-manager.sh cleanup-merged`. Report cleanup results: how many worktrees were cleaned up, which branches remain active.
**Check for knowledge-base directory and load context:**
Check if `knowledge-base/` directory exists. If it does:
1. Run `git branch --show-current` to get the current branch name
2. If the branch starts with `feat-`, read `knowledge-base/project/specs/<branch-name>/tasks.md` if it exists
**If knowledge-base/ exists:**
1. Read `CLAUDE.md` if it exists - apply project conventions during implementation
2. If `# Project Constitution` heading is NOT already in context, read `knowledge-base/project/constitution.md` - apply principles during implementation. Skip if already loaded (e.g., from a preceding `/soleur:plan`).
3. Detect feature from current branch (`feat-<name>` pattern)
4. Read `knowledge-base/project/specs/feat-<name>/tasks.md` if it exists - use as work checklist alongside TodoWrite
4.5. Read `lane:` from spec.md if present. Guard file existence first:
```bash
spec_path="knowledge-base/project/specs/feat-${branch_name}/spec.md"
if [[ -f "$spec_path" ]]; then
LANE=$(awk '/^lane:/ { gsub(/^lane:[[:space:]]*"?|"?$/, ""); print; exit }' "$spec_path")
case "$LANE" in
single-domain|cross-domain|procedural) ;;
"") LANE="" ;; # legacy spec; silent skip in announce
*) echo "work: invalid lane value '$LANE' in spec; ignoring."; LANE="" ;;
esac
fi
```
Lane is **non-binding in skill logic** — `work` code does not branch on `LANE`. Operators MAY use the announced lane as a heuristic when picking work Tier 0/A/B/C in Phase 2; binding behavior is deferred per Non-Goal #2.
5. Announce: `"Loaded constitution and tasks for \`feat-<name>\`"` — append `" (lane=<value>)"` when `LANE` is non-empty.
**If knowledge-base/ does NOT exist:**
- Continue with standard work flow (use input document only)
### Phase 0.5: Pre-Flight Checks
Run these checks before proceeding to Phase 1. A FAIL blocks execution with a remediation message. A WARN displays and continues. If all checks pass, proceed silently.
**Environment checks:**
1. Run `git branch --show-current`. If the result is empty (detached HEAD), FAIL: "Detached HEAD state -- checkout a feature branch or create a worktree." If the result is the default branch (main or master), FAIL: "On default branch -- create a worktree before starting work. Run: `bash ./plugins/soleur/skills/git-worktree/scripts/worktree-manager.sh feature <name>`"
2. Run `pwd`. If the path does NOT contain `.worktrees/`, WARN: "Not in a worktree directory. You can create one via `git-worktree` skill in Phase 1."
3. Run `git status --short`. If output is non-empty, WARN: "Uncommitted changes detected. Consider committing or stashing before starting new work."
4. Run `git stash list`. If output is non-empty, WARN: "Stashed changes found. Review stash list to avoid forgotten work."
**Scope checks:**
5. If a plan file path was provided as input (ends in `.md` or starts with a path-like pattern), verify it exists and is readable. If not, FAIL: "Plan file not found at the specified path." If the input appears to be a text description rather than a file path, WARN: "Input appears to be a description, not a file path. Scope validation limited."
6. Run `git diff --name-only HEAD...origin/main` to identify files that diverged between this branch and main. If output is non-empty, WARN: "Branch has diverged from main in [N] files: [file list]. Consider merging main before starting." If the git command fails (e.g., offline, no remote), skip this check silently. **For plans that edit AGENTS.\* (high-collision file class), `plugins/soleur/skills/ship/SKILL.md` (Phase 5.5 gates), OR any path under `docs/legal/**` / `knowledge-base/legal/**` (legal-doc cross-document gate; weekly compliance PRs collide on the same 4-file set), FAIL HARD instead of WARN — fetch + rebase BEFORE Phase 1 (`git fetch origin main && git rebase origin/main`); sibling PRs landing mid-session reliably obsolete plan-quoted budget baselines and trim-target line numbers. Applying-then-rebasing duplicates sibling work and requires full reassessment.** See `knowledge-base/project/learnings/best-practices/2026-05-20-rebase-before-applying-agents-md-plan-edits.md` and `knowledge-base/project/learnings/2026-05-25-closed-field-list-must-classify-at-value-shape-not-column-name.md` §Session Errors #5 (PR #4351 — 10 commits behind including #4353 legal-doc lockstep; caught at review time, not Phase 0.5).
7. If a plan file was provided (check 5 passed), scan for a `## Domain Review` or `## UX Review` heading (both are accepted for backward compatibility). If NEITHER heading found: scan the plan content for UI file patterns (page.tsx, layout.tsx, template.tsx, .jsx, .vue, .svelte, .astro, +page.svelte, app/, pages/, components/, layouts/, routes/). If UI patterns found, WARN: "Plan references UI files but has no Domain Review section. Consider running /soleur:plan to add domain review before implementing." If either heading IS present: pass silently.
**Design artifact checks:**
8. Check if prior phases produced design artifacts. Search the repo for design files matching the feature name: `git ls-files '*.pen' '*.fig' '*.sketch' | grep -i "<feature-name>"` and check `knowledge-base/product/design/` for related files. If design artifacts exist AND the current tasks include UI/page implementation (patterns: `.njk`, `.html`, `.tsx`, `.jsx`, `.vue`, `.svelte`, `pages/`, `components/`, `layouts/`): store the artifact paths as `DESIGN_ARTIFACTS` for use in Phase 2.
**Specialist review checks:**
9. If a plan file was provided (check 5 passed) and a `## Domain Review` section exists with a `### Product/UX Gate` subsection: check whether domain leader assessments recommended specialists (copywriter, ux-design-lead, conversion-optimizer) that are NEITHER listed in `**Agents invoked:**` NOR in `**Skipped specialists:**`. If the `**Decision:**` field says `reviewed (partial)`, WARN: "Domain review was partial — some specialist agents failed. Review the Domain Review section before proceeding." If any recommended specialist is missing from both fields: **Interactive mode:** FAIL with message listing the missing specialists and options: (a) "Run \<specialist\> now" — invoke the specialist agent directly, update the plan file's `**Agents invoked:**` field, then continue; (b) "Skip with justification" — prompt for reason, add to the plan file's `**Skipped specialists:**` field, then continue. **Pipeline mode (headless/one-shot):** auto-invoke each missing specialist agent. If the agent succeeds, add to `**Agents invoked:**`. If it fails, add to `**Skipped specialists:**` with note `(auto-skipped — agent unavailable in pipeline)` and WARN. Do not FAIL in pipeline mode. If all recommended specialists are accounted for (in `**Agents invoked:**` or `**Skipped specialists:**`): pass silently.
**UX-skip-on-UI-plan hard gate (within check 9):** When `ux-design-lead` appears in `**Skipped specialists:**` AND the plan's `## Files to Create` or `## Files to Edit` contains files matching `components/**/*.tsx`, `app/**/*.tsx`, or `app/**/*.jsx` (UI file patterns), FAIL with: "Plan adds UI components but ux-design-lead was skipped. Invoke the specialist or provide an explicit override naming the specific UI surfaces being shipped without review." This overrides the "all accounted for → pass silently" branch. A documented skip of UX review on a UI-heavy plan is a process gap, not process compliance. See `knowledge-base/project/learnings/workflow-patterns/2026-05-26-ux-design-review-skip-must-fail-hard-on-ui-plans.md`.
**UX artifact commit checkpoint (after each specialist in check 9):** After each specialist agent completes successfully (interactive "Run specialist now" or pipeline auto-invoke), commit the output:
1. Run `git status --short` to discover new/modified files from the specialist
2. Stage specialist output files: `git add <discovered files>`
3. Commit: `git commit -m "wip: <specialist-name> artifacts for <feature-name>"`
Each specialist gets its own commit so partial progress is preserved if a later specialist fails. Do not commit on specialist failure.
**On FAIL:** Display the failure message with remediation steps and stop. Do not proceed to Phase 1.
**On WARN only:** Display all warnings together and proceed to Phase 1.
**On all pass:** Proceed silently to Phase 1.
### Phase 1: Quick Start
**Pipeline detection:** If `$ARGUMENTS` contains a file path (ends in `.md` or matches a path-like pattern), this skill is running in **pipeline mode** (invoked by one-shot or another orchestrator). In pipeline mode, skip all interactive approval gates and proceed directly. If `$ARGUMENTS` is empty or a plain text description, this is **interactive mode** — keep the approval gates below.
1. **Read Plan and Clarify**
- Read the work document completely
- Review any references or links provided in the plan
- Before proceeding, verify the plan does not contradict conventions in AGENTS.md and constitution.md: file format (markdown tables not YAML), kebab-case naming, directory structure (agents recurse, skills flat), required frontmatter fields, shell script conventions
- **Plan-quoted numbers are preconditions to verify, not facts.** When the plan quotes a current measurement (`bun test … reports X`, `wc -c < AGENTS.md = N`, "cumulative ~Y words; ~Z headroom", `git ls-files | wc -l`), re-run the measurement at /work start before depending on it. Plans authored hours-or-days earlier observe a moving target; parallel branches landing in `main` invalidate the measurement. PR #3501 plan claimed `~186 word headroom` against an actual `15` and required inline trim of the gate description. See `knowledge-base/project/learnings/2026-05-10-handshake-schema-drift-and-stale-precondition-budgets.md`.
- **Counts written into the artifact (workflow header, script comment, AC expected-N) must be derived from the as-written file, not from plan-prose estimates.** Plan §Phase X says "~40 resources after expansion" → /work runs the grep → the actual count goes into the workflow header comment AND into AC4's expected N. Cheapest gate: after writing the artifact, re-run the canonical count command against the as-written file (`grep -cE '^[[:space:]]+-target=' <workflow>`, `wc -l <list>`, etc.) and copy the integer into every comment/AC reference. Plan-prose mental tallies drift by ±1-2 during expansion; multi-agent review reliably catches it (P3 polish) but the inline grep at write-time is free. **Why:** PR #4122 — workflow header carried "68 explicit targets" from plan §Phase 0.3 mental tally while `grep` returned 67; caught by `code-quality-analyst` + `pattern-recognition-specialist` at review. See `knowledge-base/project/learnings/best-practices/2026-05-20-plan-time-pr-vs-issue-disambiguation-and-self-derived-counts.md`.
- **Discrete-enumeration re-lockstep (when applicable).** When the plan asserts paraphrased lockstep across discretely-enumerated documents — e.g., "files in lockstep at sections (a)-(N)", "all 7 entries match in both files", "list is (i) through (v) on both sides" — run a fresh `grep -cE '^- \*\*\([a-z]\)\*\* '` (or equivalent, using `[a-z]` NOT `[a-N]`) across each file BEFORE the first letter-inserting Edit. Counts AND letter-sets must match; anything else means the plan's lockstep claim is paraphrase-from-stale-read. PR #3755 (#3708): plan asserted DPD §(a)-(k) lockstep; canonical actually had §(l) DSAR; AC1 caught the §(l) collision but the cheaper gate is re-lockstep at /work-start. See `knowledge-base/project/learnings/2026-05-14-discrete-enumeration-relockstep-and-pr-introduced-asymmetry.md`.
- **Sequential-section insertion anchor.** When inserting a new `### N.M`-numbered section, the Edit anchor MUST target the LAST `### N.M` block before the desired slot (not the lexically-adjacent one). Confirm with `grep -nE '^### [0-9]+\.[0-9]+ ' <file> | tail -3`; the new `M` must be greater than the picked anchor's `M`. PR #3755 (#3708): §5.10 was anchored on §5.9-Resend's block start and landed BEFORE §5.9. One-grep prevention. Same learning file.
- **Write-boundary sentinel sweep (when applicable).** If the plan introduces a sentinel/guard that asserts a property at write sites (e.g., `assertWriteScope` for cross-tenant integrity, GDPR write-boundary checks), enumerate ALL write sites where the property applies — not just diff sites. Run `git grep -nE '\.from\("<table>"\)\.insert\(' <scope>` (or the equivalent for the boundary type) at Phase 0 and verify every match is sentinel-gated, then file follow-up tasks for any uncovered sites BEFORE entering Phase 1. See `knowledge-base/project/learnings/2026-05-12-type-widening-cascades-and-write-boundary-sentinels.md`; hard rule `hr-write-boundary-sentinel-sweep-all-write-sites`. **Why:** PR-A2 #3603 — sentinel placed at assistant-row write but not user-row write at `cc-dispatcher.ts:1008`; same service-role-bypass surface.
- **Type-widening cross-consumer grep (when applicable).** When the PR widens a producer-side shared type whose payload crosses an `unknown`/`any`/jsonb boundary (compiler cannot enforce optionality at the consumer), `git grep -nE '<field-name-pattern>' apps/` across every consumer and verify each respects the new optionality. For `Message`-class fields the canonical grep is `git grep -nE '\bmessage\.usage\.(input_tokens|output_tokens|completed_actions)\b' apps/` (adapt per field family). See learning `2026-05-12-type-widening-cascades-and-write-boundary-sentinels.md`; hard rule `hr-type-widening-cross-consumer-grep`. **Why:** PR-A2 #3603.
- **Sweep-class fixes use grep-enumerated work-lists, not intuited ones.** When a plan declares a multi-file sweep with a verification grep (e.g., AC5-style `git grep -nE '<pattern>' <scope> | grep -vE '<safe-form>' | wc -l = 0`), run the grep ONCE at Phase 0 to enumerate the authoritative work-list (write the hits to `/tmp/sweep-targets.txt`), fix each line, then re-run the grep after each batch. The plan's narrative enumeration of "files X, Y, Z" is a starting hypothesis; the grep result is the work-list. Same applies to regex widenings: enumerate the configs/verbs/paths invoked by the IN-SCOPE runbooks, not the configs the incident occurred against — the incident is one data point; the trap-class config-set is the full union of every config the runbooks touch. **Why:** PR #4031 — initial sweep handled 9 named runbook hits but missed 2 buried in deeper sections of the same files; widened regex covered `(prd|prd_terraform|dev|ci)` per the plan's leak-footprint enumeration but missed `prd_orchestration` which 2 in-scope runbooks operate against. Pattern-recognition + security-sentinel caught both at multi-agent review. See `knowledge-base/project/learnings/best-practices/2026-05-18-sweep-class-fixes-grep-enumerated-not-intuited.md`.
- **Interactive mode only:** If anything is unclear or ambiguous, ask clarifying questions now. Get user approval to proceed. **Do not skip this** - better to ask questions now than build the wrong thing.
- **Pipeline mode:** Skip clarifying questions and approval. Proceed directly to step 2.
2. **Setup Environment**
First, check the current branch by running `git branch --show-current`. Then determine the default branch by running `git symbolic-ref refs/remotes/origin/HEAD` and extracting the branch name. If that fails, check whether `origin/main` exists (fallback to `master`).
**If already on a feature branch** (not the default branch):
- **Interactive mode only:** Ask: "Continue working on `[current_branch]`, or create a new branch?"
- **Pipeline mode:** Continue on current branch without asking.
- If continuing, proceed to step 3
- If creating new, follow the worktree creation instructions below
**If on the default branch**, you MUST create a worktree before proceeding. Never edit files on the default branch -- parallel agents cause silent merge conflicts, and this repo uses `core.bare=true` where `git pull` and `git checkout` are unavailable.
Create a worktree for the new feature:
```bash
SOLEUR_SKILL_NAME=work SOLEUR_EXPECTED_DURATION_MIN=240 \
bash ./plugins/soleur/skills/git-worktree/scripts/worktree-manager.sh --yes create feature-branch-name
```
Then `cd` into the worktree path printed by the script. The worktree manager handles bare-repo detection, branch creation from latest origin/main, .env copying, and dependency installation. The env vars wire a session lease so sibling cleanup-merged invocations refuse to reap this worktree.
**Phase Exit (release lease).** At the end of the workflow — after `/soleur:ship` returns OR if you exit without shipping — release the lease so a sibling `cleanup-merged` can reap the worktree once it's actually merged:
```bash
bash .claude/hooks/lib/session-state.sh release_lease "$(basename "$PWD")"
```
The release is a no-op if the lease was already removed by the multi-signal trap (EXIT/INT/TERM/HUP fires on abnormal exit). Stale leases get swept after 24 hours regardless.
Use a meaningful name based on the work (e.g., `feat-user-authentication`, `fix-email-validation`).
3. **Create Todo List (TDD-First Structure)**
Structure tasks as RED/GREEN/REFACTOR units, not as "implement everything, then test":
- For each feature requirement with Acceptance Criteria or testable behavior:
- Create a **RED task**: "Write failing test for [feature]" — the test file with at least one failing test
- Create a **GREEN task**: "Implement [feature] to pass tests" — blocked by its RED task
- Group these as a TDD unit with `blockedBy` dependency (GREEN blocked by RED)
- Infrastructure-only tasks (config files, CI, scaffolding, legal docs) are exempt from RED/GREEN pairing — create them as standalone tasks
- Place a final "Run full test suite and lint" task at the end, blocked by all other tasks
- Keep tasks specific and completable
**Anti-pattern to avoid:** Creating a task list like `[implement A, implement B, implement C, ..., write tests, lint]`. This structure guarantees TDD violation because the agent executes tasks in order. The correct structure is `[RED: test A, GREEN: implement A, RED: test B, GREEN: implement B, ..., lint]`.
**Post-creation validation (HARD GATE):** After creating all tasks, scan the task list for any non-exempt implementation task (GREEN) that does NOT have a corresponding RED test task in its `blockedBy` list. If found, restructure the task list before proceeding. Do not start Phase 2 with an invalid task structure. **Why:** In PR #2428, the agent created flat tasks ("Fix X", "Write tests") and started implementation before tests — the user had to intervene and force a restructure. The anti-pattern instruction was not enough without a validation gate.
### Phase 2: Execute
1. **Execution Mode Selection** (HARD GATE — must complete before executing ANY task)
**Do NOT execute any task before completing this analysis.** Analyze independence first, select the execution tier, then begin. Starting sequential execution "because the first tasks feel simple" is a workflow violation — it forfeits parallelization savings on the remaining tasks.
Before starting the sequential task loop, check for parallelization opportunities:
**Step 0: Tier 0 pre-check (Lifecycle Parallelism)**
Read the plan. Apply a single judgment: "Does this plan have distinct code and test workstreams that can be assigned to separate agents with non-overlapping file scopes?"
- If yes (interactive mode): offer Tier 0 to the user
- If yes (pipeline mode): auto-select Tier 0 without prompting
- If declined or ineligible: fall through to Step 1 below
**Read `plugins/soleur/skills/work/references/work-lifecycle-parallel.md` now** for the full Tier 0 protocol (offer/auto-select, generate contract, spawn 2 agents, collect/commit, test-fix-loop, docs). If Tier 0 executes, proceed directly to Phase 3 after completing Step 06 of the protocol. If declined, fall through to Step 1.
---
**Step 1: Analyze independence**
Read the TaskList. Identify tasks that have no `blockedBy` dependencies and reference
different files or modules (no obvious file overlap). Count the independent tasks.
If fewer than 3 independent tasks exist, skip to **Tier C: Sequential** below.
If 3+ independent tasks exist, proceed through the tiers in order (A, then B, then C).
Each tier either executes or falls through to the next.
---
**Pipeline mode override:** If running in pipeline mode (plan file argument detected in Phase 1), auto-select Tier 0 if eligible (Step 0 above). If Tier 0 is ineligible, skip Tier A entirely and auto-accept Tier B without prompting. Do not present "Run as Agent Team?" or "Run in parallel?" questions -- proceed directly to Step B2 of the Subagent Fan-Out protocol if 3+ independent tasks exist, otherwise fall through to Tier C.
---
**Tier A: Agent Teams** (highest capability, ~7x token cost)
**Read `plugins/soleur/skills/work/references/work-agent-teams.md` now** for the full Agent Teams protocol (offer, activate, spawn teammates, monitor/commit/shutdown). If declined or failed, fall through to Tier B.
---
**Tier B: Subagent Fan-Out** (fire-and-gather, moderate cost)
**Read `plugins/soleur/skills/work/references/work-subagent-fanout.md` now** for the full Subagent Fan-Out protocol (offer, group/spawn, collect/integrate). If declined, fall through to Tier C.
---
**Tier C: Sequential** (default)
Proceed to the task execution loop below.
2. **Task Execution Loop**
**Design Artifact Gate (before first UI task):** If `DESIGN_ARTIFACTS` was set in Phase 0.5, spawn the `ux-design-lead` agent with the artifact paths and ask it to produce an **implementation brief** (see ux-design-lead "Wireframe-to-Implementation Handoff" workflow). The brief is a structured description of every section, its content, and its layout — this becomes the binding input for all UI tasks. Do not write any markup until the brief is received.
**UX artifact commit checkpoint (after Design Artifact Gate):** After the implementation brief is received, commit before proceeding to UI tasks:
1. Run `git status --short` to discover the implementation brief and any generated design files
2. Stage output files: `git add <discovered files>`
3. Commit: `git commit -m "wip: UX implementation brief for <feature-name>"`
This checkpoint ensures the implementation brief survives session crashes.
For each task in priority order:
```text
while (tasks remain):
- Mark task as in_progress in TodoWrite
- Read any referenced files from the plan
- If task creates UI/pages: verify implementation brief exists (HARD GATE)
- TDD GATE: (see below)
- Look for similar patterns in codebase
- RED: Write failing test(s) for this task's acceptance criteria
- GREEN: Write minimum code to make the test(s) pass
- REFACTOR: Improve code while keeping tests green
- Run full test suite after changes
- Mark task as completed in TodoWrite
- Mark off the corresponding checkbox in the plan file ([ ] → [x])
- Evaluate for incremental commit (see below)
```
**No mid-plan pause gates (HARD GATE).** A multi-phase plan
(`tasks.md` Phase 0 through Phase N) is a SINGLE execution unit.
Do NOT insert "Pause for review or continue?" prompts between
phases. Do NOT end a turn after one phase commits with "Continue
into Phase N+1 next turn?". The skill's Phase 4 handoff is the
only sanctioned stopping point — until then, chain straight
through every phase the plan defines, including phases the plan
labels "Pre-merge verification" or "Post-merge (operator)" if
they're automatable per the next gate. **Why:** the founder is a
solo operator; every "continue or pause?" is a context switch
that defeats the entire point of a multi-phase plan. Pipeline
mode (file-path arg in Phase 1) means pipeline mode for the WHOLE
plan, not per-phase.
**Operator-step automation gate (HARD GATE).** Before treating
any task in `tasks.md` as "operator-driven" (apply migration,
verify pg_cron, verify Storage bucket, run end-to-end smoke,
`gh pr ready`, `gh pr merge --auto`), check whether it is
automatable via a loaded MCP server or CLI:
- Supabase migrations + `cron.job` queries + Storage bucket
existence + RLS spot-checks → `mcp__plugin_supabase_supabase__*`
**with Doppler `DATABASE_URL_POOLER` fallback when MCP is
unavailable** — see "Supabase fallback chain" below.
- `gh pr ready` / `gh pr merge --squash --auto` / `gh issue close`
→ Bash via `gh` CLI
- End-to-end UI flow → Playwright MCP (`mcp__playwright__*`)
- Cloudflare DNS / WAF / Workers → `mcp__plugin_soleur_cloudflare__*`
- Live Stripe state → `mcp__plugin_soleur_stripe__*`
If automatable, EXECUTE it inline as part of the work pipeline —
never list it back to the operator. The /ship skill already
handles `gh pr ready` + auto-merge + migration verification (see
`plugins/soleur/skills/ship/SKILL.md`); chain to `/soleur:ship`
at Phase 4 and let it run. For migration **apply** to dev (vs
verify), invoke `mcp__plugin_supabase_supabase__apply_migration`
inline at the phase where the migration lands, not as a
post-merge todo. **Why:** see ship/SKILL.md:1027 ("Every 'please
run this manually' is a context switch") and ship/SKILL.md:1177
(PR #1375 — migration verification was left as a manual
"post-merge todo" instead of being executed; deployed code
expected the new schema and broke). Same class as the
Playwright-first audit in Phase 4: if a tool exists, use it.
**Supabase fallback chain (when MCP OAuth fails).** The Supabase
MCP OAuth flow at `https://api.supabase.com/v1/oauth/authorize`
intermittently rejects valid URLs at the dashboard `auth_id`
handoff (cause: external — Supabase-side). When that happens, do
NOT fall back to "paste this SQL into the dashboard SQL editor"
handoff — that's a manual-step rationalisation that violates
`hr-never-label-any-step-as-manual-without`. Instead walk down the
`hr-exhaust-all-automated-options-before` priority chain:
(1) Doppler `DATABASE_URL_POOLER` — already provisioned for every
env; the migration apply path. (2) Verify the project ref in the
URL matches the plan's stated dev/prd refs — Doppler is the
source of truth (plan-quoted project refs are preconditions to
verify, never facts; the plan can drift). (3) Rewrite the URL's
port `:6543` → `:5432` so the pooler runs in session mode (multi-
statement DDL works; transaction mode rejects with SQLSTATE 42601
"cannot insert multiple commands into a prepared statement").
(4) Apply via `pg` (node-pg, bun-installed in `/tmp` if missing)
wrapped in `BEGIN; <migration>; COMMIT;`. The direct DB host
`db.<ref>.supabase.co:5432` is IPv6-only and typically
unreachable from operator/CI networks; the pooler is IPv4.
(5) Post-apply, verify schema via the same connection — RLS
enabled, policy_count, trigger names, RPC signatures + SECURITY
DEFINER flag, UNIQUE constraints. Write the verification artifact
to `knowledge-base/project/specs/feat-<name>/migration-checklist.md`.
**Why:** PR #3853 / #3205 — Supabase MCP OAuth was rejecting URLs
at the auth_id handoff; the agent first proposed "paste SQL into
dashboard" (manual-step violation), then pivoted to Playwright-
first audit on dashboard navigation (correct), then discovered
Doppler had the working `DATABASE_URL_POOLER` and applied via
pg directly — the path it should have taken at step 1.
**Pre-apply collision check (always, even on first attempt).**
Before invoking pg apply (or `supabase migration up`) against any
shared env, run `git fetch origin main && git ls-tree origin/main
-- apps/web-platform/supabase/migrations/ | awk '{print $4}' |
grep -oE '^[0-9]{3}_[^.]+' | sort -u`. For each LOCAL migration
file the branch introduces, assert no DIFFERENT filename with the
same 3-digit prefix exists in that list. A collision means a
sibling PR is landing the same number window; renumber FIRST,
then apply under the final filename. **Why:** PR #4225 — applied
053–057 in the morning; PR #4251 landed `054_schema_migrations_
content_sha.sql` 10 hours later and main's CI drift probe flagged
the entire branch; the recovery (renumber 054→058, 055→059, 056→060,
057→061 + reconcile `public._schema_migrations` on both dev + prd
via `git hash-object` content_sha) took ~30 min and could have been
zero-cost if the operator had grepped origin/main first.
**Tracking row in the SAME transaction as the migration body.**
The project's canonical `apps/web-platform/scripts/run-migrations.sh`
writes `INSERT INTO public._schema_migrations (filename, content_sha)
VALUES ('<basename>', '<git-hash-object>')` in the same transaction
as the migration SQL. The Doppler+pg fallback MUST mirror this —
bare `BEGIN; <migration>; COMMIT;` produces a phantom-applied state
where the schema reflects the migration but `_schema_migrations`
does not, and the next deploy attempts re-apply (failing on
non-idempotent statements like `CREATE TRIGGER`). The reconciliation
pattern (UPSERT with `ON CONFLICT (filename) DO UPDATE SET
content_sha = EXCLUDED.content_sha`) is the recovery shape — but
doing it inline is cheaper.
**PostgREST schema cache reload via session-mode pooler does NOT
work.** `NOTIFY pgrst, 'reload schema'` over a `:5432` pooler
connection does not reach PostgREST's `LISTEN` (PgBouncer
multiplexes; LISTEN/NOTIFY channel scope is bound to backend
process identity, not session). 90 attempts over 5 minutes
returned `PGRST205`. After a direct-pg apply: either wait for the
natural ~10-min schema poll cycle, OR use the Supabase Management
API to restart PostgREST. The direct DB host
(`db.<ref>.supabase.co:5432`) is IPv6-only and typically
unreachable from operator networks, so the canonical "NOTIFY via
direct connection" workaround documented upstream isn't available.
**TDD Gate (HARD GATE):** Before writing ANY implementation code for a task, determine if the task has testable behavior:
Emit rule-application telemetry (records that the TDD gate was reached — see AGENTS.md `cq-write-failing-tests-before`):
```bash
source "$(git rev-parse --show-toplevel)/.claude/hooks/lib/incidents.sh" && \
emit_incident cq-write-failing-tests-before applied \
"Write failing tests BEFORE implementation code whe"
```
1. **Check:** Does the plan have a "Test Scenarios" or "Acceptance Criteria" section that covers this task? If yes, this task requires test-first.
2. **Exempt:** Infrastructure-only tasks (config files, CI workflows, scaffolding directories, dependency installs) are exempt. If the task only creates/modifies config, it skips to Infrastructure Validation below.
3. **Enforce:** For non-exempt tasks, write the failing test file FIRST. The test must:
- Import the component/function/module that will be created (the import will fail — that is correct)
- Assert the specific behavior from the acceptance criteria
- Be runnable via the project's test command (even if it fails due to missing implementation)
4. **Verify RED:** Run the test. It must fail (missing module, assertion failure, etc.). If it passes, the test is not testing new behavior — rewrite it. **For gating/sequencing primitives (semaphores, locks, queues, ordering guarantees), the test must distinguish gate-absent from gate-present: add an intermediate-state assertion that would fail without the primitive (e.g., `count === 2` while two slots are held) in addition to the final-state assertion. A test that passes identically with and without the primitive isn't testing the primitive.** See `knowledge-base/project/learnings/test-failures/2026-04-18-red-verification-must-distinguish-gated-from-ungated.md`. **Test-environment fidelity:** if the SUT's buggy code lives behind a guard (`if [[ -d "$X" ]]`, `if (cache.has(key))`, etc.), the harness MUST seed the precondition the guard requires — otherwise both buggy and fixed paths short-circuit identically and any negative-space assertion passes vacuously. See `knowledge-base/project/learnings/test-failures/2026-04-22-red-test-must-simulate-suts-preconditions.md`. **Early-exit shadowing:** if the SUT has a guarded fast path (substring strip like `replaceAll(arg, "")`, cache-hit, env-flag short-circuit) that handles a superset of inputs the slow path under test handles, RED inputs MUST choose identities ONLY the slow path can produce. Sharing a fixture across the fast/slow boundary lets the fast path scrub first and the regex/branch under test never fires — the assertion passes without testing the fix. Add an invariant guard test asserting the fast/slow fixtures do not collide. See `knowledge-base/project/learnings/2026-05-04-vacuous-red-via-shared-fixture-and-toolchain-pinning.md`. **In-component state machines (RTL):** when the gate-under-test is component-local state (`useState`/`useRef`/`useReducer`), drive the SUT through state transitions with `result.rerender(<C />)` — never `unmount()` + fresh `render()`. Remount resets the in-component bookkeeping that IS the gate, producing vacuous green. See `knowledge-base/project/learnings/test-failures/2026-05-11-rerender-not-remount-for-in-component-state-machine-tests.md`.
5. **Only then:** Write the minimum implementation to make the test pass (GREEN).
6. **Refactor:** Improve code while keeping tests green.
Skipping this gate — writing implementation before tests — is a workflow violation equivalent to committing directly to main. The rationalization "this is simple enough to not need test-first" is exactly the reasoning TDD is designed to prevent.
- When adding MCP tools to an existing registration block in agent-runner.ts, verify each tool's prerequisites are independent of the block's guard condition. Write a test that validates the new tool works WITHOUT the existing block's prerequisites (e.g., Plausible tools work without GitHub installation).
- When adding route handler tests that require `vi.mock()`, create a separate test file from existing unit tests that import the real module. Vitest hoists all `vi.mock()` calls to the top of the file, clobbering real imports for the entire file regardless of describe block scope.
- When creating test files with `vi.mock()` factories that reference shared variables, use `vi.hoisted()` from the start -- vitest hoists `vi.mock` to the top of the file before `const`/`let` declarations execute.
- When mocking `child_process.spawn`, `fetch`, or any constructor returning an event-emitter-like object, use `mockImplementation(() => factory(...))` rather than `mockReturnValue(factory(...))`. `mockReturnValue` evaluates the factory eagerly at test-setup time; any `queueMicrotask` / `setTimeout` / `setImmediate` scheduled inside the factory fires BEFORE the SUT attaches its listeners, producing empty event data or an "uncaught error" test timeout. See `knowledge-base/project/learnings/test-failures/2026-04-17-vitest-mockReturnValue-eager-factory-async-event-race.md`.
- When using `vi.doMock("specifier", () => { throw new Error("X") })` to simulate a module-init failure, do NOT assert on the inner error message via the SUT's caller. Vitest wraps factory throws with its own synthetic Error (`"[vitest] There was an error when mocking a module..."`) and the inner string is unobservable. Assert on the SUT's observable contract (return shape, observability mirror call) instead — the throw is a *trigger*, not a *contract*. See `knowledge-base/project/learnings/2026-05-07-vitest-domock-factory-throw-wrapped-message.md`.
- To prove a cache-hit skips work (not just that the response status is correct), wrap the real implementation in a spy via `vi.importActual` rather than stubbing the return value: `vi.mock("@/module", async () => { const actual = await vi.importActual(...); return { ...actual, expensiveFn: (...args) => { spy(...args); return actual.expensiveFn(...args); } })`. Stubbed returns break any downstream behavior that depends on the real output (hash-match, SQL row shape, etc.); wrapping preserves the contract while exposing call counts for assertions like `expect(spy).toHaveBeenCalledTimes(1)` across a HEAD+GET sequence. **Why:** In PR #2515, verifying that HEAD populates `shareHashVerdictCache` so a follow-up GET skips the SHA-256 drain required counting `hashStream` calls, not stubbing its return — a stubbed return would have broken the post-drain hash-equality check and masked the very regression the test was meant to catch.
- When testing decorative images (alt="") with happy-dom, use container.querySelector instead of screen.getAllByRole("img", { hidden: true }) -- happy-dom excludes presentational elements from role queries even with hidden: true.
- When asserting against a conditional render branch in a component test, grep the test file's `vi.mock(...)` factories for the inputs the branch reads and confirm the mock returns values that activate the target branch. Mocks that simplify (e.g., `getDisplayName: (id) => id.toUpperCase()`) often skip production branches like `leader.title.includes(displayName)` — assertions on the skipped branch fail for non-bug reasons. **Why:** PR #3427 — see `knowledge-base/project/learnings/2026-05-07-test-assertion-must-verify-mock-activates-branch.md`.
- When adding `sessionStorage` usage to React components, ensure the component's test file includes `sessionStorage.clear()` in its `beforeEach` block. Shared jsdom environments leak sessionStorage between tests, causing ordering-dependent failures.
- When asserting on `vi.getTimerCount()`, remember that `vi.useFakeTimers()` mocks every timer-like API by default — including `requestAnimationFrame`, `setImmediate`, `queueMicrotask`, `requestIdleCallback`. The count is a SUM across all fake timer types, not just `setTimeout`. Prefer stability assertions (`count before N extra calls === count after`) over magnitude assertions (`count === 1`) so refactors that add a well-behaved rAF or microtask don't falsely read as leaks. See `knowledge-base/project/learnings/test-failures/2026-04-17-vitest-getTimerCount-counts-requestAnimationFrame.md`.
- When a component exports an interface that a test harness consumes (e.g., `ChatInputQuoteHandle`), have the test import it via `type X = ExportedInterface` — never shadow with a local duplicate. Duplicate interfaces silently drift when the exported type gains a method; the `tsc --noEmit` failure surfaces only at build time.
- When adding a new npm dependency, check the installed major version (`node -e "console.log(require('<pkg>/package.json').version)"`) and read the type definitions before using API from docs or training data. Library APIs change across major versions (e.g., `react-resizable-panels` v4 uses `Group`/`Separator`/`orientation`/`useDefaultLayout`, not v2's `PanelGroup`/`PanelResizeHandle`/`direction`/`autoSaveId`).
- For sizing APIs from third-party libraries, always pass **explicit units as strings** (e.g., `"18%"`, `"100px"`, `"1rem"`) rather than bare numbers. Docstrings may claim a default unit but runtime parsers often treat numbers as pixels. **Why:** `react-resizable-panels` v4 doc said "Percentage of the parent Group (0..100)" for numeric sizes, but the runtime treated `18` as 18px, producing a ~18px-wide sidebar in production. Explicit units make intent visible at the call site and survive library version upgrades.
**Test environment setup:** If the project's test runner cannot run the type of test needed (e.g., React component tests require jsdom but vitest is configured for node), set up the test environment BEFORE starting the task. This is part of RED — the test infrastructure must exist for the test to fail properly.
- When configuring bun preload scripts that register DOM globals (e.g., happy-dom), use dynamic `await import()` for all subsequent dependencies — static ES imports are hoisted before any imperative code, causing libraries like @testing-library/react to initialize without DOM globals. See `knowledge-base/project/learnings/test-failures/2026-04-03-bun-test-dom-preload-execution-order.md`.
- When a test file calls a SUT that lazy-imports a heavy module (`pdfjs-dist`, `sharp`, `puppeteer`, `playwright`, `@xenova/transformers`, `onnxruntime`), pre-warm the module in `beforeAll(async () => { await import("<module>"); }, 30_000)`. The cold-start cost (~5-10s on CI runners) otherwise lands on the first `it()` and blows the default 5s vitest timeout — the second test in the same file runs at warm ~9ms because subsequent calls hit the module cache. Cheapest detection: `git grep -lE '(pdfjs-dist|sharp|puppeteer|playwright|@xenova/transformers|onnxruntime)' -- '*.test.ts'` and check for sibling `beforeAll`. **Why:** PR #3681 `pdf-text-extract.test.ts` cold-start flake (7s vs 9ms, #3687).
- When uploading files via Playwright MCP, save files to repo-accessible paths (not `/tmp/`). Playwright MCP restricts file access to the repo root. When Google Search Console offers Cloudflare auto-verification, prefer "Any DNS provider" manual flow — the popup OAuth flow opens an external tab that crashes the Playwright browser context.
- **Vendor-token extraction via Playwright MUST use `browser_evaluate(filename: ...)` from the FIRST attempt** — the return value otherwise enters the conversation transcript and the token is leaked even after revocation. AND the `filename` parameter JSON-encodes the result (surrounding quotes), so the canonical pipe is `python3 -c "import sys,json; sys.stdout.write(json.loads(open('<path>').read()))" | doppler secrets set <KEY> --no-interactive`. Validate via the vendor's API (HTTP 200 + length check) before shredding the file — some vendors silently tolerate quoted tokens via `Authorization: Bearer "abc"`, but Terraform's HCL parser does not. For `●●●`-masked UI tokens (Doppler personal tokens), click the in-page copy button via `browser_evaluate`, then `xclip -selection clipboard -o > <path>`; clear with `xclip -i </dev/null`. **Doppler TF var storage convention:** drop the `TF_VAR_` prefix from the secret name — `--name-transformer tf-var` ADDS the prefix at injection time (`DOPPLER_TOKEN_TF` → `TF_VAR_doppler_token_tf`; storing the already-prefixed `TF_VAR_DOPPLER_TOKEN_TF` produces `TF_VAR_tf_var_doppler_token_tf`). See [`2026-03-21-doppler-tf-var-naming-alignment.md`](../../../../knowledge-base/project/learnings/2026-03-21-doppler-tf-var-naming-alignment.md). **Why:** PR #3973 (#3960) — full pattern + recovery flow at [`2026-05-18-vendor-token-mint-and-oci-image-content-carrier-patterns.md`](../../../../knowledge-base/project/learnings/2026-05-18-vendor-token-mint-and-oci-image-content-carrier-patterns.md).
- After any `Write` whose hook output emits a warning (security, style, rule), immediately `Read` the file to verify the full content landed. PreToolUse hooks that print error output but return non-blocking status can still cause partial writes — detecting this only when tests fail wastes a debug round. See `knowledge-base/project/learnings/2026-04-15-kb-share-binary-files-lifecycle.md`.
- When adding source-reading regex tests (`readFileSync(path)` + `expect(src).toMatch(...)`) as a negative-space regression gate after an extraction, put them in a standalone `*.test.ts` file — never add them to an existing test file that already mocks `node:fs` or `node:path`. The existing `vi.mock("node:fs", ...)` factory likely omits `readFileSync`, and the new test will fail at collection with "No `readFileSync` export is defined" before any assertion runs. Also trim the gate to only the assertion that cannot be expressed behaviorally — usually the negative "symbol-not-present" check. Positive assertions (import regex, await-call regex) duplicate coverage that mock-based behavioral tests already provide and are brittle to barrel re-exports, aliases, and whitespace. See `knowledge-base/project/learnings/best-practices/2026-04-17-regex-on-source-delegation-tests-trim-to-negative-space.md`.
- When a bun-test file mutates `process.env.*` or `globalThis.*`, capture originals at module top-level (before any `describe`) and restore in `afterEach` using `delete` when the original was `undefined` — `bun test` runs every file in a single OS process, so mutations leak to sibling files and to any `spawnSync` subprocess launched after the mutation. Vitest isolates files in workers by default; bun does not, and has no built-in `stubEnv`/`unstubAllEnvs` equivalent. **Why:** PR #2579 — `bot-fixture-helpers.test.ts` stubbed `SUPABASE_URL` in `beforeEach` with no restore, causing 4 integration tests in `bot-fixture.test.ts` (same run) to ConnectionRefused against the stub host. See `knowledge-base/project/learnings/test-failures/2026-04-18-bun-test-env-var-leak-across-files-single-process.md`.
- When a vitest test asserts a `process.env.X === "true"`-gated default-off path, add `vi.stubEnv("X", "")` to `beforeEach` regardless of whether the current Doppler/CI config injects the var. `vi.unstubAllEnvs()` reverts `vi.stubEnv` writes only — it CANNOT delete a process-inherited env var (Doppler dev / CI secrets / `direnv` / devcontainer envs). The test passes locally with plain `npx vitest run` and fails deterministically under `doppler run -p soleur -c dev -- npx vitest run` when the dev config flips the flag on. Tests that need the flag on continue to call `vi.stubEnv("X", "true")` in their own `it()` bodies — the local stub overrides the beforeEach default (overwrite-semantics). **Why:** PR #4141 (#4128) — `cc-dispatcher.test.ts > T-W4-basic-off` failed 1/1 under Doppler dev because `CC_PERSIST_USAGE=true` injection survived `unstubAllEnvs()`. See `knowledge-base/project/learnings/test-failures/2026-05-20-vitest-unstub-does-not-clear-process-inherited-env-vars.md`.
- When a test uses retry-on-flake logic (network, LLM non-determinism, timing), collect every attempt into an array and assert the invariant across ALL attempts — not just the last. Early-return after retry silently drops first-attempt failures. If the retry exists to force a precondition (tool invocation, tool output presence), assert the precondition WAS met on the final attempt; a refusal on retry is a hard failure, not a silent pass. **Why:** PR #2610 FR2-smoke/FR8/FR9 originally used `if (!condition) { retry; return; }` which let attempt-1 leaks slip through. See `knowledge-base/project/learnings/test-failures/2026-04-19-retry-once-early-return-masks-first-attempt-failures.md`.
- When adding Eleventy `_data/*.js` files: (a) name the file in camelCase matching a valid JS identifier — kebab-case filenames produce hyphenated template variables that Nunjucks dotted access cannot resolve; (b) keep the module **default-export-only** — sibling `export` statements silently disable Eleventy's data-module registration (no error, no warning, the benchmark log omits the file); attach test helpers as properties on the default export. Verify each new `_data/*.js` appears in the build's `Benchmark ... (Data) ...` log. **Why:** PR #2596 — see `knowledge-base/project/learnings/build-errors/2026-04-18-eleventy-data-module-loading-and-nunjucks-null-test.md`.
- Nunjucks has **no** `is null` / `is not null` test — the parser accepts `{% if x is not null %}` but evaluates it unpredictably for numeric values. To distinguish `undefined` / `null` / `0`, precompute a boolean in the `_data/*.js` module (e.g., `{ stars, showStars: stars != null }`) or accept a truthy guard. **Why:** PR #2596 — see same learning file.
- When a work task ports a TS regex normalizer to SQL (or vice versa) for a backfill migration, run every fixture from the TS unit test file through the SQL expression BEFORE committing the migration. Cheapest shape: a `WITH fixtures AS (VALUES (<input>, <expected>), ...) SELECT input, expected, <sql-expr> AS actual, expected = <sql-expr> AS ok FROM fixtures` query. The WHERE-clause idempotence guard (`col <> <normalized-expr>`) is necessary but not sufficient — it only catches drift on re-runs, not on first apply. Idempotence fixtures must include at least one repeated-suffix case per strip-class (`.git.git`, trailing `//`) so a `\.git$` that should be `(\.git)+$` is forced to fail. **Why:** PR #2817 — migration 031 had a P1 operator-precedence bug (`.git` stripped before trailing `/`) and a P2 non-idempotency bug (`bar.git.git`), both caught only at multi-agent review. See `knowledge-base/project/learnings/best-practices/2026-04-22-ts-sql-normalizer-parity-when-shipping-backfill-migration.md`.
**IMPORTANT**: Always update the original plan document by checking off completed items. Use the Edit tool to change `- [ ]` to `- [x]` for each task you finish. This keeps the plan as a living document showing progress and ensures no checkboxes are left unchecked.
3. **Incremental Commits**
After completing each task, evaluate whether to create an incremental commit:
| Commit when... | Don't commit when... |
|----------------|---------------------|
| Logical unit complete (model, service, component) | Small part of a larger unit |
| Tests pass + meaningful progress | Tests failing |
| About to switch contexts (backend → frontend) | Purely scaffolding with no behavior |
| About to attempt risky/uncertain changes | Would need a "WIP" commit message (exception: UX artifacts use `wip:` prefix) |
| UX specialist produces artifacts (wireframes, copy, brief) | Specialist is still generating (mid-output) |
| Domain leader review cycle completes (feedback applied) | Review feedback not yet incorporated |
| Brand guide alignment pass completes | Alignment still in progress |
- When lefthook hangs during commit in a worktree (common with `core.bare=true` repos), verify typecheck and tests pass manually, then use `LEFTHOOK=0 git commit`. Always check for stalled lefthook processes (`pgrep -fa lefthook`) before retrying.
- **When a commit needs a machine-readable trailer (`Allowlist-Widened-By:`, `Signed-off-by:`, `Reviewed-by:`, etc. — anything downstream parses via `git log --format='%(trailers:key=NAME,valueonly)'`), keep the FINAL paragraph as a pure contiguous block of `Token: value` lines.** Two silent-drop shapes: (a) blank line between the new trailer and `Co-Authored-By:` makes the former part of the body, not a trailer; (b) ANY non-key:value line in the final paragraph (e.g., `Closes #3877.`, `Refs #3874 (precedent).`) invalidates the WHOLE block — both legitimate trailer lines below it drop silently. Put `Closes`/`Refs`/`Fixes` in mid-body prose; GitHub auto-close still works anywhere in the body. Verify locally with `git interpret-trailers --parse < <(git log -1 --format=%B)` — empty output for a trailer that should exist is a hard fail. See `knowledge-base/project/learnings/2026-05-16-git-trailer-parser-requires-contiguous-key-value-block.md`.
**Heuristic:** "Can I write a commit message that describes a complete, valuable change? If yes, commit. If the message would be 'WIP' or 'partial X', wait."
**UX artifact heuristic:** "Did a specialist just produce or revise artifacts? If yes, commit with `wip: UX <description> for feat-X`. UX artifacts are high-effort and low-recoverability -- err on the side of committing too often rather than too rarely."
The `wip:` prefix is intentional -- UX artifacts are valuable at every revision stage, and WIP commits are squashed on merge with no impact on final git history. Do not run compound before UX WIP commits -- compound runs once in Phase 4.
**Compound-before-commit scope:** AGENTS.md Workflow Gates says "Before every commit, run compound." Within this skill, that rule applies to the **final Phase 4 commit** (the one that closes the feature), not to Phase 2 incremental commits. Running compound per incremental commit is recursive (compound creates commits) and defeats the point of incremental checkpoints. A single compound at Phase 4 covers the whole feature's session-error inventory and learnings.
**Commit workflow:**
```bash
# 1. Verify tests pass (use project's test command)
# Examples: bin/rails test, npm test, pytest, go test, etc.
# 2. Stage only files related to this logical unit (not `git add .`)
git add <files related to this logical unit>
# 3. Commit with conventional message
git commit -m "feat(scope): description of this unit"
```
**Handling merge conflicts:** If conflicts arise during rebasing or merging, resolve them immediately. Incremental commits make conflict resolution easier since each commit is small and focused.
**Note:** Incremental commits use clean conventional messages without attribution footers. The final Phase 4 commit/PR includes the full attribution.
4. **Follow Existing Patterns**
- The plan should reference similar code - read those files first
- Match naming conventions exactly
- Reuse existing components where possible
- Follow project coding standards (see CLAUDE.md)
- When in doubt, grep for similar implementations
- **Before writing a new format, date, or util helper in any app, `ls` + grep the app's canonical `lib/` directory (e.g., `apps/web-platform/lib/`) for equivalents.** Canonical helpers are often single-purpose small files named by verb (`relative-time.ts`, `format-currency.ts`); typecheck and tests will not catch duplicated logic. See `knowledge-base/project/learnings/2026-04-17-grep-lib-before-writing-format-helpers.md`.
- **When the plan's specified path is wrong and you correct it during implementation, immediately `git grep` the corrected path's basename across the diff scope and fix EVERY secondary citation in the same edit cycle.** The plan often appears as an authoritative path source in multiple secondary artifacts (Article 30 register entries, runbooks, ADRs, README references); fixing only the primary landing site leaves a silent drift the reviewer must catch. The plan is authoritative for intent, never for paths (`hr-when-a-plan-specifies-relative-paths-e-g`). **Why:** PR #4287 — plan §6.2 named `knowledge-base/engineering/runbooks/cron-retention-monitor.md`; runbook landed at the correct `engineering/ops/runbooks/...` but the PA-20 Article 30 entry cited the plan's wrong path; caught at multi-agent review by `git-history-analyzer`. See `knowledge-base/project/learnings/2026-05-22-or-semantics-allowlist-inverse-lint-and-keyset-cursor-tiebreak.md`.
- **Plan-prescribed redaction filters for captured-real fixtures are intent, never authority. Audit the filter against every secret-class the artifact can contain before executing it.** When a plan instructs "capture real provider output → run jq redaction → commit as fixture" (terraform-show-json, supabase log dumps, sentry event payloads, vendor API captures), the prescribed jq filter is a starting point — not a sufficient scrub. Same shape as `hr-when-a-plan-specifies-relative-paths-e-g`: plan is authoritative for intent (which fields to scrub), never for completeness (which fields exist). Concretely: for `terraform show -json` captures, **always** prepend `del(.variables)` regardless of the plan's prescribed filter — terraform-show-json embeds plan-input variables verbatim including `sensitive=true` declarations (`sensitive` masks render-time text output, NOT JSON serialization). After redaction, run a mandatory canonical scan: `! grep -qE 'BEGIN [A-Z ]*PRIVATE KEY|ghp_[A-Za-z0-9_]{20,}|ghs_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,}|sk-ant-api03-[A-Za-z0-9_-]{20,}|sk_(test|live)_[A-Za-z0-9]{20,}|sbp_[A-Za-z0-9]{20,}|xoxb-[A-Za-z0-9-]{20,}|AKIA[0-9A-Z]{16}|dp\.(pt\|st\|sa\|ct)\.[A-Za-z0-9_-]{40,}' <fixture>` — must return rc=1 (no matches). A token-prefix-only scan (the plan's typical `no token values, no actor IDs` framing) misses PEM headers. **Why:** PR #4420 — the captured-real fixture initially shipped with the full GitHub App RSA private key embedded under `.variables.github_app_private_key.value`; caught at post-implementation multi-agent review by `security-sentinel` AFTER 9 other agents had read the file. Full incident post-mortem at `knowledge-base/project/learnings/security-issues/2026-05-25-terraform-show-json-leaks-sensitive-variables-into-fixtures.md`. Backstop: PreToolUse hook `git-commit-secret-scan.sh` runs gitleaks on the staged index at every `git commit` regardless of `.git/hooks/pre-commit` installation state.
- **Before writing data-layer tests that use new PostgREST operators, read the shared mock helper (e.g., `apps/web-platform/test/helpers/mock-supabase.ts`) to confirm it covers every operator the code under test uses.** If not, extend it at the START of Phase 2, not after the first cryptic test failure.
- **When extending a Supabase wrapper module (e.g., `apps/web-platform/server/conversation-writer.ts`) with a new chained method (`.eq`, `.select`, `.in`, `.maybeSingle`, etc.), grep `apps/web-platform/test/` for every supabase mock chain — both shared helpers (`test/helpers/*-mocks.ts`) AND inline `vi.mock("@supabase/supabase-js", ...)` setups — and extend each one in the same edit cycle.** `tsc` is silent on chain-shape drift; only the full vitest suite catches it. Recursive-by-default mock chains (every chained call returns the same chain object) survive future extensions transparently. Same class as `cq-raf-batching-sweep-test-helpers` and `cq-preflight-fetch-sweep-test-mocks` but for the data-layer fluent API. See `knowledge-base/project/learnings/best-practices/2026-04-27-wrapper-extension-test-mock-chain-sweep.md`.
- **Before writing fixture-setup mutations (`upsert`/`insert`) in integration tests for a table whose parent has an `ON INSERT` trigger, grep migrations for `after insert on auth.users` (or equivalent) and replace the mutation with a SELECT assertion.** Redundant mutations mask trigger regressions — production signup silently breaks while the test keeps passing. Turn the setup step into a canary: read the row the trigger should have created, assert it exists. See `knowledge-base/project/learnings/security-issues/2026-04-18-rls-for-all-using-applies-to-writes.md`.
- **When extracting a pure reducer out of a React hook, migrate ALL companion state (refs the reducer reads or writes) to the reducer's state boundary in the same change.** A half-extraction — pure function plus mutable ref inside a `setState` updater — advertises purity the call site doesn't honor and recreates the StrictMode/concurrent-rendering hazard the extraction was meant to eliminate. See `knowledge-base/project/learnings/best-practices/2026-04-14-pure-reducer-extraction-requires-companion-state-migration.md`.
- **When extracting a module-level `export const NAME = ...` binding to a new module and re-exporting from the source, grep the source file for internal `NAME` references and add a sibling `import { NAME } from "./new-module"` — the re-export alone does NOT put `NAME` back in local scope.** `tsc --noEmit` flags this as TS2304 but ESM-friendly bundlers may silently swap in `undefined`. Same class as `cq-ref-removal-sweep-cleanup-closures` but for module-level bindings. **Why:** #2653 — see `knowledge-base/project/learnings/2026-04-19-enoent-on-optional-mount-should-not-alarm.md` session errors.
- **When a caller of `reportSilentFallback` runs in an environment where the "error" path is a known degraded state (e.g., `readdir` on an optional mount that doesn't exist in dev/CI), filter the error code before paging.** ENOENT on a configured-but-optional path is not a silent fallback — it's a documented zero, and routing it through Sentry exposes every request to any bug in the alarm pipeline itself. Only page on truly unexpected errors (EACCES, I/O, pathological). **Why:** #2653 — same learning file.
- **After any content-move or template port that preserves `{{ site.url }}<path>` interpolations from the source, build the site and grep rendered output for host-letter concatenation artifacts.** `{{ site.url }}` + path-without-leading-slash produces `https://soleur.aiblog/...` when `site.url` has no trailing slash — Eleventy emits it without warning; source-grep cannot detect it (the source diff is plausibly "consistent with the original"). Cheapest gate: `grep -oE "https://${HOST}[a-zA-Z]" _site/<page>/index.html` — every hit is broken. **Why:** #2705 — see `knowledge-base/project/learnings/best-practices/2026-04-21-eleventy-site-url-concatenation-broken-without-leading-slash.md`.
- **When editing a `"use client"` component or a `lib/` module reachable from client code, never import from `@/server/observability` or any `@/server/*` module that transitively pulls `pino`.** `next.config.ts` `serverExternalPackages` only externalizes for the server chunk; pino will bundle into the browser. Use `@/lib/client-observability` (a thin `@sentry/nextjs`-only shim) or add a new shim with the same signature. Verify with `grep -rn "@/server" <new-or-edited-file>`. **Why:** PR #2860 — see `knowledge-base/project/learnings/2026-04-23-render-time-scrub-sentinels-and-client-bundle-boundaries.md`.
- **Any textual tokenize-scrub-restore pipeline (stash regex matches under placeholders, scrub the remainder, restore from an index) must use a per-call random sentinel (≥24 bits of entropy) and THROW on out-of-range restore indices.** Human-readable placeholders (` PRESERVED_N `, `__TOKEN_N__`) are a substitution oracle — assistant-controlled prose containing the literal splices in stashed content, and `?? ""` fallback silently deletes the literal. Pattern: `SOLEUR_PRES_${8hexchars}_${i}`. **Why:** PR #2860 — same learning file.
- **Debounce/throttle "not-yet-fired" sentinels must be `undefined` or `-Infinity`, never `0`.** Combined with `vi.useFakeTimers({ now: 0 })`, a `0` default produces `Date.now() - 0 >= threshold` = false on the first fire, starving the very path the debounce was supposed to time. Use `if (last === undefined || now - last >= THRESHOLD_MS)`. **Why:** same learning file, session error #3.
- **When the diff touches `bun.lock` AND the bump is intended to be transitive-only (e.g., a Dependabot security bump), use the surgical-lockfile-edit pattern in [work-lockfile-bumps.md](./references/work-lockfile-bumps.md) as the first attempt.** Never `bun update <pkg>` (elevates the target to a direct dep) or bare `bun update` (bumps every direct caret-ranged dep). Validate with `bun install --frozen-lockfile`. **Why:** PR #3488 — three failed bun invocations rediscovered the constraint at task time.
- **When a new call site needs coverage by a boundary-enforcing drift-guard whose walk array (`*_DIRS`/`*_PATHS`/`*_GLOBS`) does NOT include the new file's directory, extract the call site into the existing scope — do NOT widen the walk.** The guard encodes an architectural convention ("auth verbs live in `app/(auth)` + `components/auth/`", "CSRF coverage applies to `app/api/`", etc.). Widening the array to absorb one new call site (e.g., adding `app/(dashboard)` because a single `(dashboard)/layout.tsx` calls `signOut`) inverts the convention into "any file in this whole route group that happens to call the verb must carry the guard's tags." The shortest path leaves a worse architecture. Refactor to a hook/util living in the existing scope (`components/auth/use-sign-out.ts`) so the guard's directional rule is preserved. **Why:** PR #3576 — see `knowledge-base/project/learnings/2026-05-11-drift-guard-scoping-extract-call-site-not-widen-walk.md`.
- **When a migration changes a SECURITY DEFINER RPC's signature for which prod callers exist, prefer overloading (additive `CREATE OR REPLACE` with a new parameter list) over `DROP FUNCTION` + `CREATE`.** Postgres distinguishes overloads by parameter list; supabase-js sends named-arg PostgREST envelopes that route to whichever overload matches by parameter name. Overloading is rolling-deploy-safe: (a) prd-schema-without-app keeps the v1 signature alive for old pods; (b) prd-app-without-schema keeps writes succeeding because the v1 signature still exists. DROP+CREATE creates a window where one direction silently zeros the write path. Drop the v1 in a follow-up migration after the old build ages out. **Why:** PR #3626 — see `knowledge-base/project/learnings/2026-05-12-stub-handlers-as-silent-undercount-vectors.md`.
- **Stub event handlers ("wire in Stage N when X lands") in dispatcher/router code are silent telemetry-loss vectors.** A no-op handler that satisfies the type system, sits next to fully-wired siblings, and has no error path is invisible to skim-review and Sentry alike. Either throw `Error("handler not yet wired: <name>")` until the wiring lands, OR fan out to an instrumentation counter so a "stub still present" alert can fire. **Why:** same PR #3626 — `cc-dispatcher.ts:1202` `onResult` shipped as a no-op for 3 weeks (originally added 2026-04-24 #2858), under-counting API cost by 60-90% for every cc-soleur-go conversation while the legacy path's wiring made the surface look complete.
- **When extending or mirroring a parallel runner/dispatcher/writer path (e.g., cc-dispatcher mirroring agent-runner), grep BOTH role-side persistence calls in the new path AND the reference path before declaring the implementation done.** If the new path has only one role's `from("messages").insert(...)` (or equivalent persist) and the reference has multiple, the asymmetry will land as a UI bug downstream via the resume hydration code (`api-messages.ts` → reducer state → `isClassifying`-style gates). Cheapest gate at work time: `grep -n "saveMessage\|messages.*insert" <new-path>` + `grep -n "saveMessage" <reference-path>`. Role-count parity must match or the divergence must be documented with rationale. **Why:** PR #3286 — cc-dispatcher persisted only the user role; agent-runner.ts:1079 persisted both; the gap surfaced as a "Continue thread" routing-chip regression that PR #3251 made visible. See `knowledge-base/project/learnings/integration-issues/2026-05-05-cc-dispatcher-assistant-persistence-asymmetry.md`.
5. **Test Continuously**
- **RED**: Write a failing test before implementing any new behavior
- **GREEN**: Write the minimum code to make the test pass
- **REFACTOR**: Improve code while keeping tests green
- Run the full test suite after each RED/GREEN/REFACTOR cycle. When running test suites via Bash, always capture both failure details AND summary in a single run — use `grep -E "(FAIL|ERROR|Test Files|Tests )"` or `| tail -30`, never `| tail -10` which discards failure names and forces a wasteful second run. **Why:** In PR #2430, `| tail -10` discarded failing test names, requiring a full re-run just to identify which 2 of 1580 tests failed.
- The agent harness's `bash -c` does NOT inherit `set -o pipefail`, so `bash <test-script> 2>&1 | tail -N` reports `tail`'s exit (always 0) and silently swallows the test runner's non-zero exit. For aggregate test scripts whose pass/fail signal is load-bearing ([scripts/test-all.sh](../../../../scripts/test-all.sh), `bun test`, `pytest`, `go test ./...`), prefer `bash <script> > /tmp/<script>.log 2>&1; rc=$?; echo "EXIT=$rc"` and inspect `rc` explicitly; only then `tail` or `grep` the log for context. **Why:** PR #4011 — a `bash test-all.sh 2>&1 | tail -40` invocation reported exit 0 while the runner exited 1 (3 pre-existing failed suites); the false-pass nearly chained through to ship. See `knowledge-base/project/learnings/2026-05-18-test-all-tail-masking-and-monitor-exit-condition-tightness.md` §1.
- When running test/lint/budget commands from inside a worktree pipeline, chain `cd <worktree-abs-path> && <cmd>` in a single Bash call. The Bash tool does NOT persist CWD across calls; a prior `cd /tmp/... && git clone ...` leaves subsequent commands running against the bare repo root (where tracked files exist as stale synced copies), producing wrong pass/fail counts that look like real regressions. **Why:** PR #2683 `bun test` reported 1005/1 (baseline-state result) from bare root after a drifted CWD; worktree re-run was 1006/0. See `knowledge-base/project/learnings/bug-fixes/2026-04-19-admin-ip-drift-misdiagnosed-as-fail2ban.md` session errors.
- When the project pins a test runner via `devDependencies` (e.g., `vitest@3.2.4`), invoke `./node_modules/.bin/<tool>` — never `npx <tool>`. `npx` resolves to its own cache and silently major-version-jumps (e.g., installing vitest 4.x against a vitest 3.2.4 config), producing `Could not resolve 'vitest/config'` and `Unexpected JSX expression` parse errors that look like real regressions. **Why:** PR #3186 — `npx vitest` installed 4.x and rolldown rejected the project's JSX config; switching to `./node_modules/.bin/vitest` (3.2.4) restored a passing run. See `knowledge-base/project/learnings/2026-05-04-plan-precedent-search-must-include-lib-helpers.md` session errors.
- Fix failures immediately -- never move to the next task with failing tests
- When a class becomes hard to test (too many dependencies), extract an interface and inject dependencies. See the `/atdd-developer` skill for detailed TDD guidance.
6. **Infrastructure Validation**
When any task modifies files in `apps/*/infra/`, run these checks after each change (in addition to or instead of the app test suite):
1. **cloud-init schema**: For each modified `cloud-init.yml`:
`cloud-init schema -c <file>` -- validates YAML syntax AND cloud-init schema in one step. Warnings about missing datasource are expected; only non-zero exit codes are failures. If `cloud-init` is not installed locally, warn and continue.
2. **Terraform format**: For each infra directory with modified `.tf` files:
`terraform fmt -check <dir>` -- exit 0 means formatted; exit 3 means violations. Fix with `terraform fmt <dir>`.
3. **Terraform validate**: For each infra directory with modified `.tf` files:
`terraform init -backend=false` then `terraform validate` -- catches HCL syntax errors and undefined references without requiring provider credentials.
These checks replace the "tests may be skipped" exemption for infra files. If any check fails, fix before proceeding to the next task.
- When cloud-init has `lifecycle { ignore_changes = [user_data] }`, changes to cloud-init templates are never applied to existing servers. Use a `terraform_data` provisioner with `remote-exec` to bridge the gap. Verify systemd services use `EnvironmentFile=` directives (not `/etc/environment`) for token injection.
- When fixing syscall-level issues in Docker containers, test with `--privileged` first to establish a working baseline, then remove privileges one at a time. Docker's seccomp `includes.caps` is compile-time (evaluated when building BPF filter), not runtime -- processes gaining capabilities inside user namespaces do NOT gain access to capability-gated seccomp rules.
- When a `terraform_data` provisioner writes a systemd unit or config file via `remote-exec` heredoc, extract the content to a standalone file and use `file()` in both `triggers_replace` and a `file` provisioner. Inline heredoc strings desync from the trigger hash -- partial strings in `triggers_replace` silently skip re-provisioning when the unit content changes.
- When adding or removing files from a `triggers_replace` hash in `server.tf`, grep for `TRIGGER_FILES` in `plugins/soleur/test/` and `DEPLOY_PIPELINE_FIX_TRIGGERS` in `plugins/soleur/skills/ship/SKILL.md` — update all three locations in the same commit. The drift guard test catches this post-merge but costs a hotfix PR. **Why:** #4492 added 2 files to `triggers_replace` without updating the test array; CI failed post-merge (#4493 hotfix).
- When referencing `cloudflare_zero_trust_access_service_token.*.client_secret` (or any provider-managed credential attribute) in a Terraform `environment {}` block, check the provider docs for write-only attributes. The Cloudflare provider's `client_secret` is available at creation but empty on subsequent `terraform refresh`. Use Doppler variables instead of state references for credentials. **Why:** #4492 → #4494.
- When HMAC-signing a payload and sending it via curl, always use `--data-binary @file` (not `-d @file`). curl's `-d` strips newlines from the file content, creating a mismatch between what `openssl dgst` hashed (with newlines) and what the server receives (stripped). **Why:** #4492 → #4495.
- When writing a webhook handler that runs inside a systemd service's mount namespace (`ProtectSystem=strict`), cross-check every destination path against the service unit's `ReadOnlyPaths`/`ReadWritePaths` at implementation time. SSH provisioners run outside the namespace; webhook handlers run inside. `terraform validate` and sandbox test suites do not catch namespace conflicts. **Why:** #4492 P1 review finding.
7. **Track Progress**
- Keep TodoWrite updated as you complete tasks
- Note any blockers or unexpected discoveries
- Create new tasks if scope expands
- Keep user informed of major milestones
8. **GDPR / Compliance Gate (single pass, end of Phase 2)**
[skill-enforced: gdpr-gate at work Phase 2 exit]
After the per-task RED/GREEN/REFACTOR loop completes and before Phase 2.5, run `/soleur:gdpr-gate` once against the cumulative diff `git diff main...HEAD`. Same advisory-only output and Critical-finding escalation as plan Phase 2.7. **Never per-task** — token budget is ≤4k per invocation, single pass per phase per ADR-026 TR3.
Skip silently if the cumulative diff does not match the `hr-gdpr-gate-on-regulated-data-surfaces` canonical regex.
9. **Full-Suite Exit Gate (single pass, end of Phase 2)**
[skill-enforced: work Phase 2 exit]
Before entering Phase 3, run `bash scripts/test-all.sh` once. Touched-file tests are the inner loop; `test-all.sh` is the exit gate — it discovers orphan test suites (sibling files covering the same script — e.g., an untouched `tests/scripts/test-rule-metrics-aggregate.sh` alongside the touched `rule-metrics-aggregate.test.sh`) that the touched-file set never sees. Symmetric to the ship Phase 5.5 Review-Findings Exit Gate; catches the gap that PR #3512 surfaced post-merge-queue when an untouched orphan suite's fixture broke under a tightened predicate. **Why:** see issue #3533.
### Phase 2.5: Research Validation Loop (knowledge-base deliverables only)
Rule source: AGENTS.md — migrated 2026-04-21 (PR #2754). When a research sprint produces recommendations, run the cascade-validate loop [id: wg-when-a-research-sprint-produces] [skill-enforced: work Phase 2.5]. **"Findings written" is NOT done — "findings applied, validated, and all documents reflect the final state" is done.** The full body of that rule lives here; AGENTS.md retains a one-line pointer preserving the `[id: ...]` tag.
**Trigger:** This phase runs when the plan's deliverables are knowledge-base research artifacts (findings, analysis, audits, research briefs) that produce recommendations targeting other existing documents. Skip for code-only plans.
**Detection:** After Phase 2 completes, scan the outputs for recommendation patterns — "should rewrite," "needs updating," "add to," "change X in Y.md," or any finding that names a specific target file. If found, enter the loop.
**The loop:**
```text
while (recommendations exist that haven't been applied):
1. CASCADE: Apply all recommendations to their target artifacts
- Rewrite questions in interview guides
- Update framings in brand guide
- Add alternatives to pricing strategy
- Any finding that names a file → edit that file
2. VALIDATE: Re-run the same research methodology against updated artifacts
- Use the same personas/parameters as the original run
- Produce a before/after comparison (original → current)
3. CHECK: Did the validation surface NEW weak spots or recommendations?
- If yes → apply fixes, loop back to step 2
- If no (at synthetic ceiling) → exit loop
4. UPDATE BRIEF: Update the research brief with final validated results
- Executive summary reflects current state, not original findings
- Recommendations marked as "Applied" with results
- Add Cascade Status section tracking all changes to all files
5. SUMMARIZE: Present founder summary
- Key findings table
- All files changed table (file, what changed, before/after metrics)
- Remaining limitations (structural, not fixable)
```
**Exit condition:** The loop exits when a validation round produces no new actionable recommendations — only structural limitations that can't be fixed by rewording (e.g., a persona's archetype inherently produces flat responses to a specific question type).
**Max iterations:** 3 rounds. If the third round still produces actionable recommendations, present them to the user rather than looping indefinitely. Synthetic-on-synthetic validation has diminishing returns.
**Why this matters:** Without this loop, research sprints produce findings that sit in briefs without updating the documents they target. The founder has to manually ask "was any action taken?" after each round. This loop makes cascade + validate + re-cascade automatic.
### Phase 3: Quality Check
1. **Run Core Quality Checks**
Always run before submitting:
```bash
# Run full test suite (use project's test command)
# Examples: bin/rails test, npm test, pytest, go test, etc.
# Run linting (per CLAUDE.md)
# Use linting-agent before pushing to origin
```
- **Run `npx tsc --noEmit` in the app package alongside the test suite.** Vitest type-checks test files lazily, so TS errors in tests pass the suite locally but fail CI. A standalone tsc pass catches them at the work-phase gate instead of deferring to review.
- **When extracting enforcement logic (auth, CSRF, validation) from route files into a shared helper, update negative-space tests in the same commit.** Route-level detection must prove helper invocation AND failure early-return — not just import presence. Add direct assertions on the helper file for every invariant that moved into it. See `knowledge-base/project/learnings/best-practices/2026-04-15-negative-space-tests-must-follow-extracted-logic.md`.
- **When adding git operations that contact remotes in Next.js API routes, include the credential helper pattern from `session-sync.ts`** (search `credential.helper`). Bare `git pull`/`git push`/`git fetch` fail silently on private repos. See `knowledge-base/project/learnings/integration-issues/kb-upload-missing-credential-helper-20260413.md`.
2. **Consider Reviewer Agents** (Optional)
Use for complex, risky, or large changes:
- **code-simplicity-reviewer**: Check for unnecessary complexity
- **kieran-rails-reviewer**: Verify Rails conventions (Rails projects)
- **performance-oracle**: Check for performance issues
- **security-sentinel**: Scan for security vulnerabilities
Run reviewers in parallel with Task tool:
```text
Task(code-simplicity-reviewer): "Review changes for simplicity"
Task(kieran-rails-reviewer): "Check Rails conventions"
```
Present findings to user and address critical issues.
3. **Final Validation**
- All TodoWrite tasks marked completed
- All tests pass
- Linting passes
- Code follows existing patterns
- Figma designs match (if applicable)
- No console errors or warnings
### Phase 4: Handoff
Implementation is complete. Before handing off, run the **Playwright-first audit**, then determine invocation mode.
#### Playwright-First Audit
Scan any "next steps", "setup instructions", or "to use this" text you are about to output. For each step that involves a browser action (account creation, credential generation, settings configuration, form submission, vendor support tickets, OAuth flow, portal navigation):
1. **Classify:** Is this step automatable via Playwright MCP, or is it genuinely manual (CAPTCHA, interactive OAuth consent, hardware MFA token, payment-card entry)?
2. **If automatable:** Do not list it as a manual step. Either execute it now via Playwright MCP, or note it as "automatable via Playwright — will execute next."
3. **If genuinely manual:** Drive the flow via Playwright up to the manual gate (e.g., navigate to the OAuth consent screen), then hand off only that single interaction to the user.
If you catch yourself writing phrases like "set up X in the browser", "go to the portal and...", "manually configure...", "paste this ticket body into the support form", or "the operator pastes + submits" — stop and attempt Playwright first. This audit is mandatory; skipping it is a deviation.
**Vendor support ticket submissions are Playwright-driveable** — they are NOT operator-handoff by default. Most vendor support surfaces today are Intercom / Zendesk / HelpScout chat widgets (`help.<vendor>.io`, `support.<vendor>.com`, `<vendor>.zendesk.com`) where the AI assistant routes to a human team. The full submission flow — opening the widget, accepting cookies, starting a conversation, sending the ticket body, requesting human escalation if the AI gives a stock policy answer — runs entirely under Playwright. The only legitimate manual gates are:
- **Email-OTP verification** when the vendor sends a one-time passcode to the operator's inbox before routing to a human reviewer.
- **SMS-OTP** — same shape as email-OTP but delivered to the operator's phone (e.g., banks, telcos, account-recovery flows). Same handoff: "check your messages for the code, tell me the digits."
- **Authenticator-app TOTP** — operator reads a rolling 6-digit code from Authy / 1Password / Google Authenticator / Microsoft Authenticator. Playwright cannot reach the authenticator source.
- **WebAuthn / passkey / U2F browser prompts** — the OS or browser surfaces a native dialog (passkey selection, Touch ID, Windows Hello) that Playwright cannot synthesize. Distinct from hardware MFA: passkeys are software-resident and increasingly the default on Google / GitHub / Stripe / Apple.
- **Push-based MFA** (Duo Push, Okta Verify, Authy Push, Microsoft Authenticator notification) — operator approves on a separate device; no DOM interaction available to Playwright.
- **Payment-card entry** in Stripe / similar widgets (cross-origin iframe sandbox; even if Playwright could reach in, card entry is the explicit operator decision-and-ack point).
- **CAPTCHA / "I am not a robot"** challenges (intentional bot-detection).
- **Hardware MFA token tap** — physical YubiKey / Titan / Solokey touch (operator-side device).
Never quote OTP digits, TOTP codes, or any other ephemeral authentication value in a committed file — capture only the fact-of-verification + UTC timestamp. The specific code is dead immediately, but quoting it normalizes "paste secrets into the runbook" as a pattern and the next vendor's code may NOT be single-session (some flows reuse codes within a TTL window).
Drive the flow up to one of these gates, hand off the single interaction (e.g., "check your inbox for the OTP and tell me the code"), then resume. Never list "operator pastes + submits" as a step — that's a Playwright-first-audit violation. Vendor support tickets typically do not return a numeric ticket ID; capture (a) the submission UTC timestamp, (b) the AI-classifier auto-title (often surfaced in the messages list), and (c) any human-team routing label as the audit baseline. The conversation thread on the vendor side IS the canonical ticket; async response arrives via email to the operator. **Why:** PR #3946 PR-γ §17 Sentry refund + forensics tickets — the original plan listed them as "NOT Playwright-driveable" (operator handoff to paste-and-submit); after operator pushback, both tickets were driven via Sentry's Intercom widget at `help.sentry.io` with only the email-OTP step handed off. See learning `knowledge-base/project/learnings/2026-05-17-vendor-support-tickets-are-playwright-driveable.md`.
#### Phase 4 Entry-Guard
Before emitting `## Work Phase Complete` (one-shot mode) or chaining into the post-implementation pipeline (direct mode), assert at least one commit exists beyond `origin/<branch>`. An empty diff hands review agents nothing to analyze and produces no signal. Run BEFORE the Invocation Mode branch so both paths are covered.
**Procedure (distinct exit codes signal distinct operator actions):**
1. Probe the commit count:
```bash
BRANCH=$(git rev-parse --abbrev-ref HEAD)
if [[ -z "$BRANCH" || "$BRANCH" == "HEAD" ]]; then
echo "[work-phase-4-guard] detached HEAD — checkout a feature branch before chaining to review." >&2
exit 1
fi
N=$(git rev-list "origin/${BRANCH}..HEAD" --count 2>/dev/null || echo 0)
```
2. If `N == 0`, **stop and run Phase 2 step 3** (stage logical-unit files, write conventional commit message). Do not chain through this block as a single bash invocation — the commit is an explicit action the agent must perform between probes:
```bash
if [[ "$N" == "0" ]]; then
echo "[work-phase-4-guard] no commits beyond origin/${BRANCH} — pause and run Phase 2 step 3 incremental commit before continuing." >&2
exit 2 # PAUSE — orchestrator should re-enter Phase 4 after the commit lands
fi
```
3. After the incremental commit lands, re-enter the Phase 4 entry-guard. If `N == 0` on the second probe (commit failed silently or no diff exists), HALT:
```bash
if [[ "$N" == "0" ]]; then
echo "[work-phase-4-guard] empty diff vs origin/${BRANCH} after Phase 2 step 3 — investigate before continuing." >&2
exit 1 # HALT — do NOT emit "## Work Phase Complete"
fi
```
**Form rationale.** `git rev-parse --abbrev-ref HEAD` matches `ship/SKILL.md:619` precedent and returns the literal `HEAD` on detached state (vs. `git branch --show-current` which returns empty), so the detached-HEAD guard catches both shapes. `git rev-list ... --count` returns a clean integer ready for `[[ "$N" == "0" ]]`; the `wc -l` shape requires a `tr -d` strip and is whitespace-padded. Precedent: `plugins/soleur/skills/ship/SKILL.md:619`, `.claude/hooks/ship-unpushed-commits-gate.sh`. **Distinct exit codes** (`2 = pause-and-commit`, `1 = halt-and-investigate`) let one-shot orchestrators distinguish the two recovery paths rather than treating both as opaque non-zero failures.
#### Post-Merge Section Self-Audit (HARD GATE)
After drafting the PR body and BEFORE `gh pr ready` / `gh pr merge --auto`, scan every line under headings matching `^##\s+(Post-?merge|Operator|Follow-?ups?)` (case-insensitive). For each bullet, classify and resolve **before** marking ready:
| Pattern | Action |
|---|---|
| Doppler/env-var verification | Inline-execute via `doppler secrets get <KEY> -p soleur -c <env> --plain`; if missing, set from a known source via `doppler secrets set` or update the handler to read the existing canonical name. |
| `Within Nh of merge: file <issue>` | File the issue NOW via `gh issue create` using the template the bullet describes; replace the bullet with `Done: #<num>`. |
| `gh issue close` / `gh issue comment` on existing issues | Run NOW via `gh` CLI. |
| Sentry / Better Stack / monitor verification | Replace with the monitor's own auto-page mechanism (`failure_issue_threshold = 1` is the verification — no operator gaze required per `hr-no-dashboard-eyeball-pull-data-yourself`). If active verification is still wanted, create a one-time scheduled workflow via `/soleur:schedule create --once --at <date>` with a self-disabling `verify-and-close-or-file-issue` body. |
| Genuinely operator-only (CAPTCHA, SSO consent, payment-card entry, hardware MFA, K-bis-style first-onboarding) | File a `type/chore` issue carrying the literal `deferred-automation` sentinel via `gh issue create --label type/chore --body "deferred-automation backlog item; re-evaluate when: <criterion>" ...`, then add `Tracks #N` to the bullet in the PR body. |
| Anything else | Inline-execute. Default-deny on "operator should later …" phrasing. |
After resolution, re-scan; the section MUST contain zero unaccompanied operator/manual bullets. The `ship-operator-step-gate.sh` PreToolUse hook enforces this mechanically at `gh pr ready` / `gh pr merge --auto` — the gate's deny message lists each undeferred match. Override via `SOLEUR_SKIP_OPERATOR_STEP_GATE=1` is reserved for the rare attestation case.
**Why:** PR #4227 (TR9 PR-3) shipped with a "Post-merge" section listing four operator items (Doppler secrets check, T+90 min Sentry verify, T+24h auto-resolve verify, file follow-up issue within 48h) — all four were inline-automatable; the agent had hard rules forbidding the deferral (`hr-exhaust-all-automated-options-before`, `hr-never-label-any-step-as-manual-without`, `wg-block-pr-ready-on-undeferred-operator-steps`) and still wrote the bullets. The gate existed at `/ship` Phase 5.5 but the agent reached `gh pr ready` directly. This self-audit + the hook close both halves of that bypass. See `knowledge-base/project/learnings/best-practices/2026-05-21-post-merge-section-self-audit.md`.
#### Invocation Mode
**If invoked by one-shot** (the conversation contains `soleur:one-shot` skill output earlier): Output exactly `## Work Phase Complete` and then **immediately invoke** `skill: soleur:review` (step 4 of the one-shot sequence). Do NOT end your turn after outputting the marker — you ARE the orchestrator, so you must continue executing one-shot steps 4 through 10 in order. The marker is a progress signal, not a stopping point.
**If invoked directly by the user** (no one-shot orchestrator): Continue through the post-implementation pipeline automatically. Do NOT stop and wait — the earlier learning "Workflow Completion is Not Task Completion" applies. Run these steps in order, forwarding `--headless` if `HEADLESS_MODE=true`:
1. `skill: soleur:review` (or `skill: soleur:review --headless` if headless) — catch issues before shipping
2. `skill: soleur:resolve-todo-parallel` — resolve any review findings (no `--headless` needed; this skill has no interactive prompts)
3. `skill: soleur:compound` (or `skill: soleur:compound --headless` if headless) — capture learnings before committing
3.5. Display: "Tip: After shipping, run `/clear` to reclaim context headroom for the next task."
4. `skill: soleur:ship` (or `skill: soleur:ship --headless` if headless) — commit, push, create PR, merge
---
## Key Principles
### Start Fast, Execute Faster
- Get clarification once at the start, then execute
- Don't wait for perfect understanding - ask questions and move
- The goal is to **finish the feature**, not create perfect process
### The Plan is Your Guide
- Work documents should reference similar code and patterns
- Load those references and follow them
- Don't reinvent - match what exists
### Test As You Go
- Run tests after each change, not at the end
- Fix failures immediately
- Continuous testing prevents big surprises
### Quality is Built In
- Follow existing patterns
- Write tests for new code
- Run linting before pushing
- Use reviewer agents for complex/risky changes only
### Review Before You Ship
- Use `skill: soleur:review` after completing implementation
- Catches issues before they reach PR reviewers
- Faster feedback than waiting for human review
- Builds confidence that your code is solid
### Compound Your Learnings
- Use `skill: soleur:compound` before creating a PR
- Document debugging breakthroughs, non-obvious patterns, and framework gotchas
- Even "simple" implementations can yield valuable insights
- Future-you and teammates will thank present-you
### Ship Complete Features
- Mark all tasks completed before moving on
- Don't leave features 80% done
- A finished feature that ships beats a perfect feature that doesn't
## Quality Checklist
Before entering Phase 4, verify these Phase 2-3 items are complete:
- [ ] All clarifying questions asked and answered
- [ ] All TodoWrite tasks marked completed
- [ ] Tests pass (run project's test command)
- [ ] New source files have corresponding test files
- [ ] Linting passes (use linting-agent)
- [ ] Code follows existing patterns
- [ ] Figma designs match implementation (if applicable)
After Phase 4 handoff (one-shot only), the same agent continues executing one-shot steps 4-10 (`/review`, `/qa`, `/compound`, `/ship`, `/test-browser`, `/feature-video`).
## When to Use Reviewer Agents
**Don't use by default.** Use reviewer agents only when:
- Large refactor affecting many files (10+)
- Security-sensitive changes (authentication, permissions, data access)
- Performance-critical code paths
- Complex algorithms or business logic
- User explicitly requests thorough review
For most features: tests + linting + following patterns is sufficient.
## Common Pitfalls to Avoid
- **Analysis paralysis** - Don't overthink, read the plan and execute
- **Skipping clarifying questions** - Ask now, not after building wrong thing
- **Ignoring plan references** - The plan has links for a reason
- **Testing at the end** - Test continuously or suffer later
- **Forgetting TodoWrite** - Track progress or lose track of what's done
- **80% done syndrome** - Finish the feature, don't move on early
- **Over-reviewing simple changes** - Save reviewer agents for complex work
- **Silent plan omissions** - When dropping a conditional plan item, document why in the commit or plan
- **Research without cascade-validate loop** - For knowledge-base research deliverables, Phase 2.5 enforces: cascade findings into source artifacts → re-run validation → cascade again if new weak spots emerge → update brief with final results → present founder summary. "Findings written" is not "done" — "findings applied, validated, and all documents reflect the final state" is done. See Phase 2.5.
- **Missing founder summary** - After completing research, analysis, or audit work, present a concise summary: key findings table + all files changed table (file, what changed, before/after metrics if applicable). The founder needs to review what changed, not just what was discovered.
- **Incomplete replace_all** - After any `replace_all` Edit operation, grep the file to verify zero remaining matches before proceeding to the next task. `replace_all` can miss occurrences with different surrounding context (whitespace, indentation).
- **Encoded-blob value sweep** - When removing a value from a file that contains base64, hex, JSON-string-escape, or URL-encoded forms (JWT fixtures, encoded config snapshots, request payloads), source-form `grep` is insufficient. After substitution, decode each blob and grep the **decoded** form for the removed value. **Why:** PR #3054 — `replace_all "ifsccnjhymdmidffkzhl"` returned 0 source hits but `JWT_LOG_INJECT_U2028`'s base64 payload still encoded the dev Supabase ref; the secret scanner would have re-fired. See `knowledge-base/project/learnings/security-issues/2026-04-29-jwt-fixture-reminting-decode-verify.md`.
- **Local verification without Doppler** - For env-var-reading apps, use a single Bash call: `cd <abs-path> && doppler run -p soleur -c dev -- npm run <script>` (for `apps/web-platform`, `cd apps/web-platform && doppler run -p soleur -c dev -- npm run dev`). Prevents: (a) skipping `doppler run` (missing secrets), (b) invoking transitive binaries under `doppler run` (not on PATH), (c) relying on prior CWD (shell state doesn't persist). If port 3000 is already bound by another dev server (the user may have one running), start on an alternate port via `PORT=3099 doppler run ... npm run dev` rather than killing the existing process. (ex-`cq-for-local-verification-of-apps-doppler`; #2350 hit all three failure modes in sequence; PR #3199 added the alt-port fall-through after the stale `./scripts/dev.sh` reference broke startup)
- **Closes-after-apply deferral missed in commit messages** - When a plan's `## Risks` (or `## Sharp Edges`) section names an explicit Closes-after-apply deferral (issue stays open until a post-merge PM step proves green — workflow first-run, terraform apply, deploy probe, etc.), commit messages AND PR body MUST default to `Ref #N`, not `Closes #N`, regardless of whether the commit body's `Closes` placement is technically `wg-use-closes-n-in-pr-body-not-title-to`-legal. Auto-close fires at merge time, decoupled from whether the proof artifact actually lands green. Detection: grep the plan for `Closes-after-apply`, `manual close after`, `Ref #N` + `close manually`, `type: ops-remediation`, or any explicit per-PM closure-link instruction. On match, emit `Ref #N` + 1-line WARN. The author manually `gh issue close N --comment "<run URL>"` post-PM. **Why:** PR #3551 — initial commit message used `Closes #3060` against plan §R6's `Ref #3060 + manual close after PM1 confirms first green run` directive; caught pre-push via self-audit, amended. See `knowledge-base/project/learnings/2026-05-11-plan-r6-closes-after-apply-deferral-pattern.md`.
- **Parallel `gh issue create` scrambles ID-to-title mapping** - When a /work task files N related GitHub issues (e.g., a deferral-issues batch), `gh issue create ... &` + `wait` returns URLs in completion order, not start order. The first `gh` job to FINISH gets `#N`, the next gets `#N+1`, etc. — independently of which title started first. Worse, a transient GraphQL error on one parallel job is easy to misattribute to the wrong title. **Either serialize the calls** (~1.5–3s each is cheap for ≤5 issues), **or write each result to a name-keyed file** (`echo "$url" > "/tmp/issue-$short_name.url"`) so the title→ID mapping is explicit. **Always run `gh issue view <N> --json title` reconciliation before citing IDs in any artifact** (agent body, README, SKILL.md, plan). The cost of catching wrong IDs at PR review is ~30 minutes of recovery (close duplicate, file missing, edit artifacts, force-push); the cost of post-creation reconciliation is N seconds. **Why:** PR #4288 — 5 deferral issues filed in parallel; 3 of 5 IDs ended up inverted in the agent body, 1 was dropped (GraphQL error), 1 was a duplicate retry. See `knowledge-base/project/learnings/2026-05-22-parallel-gh-issue-create-scrambles-id-mapping-and-review-agent-producer-consumer-symmetry.md`.
- **Relaunching a long-running background bash before verifying it died** - When a `run_in_background: true` task seems unresponsive, do NOT relaunch until ALL three checks confirm death: (1) broad `ps -ef \| grep -E '<substring>' \| grep -v grep` (never `pgrep -fa 'pattern$'` — anchored patterns miss processes wrapped in `doppler run -- bash ...`), (2) cache/output file size stopped growing over a 30+ second window, (3) the harness's `<task-notification>` has fired with a definitive `status` field. Log file `mtime` is NOT a liveness signal — long-running scripts buffer output between API calls. Relaunching prematurely concurrent-runs against the same API key and wastes paid spend. **Why:** PR #4156 — bench 1 was running fine the whole ~75 min, but `pgrep` with anchored pattern + stale log mtime led to two redundant bench launches (extra ~$2-3 Anthropic spend). See `knowledge-base/project/learnings/workflow-issues/2026-05-20-long-running-bench-verify-process-before-relaunch.md`.
No comments yet. Be the first to comment!