diff --git a/.claude/skills/conciv-issue/SKILL.md b/.claude/skills/conciv-issue/SKILL.md new file mode 100644 index 000000000..24fcab441 --- /dev/null +++ b/.claude/skills/conciv-issue/SKILL.md @@ -0,0 +1,80 @@ +--- +name: conciv-issue +description: Use when asked to pick up a GitHub issue, work the issue backlog down, burn down issues, triage open tickets, groom an issue for an agent, pick up a ticket, or dispatch an agent to fix a ticket and close it with a PR. +--- + +# Working the conciv issue backlog + +## Overview + +Goal: open-issue count trends to zero. The unit of work is one lane: pick one issue, groom it, +dispatch one agent in one worktree, verify, open one PR whose merge auto-closes the issue. Never +batch several issues into one PR, and never merge — the user merges. + +The orchestrator (this session) picks, grooms, verifies, and reports. Agents implement. The +orchestrator never edits product code. + +## Step 0: pick one issue + +```bash +gh issue list --state open --limit 60 --json number,title,labels +``` + +Selection order: + +1. `ready-for-agent` and unassigned, smallest first — pre-groomed and grabbable. +2. Ungroomed issues with a clear defect and reproduction. +3. Everything else needs grooming first (that grooming is itself a valid lane outcome). + +Skip: `agent:blocked-external`, `agent:needs-info`, `agent:in-progress`, `agent:implemented-in-pr`, +epics/RFCs, and anything already linked to an open PR (`gh pr list --search "NNN in:body"`). If +every remaining issue is blocked, say so and stop; do not force a lane. + +Lifecycle labels, transitions, and type-label grooming rules: `references/labels.md`. + +## Step 1: classify and groom + +Read the full issue and comments (`gh issue view NNN --comments`). Verify every claim in the body +against current `origin/main` — issues go stale; file:line citations rot. + +Required shape before dispatch, by type: bug — RCA plan first, evidence-only Phase 1, reproduce +before fix; no repro = comment findings, label `agent:needs-info`, next issue. Flake — same +reproduce-before-fix bar, HARD, no exceptions. Feature/refactor — acceptance criteria enumerable as +greps/tests; if missing, groom them into the body via `gh issue edit`. Question — answer with +evidence in a comment, label `agent:answered`, close if resolved. + +If grooming reveals the issue is already fixed on main, prove it (grep/test), comment, label +`agent:already-resolved`, close. That counts as a completed lane. + +## Step 2: dispatch one agent + +- Worktree per issue: flat name `issue-NNN` (no `+` in path), pinned base `origin/main`. +- Agent type by shape: `conciv-frontend` (Solid/ui-kit/widget UI), `conciv-implementer` + (server/core, judgment needed), `conciv-mechanic` (fully-specced mechanical). Model per global + rules: sonnet default; opus only for adversarial review or design-heavy contract work. State the + model in the dispatch. +- Dispatch is definition-of-done shaped, with the orchestrator pre-deciding the fix's design so + the agent never stalls on a choice. Full skeleton: `references/dispatch-template.md`. +- Label the issue `agent:in-progress` at dispatch; run in background, no blocking waits. + +## Step 3: verify and hand off + +1. Review the agent's full diff yourself. Judge mechanism, not just correctness. +2. Re-run gates with `--force` (turbo cache greens are claims about old inputs). +3. Non-trivial diff → run the conciv-review skill on the branch before calling it done. +4. Confirm PR body carries `Fixes #NNN`, CI is green, then label the issue + `agent:implemented-in-pr` and report the PR link to the user. The user merges; merge auto-closes + the issue. + +A full lane, checkpoint by checkpoint (including where three review rounds and one RCA earned +their keep): `references/worked-example-316.md`. + +## Red flags — stop the lane + +- "I'll fix these three related issues in one PR" — one issue, one PR. +- "The bug is obvious, skip the reproduction" — no repro, no fix. +- "CI is green, I'll merge it" — user merges. Always. +- "The issue body says the code does X" — verify against main first; bodies rot. +- Closing an issue by hand when a PR exists — `Fixes #NNN` closes it on merge; manual close loses + the audit trail. +- Working in the main repo checkout — every lane gets its own worktree. diff --git a/.claude/skills/conciv-issue/references/dispatch-template.md b/.claude/skills/conciv-issue/references/dispatch-template.md new file mode 100644 index 000000000..9d2bc5ebf --- /dev/null +++ b/.claude/skills/conciv-issue/references/dispatch-template.md @@ -0,0 +1,85 @@ +# Dispatch template + +A definition-of-done dispatch is a fill-in template, not free text. Every section below exists to +close one specific way lanes have gone wrong before. Fill in every section; do not skip one because +"it's obvious" — the agent reading the dispatch has none of this session's context. + +## Setup + +Why: an agent with no worktree, or a worktree branched from a stale local `main`, produces a diff +that can't land cleanly and wastes the whole lane. + +```bash +git -C /Users/omrikatz/Public/web/aidx worktree add \ + /Users/omrikatz/Public/web/aidx/.claude-worktrees/issue-NNN -b issue-NNN origin/main +``` + +- `pnpm install` in the new worktree if the lockfile changed since the last worktree was cut. +- HARD warning to include verbatim in the dispatch: Bash cwd snaps back to the repo root between + tool calls — pin the absolute worktree path in every command, never rely on a prior `cd`. + +## The fix, decided shape + +Why: an agent with a genuine design choice in front of it either stalls asking for clarification +(defeating background dispatch) or picks silently and produces a diff the orchestrator has to +re-litigate. The orchestrator makes every judgment call before dispatch and states it as a ruling. + +Enumerate each ruling explicitly, e.g.: + +- "Use option 2 from the issue body (targeted guard in `guards.ts`), not option 1 + (`changeset status` in CI) — rejected for its zero-changeset exit-code quirk." +- "New state belongs in a store, not a parallel signal — see `references/labels.md` if the issue + touches Solid state." +- Any file/module boundary the agent must not cross. + +## Constraints + +Repo law, restated because agents drift toward convenient defaults under pressure: + +- Functions, not classes. Zero comments in TS/JS. No `any`/`as`/non-null assertion/IIFE. +- oxfmt formatting (no semicolons, single quotes, trailing commas). +- Verify every API call against the resolved source in `node_modules` (or the package's own + `src/`), never guess a signature from memory or training data. + +## Dependencies + +NO new dependencies unless the dispatch names a package the USER approved verbatim. If a +dependency seems needed mid-implementation, the agent stops and reports back instead of installing +it. + +Why this rule exists: an agent's inference that "the user would probably approve this" is +fabricated consent, not real consent — only an explicit prior approval counts. + +## Acceptance criteria + +Verbatim from the groomed issue body, each phrased so it is checkable as a grep or a test run — +not a prose restatement the orchestrator has to reinterpret at verification time. + +## Gates + +Run from the worktree, in this order: + +```bash +pnpm -C exec turbo run typecheck --filter= --force +env TURBO_CONCURRENCY=1 VITEST_MAX_FORKS=1 \ + pnpm -C exec turbo run test --concurrency=1 --filter= --force +pnpm lint +pnpm format:check +pnpm exec fallow audit --changed-since origin/main --format json +``` + +- Fallow: 0 `introduced` findings. JSON runtime errors (`{"error": true, ...}`) are non-blocking. +- Changeset decision: needed when a published package's runtime behavior changes; the + `check-changesets` CI gate (verify-changesets.yml) enforces coverage. If the PR intentionally + ships no release note (docs-only, internal tooling, a private package), apply the `no-changeset` + label instead of adding one. + +## Deliverable + +- Conventional commits, each ending with `Co-Authored-By: Claude Fable 5 `. +- Plain push (never force-push a fresh branch that has no upstream history to protect). +- `gh pr create` with a body that explains the mechanism of the fix (not just "fixes the bug"), + the exact line `Fixes #NNN`, and the Claude Code generated-by footer. +- Never merge — the user merges. +- Final report shape: a per-acceptance-criterion table (met / how verified), gate command outputs, + commit SHAs, and any deviations from the dispatch with a one-line reason each. diff --git a/.claude/skills/conciv-issue/references/labels.md b/.claude/skills/conciv-issue/references/labels.md new file mode 100644 index 000000000..b047b9046 --- /dev/null +++ b/.claude/skills/conciv-issue/references/labels.md @@ -0,0 +1,49 @@ +# Labels + +Verified against the live label set (`gh label list --limit 60`) as of this writing. If a label +referenced here is missing, re-run that command — labels do drift. + +## Lifecycle labels + +The nightly issue tracker and the conciv-issue lane both read and write these. `agent:*` labels +plus `ready-for-agent` and `agent-triaged` track where an issue sits in the pipeline. + +| Label | Meaning | Who sets it | What the lane does on seeing it | +| ------------------------- | -------------------------------------------------------------------- | ------------------------------------------ | -------------------------------------------------------------------------------------- | +| `agent-triaged` | Nightly issue tracker has investigated this issue. | Nightly tracker | Informational; does not gate picking. | +| `ready-for-agent` | Ticket is fully specified and agent-grabbable. | Nightly tracker, or a human after grooming | Top of the pick order (Step 0). | +| `agent:in-progress` | Nightly agent (or this lane) is investigating/working it right now. | Lane, at dispatch | Skip — already claimed. | +| `agent:needs-info` | Nightly agent needs a reproduction or detail from the reporter. | Lane, after a failed-repro grooming pass | Skip until the reporter adds detail. | +| `agent:blocked-external` | Blocked on an upstream or a product decision. | Lane or human | Skip — not actionable by an agent. | +| `agent:already-resolved` | Nightly agent (or this lane) found this already fixed on main. | Lane, after proving it with a grep/test | Terminal — issue gets closed. | +| `agent:answered` | Nightly agent (or this lane) answered a question with evidence. | Lane, for `question`-type issues | Terminal if the asker's question is resolved; close. | +| `agent:fix-proposed` | Nightly agent opened a PR that fixes this. | Nightly tracker | Treat like `agent:implemented-in-pr` for picking purposes — skip, a PR already exists. | +| `agent:implemented-in-pr` | Implemented and gated locally; awaiting PR merge + full CI evidence. | Lane, at Step 3 hand-off | Skip — this lane's own terminal state; do not re-dispatch. | + +Transitions this lane performs itself: + +- At dispatch (Step 2): add `agent:in-progress`. +- At hand-off (Step 3): remove `agent:in-progress`, add `agent:implemented-in-pr`. +- If grooming proves the issue already fixed: add `agent:already-resolved`, close. +- If grooming can't establish a repro for a bug/flake: add `agent:needs-info`, comment the + findings, move to the next issue. + +## Type labels + +Type labels describe _what kind_ of issue this is and change how Step 1 grooming proceeds. + +| Label | Meaning | How it shapes grooming | +| --------------------- | ------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `bug` | Something isn't working. | Requires an RCA plan before dispatch: evidence-only Phase 1, reproduce before fix. No repro → `agent:needs-info`, not a dispatch. | +| `enhancement` | New feature or request. | Needs acceptance criteria enumerable as greps/tests; write them into the issue body if missing. | +| `documentation` | Docs-only change. | Lighter gate: no code test suite required, but still one lane/one PR; likely `no-changeset`. | +| `question` | Further information is requested. | Answer with evidence in a comment; label `agent:answered`; close if resolved. Not a code dispatch. | +| `flake` | Intermittent test or CI failure. | Reproduce-before-fix is HARD — same bar as `bug`, no exceptions. A fix without a captured reproduction or an identified mechanism is not dispatchable. | +| `refactor` | Internal cleanup, no behavior change. | Acceptance criteria are usually "tests still pass, behavior unchanged" — verify there is no user-visible delta before treating it as done. | +| `ci` | CI, build, or release infrastructure. | Gates are still typecheck/test/lint/fallow on the touched package, plus verifying the workflow YAML change against a real CI run (cached-green claims don't count for workflow files). | +| `testing` | Test coverage or test infrastructure. | Acceptance criteria are the new/fixed tests themselves; still needs a real failing-before/passing-after pair. | +| `dx` | Developer experience and tooling. | Same as `enhancement`; verify the tooling claim against a live run, not just reading the script. | +| `epic` | Umbrella issue tracking a set of sub-issues. | Never dispatch directly — pick a linked sub-issue instead; an epic has no single mergeable PR. | +| `security` | Security hardening or vulnerability. | Treat like `bug` (RCA required) plus: never weaken an existing security gate to make a test pass. | +| `needs-investigation` | Needs RCA or a design decision before it is actionable. | Not dispatchable as-is. Grooming produces the RCA or surfaces the design question; the design question itself goes back to the user, it is not the lane's call. | +| `no-changeset` | PR intentionally ships no release note. | Applied to the resulting PR (not the issue) when the fix touches only private/internal code or is docs-only; lets the changeset-coverage CI gate pass without a changeset. | diff --git a/.claude/skills/conciv-issue/references/worked-example-316.md b/.claude/skills/conciv-issue/references/worked-example-316.md new file mode 100644 index 000000000..aa01d16c8 --- /dev/null +++ b/.claude/skills/conciv-issue/references/worked-example-316.md @@ -0,0 +1,97 @@ +# Worked example: issue #316 → PR #468 + +Facts below are verified against the live issue/PR (`gh issue view 316`, `gh pr view 468`) at +write time, not reconstructed from memory. Use this as the shape a real lane takes, including the +parts that don't fit on the happy path. + +## Pick + +#316 carried `ready-for-agent` and `ci`. The body was already groomed: a failing `Release` +workflow run, a two-commit root cause table (`e93318a3` reverted the CLI package name, `83272f77` +added a changeset still naming the old one), and three ranked suggested fixes with an explicit +caveat on option 1 (`changeset status` has zero-changeset exit-code quirks). This is exactly the +shape Step 0's pick order rewards — smallest pre-groomed issue, grab it. + +## Groom-verify + +Grooming still re-checked the issue's claims against current `origin/main` rather than trusting +the body verbatim: confirmed `packages/cli/package.json` was in fact `@conciv/cli`, confirmed no +workspace package named bare `conciv` existed, and confirmed `ci.yml`'s `repo-checks` job really +had no `changeset` invocation anywhere (`grep -rn changeset .github/workflows/*.yml` turned up only +`release.yml`). The issue picked option 2 (a targeted `conciv-publish` guard, shaped like the +existing `assertPublicSet` drift guard) over option 1 for the reason the body itself gave. + +## Dispatch + +Decided shape handed to the agent: build `assertChangesetsResolve` in `packages/publish/src/guards.ts`, +same shape as `assertPublicSet`; expose it as `conciv-publish check-changesets`; call it at the +start of `conciv-publish version` so the release job fails fast with a readable message instead of +changesets' raw stack trace; wire it into `repo-checks` in `ci.yml`. That single ruling (guard, not +`changeset status`) meant the agent never had to re-litigate the issue's own trade-off analysis. + +## Review rounds + +The PR went through multiple hardening passes before it was mergeable, each catching something the +previous pass missed — this is the part of the lane that Step 3 ("review the full diff yourself, +judge mechanism not just correctness") exists for: + +- Early rounds tightened the trust model of `conciv-publish` itself: deleted an upward + directory-walk `findRoot` in favor of an explicit `assertWorkspaceRoot` guard, closed several + fail-open branches in manifest/changeset reading into fail-closed ones (malformed manifest, + symlinked changeset file, duplicate package entry), and swapped a hand-rolled frontmatter regex + for `@changesets/parse` — the same library `changeset version` itself uses — so the guard accepts + exactly what the real release step accepts. +- A dedicated **codex + adversarial review round** on the PR (documented in the PR body under "Fix + wave") found several issues that would have shipped a gate that looked green but did nothing + real: **H1**, the coverage check counted every `.changeset/*.md` file present in the directory at + HEAD, not just the ones the PR itself added — so a changeset already committed at the merge base + silently satisfied coverage for an unrelated PR, making the gate vacuous. **H2**, a live CI-red + bug: the hand-rolled `packages/` + `packages/extensions/` directory scan used to enumerate + workspace packages was blind to `apps/*`, so a real changeset naming `@conciv/app` failed + name-validation on an actual PR. **H3**, a zizmor-flagged unsound Actions ternary: + `fetch-depth: ${{ github.event_name == 'pull_request' && 0 || 1 }}` always evaluated to `1` + because `0` is falsy in GitHub Actions expressions, silently defeating the intended + full-history checkout on PR runs. Fixes: PR-added-only coverage counting (`git diff --name-status` + scoped to `Added` files), a pleb-ported generator tree walk over `pnpm-workspace.yaml`'s real + glob list replacing the hand-rolled scan, and removing the ternary entirely once the coverage + step moved to its own always-full-history workflow. + +## The RCA + +Even after that hardening landed, the PR's own CI run produced a false positive: `check-changesets +--require-coverage` reported every published package as touched, when the PR's actual diff only +touched `packages/publish` (private), workflow files, and the lockfile. The RCA phase evaluated two +suspects — a bug in the new dependency-attribution logic (H4), and a root-package inversion in the +workspace walk — and refuted both with evidence before accepting either. The real mechanism: +`pull_request`'s default `actions/checkout` ref is the _ephemeral merge_ of the PR head into +whatever `main` currently is, not the PR branch alone, while `--base` was pinned to a base-SHA +snapshot taken when the event fired. Between that snapshot and the job actually running, an +unrelated `chore: version packages` PR landed on main and bumped every package's manifest and +changelog — a real diff, just not this PR's diff. Confirmed locally by reproducing the exact +`git diff --name-status --no-renames ...` and seeing the unrelated release +bump appear. Fix: pin the checkout to `ref: ${{ github.event.pull_request.head.sha }}` so both +sides of the diff are fixed commits, immune to whatever lands on main afterward. + +## Lessons + +- Verify claims against the actual consumer's own libraries, not an assumption about how they + behave — `@changesets/parse` for parsing, real `pnpm-workspace.yaml` glob semantics (via a + faithful port of pleb's resolver, plus a parity test against `pnpm ls -r --depth -1 --json`) for + workspace enumeration. +- Fail closed everywhere a guard has a choice between silently tolerating and throwing — a fail-open + branch in security- or release-critical code is a bug waiting for the one input that exercises it. +- Every new dependency in this PR (`minimatch`, `js-yaml`, later `@manypkg/get-packages` and then + its removal in favor of the pleb port) was verbatim-approved by the repo owner before landing — + none were inferred. + +## Checkpoint list for a future lane + +1. Pick order respected; issue re-verified against current main, not trusted as written. +2. Dispatch states a decided shape, not an open design question. +3. Full diff reviewed by the orchestrator before hand-off, not just "tests pass." +4. Where a gate's own correctness matters (CI/release code), prove it fails on the bad input, not + just that it passes on the good one — a coverage gate that never goes red on a real miss is not + verified. +5. A false-positive or a flaky-looking gate result gets a real RCA (competing suspects, evidence, + refutation) before a fix — never patched on a hunch. +6. Every new dependency has explicit owner approval on record.