diff --git a/.claude/agents/conformance-auditor.md b/.claude/agents/conformance-auditor.md index aae806f1..838cf93b 100644 --- a/.claude/agents/conformance-auditor.md +++ b/.claude/agents/conformance-auditor.md @@ -12,19 +12,22 @@ color: yellow You answer one question: **does the diff match the plan?** Not whether the code is good, not whether the plan was a good plan. Fidelity, and nothing else. -You are given a worktree path, a plan file, and an output path. Read both, read the diff, write -your report to the output path, and report a short summary. You do not repair what you find, -and that is deliberate — a deviation you quietly repair is a deviation nobody ever sees. -Report it. +You are given a worktree path, a plan file, a base ref, and an output path. Read both, read the +diff, write your report to the output path, and report a short summary. You do not repair what +you find, and that is deliberate — a deviation you quietly repair is a deviation nobody ever +sees. Report it. ## Getting the diff +The base ref is the branch this change is proposed against, and it is given to you — do not +assume it: + ```bash -git -C diff upstream/stable...HEAD -git -C log --oneline upstream/stable..HEAD +git -C diff ...HEAD +git -C log --oneline ..HEAD ``` -Three dots. You want what the branch added, not what `stable` moved on to. Read the changed +Three dots. You want what the branch added, not what the base moved on to. Read the changed files themselves where the diff alone does not tell you whether a step was really done — a plan step that says "return `Maybe` instead of null" is not satisfied by a signature change if the call sites still null-check. diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 0132b517..ada4f1ce 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -13,15 +13,16 @@ You review a diff under a **lens** given in your prompt — a name and a descrip look for. One definition serves every lens; the prompt decides which one you are. If no lens is given, review generally: correctness first, then everything else. -You are given a worktree path, a lens, and an output path. Write your findings to the output -path and report a short summary. Report what is wrong; someone else fixes it. +You are given a worktree path, a lens, a base ref, and an output path. Write your findings to the +output path and report a short summary. Report what is wrong; someone else fixes it. ## Scope -Review the diff, not the repository: +Review the diff, not the repository. The base ref is the branch this change is proposed against, +and it is given to you — do not assume it: ```bash -git -C diff upstream/stable...HEAD +git -C diff ...HEAD ``` Read the surrounding code freely — you cannot judge a change without it — but a problem that diff --git a/.claude/skills/crypter-change/SKILL.md b/.claude/skills/crypter-change/SKILL.md index ba547c09..cc22ddc1 100644 --- a/.claude/skills/crypter-change/SKILL.md +++ b/.claude/skills/crypter-change/SKILL.md @@ -71,8 +71,9 @@ git fetch origin docker exec crypter-pipeline crypter-workspace create {run-id} ``` -Fetch first — the workspace takes `upstream/stable` from your `origin/stable`, so a stale -remote-tracking ref puts the whole run on an old base. **If either fails, stop and say so.** +Fetch first — the workspace takes its `origin/stable` from yours, so a stale remote-tracking ref +puts the whole run on an old base. A change of your own targets `stable`, which is what `create` +uses when no `--base` is given. **If either fails, stop and say so.** The workspace holds only committed history. Uncommitted work in your checkout is not visible to the container and never reaches the branch. @@ -97,7 +98,7 @@ Keep the title and description it reports; `crypter-step-open-pull-request` need ```bash docker exec -w /work/{run-id} crypter-pipeline \ - claude --permission-mode auto -p "/crypter-devcontainer-examine {run-id} {branch} /plans/{run-id}/plan.md" + claude --permission-mode auto -p "/crypter-devcontainer-examine {run-id} {branch} origin/stable /plans/{run-id}/plan.md" ``` It writes `.claude/runs/{run-id}/conformance.md` and `.claude/runs/{run-id}/findings/{lens}.md`. diff --git a/.claude/skills/crypter-devcontainer-examine/SKILL.md b/.claude/skills/crypter-devcontainer-examine/SKILL.md index 287dbbe4..df2dbe59 100644 --- a/.claude/skills/crypter-devcontainer-examine/SKILL.md +++ b/.claude/skills/crypter-devcontainer-examine/SKILL.md @@ -1,6 +1,6 @@ --- name: crypter-devcontainer-examine -description: Review a diff in the pipeline container and write findings to the host. Invoked as /crypter-devcontainer-examine {run-id} {ref} [plan-path] by the crypter-change and crypter-review skills. +description: Review a diff in the pipeline container and write findings to the host. Invoked as /crypter-devcontainer-examine {run-id} {ref} {base-ref} [plan-path] by the crypter-change and crypter-review skills. --- # Crypter devcontainer examine @@ -12,8 +12,14 @@ The ref already exists in the run's workspace at `/work/{run-id}`. ## Setup -You are given a run id, a ref, and optionally a plan path: -`/crypter-devcontainer-examine {run-id} {ref} [plan-path]`. +You are given a run id, a ref, a base ref, and optionally a plan path: +`/crypter-devcontainer-examine {run-id} {ref} {base-ref} [plan-path]`. + +The base ref is the branch the change is proposed against, named as the workspace knows it — +`origin/stable` for work built here, `origin/main` for a pull request that targets `main`. Every +agent below diffs against it. **Pass it on as given; never substitute a default.** A base that +does not match the pull request produces a diff nobody asked about, and the emptiest version of +that failure — a base identical to the ref — reads as four lenses finding nothing wrong. Two review phases run here, and the plan path decides whether the first one applies: @@ -50,13 +56,13 @@ Every agent gets the workspace path and works by absolute path inside it. Never ## 2. Plan adherence -Given a plan path, invoke `conformance-auditor` with it, the workspace, and +Given a plan path, invoke `conformance-auditor` with it, the workspace, the base ref, and `/runs/{run-id}/conformance.md`. It reports where the diff and the plan diverge. ## 3. Code review -Invoke `reviewer` once per lens, in parallel — they do not interact. Each gets the workspace and -`/runs/{run-id}/findings/{lens}.md`. +Invoke `reviewer` once per lens, in parallel — they do not interact. Each gets the workspace, the +base ref, and `/runs/{run-id}/findings/{lens}.md`. | Lens | Brief | |---|---| diff --git a/.claude/skills/crypter-devcontainer-implement/SKILL.md b/.claude/skills/crypter-devcontainer-implement/SKILL.md index 546a4740..53090e79 100644 --- a/.claude/skills/crypter-devcontainer-implement/SKILL.md +++ b/.claude/skills/crypter-devcontainer-implement/SKILL.md @@ -26,10 +26,10 @@ end. ## 1. Branch -The workspace is checked out at `upstream/stable`, so build from there: +The workspace is checked out at `origin/stable`, so build from there: ```bash -git -C /work/{run-id} checkout -b {branch} refs/remotes/upstream/stable +git -C /work/{run-id} checkout -b {branch} refs/remotes/origin/stable ``` **If this fails, stop and say so.** A branch cut from the wrong base leaves the diff and the diff --git a/.claude/skills/crypter-review/SKILL.md b/.claude/skills/crypter-review/SKILL.md index 30a25983..445dc719 100644 --- a/.claude/skills/crypter-review/SKILL.md +++ b/.claude/skills/crypter-review/SKILL.md @@ -66,19 +66,26 @@ Read its title, description and diff with whatever GitHub access this session ha CLI, or the GitHub MCP server's `pull_request_read`. What the author says it does is context for reading the diff, and worth carrying into your report where the two disagree. +Take its **base branch** from the same read — `base.ref` from `pull_request_read` with method +`get`, or `.baseRefName` from `gh pr view`. Most pull requests here target `stable`, but a +release targets `main`, and nothing about the number tells you which. Everything below diffs +against the branch the pull request actually names. + ## 2. Fetch it into a workspace The container has no network remote. It clones from your repository through a read-only mount, so the pull request head goes into your repository first and travels across from there: ```bash -git fetch origin +refs/pull/{number}/head:refs/pr/{number} +git fetch origin +refs/pull/{number}/head:refs/pr/{number} {base-branch} docker exec crypter-pipeline crypter-workspace create pr-{number} \ - '+refs/pr/{number}:refs/heads/pr-{number}' + --base {base-branch} '+refs/pr/{number}:refs/heads/pr-{number}' ``` The refspec is forced, so reviewing a pull request again after its author rebased or amended -picks up the new head instead of being rejected. +picks up the new head instead of being rejected. The base branch is fetched alongside it because +the workspace clones your repository, and a base you have never fetched is not there to diff +against. **If either fails, stop and say so.** @@ -88,7 +95,7 @@ The workspace lasts for this review and no longer. ```bash docker exec -w /work/pr-{number} crypter-pipeline \ - claude --permission-mode auto -p "/crypter-devcontainer-examine pr-{number} pr-{number}" + claude --permission-mode auto -p "/crypter-devcontainer-examine pr-{number} pr-{number} origin/{base-branch}" ``` No plan path. A pull request raised elsewhere has no plan to hold it against, so the plan diff --git a/.devcontainer/workspace.sh b/.devcontainer/workspace.sh index 1097dfbf..69a4ad77 100755 --- a/.devcontainer/workspace.sh +++ b/.devcontainer/workspace.sh @@ -12,7 +12,7 @@ set -euo pipefail host_git="/host-git" usage() { - echo "usage: crypter-workspace create {run-id} [refspec]" >&2 + echo "usage: crypter-workspace create {run-id} [--base {branch}] [refspec]" >&2 echo " crypter-workspace remove {run-id}" >&2 exit 64 } @@ -20,6 +20,7 @@ usage() { subcommand="${1:-}" run_id="${2:-}" [[ -n "${subcommand}" && -n "${run_id}" ]] || usage +shift 2 # The run id becomes a path under /work that `remove` deletes recursively, so it has to be a # plain name before it is used as one. @@ -28,6 +29,22 @@ if [[ ! "${run_id}" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then exit 64 fi +# The branch the run is built or reviewed against. A pull request states its own, so the caller +# passes what it read rather than letting this default stand in for it. +base="stable" +if [[ "${1:-}" == "--base" ]]; then + base="${2:-}" + [[ -n "${base}" ]] || usage + shift 2 +fi + +# The base reaches git as a ref, where a leading dash would be read as an option instead. +if [[ ! "${base}" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]]; then + echo "Base branch '${base}' is not a plain branch name" >&2 + exit 64 +fi + +refspec="${1:-}" workspace="/work/${run_id}" case "${subcommand}" in @@ -45,26 +62,43 @@ case "${subcommand}" in exit 1 fi + # Resolve the base before anything is created, so a base that is not there leaves nothing + # behind to remove first. + if ! git -C "${host_git}" rev-parse --verify --quiet "refs/remotes/origin/${base}" >/dev/null + then + echo "The host repository has no origin/${base}. Fetch it there and try again." >&2 + exit 1 + fi + # --no-hardlinks because the mount is read-only and owned by another uid, which is exactly # the case where git's hardlink optimisation is unavailable. Copying is predictable. git clone --quiet --no-hardlinks "${host_git}" "${workspace}" - # The agents diff against upstream/stable. Take it from the host's own remote-tracking ref - # so it reflects the org repository rather than whatever branch the host has checked out. - git -C "${workspace}" fetch --quiet origin \ - '+refs/remotes/origin/stable:refs/remotes/upstream/stable' - - if [[ -n "${3:-}" ]]; then - git -C "${workspace}" fetch --quiet origin "${3}" + # A clone maps the source's local branches into origin/*, so origin/stable here would mean + # whatever the host has checked out rather than what the org repository holds. Point the + # remote at the host's own remote-tracking refs instead, so origin/{branch} means the same + # thing in a workspace as it does on the host. Configuring the refspec rather than fetching + # it once keeps a later bare `git fetch` from putting the host's local branches back. + git -C "${workspace}" config remote.origin.fetch \ + '+refs/remotes/origin/*:refs/remotes/origin/*' + git -C "${workspace}" fetch --quiet --prune origin + + if [[ -n "${refspec}" ]]; then + git -C "${workspace}" fetch --quiet origin "${refspec}" fi - git -C "${workspace}" checkout --quiet -B stable refs/remotes/upstream/stable + # The clone takes origin/HEAD from the host's checked-out branch, which is the one thing in + # the origin namespace that would still mean the host rather than the org. Point it at the + # base, so a bare `origin` resolves to what the run is measured against. + git -C "${workspace}" remote set-head origin "${base}" + + git -C "${workspace}" checkout --quiet -B "${base}" "refs/remotes/origin/${base}" git -C "${workspace}" config user.name "${CRYPTER_GIT_NAME}" git -C "${workspace}" config user.email "${CRYPTER_GIT_EMAIL}" - echo "Workspace ready at ${workspace}" - git -C "${workspace}" log --oneline -1 refs/remotes/upstream/stable + echo "Workspace ready at ${workspace}, based on ${base}" + git -C "${workspace}" log --oneline -1 "refs/remotes/origin/${base}" ;; remove) diff --git a/Documentation/Development/Agentic Development Pipeline.md b/Documentation/Development/Agentic Development Pipeline.md index 0a19a8d7..255b6110 100644 --- a/Documentation/Development/Agentic Development Pipeline.md +++ b/Documentation/Development/Agentic Development Pipeline.md @@ -61,15 +61,18 @@ The orchestrator owns the lifecycle. It creates the workspace in its setup and r the run ends; the container skills use it and never create or destroy one. ```bash -docker exec crypter-pipeline crypter-workspace create {run-id} [refspec] +docker exec crypter-pipeline crypter-workspace create {run-id} [--base {branch}] [refspec] docker exec crypter-pipeline crypter-workspace remove {run-id} ``` -The org repository has two names as a result. Your session reaches it as `origin`, the remote -your checkout already has. Inside a workspace it is `upstream/stable`, a ref the create step -copies from your `origin/stable` so the agents always diff against the org's current code rather -than whatever branch you have checked out. Fetch before creating a workspace, or the run starts -on a stale base. +The org repository is `origin` on both sides. A clone would otherwise map your local branches +into the workspace's `origin/*`, so the create step points the remote at your remote-tracking +refs instead, and `origin/stable` in a workspace means what it means in your checkout. Fetch +before creating a workspace, or the run starts on a stale base. + +`--base` is the branch the run is built or reviewed against, `stable` when it is not given. A +pull request states its own base, and a release states `main`, so the review skills pass what +they read rather than assuming. This document covers the setup you need before the container will start.