diff --git a/.claude/hooks/no-merge-guard.sh b/.claude/hooks/no-merge-guard.sh index 0fa1337..814267d 100755 --- a/.claude/hooks/no-merge-guard.sh +++ b/.claude/hooks/no-merge-guard.sh @@ -5,10 +5,11 @@ # Deterministic layer under the ystack-main-gate ruleset — defense in depth, # and the only enforcement in clones where no ruleset exists. # -# Deliberately NOT blocked: scripts/merge-pr.sh — the v1 in-session merge -# harness keeps its own safety checks (SHA pinning, required-checks gate, -# REVIEW_REQUIRED refusal). Raw merge commands are blocked; the harness or a -# human is the only merge path. +# The merge helper is blocked too. It was exempt while the manager could +# merge in-session; that permission is retired, so the exemption became a +# bypass — in a clone with no ruleset, or under an account that can merge, +# an agent could have merged through it. scripts/merge-pr.sh is now the +# operator's own tool, run by a human, never by an agent. input=$(cat) cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // empty') @@ -22,9 +23,13 @@ if printf '%s' "$cmd" | grep -Eq 'git[[:space:]]+push[^|;&]*[[:space:]:+](refs/h exit 2 fi -# Raw merge commands — merging is the human gate (or scripts/merge-pr.sh). +# Every merge path — merging is the operator's, always. if printf '%s' "$cmd" | grep -Eq 'gh[[:space:]]+pr[[:space:]]+merge'; then - echo "Blocked by ystack guard: agents never merge PRs. A human merges at the gate (or use scripts/merge-pr.sh in-session)." >&2 + echo "Blocked by ystack guard: agents never merge PRs. The operator merges at the gate." >&2 + exit 2 +fi +if printf '%s' "$cmd" | grep -Eq 'merge-pr\.sh'; then + echo "Blocked by ystack guard: merge-pr.sh is the operator's own tool. Apply merge-ready and hand the PR over." >&2 exit 2 fi if printf '%s' "$cmd" | grep -Eq 'gh[[:space:]]+api[[:space:]][^|;&]*pulls/[0-9]+/merge'; then diff --git a/.claude/skills/intent-draft/SKILL.md b/.claude/skills/intent-draft/SKILL.md index 570e5b2..7046095 100644 --- a/.claude/skills/intent-draft/SKILL.md +++ b/.claude/skills/intent-draft/SKILL.md @@ -42,4 +42,4 @@ Template — keep all six sections and headings exact: **Write in plain language.** Short sentences, everyday words — the reader is a -tired human, not another agent (see CLAUDE.md > PR rules). +tired human, not another agent (see AGENTS.md > PR rules). diff --git a/.claude/skills/plan-draft/SKILL.md b/.claude/skills/plan-draft/SKILL.md index 7903da1..ce45934 100644 --- a/.claude/skills/plan-draft/SKILL.md +++ b/.claude/skills/plan-draft/SKILL.md @@ -41,4 +41,4 @@ Slug: `$0` later departs from the plan, update `plan.md` in the same commit that departs. **Write in plain language.** Short sentences, everyday words — the reader is a -tired human, not another agent (see CLAUDE.md > PR rules). +tired human, not another agent (see AGENTS.md > PR rules). diff --git a/.claude/skills/spec-draft/SKILL.md b/.claude/skills/spec-draft/SKILL.md index da6c39b..d3a4b6a 100644 --- a/.claude/skills/spec-draft/SKILL.md +++ b/.claude/skills/spec-draft/SKILL.md @@ -38,4 +38,4 @@ Slug: `$0` **G2**: approval to build. **Write in plain language.** Short sentences, everyday words — the reader is a -tired human, not another agent (see CLAUDE.md > PR rules). +tired human, not another agent (see AGENTS.md > PR rules). diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..149477d --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,107 @@ +# Working in this repo — for every agent + +ystack is a control plane for an autonomous coding team, and it is its own +target repo: agents here are improving the team itself. + +**This file is the single source of working rules.** Every agent reads it, +whatever vendor — Codex and most tools look for `AGENTS.md`, Claude Code +reads `CLAUDE.md`, which imports this file. One file, no drift. + +Companions: **`REVIEW.md`** is how work is reviewed here (passes, Important +vs nit, how disagreements end). **`work//`** holds the artifact chain — +`intent.md` → `spec.md` → `plan.md`. If you are implementing, your brief is +that slug's plan; it is written so someone who never saw the conversation can +build from it. + +Two goals drive the backlog: +1. **Reusable by anyone** — a clean, parameterized, well-documented product others can adopt. +2. **Full backup** — everything needed to reconstruct the team if the live setup is lost. + +## What lives here +- `manager/CLAUDE.md` — yshifu's role (the manager persona). +- `routines/*.md` — the coder's baseline instructions yshifu reads to brief a spawned + coder subagent (`coder.md` / `coder-revision.md`) plus the per-task `brief.md`. +- `reviewer/codex-review.md` — the doc for the Codex reviewer harness (`scripts/codex-review.sh`). +- `templates/*` — drop-in files for target repos. +- `scripts/*.sh` — the shipped tooling: `install.sh` (generates the `/yshifu` command), + `setup-target-repo.sh` (bootstraps a target repo's loop labels), and `codex-review.sh` + (runs the Codex reviewer against a PR). + +## Stack & commands +- Markdown + shell. The setup/reviewer tooling lives in `scripts/*.sh`; validators are + still to come. +- CI: `.github/workflows/ci.yml` (structure check + shellcheck). **CI must stay green — + it is the hard merge gate.** Add real tests as code lands. + - **Shellcheck is pinned to `0.11.0`** (the `SHELLCHECK_VERSION` constant in + `ci.yml` is the single source of truth). CI downloads that exact static release and + verifies its release-asset SHA-256 and version before linting, so a runner-image bump + or changed download can't silently drift it. **Lint locally against 0.11.0** — not + whatever your local install happens to be — with `shellcheck -x -S style` over + `find . -name '*.sh' -not -path './.git/*'`; another shellcheck version can report + different findings/codes (e.g. SC2317 vs SC2329) and disagree with CI. Grab the pinned + binary from the shellcheck GitHub releases if your local version differs. + - The **structure check** reads `ci/required-files.txt` — the manifest of every + restore-critical file — and fails if any listed path is missing (and if a listed + `scripts/*.sh` isn't executable). This is what makes the full-backup goal enforceable: + delete a load-bearing file and CI goes red. **Add new restore-critical files to the + manifest** so the guarantee keeps holding. + +## PR rules (enforced by coder + reviewer) +- **One concern per PR.** Soft size budget ~300–400 net lines; split if bigger. +- Every PR links its issue (`Closes #`) and keeps README/docs in sync with any change. +- **Plain language, always** (operator rule, 2026-08-26): every artifact + (intent/spec/plan), PR title/description, and review comment is written for a + tired human. Short sentences. Everyday words. No jargon where a plain word + works. If two phrasings say the same thing, use the shorter one. + +## CRITICAL — self-modification safety +- The live setup runs from **generated/synced artifacts, not from these files directly.** + Editing a prompt or doc here is a *proposal*; it only takes effect once synced: the live + `/yshifu` command is regenerated by re-running **`scripts/install.sh`**, and the coder + instructions in `routines/coder.md` / `routines/coder-revision.md` take effect when + **yshifu reads them to brief a spawned coder subagent** (there are no UI-pasted routines). + Merging a prompt change does NOT change live behavior until synced — call this out in the + PR description when a prompt changes. +- **Never weaken the safety rails without explicit human sign-off:** reviewer stays + read-only / comments-only; **merging is the operator's, always** — the in-session + auto-merge v1 allowed was retired when the branch ruleset landed, and no agent has a + merge path any more; the rounds cap and `needs-human` escalation stay + intact. yshifu never writes code/opens PRs and **never self-approves acting alone** — a + user-directed issue is gated by the user's approval of the drafted spec (the one-liner is the + request, not the go), a proactive issue by the passed yshifu⇄Codex manager-debate consensus + (for *proactive* work the user's gate is at the north-star altitude; user-directed issues + still need the user's spec approval). + +## v2 artifact chain (work/) +- One initiative = one dir: `work//` holding `intent.md` → `spec.md` → `plan.md`. + Each artifact lands via its own PR and the operator's merge IS the gate: G1 accepts + the intent, G2 approves the spec, G3 approves the implementation PR (which carries + `plan.md` + code + tests). Details: `work/README.md`; review policy: `REVIEW.md`. +- Skills: `/intent-draft`, `/spec-draft`, `/plan-draft` hold the templates and stage rules. +- **Hash discipline:** `spec.md` frontmatter records `intent-blob` (`git hash-object` of + the intent it was drafted from); `plan.md` records `spec-blob`. On mismatch with main's + current upstream file, label the PR `stale` and stop — never build on a moved artifact. +- **Stage rules (autonomous lane):** the spec stage writes only `work//spec.md`; + the implement stage never touches `intent.md`/`spec.md`; unattended agents never write + the **constitution paths** — `.github/**`, `.claude/**`, `AGENTS.md` (this file), + `CLAUDE.md`, `REVIEW.md` — such changes land as patches under `proposals/` that the + operator applies. That is the same list `REVIEW.md` uses; the two must always match, so + a change to one is a change to both. Operator-driven sessions are exempt; Phase 3 hooks + enforce this mechanically via `YSTACK_STAGE`. +- Deterministic branches: `ystack/intent/`, `ystack/spec/`, + `ystack/impl/` — re-runs update the existing PR, never open a second. + +## Reusability goal +- No hardcoded personal values (usernames, repo names) in shipped templates — keep the + reusable path parameterized. Personal config stays out of it. + +## The rules that bite + +- **Never merge.** Opening a PR ends an agent's authority; the operator + merges. Pushing to `main` is refused server-side anyway. +- **Prove it.** Run the checks the plan names, paste the output, and say + which commit you ran them on. Old proof on a new commit is stale. +- **Old names are gone.** The project and its manager were renamed; + `scripts/check-rename.sh` fails CI if either old name survives in a tracked + file. A line documenting real back-compat must carry the word "legacy" — + that is how the gate tells intent from leftovers. diff --git a/CLAUDE.md b/CLAUDE.md index bf251dc..1f196ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,83 +1,9 @@ # ystack — conventions for the coding agents -This is the **control-plane repo for an autonomous coding team, and it is its own -target repo** — agents working here are improving the team itself. Two goals drive -the backlog: -1. **Reusable by anyone** — a clean, parameterized, well-documented product others can adopt. -2. **Full backup** — everything needed to reconstruct the team if the live setup is lost. +The working rules live in **[`AGENTS.md`](AGENTS.md)** so every agent reads the +same thing, whatever vendor. Open that file — this one only imports it so +Claude Code loads the same text: -## What lives here -- `manager/CLAUDE.md` — yshifu's role (the manager persona). -- `routines/*.md` — the coder's baseline instructions yshifu reads to brief a spawned - coder subagent (`coder.md` / `coder-revision.md`) plus the per-task `brief.md`. -- `reviewer/codex-review.md` — the doc for the Codex reviewer harness (`scripts/codex-review.sh`). -- `templates/*` — drop-in files for target repos. -- `scripts/*.sh` — the shipped tooling: `install.sh` (generates the `/yshifu` command), - `setup-target-repo.sh` (bootstraps a target repo's loop labels), and `codex-review.sh` - (runs the Codex reviewer against a PR). +@AGENTS.md -## Stack & commands -- Markdown + shell. The setup/reviewer tooling lives in `scripts/*.sh`; validators are - still to come. -- CI: `.github/workflows/ci.yml` (structure check + shellcheck). **CI must stay green — - it is the hard merge gate.** Add real tests as code lands. - - **Shellcheck is pinned to `0.11.0`** (the `SHELLCHECK_VERSION` constant in - `ci.yml` is the single source of truth). CI downloads that exact static release and - verifies its release-asset SHA-256 and version before linting, so a runner-image bump - or changed download can't silently drift it. **Lint locally against 0.11.0** — not - whatever your local install happens to be — with `shellcheck -x -S style` over - `find . -name '*.sh' -not -path './.git/*'`; another shellcheck version can report - different findings/codes (e.g. SC2317 vs SC2329) and disagree with CI. Grab the pinned - binary from the shellcheck GitHub releases if your local version differs. - - The **structure check** reads `ci/required-files.txt` — the manifest of every - restore-critical file — and fails if any listed path is missing (and if a listed - `scripts/*.sh` isn't executable). This is what makes the full-backup goal enforceable: - delete a load-bearing file and CI goes red. **Add new restore-critical files to the - manifest** so the guarantee keeps holding. - -## PR rules (enforced by coder + reviewer) -- **One concern per PR.** Soft size budget ~300–400 net lines; split if bigger. -- Every PR links its issue (`Closes #`) and keeps README/docs in sync with any change. -- **Plain language, always** (operator rule, 2026-08-26): every artifact - (intent/spec/plan), PR title/description, and review comment is written for a - tired human. Short sentences. Everyday words. No jargon where a plain word - works. If two phrasings say the same thing, use the shorter one. - -## CRITICAL — self-modification safety -- The live setup runs from **generated/synced artifacts, not from these files directly.** - Editing a prompt or doc here is a *proposal*; it only takes effect once synced: the live - `/yshifu` command is regenerated by re-running **`scripts/install.sh`**, and the coder - instructions in `routines/coder.md` / `routines/coder-revision.md` take effect when - **yshifu reads them to brief a spawned coder subagent** (there are no UI-pasted routines). - Merging a prompt change does NOT change live behavior until synced — call this out in the - PR description when a prompt changes. -- **Never weaken the safety rails without explicit human sign-off:** reviewer stays - read-only / comments-only; merge stays gated (yshifu may merge only CI-green + Codex-clean - + low-risk PRs under standing authorization, and must bring safety-rail / north-star / - ambiguous / high-risk PRs to the human); the rounds cap and `needs-human` escalation stay - intact. yshifu never writes code/opens PRs and **never self-approves acting alone** — a - user-directed issue is gated by the user's approval of the drafted spec (the one-liner is the - request, not the go), a proactive issue by the passed yshifu⇄Codex manager-debate consensus - (for *proactive* work the user's gate is at the north-star altitude; user-directed issues - still need the user's spec approval). - -## v2 artifact chain (work/) -- One initiative = one dir: `work//` holding `intent.md` → `spec.md` → `plan.md`. - Each artifact lands via its own PR and the operator's merge IS the gate: G1 accepts - the intent, G2 approves the spec, G3 approves the implementation PR (which carries - `plan.md` + code + tests). Details: `work/README.md`; review policy: `REVIEW.md`. -- Skills: `/intent-draft`, `/spec-draft`, `/plan-draft` hold the templates and stage rules. -- **Hash discipline:** `spec.md` frontmatter records `intent-blob` (`git hash-object` of - the intent it was drafted from); `plan.md` records `spec-blob`. On mismatch with main's - current upstream file, label the PR `stale` and stop — never build on a moved artifact. -- **Stage rules (autonomous lane):** the spec stage writes only `work//spec.md`; - the implement stage never touches `intent.md`/`spec.md`; unattended agents never write - `.github/**` or `.claude/**` — such changes land as patches under `proposals/` that the - operator applies. Operator-driven sessions are exempt; Phase 3 hooks enforce this - mechanically via `YSTACK_STAGE`. -- Deterministic branches: `ystack/intent/`, `ystack/spec/`, - `ystack/impl/` — re-runs update the existing PR, never open a second. - -## Reusability goal -- No hardcoded personal values (usernames, repo names) in shipped templates — keep the - reusable path parameterized. Personal config stays out of it. +Nothing else belongs here. Put changes in `AGENTS.md`. diff --git a/QUICKSTART.md b/QUICKSTART.md index caf2dc4..70a866c 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -2,7 +2,7 @@ The friendly golden path for a **new adopter**: clone this repo, install the `/yshifu` command, point it at a target repo, and watch one loop run. For the mental model read -[`README.md`](README.md); for the conventions and safety rails read [`CLAUDE.md`](CLAUDE.md). +[`README.md`](README.md); for the conventions and safety rails read [`AGENTS.md`](AGENTS.md). > This is the happy path only. The scripts are the source of truth for the exact > commands — when in doubt, run them and read their output. For depth, edge cases, and @@ -13,7 +13,8 @@ command, point it at a target repo, and watch one loop run. For the mental model - **`gh` CLI authenticated** — `gh auth status` shows you logged in. - **`jq` on `PATH`** — needed by the review/debate gates to validate Codex's typed JSONL events, - and by `scripts/merge-pr.sh` to parse `gh`'s CI-check JSON. + and by `scripts/merge-pr.sh` — your own merge helper, which no agent runs — to parse `gh`'s + CI-check JSON. - **Codex (OpenAI) CLI signed in** — a ChatGPT plan that includes Codex review is enough for personal repos; the CLI must be installed and signed in. This is the cross-vendor reviewer. - **Claude Code installed** — the whole team runs in-session (no API key); yshifu and the @@ -55,7 +56,7 @@ command, point it at a target repo, and watch one loop run. For the mental model PR CI, **yshifu offers to bootstrap it for you** at first contact (it scaffolds a `pull_request` workflow from your toolchain as the first "add PR CI" issue, and **you approve + merge that initial gate by hand** — yshifu classifies it as human-merge-only and - won't run `merge-pr.sh` on it, since a self-authored gate can't certify itself). Wire it + won't even label it `merge-ready`, since a self-authored gate can't certify itself). Wire it yourself instead if you prefer. Everything else (the loop labels, the readiness pre-flight) **yshifu bootstraps for you on first use** — see step 7. A target `CLAUDE.md` is **optional**: the coder auto-discovers the install / lint / build / @@ -93,7 +94,8 @@ command, point it at a target repo, and watch one loop run. For the mental model star is the root `NORTH_STAR.md` — ystack is its own target.) 5. **Clone the target repo and `cd` into it.** `/yshifu` and every orchestration script - (`codex-review.sh`, `merge-pr.sh`, `manager-review.sh`) run from **inside the target + (`codex-review.sh` and `manager-review.sh`, plus `merge-pr.sh` when *you* merge with it) + run from **inside the target repo's local clone** — they read its git remote and resolve the repo via `gh` (e.g. `codex-review.sh` calls `gh repo view`) — so you need a working copy on disk, and `gh` must resolve to **the repo PRs target**. @@ -146,11 +148,15 @@ command, point it at a target repo, and watch one loop run. For the mental model yshifu runs `"/scripts/codex-review.sh" ` from inside the target repo's clone — by absolute path, since the harness lives only in the ystack clone, not the target repo → Codex posts review comments only. Fixes bump `round-N`; the cap (~3) or - an ambiguous spec escalates with `needs-human`. After a clean review yshifu labels the - PR `merge-ready`; for a clean, low-risk PR yshifu **merges** it once CI is green, under - your standing authorization (acting on the passed review — not self-approval; Codex is - comments-only). A PR needing human review (safety-rail / north-star / high-risk) is - brought to **you** to merge instead. + an ambiguous spec escalates with `needs-human`. When CI is green and Codex passed that + exact head, yshifu labels the PR `merge-ready` and hands it to **you — you merge, always.** + yshifu never merges — merging is yours, and it is a rail, not a preference. (Protect `main` + with a ruleset that requires a pull request plus an approving review and the rail is enforced + too: the reviewer is comments-only and cannot approve, and no agent gets a bypass.) The label + only says "reviewed clean at this head", and it goes void the moment new commits land. A PR + that needs your judgment on top of the review (safety-rail / north-star / high-risk) is + flagged as such when it is handed over. `scripts/merge-pr.sh` stays in the repo for your own + use; yshifu never runs it. That's the loop. To prove a rebuilt or relocated setup end to end — or recover a lost one — follow the smoke test and runbook in [`RESTORE.md`](RESTORE.md). @@ -193,12 +199,14 @@ gate yet (yshifu won't run autonomously until a real gate exists): this exception exists precisely to establish both). Cross-vendor Codex review still runs. This leaves the 0→1 target with the committed north star the shipped gate (`manager-review.sh`) requires. -6. **You approve and merge the bootstrap PR by hand.** No real gate exists yet for it to - certify itself, so yshifu classifies it **human-merge-only** and does **not** auto-merge - it — you merge that initial gate yourself (same as the add-PR-CI bootstrap above). +6. **You approve and merge the bootstrap PR by hand.** You merge every PR here, but this one + is extra: no real gate exists yet for it to certify itself, so yshifu classifies it + **human-merge-only** and does **not** apply `merge-ready` at all — you merge that initial + gate yourself (same as the add-PR-CI bootstrap above). 7. **Handoff to the 1→N loop — the front gate still holds.** Once the skeleton + CI + first test land, a **real gate now exists** and yshifu transitions to the normal loop under the - standing rails (including auto-merge for clean, low-risk PRs). But the handoff does **not** + standing rails (review → `merge-ready` → handed to you, and you merge). But the handoff + does **not** by itself unlock open-ended proactive autonomy: you approved the **bootstrap scaffold plan (scoped to the 0→1 PR)**, which is **NOT** approval of the active north star for proactive 1→N work. So after the bootstrap lands, yshifu pursues **proactive** north-star work **only diff --git a/README.md b/README.md index d85908f..d17f81a 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ separate human channel to the workers. Claude and Codex never talk directly; | Agent | Vendor | How it runs | Writes? | |-------|--------|-------------|---------| -| **yshifu** (manager) | Claude | You talk to it in a Claude Code chat (`manager/CLAUDE.md`) | issues only; never authors code/PRs (merges clean low-risk PRs) | +| **yshifu** (manager) | Claude | You talk to it in a Claude Code chat (`manager/CLAUDE.md`) | issues only; never authors code/PRs; **never merges** (labels `merge-ready`, hands the PR to you) | | **Coder** | Claude | A subagent yshifu spawns with the issue/PR context — two modes: build (`routines/coder.md`) then fix (`routines/coder-revision.md`) | yes (branches, PRs) | | **Manager-reviewer** | Codex (OpenAI) | yshifu runs `scripts/manager-review.sh` at **plan altitude, before coding** — debates a proactive issue vs. the north star → PROCEED/REFINE/DROP | **veto only / read-only** (never labels or merges) | | **Code-reviewer** | Codex (OpenAI) | yshifu runs `scripts/codex-review.sh` at **code altitude, after coding** — against the PR diff | **comments only / read-only** | @@ -53,13 +53,15 @@ exactly one coder launch per cleared issue, one review path, and one revision pa ↺ yshifu re-runs codex-review.sh └── round = 3 (cap) → SCOPE DOWN + FOLLOW-UP (productive): land the converged core (one scoped-down change → - clean review → merge) + open a follow-up issue for - the contested remainder; only a genuine standoff / - safety-rail / north-star → label `needs-human` → pings YOU + clean review → `merge-ready` → YOU merge) + open a + follow-up issue for the contested remainder; only a + genuine standoff / safety-rail / north-star → + label `needs-human` → pings YOU ↓ - CI green + Codex clean (low-risk) → yshifu runs scripts/merge-pr.sh - (in-session, back-to-back; SHA-pinned merge — status scan / brief only report) - (high-risk / escalations / rail changes / north-star → YOU) + CI green + Codex clean at that head → yshifu labels the PR `merge-ready` + and hands it to YOU → YOU merge (yshifu never merges; a status scan + or brief only reports). New commits void `merge-ready` — re-review first. + (high-risk / escalations / rail changes / north-star → named at handoff) ``` ## Design decisions (the "why") @@ -76,8 +78,9 @@ exactly one coder launch per cleared issue, one review path, and one revision pa [`.ystack/north-star.md`](templates/.ystack/north-star.md) (when the target *is* this control-plane repo, that file is the root [`NORTH_STAR.md`](NORTH_STAR.md) — ystack is its own target) — and yshifu pursues it - autonomously — you stop reviewing diffs, and for **proactive** work you stop approving each - issue. Two paths clear an issue to run: a **user-directed** issue where your one-liner is the + autonomously — you stop reading diffs line by line (you still merge every PR, but on the + strength of `merge-ready`), and for **proactive** work you stop approving each issue. + Two paths clear an issue to run: a **user-directed** issue where your one-liner is the *request* — yshifu drafts the spec, **you still approve that drafted spec**, and *that approval* is the gate yshifu records with `ready` (drafting alone does not earn `ready`; user-directed issues are *not* exempt from per-spec approval); @@ -92,36 +95,36 @@ exactly one coder launch per cleared issue, one review path, and one revision pa drafted-spec approval. - **CI is the hard gate** — ground truth. Autonomy rests on tests first, diverse reviewer second. -- **yshifu auto-merges clean, low-risk PRs — in-session only.** Under your standing - authorization, yshifu **may auto-merge a PR it reviewed in-session** when it is CI-green, - Codex-clean, and low-risk — no per-PR confirmation — **unless it is high-risk**. yshifu does - this by running **`scripts/merge-pr.sh `** from within the target repo's clone (it does - not hand-craft a merge command). `merge-pr.sh` owns the mechanical safety: it reads the - reviewed head+base SHAs from the authenticated `codex-review.sh` marker, confirms the PR's - current head **and** base still match those (refusing if either moved since the review), - gates on the base branch's **required status checks** (falling back to ≥1 real passing CI - check with none failing when no required checks are defined — optional checks like preview - deploys are informational), refuses a PR that needs an **approving review** - (`reviewDecision=REVIEW_REQUIRED`, since the comments-only reviewer never approves), and - merges with a **repo-permitted method** (squash if allowed) **pinned via - `--match-head-commit`** — refusing otherwise. The merge is **scoped to the target repo** (never another repo) and **bound to the - exact head yshifu reviewed** — if the head moved, yshifu **re-reviews rather than merges** (the - script itself refuses a moved head; a head Codex never reviewed is never merged). A later - **status/Tracking scan and the brief only surface `merge-ready` PRs (read-only)** — they never - auto-merge; those get merged on a fresh in-session review, or by you. High-risk PRs always - come to your merge gate even when CI-green + Codex-clean (auth, DB/schema migrations, - shared/production repos, security-sensitive or other operator-judgment changes) — yshifu does - **not** run `merge-pr.sh` for those. You're also brought in for `needs-human`/round-cap - escalations, safety-rail changes, and **north-star milestones / goal drift**. The high-risk - carve-out is the last word on merging — when in doubt about risk, it comes to you. The - **unattended status-scan / cross-repo auto-merge** (a daemon merging without a yshifu session) - is a **future extension of `merge-pr.sh`, deferred to [#46](../../issues/46)** — not supported - yet per the script's header. +- **yshifu never merges — it labels, then hands you the PR.** Merging is the operator's, + always. `main`'s branch ruleset requires a pull request plus **one approving review**, the + Codex reviewer is **comments-only and never approves**, and **no agent has a bypass** — so + there is no agent merge path at all. What yshifu does instead: when a PR's **current head** + is CI-green **and** the reviewer passed **that same head**, yshifu applies **`merge-ready`** — + a label that means only *"this head passed Codex review"* — and hands the PR to you, naming + anything you should weigh. **You merge.** `merge-ready` is **void the moment new commits + land**: GitHub keeps the label across a head change, so yshifu clears it, re-runs + `codex-review.sh` on the new head, and re-applies it only on a fresh pass — a stale label is a + false green. A later **status/Tracking scan and the brief only surface `merge-ready` PRs + (read-only)** — they never merge either. High-risk PRs are handed over **with the risk named** + even when CI-green and Codex-clean (auth, DB/schema migrations, shared/production repos, + security-sensitive or other operator-judgment changes); `merge-ready` records a clean review, + it never means "merge without looking." **Gate-creating bootstrap PRs get no `merge-ready` at + all** — an "add PR CI" PR or a greenfield 0→1 scaffold *creates* the gate, so no real gate yet + exists to certify it, and the new workflow can self-report green on its own PR; you approve and + merge those by hand. You're also brought in for `needs-human`/round-cap escalations, + safety-rail changes, and **north-star milestones / goal drift**. **`scripts/merge-pr.sh` stays + in the repo for your own use** — it reads the reviewed head+base SHAs from the authenticated + `codex-review.sh` marker and refuses if either moved, gates on the base branch's **required + status checks** (falling back to ≥1 real passing CI check with none failing when none are + defined — optional checks like preview deploys are informational), refuses a PR that still + needs an **approving review** (`reviewDecision=REVIEW_REQUIRED`), stays **scoped to the target + repo**, and merges with a **repo-permitted method** (squash if allowed) **pinned via + `--match-head-commit`**. **yshifu never runs it, on any PR.** - **One rounds counter (~3), and the cap is productive.** Comments resolved or disagreement burned both count; a single push-back doesn't escalate. At the ~3-round cap yshifu **scopes down + splits** rather than dead-ending: land the part the reviewer is satisfied with (one - scoped-down final change → clean review → merge the core) and **open a follow-up issue** for - the contested remainder (logged, not lost). `needs-human` is **reserved** for when even the + scoped-down final change → clean review → `merge-ready` → you merge the core) and **open a + follow-up issue** for the contested remainder (logged, not lost). `needs-human` is **reserved** for when even the scoped-down core is contested, it's a genuine coder↔reviewer standoff, or it's a safety-rail / north-star decision — only then does the cap reach you. The cap **count** is unchanged; only how it resolves. @@ -209,7 +212,7 @@ now wired too ([#112](../../issues/112)): yshifu's instructions describe a deleg policy — context-heavy reads and multi-step polling (watching CI to completion, PR-diff summaries, review-thread collection, bulk `gh` queries) go to a `YSTACK_HANDS_MODEL` subagent via the same config-resolution mechanism, passed as the spawn's `model` -parameter, while single quick writes (one comment, one label, one merge command) stay +parameter, while single quick writes (one comment, one label, one short handoff note) stay inline; hands agents must return key raw lines plus a summary, never a bare conclusion, so yshifu's decisions rest on evidence. This is a **prompt-level** wiring: it takes effect once `scripts/install.sh` regenerates the live `/yshifu` command, not merely by @@ -229,7 +232,7 @@ reviewer/manager-review.md Codex manager-reviewer mechanism (issue-as-bus): roun scripts/install.sh Generate the /yshifu command with a repo-derived path (idempotent) scripts/codex-review.sh Codex reviewer harness: post `codex exec review` to a PR, verbatim (stamps Reviewed-head: marker) scripts/manager-review.sh Codex manager-reviewer harness: debate a proposed issue vs. the north star, post the verdict to the issue verbatim -scripts/merge-pr.sh Safe in-session merge harness: SHA-pin to reviewed head + repo-scope + required-checks gate + review-required refuse, then merge (repo-permitted method) +scripts/merge-pr.sh Safe merge harness for the OPERATOR's own use (yshifu never runs it): SHA-pin to reviewed head + repo-scope + required-checks gate + review-required refuse, then merge (repo-permitted method) scripts/setup-target-repo.sh Bootstrap a target repo's loop labels (idempotent) scripts/lib/north-star.sh Resolver: returns the active target repo's committed .ystack/north-star.md (or root NORTH_STAR.md when ystack itself is the target) scripts/doctor.sh Read-only restore + readiness self-check (install, auth, restore-critical files, north star, model config, ...) @@ -247,13 +250,14 @@ RESTORE.md Disaster-recovery runbook: rebuild the team from this - **Phase 1** — prove the in-session loop on one seeded target repo. Front gate held the judgment; merge was manual while the loop earned trust. -- **Phase 2** — live: yshifu **auto-merges clean, low-risk PRs in-session** (CI green + - Codex clean, back-to-back with the review it just ran) under standing authorization — - escalating only `needs-human`/round-cap, safety-rail changes, and north-star milestones / - goal drift. Both the **brief** and a **status / Tracking pass** are **read-only — they - surface `merge-ready` PRs, they never merge** (those get merged on a fresh in-session - review, or by you). -- **Phase 3** — widen the auto-merge envelope as the loop proves out, including the - **unattended status-scan / cross-repo auto-merge** — a future extension of `merge-pr.sh` - **deferred to [#46](../../issues/46)** (the script's header notes it is not supported yet); - always back-look high-risk work (auth, migrations, shared repos). +- **Phase 2** — live: the loop runs end to end in-session, and **you merge at the gate**. + yshifu labels a PR **`merge-ready`** when its current head is CI-green and the reviewer + passed that same head, then hands the PR to you — naming the risk on high-risk work, and + escalating `needs-human`/round-cap, safety-rail changes, and north-star milestones / goal + drift. Both the **brief** and a **status / Tracking pass** are **read-only — they surface + `merge-ready` PRs, they never merge**. No agent merges: `main` needs a pull request plus an + approving review the comments-only reviewer cannot give, and no agent has a bypass. +- **Phase 3** — widen what the loop takes on as it proves out (the autonomous lane in the v2 + chain); always back-look high-risk work (auth, migrations, shared repos). **The merge gate + does not widen** — the operator merges, in every phase. There is no agent merge path, now + or planned. diff --git a/RESTORE.md b/RESTORE.md index 0c8ac48..2a51a3f 100644 --- a/RESTORE.md +++ b/RESTORE.md @@ -11,7 +11,7 @@ it as written. > **Parameterize, don't hardcode.** Wherever you see `/` (or > ``), substitute your own target repo(s). Per the reusability rule in -> [`CLAUDE.md`](CLAUDE.md), keep personal usernames and repo names out of the shipped +> [`AGENTS.md`](AGENTS.md), keep personal usernames and repo names out of the shipped > files — supply them here at restore time, not in the templates. --- @@ -35,7 +35,7 @@ Accounts and access you need before starting: (ystack is its own target repo; add others as you adopt the team elsewhere.) Read [`README.md`](README.md) once for the mental model (the team, the loop, the -design "why") and [`CLAUDE.md`](CLAUDE.md) for the conventions and safety rails before +design "why") and [`AGENTS.md`](AGENTS.md) for the conventions and safety rails before you rebuild. --- @@ -59,9 +59,9 @@ human channel. state and open issues. yshifu **never writes code or opens PRs** and **never approves on your behalf** — it opens -issues and orchestrates the loop. yshifu **does** merge clean, low-risk PRs (CI green + -Codex review passed) under your standing authorization, and brings you anything needing -human review (safety-rail changes, north-star / goal drift, high-risk back-look). The +issues and orchestrates the loop. yshifu **never merges either**: when a PR is CI-green and +Codex passed that head, it labels the PR `merge-ready` and hands it to you, naming the risk +when there is one (safety-rail changes, north-star / goal drift, high-risk back-look). The front gate is **your approval of the drafted spec** (for a user-directed issue, your one-liner is the request → yshifu drafts the spec → you approve that drafted spec — drafting alone never earns `ready`): once you approve the spec, yshifu applies the `ready` label as the @@ -136,8 +136,8 @@ That checklist covers: `scripts/setup-target-repo.sh --check /` — which reports per label `matches` / `differs` / `missing` and exits non-zero if anything is missing or differs. - **Branch protection on `main`** — require CI status checks to pass; keep GitHub's - **native auto-merge button off** (merges go through yshifu or the human, both gated on - green CI — not a server-side trigger). Caveat: that section of `repo-setup.md` is a **UI checkbox checklist with no + **native auto-merge button off** (merging is the operator's, gated on green CI and a + `merge-ready` label — never a server-side trigger, and never an agent). Caveat: that section of `repo-setup.md` is a **UI checkbox checklist with no command** (unlike the labels loop), and **branch protection isn't available on free private repos** — it needs a paid plan or a public repo. If you can't enable it, **CI is still the hard gate** (see Safety rails); you just lose the server-side enforcement. @@ -209,9 +209,10 @@ Run **one trivial issue** through the full loop end to end, all from your yshifu - **CI** runs on the PR and goes green. 5. If there's feedback, confirm **yshifu spawns a fix-mode coder** that pushes follow-up commits and bumps the `round-N` label, then re-runs `codex-review.sh`. -6. Confirm the merge path: for a clean, low-risk PR (CI green + Codex passed) **yshifu - merges** under your standing authorization; a PR needing human review (safety-rail / - north-star / high-risk) is brought to **you** instead. +6. Confirm the merge path: for a clean PR (CI green + Codex passed at that head) yshifu + applies **`merge-ready`** and hands it over — **you merge**. Nothing else has a merge + path: `main` requires a pull request and an approving review, the reviewer is + comments-only, and no agent has a bypass. If every step above fired, the team is back. If one stage is silent: re-check `/yshifu` is installed and points at this repo (step 1), the coder instruction files are present (step @@ -225,15 +226,17 @@ installed and points at this repo (step 1), the coder instruction files are pres ## 6. Safety rails that must survive any rebuild These are load-bearing — per the self-modification safety section of -[`CLAUDE.md`](CLAUDE.md), never weaken them without explicit human sign-off: +[`AGENTS.md`](AGENTS.md), never weaken them without explicit human sign-off: - **Reviewer stays read-only / comments-only.** Codex never pushes, approves-to-merge, or merges, and is never the author. -- **Merge stays gated, and human-review carve-outs survive.** yshifu may auto-merge a PR - only when it's **CI-green + Codex-clean + low-risk** (standing authorization). It must - **not** merge — it brings the PR to you — for safety-rail changes, ambiguous specs, - anything escalated (`needs-human`/round-cap), north-star milestones / goal drift, or - high-risk back-look (auth, migrations, shared repos). Codex never approves or merges. +- **Merging is yours, always.** yshifu labels a reviewed-clean head `merge-ready` and hands + the PR over; it never merges, and the label goes void the moment new commits land. The + in-session auto-merge v1 allowed was retired when the branch ruleset landed. Carve-outs + survive as handoff duties: safety-rail changes, ambiguous specs, anything escalated + (`needs-human`/round-cap), north-star milestones / goal drift, and high-risk back-look + (auth, migrations, shared repos) are named as such when handed to you. Codex never + approves or merges either. - **Rounds cap (~3) + `needs-human` escalation stay intact.** Because each coder spawn is stateless, this state lives in the **labels** (`round-0..3`, `needs-human`), not in agent memory — so the labels (step 4) are part of the safety system, not decoration. diff --git a/REVIEW.md b/REVIEW.md index 700943a..20e183f 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -15,7 +15,7 @@ Run three passes and tag each finding with its pass: - **Compliance**: the diff matches `work//plan.md` and `spec.md` (when the PR belongs to a chain); safety rails intact — reviewer stays comments-only, round cap and `needs-human` escalation intact, no-merge guards untouched, constitution - paths (`.github/**`, `.claude/**`, `CLAUDE.md`, `REVIEW.md`) changed only by the + paths (`.github/**`, `.claude/**`, `AGENTS.md`, `CLAUDE.md`, `REVIEW.md`) changed only by the operator or via `proposals/`. ## What Important means here @@ -28,7 +28,7 @@ nits. Report at most five nits per review; summarize the rest as a count. Jargon-heavy or hard-to-follow writing in artifacts and PR text is a nit — -plain language is a repo rule (CLAUDE.md > PR rules). +plain language is a repo rule (AGENTS.md > PR rules). ## Do not report diff --git a/ci/required-files.txt b/ci/required-files.txt index 4337a1a..f1d6f6f 100644 --- a/ci/required-files.txt +++ b/ci/required-files.txt @@ -11,6 +11,7 @@ # Top-level docs README.md CLAUDE.md +AGENTS.md QUICKSTART.md RESTORE.md NORTH_STAR.md diff --git a/manager/CLAUDE.md b/manager/CLAUDE.md index 7e9a6a1..b43fe95 100644 --- a/manager/CLAUDE.md +++ b/manager/CLAUDE.md @@ -227,15 +227,17 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses is repo/environment-dependent — run it once a repo exists — not source-dependent.) - **The bootstrap PR is operator-approved + human-merged.** No real gate exists yet for it to certify itself, so — as with the add-CI PR (and per #87's lesson) — **classify it as - human-merge-only** and do **NOT** run `merge-pr.sh` on it at all; the operator approves - and merges it by hand. This is a **deliberate withholding of auto-merge**, not a reliance - on the tooling refusing (a same-repo bootstrap workflow can self-report green on its own - PR). Cross-vendor Codex review **still applies** pre-CI. + human-merge-only**: hand it to the operator to approve and merge by hand, and do **NOT** + apply `merge-ready` to it. That label says a real gate passed at this head, and here there + is none: a same-repo bootstrap workflow can self-report green on its own PR, so a green + check proves nothing. Say that plainly when you hand it over. Cross-vendor Codex review + **still applies** pre-CI. - **Handoff to 1→N — preserves the front gate; bootstrap-plan approval ≠ north-star approval.** Once the skeleton + CI + first test land (a **real gate now exists**), transition to the **normal loop** under the standing rails — the same rails that govern - any existing-project target, including the normal auto-merge policy now that a gate is - real. But the handoff **does not** unlock open-ended proactive autonomy on its own: the + any existing-project target, including the normal review → `merge-ready` → operator-merge + handoff now that a gate is real. But the handoff **does not** unlock open-ended proactive + autonomy on its own: the operator approved the **bootstrap scaffold plan (scoped to the 0→1 PR)**, which is **NOT** approval of the active north star for proactive 1→N work. So after the bootstrap lands, apply the standing front gate exactly as any target does: **pursue *proactive* north-star @@ -313,18 +315,17 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses coder **auto-discovers** from the repo's manifests (there is no CI config to read from yet). This offer is the operator's gate — you propose, they decide; you do not bootstrap silently. - **Bootstrapping the gate is human-gated — state it as a rail.** The "add CI" PR is - **always brought to the operator to approve and merge — never auto-merged**, regardless of - clean review / low-risk. **Classify it as human-merge-only** — the same category as - safety-rail / high-risk PRs — so you **do NOT run `merge-pr.sh` on it at all**, no matter - whether a check appears; the **operator approves and merges it by hand**. The human-merge is - *your deliberate withholding of auto-merge*, **not** a reliance on the tooling refusing — - do not assume the absence of checks blocks it. (In fact a same-repo bootstrap workflow can - **self-report green on its own PR** — the added `pull_request` workflow runs on the PR that - adds it and can produce a passing check `merge-pr.sh` might accept — which is exactly why - the human, not the tooling, is the gate here.) This is the **one sanctioned - human-merge-without-a-pre-existing-gate case**, precisely because it *creates* the gate + **always brought to the operator to approve and merge**, regardless of clean review / + low-risk. Every PR goes to the operator, but this one carries an extra caution: + **classify it as human-merge-only** — the same category as safety-rail / high-risk PRs — + and **do NOT apply `merge-ready` to it**, no matter what checks appear. `merge-ready` says + a real gate passed at this head, and here there is no real gate yet: a same-repo bootstrap + workflow can **self-report green on its own PR** — the added `pull_request` workflow runs + on the PR that adds it — so a green check proves nothing. Say that plainly when you hand + the PR over, so the operator judges it rather than trusting a check. This is the **one + sanctioned merge-with-no-pre-existing-gate case**, precisely because it *creates* the gate (operator + Codex review are the gate for *establishing* the gate). After it lands, CI - exists and the normal loop (including the normal auto-merge policy) applies. + exists and the normal loop applies. - **Surface what the gate actually covers — don't overstate it.** A bootstrapped gate is only as strong as the project's tests: it runs whatever exists (tests if present; otherwise lint / build only). Tell the operator **what the bootstrapped CI checks** so a weak gate @@ -332,8 +333,9 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses out of scope here — a lint-only gate is acceptable to start.) - This is a **capability plus a single human-gated bootstrapping exception**, not a relaxation of the merge gate: every other rail holds (reviewer stays comments-only, the - rounds cap and `needs-human` stand, normal PRs still merge only SHA-pinned + CI-green via - `merge-pr.sh`, and the front gates — spec approval / consensus — are unchanged). + rounds cap and `needs-human` stand, normal PRs still reach the operator only as a CI-green, + review-clean head labeled `merge-ready`, and the front gates — spec approval / consensus — + are unchanged). - **Project-understanding pass (first contact on a non-empty target, once per `/yshifu` session).** Before you draft your **first work on this target this session** — *whether it is user-directed* (to ground the spec you draft from the operator's one-liner) *or proactive* — @@ -371,7 +373,7 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses of scope here). It re-runs cheaply next session. 5. **Preserve every rail.** This adds a **read-only comprehension + grounding** behavior — it does not touch any gate: reviewer stays comments-only, CI stays the hard merge gate, - merges stay SHA-pinned + CI-green, the rounds cap and `needs-human` stand, and the front + merging stays the operator's, the rounds cap and `needs-human` stand, and the front gates (spec approval / consensus) are unchanged. Grounding a spec or a brief in the survey never substitutes for a gate. - **Run the loop in-session.** You drive the whole loop from this chat — there is exactly @@ -401,28 +403,26 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses within the target repo's clone. It posts Codex's review to the PR verbatim. 3. Read the review and decide **pass / not-pass** conservatively: - **Pass** only when nothing beyond optional / nit-level remains. Apply **`merge-ready`** - to the PR — it means **"the CURRENT head SHA passed Codex review."** You **MAY auto-merge - a PR you reviewed in-session** when it is CI-green, Codex-clean, and low-risk: **run - `"/scripts/merge-pr.sh" ` from within the target repo's clone** (same - absolute-path convention as `codex-review.sh`). Do **not** hand-craft a merge command — - `merge-pr.sh` owns the mechanical safety: it reads the reviewed head+base SHAs from the - authenticated `codex-review.sh` marker, confirms the PR's current head AND base still - match (refusing if either moved since the review), requires ≥1 real passing CI check, and - merges pinned via `--match-head-commit`, refusing otherwise. It is scoped to the target - repo (never another repo). This in-session review→merge is acting on the passed review, - **not** self-approval (Codex is comments-only and never approves). High-risk PRs (auth, - migrations, shared/production repos, security-sensitive) always go to the human merge - gate — the last word on merging; you do **not** run `merge-pr.sh` for those. **Gate-creating + to the PR once that same head is also CI-green — the label means **"the CURRENT head SHA + passed Codex review"** — then **hand the PR to the operator, who merges it.** You + never merge: see "Merge & never" below. `scripts/merge-pr.sh` stays in the repo for the + operator's own use — **you do not run it**, on any PR. High-risk PRs (auth, migrations, + shared/production repos, security-sensitive) are handed over with the risk **named** — + `merge-ready` records a clean review, it never means "merge without looking." **Gate-creating bootstrap PRs — a CI-bootstrap ("add PR CI") PR or a greenfield-bootstrap (0→1 scaffold) PR — - are also human-merge-only: do NOT run `merge-pr.sh` on them at all** (each *establishes* the - gate, so no real gate yet exists to certify it, and the added workflow can self-report green - on its own PR); the operator approves + merges by hand. See the auto-merge policy below. + get NO `merge-ready` at all** (each *establishes* the gate, so no real gate yet exists to + certify it, and the added workflow can self-report green on its own PR); hand those over as + human-judgment-only and the operator approves + merges by hand. - **`merge-ready` is void the moment new commits land.** GitHub keeps the label across a head change, but a new push (a fix round, or any contributor commit) means the reviewed - head is stale. Whenever a PR's head changes, **clear `merge-ready`**; it is only - (re)applied after a passing Codex review of the *new* current head. Never merge on a - `merge-ready` label whose review predates the current head — re-run `codex-review.sh` - on the new head first. + head is stale. Whenever a PR's head **or its base** changes, **clear `merge-ready`**; + it is only (re)applied after a passing Codex review of the *new* head against the + *current* base. The base matters as much as the head: when `main` moves the head SHA + stays the same, but the diff the reviewer read no longer exists — the retired merge + harness compared both `Reviewed-head` and `Reviewed-base` for exactly this reason. + Never leave the label standing when its review predates either — the operator merges + on the strength of that label, so a stale one is a false green. Re-run + `codex-review.sh` first. - **Not-pass is a "bounce" — diagnose before you respawn; this replaces any notion of model escalation.** A bounced round is never "try again with a bigger model" — diagnose which exit applies and take exactly one: @@ -433,9 +433,10 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses diagnosis + `routines/coder-revision.md`) **at the SAME tier** — never escalated — then re-run `codex-review.sh`. The coder bumps the `round-N` label each round. b. **Scope too big / genuinely hard** — decompose rather than push a struggling - coder harder: **file AND link the follow-up issue BEFORE merging the partial - PR**, then land the **independently-green mergeable core** (must pass CI + - review on its own and leave the repo coherent, docs in sync). The follow-up + coder harder: **file AND link the follow-up issue BEFORE the partial PR goes to + the operator**, then finish the **independently-green mergeable core** (must pass + CI + review on its own and leave the repo coherent, docs in sync) and hand that + core over for the operator's merge. The follow-up inherits the parent issue's approval only as a **strict subset** of the approved scope — anything beyond that subset goes through the normal front gate (spec approval or manager-debate) on its own. **Guard against scope-creep dressed as @@ -455,15 +456,16 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses this scope down to the part the reviewer is satisfied with, with the contested remainder split into a follow-up issue?"** - **Yes (the usual case)** → **file AND link the follow-up issue for the deferred / - contested remainder BEFORE merging anything** (log it, so the dropped scope is tracked, + contested remainder BEFORE anything is handed over** (log it, so the dropped scope is tracked, not lost — it inherits the parent issue's approval only as a strict subset of the approved scope; anything beyond that subset needs its own front-gate pass; the follow-up must meet the same link + quoted-scope + subset-statement requirements as exit (b) above). Then **direct one scoped-down final change** (the fix-mode coder lands just the agreed **independently-green mergeable core**, dropping the contested part), re-run - `codex-review.sh` for a **clean review of that scoped head**, then **merge the core** - (CI-green + Codex-clean + low-risk, per the auto-merge policy). The cap resolves by - *shipping the converged core and deferring the rest* — not endless rounds, not a stall. + `codex-review.sh` for a **clean review of that scoped head**, then **label that core + `merge-ready` once it is CI-green and hand it to the operator to merge** — the same + handoff as any passing PR. The cap resolves by *shipping the converged core and + deferring the rest* — not endless rounds, not a stall. - **No** → only then apply **`needs-human`** with a SHORT reason in the escalation comment (e.g. `round-cap` / `ambiguous-spec` / `oversized` / `failure`) and bring it to me. Reserve `needs-human` for when **even the scoped-down core is contested**, it's a genuine @@ -482,10 +484,10 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses completion and summarize failures, fetch and summarize a PR diff, collect a PR's review threads, bulk `gh` queries (a cross-repo status sweep, a label scan). - **Keep inline (no subagent):** single quick writes — posting one comment, one label - operation, one merge command. The content is your own reasoning, already formed; + operation, one short handoff note. The content is your own reasoning, already formed; spinning up a subagent for it would cost more than just making the call yourself. - **Hands agents are read-only.** Every write / side-effect — posting a comment, - applying a label, merging, pushing — stays **your own inline call**, regardless of + applying a label — stays **your own inline call**, regardless of size or how mechanical it looks. A hands subagent may read, fetch, and summarize evidence; it never performs the action itself. - **Evidence, not conclusions — a safety property.** A hands agent must return the @@ -494,15 +496,15 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses from a subagent whose work you can't audit after the fact. - **Merge-gate verdicts are exempt from this delegation — a hard carve-out.** For the Codex review pass/not-pass judgment that drives `merge-ready` (and any CI-conclusion - feeding a merge decision), a hands agent may **fetch** the review or the check + feeding that label), a hands agent may **fetch** the review or the check result, but the **pass-vs-not-pass judgment must be made by you**, over the **complete, verbatim** review text and the actual check conclusions — never over a hands-authored digest, summary, or conclusion. This holds even though "collect a PR's review threads" is listed above as delegable: delegate the fetch, never the verdict. A curated digest could omit a buried blocking finding — by mistake, or via prompt-injection from attacker-authored PR comments in the threads being read — and - `merge-pr.sh` verifies CI/SHA/marker but **not** review content, so this leg rests - entirely on your own reading. + the operator merges on the strength of `merge-ready`, with no tooling checking review + content behind you, so this leg rests entirely on your own reading. - **Rule of thumb.** Delegate when (tokens the action would add to your context) × (expected remaining turns this session) exceeds the cost of spawning a hands subagent — a read early in a long session is worth delegating even if small; the @@ -519,18 +521,14 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses Once you act on a `needs-human` item, it is cleared — the brief must not re-surface it. - **Tracking.** When I ask "status" / "what's stalled", query GitHub across my repos by **label** (the labels are the state) and report, action-first. This status/Tracking pass is - **read-only — it REPORTS, it does not merge.** Auto-merge happens only **in-session**, when - you review a PR back-to-back and merge the head you just reviewed via `scripts/merge-pr.sh` - (see the pass step above); a later status scan never picks up and merges a `merge-ready` PR. - (The unattended status-scan / cross-repo auto-merge — a daemon scanning many repos' PRs and - merging without a yshifu session — is a **future extension of `merge-pr.sh` deferred to #46**; - `merge-pr.sh`'s header notes it is not supported yet.) - - PRs labeled `merge-ready`: **surface them** — note the low-risk ones as `merge-ready` and - awaiting either an in-session review→merge or my merge (CI may have gone green after the - loop ended). `merge-ready` means the reviewed head passed, so if a PR's head changed since - the label was applied, flag it as **stale — needs a fresh Codex review of the current - head**, not merge-ready. Then list any held for me — high-risk (auth / migrations / - shared repos / security-sensitive), safety-rail, or north-star — as waiting on my merge gate + **read-only — it REPORTS, it does not merge.** No pass of yours merges, in session or out + (see "Merge & never"); a status scan surfaces a `merge-ready` PR, it never acts on one. + - PRs labeled `merge-ready`: **surface them** — they are reviewed clean at that head and + waiting on my merge (CI may have gone green after the loop ended). `merge-ready` means the + reviewed head passed, so if a PR's head changed since the label was applied, flag it as + **stale — needs a fresh Codex review of the current head**, not merge-ready. Call out the + ones that need my judgment on top of the review — high-risk (auth / migrations / + shared repos / security-sensitive), safety-rail, or north-star - anything labeled `needs-human` (the escalation comment's short reason says which: `round-cap` / `ambiguous-spec` / `oversized` / `failure`). Skip any I've already resolved — once acted on, `needs-human` is cleared, so it must not be re-reported. @@ -544,41 +542,14 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses ## Merge & never -- **Merge clean PRs in-session (auto-merge policy).** Per my standing authorization, you - **MAY auto-merge a PR you reviewed in-session** when it is CI-green, Codex-clean, and - low-risk — by running **`"/scripts/merge-pr.sh" ` from within the target - repo's clone** (same absolute-path convention as `codex-review.sh`). No per-PR confirmation - needed. **Let the script own the mechanics — never hand-craft a merge command:** `merge-pr.sh` - reads the reviewed head+base SHAs from the authenticated `codex-review.sh` marker, confirms - the PR's current head AND base still match those (refusing if either moved since the review), - requires ≥1 real passing CI check, and merges pinned via `--match-head-commit`, refusing - otherwise — so the SHA-pin and race guards are enforced mechanically, not by you. It is - scoped to the target repo (never another repo). This is acting on the passed review, not - self-approval (Codex is comments-only and never approves). A `merge-ready` label only counts - if it reflects the current head: if commits landed since the review, the label is void — clear - it and re-run `codex-review.sh` on the new head before merging (`merge-pr.sh` will itself - refuse a moved head). A later **status/Tracking scan never auto-merges** — it only - surfaces `merge-ready` PRs (read-only); those get merged on a fresh in-session review, or by - me. **High-risk PRs always go to the human merge gate** even when CI-green + Codex-clean: - auth, DB/schema migrations, shared/production repos, other security-sensitive changes, or - anything else that warrants operator judgment / a back-look — for those you do **not** run - `merge-pr.sh`. You also do **not** merge when human review is required for other reasons: - safety-rail changes, ambiguous specs, anything escalated (`needs-human`/round-cap), or - north-star milestones / goal drift — those come to me. **A CI-bootstrap ("add PR CI") PR is - also never auto-merged** — you classify it as human-merge-only and do **not** run `merge-pr.sh` - on it at all; it is always brought to me to approve and merge by hand, because it *creates* the - gate CI can't yet certify. Do not rely on the tooling refusing it — a same-repo bootstrap - workflow can even self-report green on its own PR; the human is the gate. See the first-contact - CI bootstrap above. **A greenfield-bootstrap (0→1 scaffold) PR is likewise never auto-merged** — - same treatment as the CI-bootstrap PR: classify it human-merge-only, do **not** run `merge-pr.sh` - on it at all, and always bring it to me to approve and merge by hand, because it too *establishes* - the gate (it adds the first `pull_request` CI workflow) and so no real gate yet exists for it to - certify itself. The same self-report caveat applies — the newly-added workflow can go green on its - own PR — so the human, not the tooling, is the gate. See the greenfield bootstrap (3b) above. - This high-risk carve-out is the last word on merging: when in doubt about - risk, hand it to me. **The unattended status-scan / - cross-repo auto-merge remains a future extension of `merge-pr.sh`, deferred to #46 — - `merge-pr.sh`'s header notes it is not supported yet.** +- **You never merge. The operator does.** The in-session auto-merge v1 allowed was + retired when the branch ruleset landed: the rules require an approving review that + a comments-only reviewer cannot give, and no agent has a bypass. When a PR is + CI-green and the reviewer has passed the current head, apply **`merge-ready`** and + hand it to the operator — that label means "reviewed clean at this head", nothing + more, and it is void the moment new commits land (clear it, re-review the new head, + re-apply only on a pass). `scripts/merge-pr.sh` remains in the repo for the + operator's own use; you do not run it. - **Never write code or open PRs yourself.** You create issues, not diffs. - **Never self-approve — yshifu alone can't; yshifu + Codex consensus can.** You acting *alone* never applies `ready`: a **user-directed** issue gets `ready` only as the record of @@ -588,7 +559,8 @@ a lower/non-frontier tier, **warn me once and continue** — don't block the ses by itself — is what gates proactive north-star work; my own per-issue approval moved up to the north star. (Codex is comments-only and never approves a *diff*; on the manager-debate it is veto-only and gives a verdict you weigh — consensus, not a Codex rubber-stamp, is the - gate. Merging a clean PR is acting on the passed code review, not approving it yourself.) + gate. `merge-ready` records that a review passed at this head; it is not an approval, and it + is not a merge.) - Be brief: lead with the answer, no essays. ## Notes diff --git a/proposals/README.md b/proposals/README.md index 5e4abb1..ac4324c 100644 --- a/proposals/README.md +++ b/proposals/README.md @@ -1,6 +1,6 @@ # proposals/ -Patches to the constitution paths — `.github/**`, `.claude/**`, `CLAUDE.md`, +Patches to the constitution paths — `.github/**`, `.claude/**`, `AGENTS.md`, `CLAUDE.md`, `REVIEW.md` — which unattended agents may not write directly. An autonomous-lane agent that wants to change how the machinery itself works saves diff --git a/reviewer/codex-review.md b/reviewer/codex-review.md index 939295f..0e9b8ab 100644 --- a/reviewer/codex-review.md +++ b/reviewer/codex-review.md @@ -86,8 +86,11 @@ every `gh` call, so a `GH_REPO` in the environment can't redirect the comment to **`reviewer: @ `** line recording the RESOLVED config that gated this review (see **model policy** below). The marker and reviewer lines are part of yshifu's header prefix, clearly separate from Codex's verbatim body, so the review stays - read-only / comments-only / verbatim. [`scripts/merge-pr.sh`](../scripts/merge-pr.sh) is - the first consumer: it reads the `Reviewed-head`/`Reviewed-base` markers, confirms the PR's + read-only / comments-only / verbatim. The markers tie the review to one exact commit, and + two readers use them. yshifu checks them before it applies `merge-ready` and hands the PR to + the operator — a head that has moved since the review voids the label. And + [`scripts/merge-pr.sh`](../scripts/merge-pr.sh) — the **operator's own** merge helper, which + no agent runs — reads the same `Reviewed-head`/`Reviewed-base` markers, confirms the PR's current head still equals it and that CI is green, then squash-merges pinned to that SHA (`--match-head-commit`). @@ -167,7 +170,8 @@ The script FAILS LOUDLY on a degraded/non-substantive Codex run instead of posti "clean" verdict. Real incident (2026-07-11): `codex-code-mode-host` failed to spawn (missing from a Homebrew codex install); `codex exec review` still "completed" — exit 0, in ~8-14s, at confidence ~0.05, with a generic "no actionable findings" — having done **zero** diff -inspection. Under the standing auto-merge rail, a fake "clean" would auto-merge unreviewed code. +inspection. A fake "clean" is how unreviewed code gets labeled `merge-ready` and handed to the +operator as if a reviewer had passed it. **Detection uses a structured boundary — never untyped model/tool content.** Normal `codex exec -o` writes the final answer to the requested file **and** repeats it on stdout, so @@ -199,14 +203,15 @@ is also refused, rather than posting a header-only comment with no findings. On detection: the script exits non-zero and posts an explicit DEGRADED marker comment instead — `## Codex reviewer — DEGRADED, REVIEW DID NOT RUN (cross-vendor, read-only)`, deliberately a **different** header line than the real `## Codex reviewer (cross-vendor, read-only)` one, with -NO `Reviewed-head`/`Reviewed-base` markers. That means `scripts/merge-pr.sh`'s marker parser -(which matches that exact header line plus those exact marker keys) can never mistake a -degraded run for a completed review — belt-and-suspenders on top of yshifu reading the comment -text. +NO `Reviewed-head`/`Reviewed-base` markers. That means the marker parser in the operator's +`scripts/merge-pr.sh` (which matches that exact header line plus those exact marker keys) can +never mistake a degraded run for a completed review — belt-and-suspenders on top of yshifu +reading the comment text before it labels anything `merge-ready`. **The DEGRADED comment never embeds codex's raw output verbatim (#119 P2 integrity fix).** The -DEGRADED comment is posted by, and so is authored as, the same gh-authenticated operator -`scripts/merge-pr.sh` trusts — so it is exactly the kind of comment that parser's author+header +DEGRADED comment is posted by, and so is authored as, the same gh-authenticated operator the +operator's own `scripts/merge-pr.sh` trusts — so it is exactly the kind of comment that +parser's author+header match would accept. codex's diagnostic output is untrusted (a prompt-injected PR could make it emit lines identical to the real `## Codex reviewer (cross-vendor, read-only)` header plus `Reviewed-head:`/`Reviewed-base:` markers), and the `-o` answer is doubly untrustworthy on a @@ -246,21 +251,24 @@ yshifu runs codex-review.sh (by absolute path, from the target repo's clo (script posts Codex's verdict to the PR, verbatim) ↓ yshifu reads the Codex comment - ├── pass → yshifu merges if low-risk (CI green); else hands to the human + ├── pass → yshifu labels the PR `merge-ready` (CI green + this exact head + │ passed) and hands it to the operator, who merges └── not pass → yshifu spawns coder (fix mode) to address comments ↓ yshifu re-runs codex-review.sh (bump round-N) ↺ repeat └── ~3-round cap → SCOPE DOWN + FOLLOW-UP (productive): land the converged core (one scoped-down change → - clean review → merge) + open a follow-up issue for the - contested remainder; reserve needs-human → yshifu pings you - for a genuine standoff / safety-rail / north-star + clean review → `merge-ready` → the operator merges) + + open a follow-up issue for the contested remainder; + reserve needs-human → yshifu pings you for a genuine + standoff / safety-rail / north-star ``` yshifu, not the reviewer, drives each step; Claude and Codex never talk directly — the **PR is the message bus**. Rounds + escalation live in the **labels** -(`round-0..3`, `needs-human`), not in any agent's memory. +(`round-0..3`, `merge-ready`, `needs-human`), not in any agent's memory. **No agent merges** — +the loop ends at the handoff, and the operator merges. ## Future / alternatives (not wired) diff --git a/reviewer/manager-review.md b/reviewer/manager-review.md index 46125aa..b80d700 100644 --- a/reviewer/manager-review.md +++ b/reviewer/manager-review.md @@ -323,11 +323,11 @@ never read this as a `PROCEED`. as `codex-review.sh`).** It never embeds the `-o` verdict answer (untrustworthy on a degraded run), **never embeds JSONL** (it contains private agent/command/repository payloads), and embeds only a bounded, sanitized raw-stderr tail via `cd_sanitize_snippet` — every line prefixed `> `, -which breaks the line anchors a marker parser like `scripts/merge-pr.sh`'s would require. This -comment is posted by, and authored as, the same gh-authenticated operator, so it must never be -able to carry an unneutralized marker-shaped line even though `merge-pr.sh` only reads PR -comments today — defense-in-depth against codex's diagnostic output being adversarially -influenced by the issue/repo content it read. +which breaks the line anchors a marker parser like the one in the operator's +`scripts/merge-pr.sh` would require. This comment is posted by, and authored as, the same +gh-authenticated operator, so it must never be able to carry an unneutralized marker-shaped +line even though that script only reads PR comments today — defense-in-depth against codex's +diagnostic output being adversarially influenced by the issue/repo content it read. ## The manager-reviewer prompt diff --git a/routines/brief.md b/routines/brief.md index 0b13fbc..9d13743 100644 --- a/routines/brief.md +++ b/routines/brief.md @@ -9,14 +9,13 @@ Scan my repos by LABEL (the labels are the state) and send me ONE short message, action-first: - PRs labeled `merge-ready` whose CI is now green (with links): this report is read-only — - it surfaces state, it does not merge. Report any low-risk ones as **awaiting merge** — they - get merged when yshifu reviews them **in-session** (auto-merge happens back-to-back with a - review, not on a scan), or when I merge. (A status/Tracking pass is read-only too; neither - the brief nor a scan auto-merges — that unattended path is deferred to #46.) `merge-ready` - means the reviewed head passed, so if a PR's head changed after the label was applied, flag - it as **stale — needs a fresh Codex review of the current head**. Call out the ones held - for me — high-risk (auth / migrations / shared repos / security-sensitive), safety-rail, or - north-star — as waiting on my merge gate + it surfaces state, it does not merge. Nothing else merges either: yshifu applies + `merge-ready` once CI is green and the reviewer passed that head, then hands the PR to me. + **I merge, always.** So report every one of these as **waiting on my merge**. + `merge-ready` means the reviewed head passed, so if a PR's head changed after the label was + applied, flag it as **stale — needs a fresh Codex review of the current head**. Call out + the ones that need my judgment on top of the review — high-risk (auth / migrations / + shared repos / security-sensitive), safety-rail, or north-star - Items labeled `needs-human` — the escalation comment's short reason says which (`round-cap` / `ambiguous-spec` / `oversized` / `failure`); say which and why. Skip anything I've already acted on: once resolved, `needs-human` is cleared, so diff --git a/routines/coder-revision.md b/routines/coder-revision.md index 7d68c2a..6d17d3d 100644 --- a/routines/coder-revision.md +++ b/routines/coder-revision.md @@ -21,8 +21,8 @@ yshifu has briefed you with the PR, the latest review comments, and the current commands, or if no PR-triggered CI is detectable), make exactly that change, verify locally, then push the green result (step 5) so the scoped core lands on - the branch for re-review and merge, then SKIP step 6's round bump — the PR stays at - `round-3`, do NOT add a `round-4` (no such label exists) — then post the summary comment + the branch for re-review and the operator's merge, then SKIP step 6's round bump — the PR + stays at `round-3`, do NOT add a `round-4` (no such label exists) — then post the summary comment (step 7) and stop (step 8). Otherwise (no scoped-down direction) add label `needs-human` and stop. 3. DISCOVER THE COMMANDS (do this **before** you modify or push anything): you are in the @@ -57,12 +57,13 @@ yshifu has briefed you with the PR, the latest review comments, and the current skeleton / manifest / first test / workflow on the existing branch). This is **narrow + sole-purpose** — only the greenfield-bootstrap PR; any other/feature work still stops here. **Pragmatics:** complex-matrix / secrets-or-services CI → run the runnable **core** locally (install + lint/build/unit) and rely on the PR's CI for the - rest; Install first; the PR's own CI is the ultimate gate (yshifu enforces at merge). + rest; Install first; the PR's own CI is the ultimate gate (yshifu won't hand a PR to the + operator until CI is green). 3.5. GATE — PR-TRIGGERED CI MUST EXIST (a **separate precondition** from step 3's command discovery, also run before you modify or push anything). Step 3 answers *which commands to run*; this gate answers *whether the target has the hard merge gate at all*. - Confirm the target repo has **CI that runs on pull requests** — the hard merge gate - `merge-pr.sh` enforces — detectable via **ANY** of: a GitHub Actions workflow + Confirm the target repo has **CI that runs on pull requests** — the hard merge gate — + detectable via **ANY** of: a GitHub Actions workflow (`.github/workflows/*.yml` / `*.yaml`) triggered on `pull_request`; an external CI provider's config (`.circleci/config.yml`, `.buildkite/*`, `Jenkinsfile`, `.gitlab-ci.yml`, `azure-pipelines.yml`, `.travis.yml`, etc.) wired to run on PRs; **or** recent PR @@ -74,7 +75,8 @@ yshifu has briefed you with the PR, the latest review comments, and the current the SHORT reason `failure`) — "no PR-triggered CI detected; CI is the hard merge gate, so a PR here can't be merged" — add label `needs-human`, and stop before editing or pushing. (Push-only CI does not satisfy - this gate — a PR gets no checks, so `merge-pr.sh` refuses.) **SOLE-PURPOSE ADD-CI + this gate — a PR gets no checks, so nothing can certify the PR and it never becomes + mergeable.) **SOLE-PURPOSE ADD-CI EXCEPTION** (mirrors `coder.md`): when the PR's **only** purpose is to **add PR-triggered CI** — it is *establishing* the gate — you MAY proceed despite no PR-CI existing yet, folding review feedback into the `pull_request` workflow you scaffold from the discovered @@ -114,9 +116,9 @@ yshifu has briefed you with the PR, the latest review comments, and the current lint with `shellcheck -x -S style` over `find . -name '*.sh' -not -path './.git/*'` using 0.11.0, grabbing that static release if your local version differs. Local green is necessary but not sufficient — the PR's own CI is the ultimate gate, - but you don't wait on it: **yshifu enforces PR CI at merge** (`merge-pr.sh` refuses - unless CI is green). Your job is the local green, then the push — then continue with - steps 6–7 below. + but you don't wait on it: **yshifu checks PR CI before it hands the PR to the operator** + (no `merge-ready` label until CI is green). Your job is the local green, then the push — + then continue with steps 6–7 below. 6. Bump the round label: remove `round-N`, add `round-(N+1)`. 7. Post a brief summary comment: what you changed vs. what you pushed back on. 8. Do NOT merge. Stop. diff --git a/routines/coder.md b/routines/coder.md index d0d6cc0..8c36d53 100644 --- a/routines/coder.md +++ b/routines/coder.md @@ -65,12 +65,13 @@ manager-debate consensus toward a user-approved north star (a proactive issue). not available locally, run the runnable **core** locally (install + lint/build/unit tests) and rely on the PR's CI for the rest — don't try to perfectly replicate CI, and don't block on un-runnable steps. Run the **Install** command first; the PR's - own CI remains the ultimate gate (yshifu enforces it at merge). + own CI remains the ultimate gate (yshifu won't hand a PR to the operator until CI is + green). 4. GATE — PR-TRIGGERED CI MUST EXIST (a **separate precondition** from step 3's command discovery; also a pre-work gate, so a failed gate leaves no dirty clone). Step 3 answers *which commands to run*; this gate answers *whether the target even has the hard merge gate*. Confirm the target repo actually has **CI that runs on pull requests** — the hard - merge gate `merge-pr.sh` enforces — detectable via **ANY** of: a GitHub Actions workflow + merge gate — detectable via **ANY** of: a GitHub Actions workflow (`.github/workflows/*.yml` / `*.yaml`) triggered on `pull_request`; an external CI provider's config (`.circleci/config.yml`, `.buildkite/*`, `Jenkinsfile`, `.gitlab-ci.yml`, `azure-pipelines.yml`, `.travis.yml`, etc.) wired to run on PRs; **or** recent PR @@ -84,7 +85,8 @@ manager-debate consensus toward a user-approved north star (a proactive issue). with the SHORT reason `failure`) — "no PR-triggered CI detected; CI is the hard merge gate, so a PR here can't be merged" — add label `needs-human`, and stop **before creating a branch or making any edit**. - (Push-only CI does not satisfy this gate — a PR gets no checks, so `merge-pr.sh` refuses.) + (Push-only CI does not satisfy this gate — a PR gets no checks, so nothing can certify + the PR and it never becomes mergeable.) - **SOLE-PURPOSE ADD-CI EXCEPTION.** The gate above stays for all feature work, but there is one exception: when the issue's **only** purpose is to **add PR-triggered CI** (it is *establishing* the gate that doesn't exist yet), you MAY proceed and open the PR despite @@ -97,9 +99,10 @@ manager-debate consensus toward a user-approved north star (a proactive issue). builds is acceptable. This exception is **narrow**: it applies solely to an issue whose one concern is adding PR CI; any feature issue on a CI-less repo still escalates and stops per the gate above. (yshifu's CI-bootstrap offer happens at first contact *before* feature - issues, so once the CI PR lands the normal gate is satisfied for later work. Note this - "add CI" PR is operator-approved and human-merged, not auto-merged — but that is yshifu's - concern; your job is only to open the green PR and stop.) + issues, so once the CI PR lands the normal gate is satisfied for later work. Note the + operator approves and merges this "add CI" PR by hand, and yshifu does not label it + `merge-ready` at all — but that is yshifu's concern; your job is only to open the green + PR and stop.) - **GREENFIELD-BOOTSTRAP EXCEPTION.** The greenfield analogue of the add-CI exception, extended to a **full first scaffold**: when yshifu briefs you with a **designated greenfield-bootstrap issue** — the **first change on an empty target** (no source yet, no @@ -117,7 +120,7 @@ manager-debate consensus toward a user-approved north star (a proactive issue). first test, and author a `pull_request`-triggered workflow that installs and runs the lint / build / test for that skeleton; run those same commands locally (step 9) so the workflow you author is green. **Also create + commit `/.ystack/north-star.md` with the - yshifu-provided north star:** post-98a the merge / manager-debate gate reads the target's + yshifu-provided north star:** post-98a the manager-debate gate reads the target's **committed** `.ystack/north-star.md` and FAILs on missing / `ystack-shipped-default`-marker / no-`status: active`-entry, so the 0→1 bootstrap must leave a real committed one. **yshifu's brief gives you the exact north-star text + done-signal** (drafted from the operator's command @@ -156,9 +159,9 @@ manager-debate consensus toward a user-approved north star (a proactive issue). (the lint / build / test commands you discovered in step 3), **locally**. Where the repo has a test suite, add or adjust tests to cover the change. Never open a PR with red CI. Local green is **necessary but not sufficient** — the PR's own CI is - the ultimate gate, but you don't wait on it: **yshifu enforces PR CI at merge** - (`merge-pr.sh` refuses unless CI is green). Your job is the local green, then - open the PR and stop. + the ultimate gate, but you don't wait on it: **yshifu checks PR CI before it hands the PR + to the operator** (no `merge-ready` label until CI is green). Your job is the local green, + then open the PR and stop. - **MATCH CI's PINNED TOOL VERSIONS.** When CI pins a linter/formatter/toolchain to a specific version, run **that exact version** locally — not whatever your local install happens to be. Different versions of the same tool report different findings diff --git a/scripts/setup-target-repo.sh b/scripts/setup-target-repo.sh index d9a7727..9a105d8 100755 --- a/scripts/setup-target-repo.sh +++ b/scripts/setup-target-repo.sh @@ -105,7 +105,7 @@ labels=( "round-2|4a90d9|Review-loop counter: revision 2" "round-3|1f6fc0|Review-loop counter: revision 3 (cap)" "needs-human|d93f0b|Escalation: round cap hit, ambiguous spec, oversized PR, or failure" - "merge-ready|5319e7|Current head passed Codex review; auto-merged in-session if low-risk, else awaiting your merge" + "merge-ready|5319e7|This head passed review; waiting on YOUR merge (no agent merges; void once new commits land)" "stale|e4a11b|v2 chain: artifact built from an outdated upstream (frontmatter hash mismatch); do not act on it" ) diff --git a/templates/repo-setup.md b/templates/repo-setup.md index 214be77..9fef99a 100644 --- a/templates/repo-setup.md +++ b/templates/repo-setup.md @@ -15,7 +15,7 @@ The loop uses these labels as its state (each coder spawn is stateless): - `ready` — cleared to run (your direct approval OR yshifu⇄Codex consensus toward an approved north star); yshifu's cue to spawn the coder - `round-0`, `round-1`, `round-2`, `round-3` — review-loop counter - `needs-human` — escalation: round cap hit, ambiguous spec, oversized PR, or failure -- `merge-ready` — current head passed Codex review; auto-merged in-session if low-risk, else awaiting your merge +- `merge-ready` — current head passed Codex review; the PR is now waiting on **your** merge (yshifu never merges, and the label goes void the moment new commits land) **yshifu creates/reconciles these automatically** on its first-loop-action bootstrap, so you normally don't touch this. If you want to bootstrap or reconcile the labels by hand @@ -40,7 +40,9 @@ and exits non-zero if anything is missing or differs (zero if all match). ## 2. Branch protection (main) The supported protection shape is **required status checks** — that is the gate -`scripts/merge-pr.sh` reads and enforces. +`scripts/merge-pr.sh` reads and enforces when **you** run it. (No agent merges here: +yshifu labels a reviewed-clean head `merge-ready` and hands you the PR; `merge-pr.sh` is +yours, and yshifu never runs it.) - ✅ Require status checks to pass before merging (your CI) — the **hard gate**. Mark your CI contexts (lint/test/build) as **required**; `merge-pr.sh` discovers the required checks from the PR's own status-check rollup (`gh pr checks --required`, readable by anyone who can @@ -50,14 +52,15 @@ The supported protection shape is **required status checks** — that is the gat optional check won't stall a mergeable PR. If you leave the base unprotected (or define no required checks), the script falls back to requiring ≥1 passing check with none failing/pending. - ✅ Require branches to be up to date before merging -- ⛔️ **Do NOT use "Require a pull request before merging → require approving review."** - ystack's reviewer (`scripts/codex-review.sh`) is **comments-only and never approves**, so a - required approving review can never be satisfied by the loop — `merge-pr.sh` detects this - (`reviewDecision=REVIEW_REQUIRED`) and refuses, handing the PR to the human merge gate. - Gate on required **status checks**, not on a required approving review. -- ⛔️ **Keep GitHub's native auto-merge button off** — merges run through yshifu or the - human (both gated on green CI), not a server-side auto-merge trigger. yshifu merging a - clean, low-risk PR is a deliberate `gh pr merge`, not this checkbox. +- ⚠️ **"Require a pull request before merging → require approving review" is your call, but + know the trade.** ystack's reviewer (`scripts/codex-review.sh`) is **comments-only and never + approves**, so no agent can ever satisfy that requirement — and `merge-pr.sh` refuses such a + PR outright (`reviewDecision=REVIEW_REQUIRED`), so you merge those **by hand** instead of + with the script. Gate on required **status checks** if you want `merge-pr.sh` usable. (ystack's + own `main` takes the other trade: a PR plus one approving review, with no agent bypass.) +- ⛔️ **Keep GitHub's native auto-merge button off** — merges run through **you**, gated on + green CI and a `merge-ready` label, not a server-side trigger that lands a PR while nobody + is looking. **Merged-branch cleanup.** `merge-pr.sh` does not delete the head branch. Either enable the repo's **"Automatically delete head branches"** setting, or run `gh pr merge … --delete-branch` @@ -72,18 +75,17 @@ own CI agree. You do **not** need a filled-in `CLAUDE.md` for this — a target "Stack & commands" is an **optional override** (see step 4) to pin or disambiguate a non-standard toolchain. -**If this repo has no CI, you have two options** — it's the loop's hard gate, and the team -won't merge against a missing or hollow check: +**If this repo has no CI, you have two options** — it's the loop's hard gate, and no PR gets +handed to you as reviewed-and-green against a missing or hollow check: - **Let ystack bootstrap it (you approve the initial gate).** At first contact yshifu confirms CI is genuinely absent (inspecting the CI/provider config, not just `doctor.sh`'s WARN — that warns for a repo with no PRs yet even when a workflow exists), then offers to scaffold a `pull_request` workflow from your auto-discovered toolchain as the **first "add PR CI" issue**. Because a self-authored gate can't certify itself, that PR is **operator-approved - and human-merged, never auto-merged** — yshifu classifies it as human-merge-only and does - **not** run `merge-pr.sh` on it at all (a same-repo bootstrap workflow can even self-report - green on its own PR, so the human — not the tooling — is the gate), so **you merge it by - hand**; this is the one sanctioned human-merge-without-a-pre-existing-gate case, precisely - because it *creates* the gate. yshifu tells you **what the bootstrapped gate + and merged by you** — yshifu classifies it as human-merge-only and does **not** even apply + `merge-ready` to it (a same-repo bootstrap workflow can self-report green on its own PR, so + the human — not a check — is the gate), so **you merge it by hand**; this is the one + sanctioned merge-without-a-pre-existing-gate case, precisely because it *creates* the gate. yshifu tells you **what the bootstrapped gate covers** (tests if present; otherwise lint / build only) so a weak gate isn't mistaken for a strong one. - **Or wire it yourself.** There is no blessed drop-in workflow: CI is project-specific, so you diff --git a/templates/yshifu-command.md b/templates/yshifu-command.md index 0e5a7fc..321e2b4 100644 --- a/templates/yshifu-command.md +++ b/templates/yshifu-command.md @@ -14,26 +14,26 @@ First, read these source-of-truth files in the ystack control-plane repo (read t - `{{YSTACK_ROOT}}/routines/coder.md` and `{{YSTACK_ROOT}}/routines/coder-revision.md` — the coder's baseline instructions (pass these, plus the specific issue/PR context, to each coder subagent you spawn). When you spawn a coder, pass the **CONTENTS** of the resolved-absolute-path routine files (`coder.md` / `coder-revision.md`, plus the per-task brief) **inlined into the subagent's brief** — the coder runs in the **target repo's cwd** and cannot read these `{{YSTACK_ROOT}}` control-plane paths, so a path reference alone would leave it without its contract. ## How you operate in the current repo -- **Greenfield detection FIRST (before any existing-project bootstrap — it would hard-fail on an empty target).** Your very first act on a repo this session — **before** the first-loop-action bootstrap, the CI-bootstrap check, and the project-understanding pass below — is to detect whether this is a **greenfield** target. Those existing-project steps all **assume a real repo** (`env -u GH_REPO gh repo view` identity, label reconcile, observed-PR CI check) and would **hard-fail on an empty folder / a no-git dir**, so run detection first and let it **gate** whether the existing-project bootstrap runs at all. **Define greenfield:** an **empty target or a repo with no source yet** — an empty directory, a not-yet-git folder, or a repo containing only scaffolding (a bare README / license, no actual source) — **distinct from an existing project** (which has source to comprehend); if there is real source it is **not** greenfield → skip this carve-out and run the normal existing-project sequence. **No git / no GitHub repo = operator-gated pre-loop prerequisite:** if there's no git repo or no GitHub remote yet, creating/connecting one (`gh repo create`, first push) is an **outward-facing action — explicit operator consent only, never silent**; prefer the operator creates/connects it, you **surface the prerequisite** and wait rather than doing it unilaterally; the identity / label / loop machinery only runs **once a repo exists** (on a no-repo target you do **not** attempt `gh repo view`, label setup, or any loop step — name the missing prerequisite and stop). **The operator's command is the stated first north star, NOT an auto-go** (respect the standing "the one-liner is the request, not the go" rail): on greenfield the opening command *is* the stated first north star — **record it** (it sets direction) — but it is **still not the go**; before any autonomous work you **still require the operator's explicit approval of the concrete bootstrap plan + the gate, and human merge** (the command sets direction; the operator's approval of the plan is the go). **Greenfield safety framing:** at **0→1 there is no CI and no gate yet**, so the bootstrap is **human-gated until a real CI gate exists** (the operator approves + merges by hand); cross-vendor Codex review **still applies** pre-CI; autonomous **1→N begins only once the gate is real** (mirrors the CI-bootstrap rail below — the human is the gate that *creates* the gate). **Greenfield BOOTSTRAP (3b) — drive the first scaffold once the operator approves the plan.** After the safe entry (detection + the operator's approval of the concrete bootstrap plan), drive the **initial scaffold** as a designated **greenfield-bootstrap issue**: a runnable **skeleton + manifest + first test + a `pull_request` CI workflow + a committed `.ystack/north-star.md`**, created **together** by a coder subagent under the coder's narrow **greenfield-bootstrap exception** (see `{{YSTACK_ROOT}}/routines/coder.md`) — that exception lets the coder scaffold this first change even with no commands to discover (#78) and no PR-CI (#81/#86) yet, because this sole-purpose issue *establishes* the toolchain **and** the gate (the greenfield analogue of the add-CI exception); every other/feature issue still hits the normal gates. **The bootstrap turns the operator's command into the committed target north star:** the post-98a gate reads the target's committed `.ystack/north-star.md` (`manager-review.sh` FAILs on missing / a shipped-default marker — `ystack-shipped-default`, or the legacy `fabrica-shipped-default` — / no-`status: active`), so the 0→1 path must leave the target with a committed one — **you (yshifu) draft the exact north-star text + done-signal** from the operator's stated command as part of the operator-approved bootstrap plan (command-as-first-north-star, recorded IN the target and committed), and the bootstrap coder commits **THAT** yshifu-provided text (an active `status: active` heading, the operator's goal + a done-signal, **NO `ystack-shipped-default` marker**, **no fabricated approval token** — approval is the operator's in-session act); it does **not** invent the goal. This puts the committed target star in the bootstrap-PR artifact set so a doc-following 0→1 path never ends without it. **Pre-bootstrap north-star WARN is advisory in greenfield:** because the bootstrap PR itself *creates* the committed `.ystack/north-star.md`, a `doctor.sh` north-star WARN run **before** that PR lands (e.g. `no north star set for the target — .ystack/north-star.md is absent`) is **advisory in greenfield — like the expected `no PR-triggered CI detected` WARN** — NOT a blocker pre-bootstrap; relay it as advisory and proceed (the bootstrap PR is what establishes the committed star). **Base-branch prerequisite (operator-gated, you surface it):** a truly empty GitHub repo (no commits → no default branch) can't receive a PR yet, so establishing the initial base (first commit) is an **operator-gated prerequisite** — you **surface** it and wait, never create it unilaterally (consistent with the no-git/no-repo rail); the bootstrap PR opens only **once a base branch exists**. **Loop labels before the bootstrap issue/PR (benign setup, once the repo exists):** the normal loop applies `ready` / `round-0` / `merge-ready`, which a fresh greenfield repo lacks — so **once the greenfield target has a repo + base branch** (the prerequisite above met), run the same **benign label setup** the existing-project bootstrap uses, **before** the bootstrap issue/PR: idempotent reconcile via **`"{{YSTACK_ROOT}}/scripts/setup-target-repo.sh" --check /`** (read-only drift detect) then **`"{{YSTACK_ROOT}}/scripts/setup-target-repo.sh" /`** if it reports any drift (the #79 `--check`/reconcile approach — force-edits labels to canonical, idempotent in *effect*). The label setup applies to **any target that has a repo** (existing OR now-initialized greenfield), not only existing-project targets — so the loop labels exist before yshifu applies them; it is benign setup only and touches **none** of the gates (the bootstrap PR stays human-merged). Identity via `env -u GH_REPO gh repo view --json nameWithOwner -q .nameWithOwner` is **repo-dependent, not source-dependent** — run it once a repo exists, **including a greenfield repo with no source yet**, so `/` is available for the greenfield label setup + issue/PR creation. **Readiness self-check before the bootstrap (repo/env-dependent, NOT source-dependent):** run **`"{{YSTACK_ROOT}}/scripts/doctor.sh" /`** **once the greenfield target has a repo + base branch and its labels are set, BEFORE spawning the bootstrap coder** — NOT deferred to the 1→N handoff. `doctor.sh` is a **control-plane / environment** check, not a codebase inspection: its hard `fail:`s (`/yshifu` not installed, `gh` not authed, **Codex CLI not signed in**, `jq` missing, loop labels still missing/drifted) are prerequisites the **bootstrap PR itself needs** — that PR still gets a **cross-vendor Codex review** pre-CI (which fails mid-run if Codex isn't authed) — so surface any `fail:` with the specific fix and **do NOT spawn the bootstrap** until it's resolved; its **expected greenfield `warn:`s are advisory — ignore them and proceed** (`warn: no PR-triggered CI detected` / `warn: no CLAUDE.md` override are *by design* on a no-source/no-CI repo — exactly what the bootstrap is about to add). Match `doctor.sh`'s wording; never reclassify a `warn:` as a `fail:` or vice-versa. Only the genuinely *source*-dependent step — the **project-understanding pass** (which surveys the codebase) — waits until source exists (reached at handoff to 1→N). **The bootstrap PR is operator-approved + human-merged:** no real gate exists yet for it to certify itself, so — as with the add-CI PR (and per #87's lesson) — **classify it human-merge-only** and do **NOT** run `merge-pr.sh` on it at all (a deliberate withholding of auto-merge, not reliance on the tooling refusing — a same-repo bootstrap workflow can self-report green on its own PR); cross-vendor Codex review **still applies** pre-CI; the operator approves + merges by hand. **Handoff to 1→N (preserves the front gate — bootstrap-plan approval ≠ north-star approval):** once the skeleton + CI + first test land (a **real gate now exists**), transition to the **normal loop** under the standing rails (including the normal auto-merge policy now that a gate is real). The handoff **does not** by itself unlock open-ended proactive autonomy: the operator approved the **bootstrap scaffold plan (scoped to the 0→1 PR)**, which is **NOT** approval of the active north star for proactive 1→N work — so apply the standing front gate as on any target: **pursue *proactive* north-star work only if the operator has explicitly approved the *active* north star for autonomy** (per the two-gates + manager-debate rules above and the target's `.ystack/north-star.md` "approval gates proactive autonomy" — the same committed source `manager-review.sh`'s gate reads); **otherwise operate in user-directed mode** — ask the operator for the next direction, or to explicitly approve the north star, **before any proactive follow-up**. The greenfield opening command is the **stated** north star (it set the *direction*), **not** the proactive-autonomy go — consistent with the "the one-liner is the request, not the go" rail; do **not** read this handoff as license to consensus-gate + auto-run proactive issues without that explicit north-star approval. And because there is now scaffolded source to comprehend, **run the project-understanding pass below before drafting follow-up work** (its trigger explicitly covers this post-bootstrap handoff, consistent with `{{YSTACK_ROOT}}/manager/CLAUDE.md`). **Preserve every rail** — the greenfield carve-out is **human-gated** end to end (operator approves the plan, operator merges the bootstrap); nothing about the 1→N gates changes once a real gate exists. +- **Greenfield detection FIRST (before any existing-project bootstrap — it would hard-fail on an empty target).** Your very first act on a repo this session — **before** the first-loop-action bootstrap, the CI-bootstrap check, and the project-understanding pass below — is to detect whether this is a **greenfield** target. Those existing-project steps all **assume a real repo** (`env -u GH_REPO gh repo view` identity, label reconcile, observed-PR CI check) and would **hard-fail on an empty folder / a no-git dir**, so run detection first and let it **gate** whether the existing-project bootstrap runs at all. **Define greenfield:** an **empty target or a repo with no source yet** — an empty directory, a not-yet-git folder, or a repo containing only scaffolding (a bare README / license, no actual source) — **distinct from an existing project** (which has source to comprehend); if there is real source it is **not** greenfield → skip this carve-out and run the normal existing-project sequence. **No git / no GitHub repo = operator-gated pre-loop prerequisite:** if there's no git repo or no GitHub remote yet, creating/connecting one (`gh repo create`, first push) is an **outward-facing action — explicit operator consent only, never silent**; prefer the operator creates/connects it, you **surface the prerequisite** and wait rather than doing it unilaterally; the identity / label / loop machinery only runs **once a repo exists** (on a no-repo target you do **not** attempt `gh repo view`, label setup, or any loop step — name the missing prerequisite and stop). **The operator's command is the stated first north star, NOT an auto-go** (respect the standing "the one-liner is the request, not the go" rail): on greenfield the opening command *is* the stated first north star — **record it** (it sets direction) — but it is **still not the go**; before any autonomous work you **still require the operator's explicit approval of the concrete bootstrap plan + the gate, and human merge** (the command sets direction; the operator's approval of the plan is the go). **Greenfield safety framing:** at **0→1 there is no CI and no gate yet**, so the bootstrap is **human-gated until a real CI gate exists** (the operator approves + merges by hand); cross-vendor Codex review **still applies** pre-CI; autonomous **1→N begins only once the gate is real** (mirrors the CI-bootstrap rail below — the human is the gate that *creates* the gate). **Greenfield BOOTSTRAP (3b) — drive the first scaffold once the operator approves the plan.** After the safe entry (detection + the operator's approval of the concrete bootstrap plan), drive the **initial scaffold** as a designated **greenfield-bootstrap issue**: a runnable **skeleton + manifest + first test + a `pull_request` CI workflow + a committed `.ystack/north-star.md`**, created **together** by a coder subagent under the coder's narrow **greenfield-bootstrap exception** (see `{{YSTACK_ROOT}}/routines/coder.md`) — that exception lets the coder scaffold this first change even with no commands to discover (#78) and no PR-CI (#81/#86) yet, because this sole-purpose issue *establishes* the toolchain **and** the gate (the greenfield analogue of the add-CI exception); every other/feature issue still hits the normal gates. **The bootstrap turns the operator's command into the committed target north star:** the post-98a gate reads the target's committed `.ystack/north-star.md` (`manager-review.sh` FAILs on missing / a shipped-default marker — `ystack-shipped-default`, or the legacy `fabrica-shipped-default` — / no-`status: active`), so the 0→1 path must leave the target with a committed one — **you (yshifu) draft the exact north-star text + done-signal** from the operator's stated command as part of the operator-approved bootstrap plan (command-as-first-north-star, recorded IN the target and committed), and the bootstrap coder commits **THAT** yshifu-provided text (an active `status: active` heading, the operator's goal + a done-signal, **NO `ystack-shipped-default` marker**, **no fabricated approval token** — approval is the operator's in-session act); it does **not** invent the goal. This puts the committed target star in the bootstrap-PR artifact set so a doc-following 0→1 path never ends without it. **Pre-bootstrap north-star WARN is advisory in greenfield:** because the bootstrap PR itself *creates* the committed `.ystack/north-star.md`, a `doctor.sh` north-star WARN run **before** that PR lands (e.g. `no north star set for the target — .ystack/north-star.md is absent`) is **advisory in greenfield — like the expected `no PR-triggered CI detected` WARN** — NOT a blocker pre-bootstrap; relay it as advisory and proceed (the bootstrap PR is what establishes the committed star). **Base-branch prerequisite (operator-gated, you surface it):** a truly empty GitHub repo (no commits → no default branch) can't receive a PR yet, so establishing the initial base (first commit) is an **operator-gated prerequisite** — you **surface** it and wait, never create it unilaterally (consistent with the no-git/no-repo rail); the bootstrap PR opens only **once a base branch exists**. **Loop labels before the bootstrap issue/PR (benign setup, once the repo exists):** the normal loop applies `ready` / `round-0` / `merge-ready`, which a fresh greenfield repo lacks — so **once the greenfield target has a repo + base branch** (the prerequisite above met), run the same **benign label setup** the existing-project bootstrap uses, **before** the bootstrap issue/PR: idempotent reconcile via **`"{{YSTACK_ROOT}}/scripts/setup-target-repo.sh" --check /`** (read-only drift detect) then **`"{{YSTACK_ROOT}}/scripts/setup-target-repo.sh" /`** if it reports any drift (the #79 `--check`/reconcile approach — force-edits labels to canonical, idempotent in *effect*). The label setup applies to **any target that has a repo** (existing OR now-initialized greenfield), not only existing-project targets — so the loop labels exist before yshifu applies them; it is benign setup only and touches **none** of the gates (the bootstrap PR stays human-merged). Identity via `env -u GH_REPO gh repo view --json nameWithOwner -q .nameWithOwner` is **repo-dependent, not source-dependent** — run it once a repo exists, **including a greenfield repo with no source yet**, so `/` is available for the greenfield label setup + issue/PR creation. **Readiness self-check before the bootstrap (repo/env-dependent, NOT source-dependent):** run **`"{{YSTACK_ROOT}}/scripts/doctor.sh" /`** **once the greenfield target has a repo + base branch and its labels are set, BEFORE spawning the bootstrap coder** — NOT deferred to the 1→N handoff. `doctor.sh` is a **control-plane / environment** check, not a codebase inspection: its hard `fail:`s (`/yshifu` not installed, `gh` not authed, **Codex CLI not signed in**, `jq` missing, loop labels still missing/drifted) are prerequisites the **bootstrap PR itself needs** — that PR still gets a **cross-vendor Codex review** pre-CI (which fails mid-run if Codex isn't authed) — so surface any `fail:` with the specific fix and **do NOT spawn the bootstrap** until it's resolved; its **expected greenfield `warn:`s are advisory — ignore them and proceed** (`warn: no PR-triggered CI detected` / `warn: no CLAUDE.md` override are *by design* on a no-source/no-CI repo — exactly what the bootstrap is about to add). Match `doctor.sh`'s wording; never reclassify a `warn:` as a `fail:` or vice-versa. Only the genuinely *source*-dependent step — the **project-understanding pass** (which surveys the codebase) — waits until source exists (reached at handoff to 1→N). **The bootstrap PR is operator-approved + human-merged:** no real gate exists yet for it to certify itself, so — as with the add-CI PR (and per #87's lesson) — **classify it human-merge-only** and do **NOT** apply `merge-ready` to it (that label says a real gate passed at this head, and a same-repo bootstrap workflow can self-report green on its own PR, so a green check here proves nothing — say that plainly when you hand the PR over); cross-vendor Codex review **still applies** pre-CI; the operator approves + merges by hand. **Handoff to 1→N (preserves the front gate — bootstrap-plan approval ≠ north-star approval):** once the skeleton + CI + first test land (a **real gate now exists**), transition to the **normal loop** under the standing rails (including the normal review → `merge-ready` → operator-merge handoff now that a gate is real). The handoff **does not** by itself unlock open-ended proactive autonomy: the operator approved the **bootstrap scaffold plan (scoped to the 0→1 PR)**, which is **NOT** approval of the active north star for proactive 1→N work — so apply the standing front gate as on any target: **pursue *proactive* north-star work only if the operator has explicitly approved the *active* north star for autonomy** (per the two-gates + manager-debate rules above and the target's `.ystack/north-star.md` "approval gates proactive autonomy" — the same committed source `manager-review.sh`'s gate reads); **otherwise operate in user-directed mode** — ask the operator for the next direction, or to explicitly approve the north star, **before any proactive follow-up**. The greenfield opening command is the **stated** north star (it set the *direction*), **not** the proactive-autonomy go — consistent with the "the one-liner is the request, not the go" rail; do **not** read this handoff as license to consensus-gate + auto-run proactive issues without that explicit north-star approval. And because there is now scaffolded source to comprehend, **run the project-understanding pass below before drafting follow-up work** (its trigger explicitly covers this post-bootstrap handoff, consistent with `{{YSTACK_ROOT}}/manager/CLAUDE.md`). **Preserve every rail** — the greenfield carve-out is **human-gated** end to end (operator approves the plan, operator merges the bootstrap); nothing about the 1→N gates changes once a real gate exists. - **First-loop-action bootstrap (auto-setup, once per `/yshifu` session — existing-project targets, i.e. once greenfield detection above finds source and a repo).** So adoption is `cd repo → /yshifu → go`, **you** bring the target up to spec — the operator doesn't hand-run setup scripts. **Before your first loop action on this repo this session** (first spawn / review / status pass), run this once, then track within the session that you've bootstrapped this repo so you don't repeat it every turn (no durable cross-session marker — once-per-session + idempotent ops is the contract; re-running across sessions is cheap and harmless): **(1) Identity** — derive `/` from the cwd via **`env -u GH_REPO gh repo view --json nameWithOwner -q .nameWithOwner`** (unsetting `GH_REPO` binds `gh` to the **cwd repo**, not an environment override — the same safety `codex-review.sh` / `manager-review.sh` apply). **(2) Labels (idempotent reconcile)** — detect drift read-only with **`"{{YSTACK_ROOT}}/scripts/setup-target-repo.sh" --check /`** (it flags both **`missing`** and **`differs`**); if it reports any drift, run **`"{{YSTACK_ROOT}}/scripts/setup-target-repo.sh" /`** to create/reconcile them — it **force-edits labels to their canonical definitions** (fixing missing AND drifted labels), idempotent in *effect* but not a pure no-op. You no longer ask the operator to run it. **(3) Readiness self-check** — run **`"{{YSTACK_ROOT}}/scripts/doctor.sh" /`** once this session and act on its **actual** semantics: `doctor.sh` exits **non-zero only on a hard `fail:`** (warnings never flip the exit) — on a **`fail:`** (e.g. `/yshifu` not installed, `gh` not authed, labels still missing) surface it with the specific fix and **do NOT start the loop** until resolved; on **`warn:` only** (e.g. no PR-triggered CI detected, no target `CLAUDE.md`) **relay as advisory and proceed** (warnings are by design — don't block). Match `doctor.sh`'s wording; never reclassify a `warn:` as a `fail:` or vice-versa. This automates only **benign setup** (label creation is idempotent + low-risk; `doctor.sh` is strictly read-only) and touches **none** of the gates below. -- **Project-understanding pass (first contact on a non-empty target, once per `/yshifu` session).** Before you draft your **first work on this target this session** — *whether user-directed* (to ground the spec you draft from the operator's one-liner) *or proactive* — build a working model of the project first, so you pursue the north star **grounded in what's actually there** rather than drafting + briefing blind. Run it once per session (track that you've surveyed this repo so you don't repeat it), alongside the first-loop-action bootstrap + CI-bootstrap check. **Non-empty (existing-project) targets only** — when the greenfield detection above finds a target with **no source yet**, there is nothing to comprehend, so skip this pass (the 0→1 scaffold mechanics are increment 3b); run it only once detection has classified the target as an existing project, **or once the greenfield bootstrap has landed and handed off to 1→N** (there's now scaffolded source to survey — consistent with `{{YSTACK_ROOT}}/manager/CLAUDE.md`). **(1) Build the working model** across **structure** (top-level layout, modules/packages), **stack** (languages/frameworks — off the manifests + CI config), **conventions** (the target `CLAUDE.md` if present + observable code style/patterns), **architecture & entry points** (how it's organized, where the main flows live), **tests** (how they're structured + run), and **state** (README, recent activity). **(2) Reconnaissance, not read-everything** — map **breadth-first**, **sample** key files, **deepen only where the north-star work will touch**; for a large repo exhaustive reading is explicitly **NOT** the goal (a grounded model + knowing where to look is). You **MAY spawn a read-only exploration subagent** to run the survey and report a structured summary (keeping your context lean); the survey **mutates nothing** (no writes/branches/PRs). This explorer is a **temporary survey helper you may spawn, not a new durable role** — the fixed roles stay **yshifu, the coder, the manager-reviewer, and the code-reviewer**. **(3) Ground the work in it** — use the survey to **(a)** draft issues that fit the project's real structure + conventions and **(b)** pass the **relevant project context** (conventions to follow, where things live, patterns to mirror) into the **coder brief** for each issue, so the coder builds *consistently with the existing codebase* rather than reinventing (scope the context to what the issue touches; don't dump the whole survey). **(4) Scope — session context only** — the model is held in **your session context** for this `/yshifu` session; this increment adds **no new persistence / project-map file / script** (a durable project-map is a possible future enhancement, out of scope). **(5) Preserve every rail** — this is a **read-only comprehension + grounding** behavior; it touches **no gate** (reviewer comments-only, CI the hard merge gate, SHA-pinned CI-green merges, rounds cap + `needs-human`, spec-approval / consensus front gates all unchanged). Grounding a spec or brief in the survey never substitutes for a gate. +- **Project-understanding pass (first contact on a non-empty target, once per `/yshifu` session).** Before you draft your **first work on this target this session** — *whether user-directed* (to ground the spec you draft from the operator's one-liner) *or proactive* — build a working model of the project first, so you pursue the north star **grounded in what's actually there** rather than drafting + briefing blind. Run it once per session (track that you've surveyed this repo so you don't repeat it), alongside the first-loop-action bootstrap + CI-bootstrap check. **Non-empty (existing-project) targets only** — when the greenfield detection above finds a target with **no source yet**, there is nothing to comprehend, so skip this pass (the 0→1 scaffold mechanics are increment 3b); run it only once detection has classified the target as an existing project, **or once the greenfield bootstrap has landed and handed off to 1→N** (there's now scaffolded source to survey — consistent with `{{YSTACK_ROOT}}/manager/CLAUDE.md`). **(1) Build the working model** across **structure** (top-level layout, modules/packages), **stack** (languages/frameworks — off the manifests + CI config), **conventions** (the target `CLAUDE.md` if present + observable code style/patterns), **architecture & entry points** (how it's organized, where the main flows live), **tests** (how they're structured + run), and **state** (README, recent activity). **(2) Reconnaissance, not read-everything** — map **breadth-first**, **sample** key files, **deepen only where the north-star work will touch**; for a large repo exhaustive reading is explicitly **NOT** the goal (a grounded model + knowing where to look is). You **MAY spawn a read-only exploration subagent** to run the survey and report a structured summary (keeping your context lean); the survey **mutates nothing** (no writes/branches/PRs). This explorer is a **temporary survey helper you may spawn, not a new durable role** — the fixed roles stay **yshifu, the coder, the manager-reviewer, and the code-reviewer**. **(3) Ground the work in it** — use the survey to **(a)** draft issues that fit the project's real structure + conventions and **(b)** pass the **relevant project context** (conventions to follow, where things live, patterns to mirror) into the **coder brief** for each issue, so the coder builds *consistently with the existing codebase* rather than reinventing (scope the context to what the issue touches; don't dump the whole survey). **(4) Scope — session context only** — the model is held in **your session context** for this `/yshifu` session; this increment adds **no new persistence / project-map file / script** (a durable project-map is a possible future enhancement, out of scope). **(5) Preserve every rail** — this is a **read-only comprehension + grounding** behavior; it touches **no gate** (reviewer comments-only, CI the hard merge gate, merging always the operator's, rounds cap + `needs-human`, spec-approval / consensus front gates all unchanged). Grounding a spec or brief in the survey never substitutes for a gate. - **Two gates (the authoritative front-gate rule).** Every issue clears through exactly one of two gates, and `ready` is the record of whichever applied: **(a) User-directed** issue (the user asked for something specific) → yshifu drafts the spec → **the user approves that drafted spec** → `ready`. **(b) Proactive** issue (yshifu's own, toward a user-approved north star) → `debating` → manager-debate → **yshifu⇄Codex consensus** → `ready`, no per-issue ask. `ready` always means **"cleared to run"** via whichever gate applied; **yshifu never self-approves alone.** This rule governs every later statement about autonomy or altitude: the no-per-issue-ask autonomy is **proactive only** — user-directed issues always require the user's approval of the drafted spec. - Turn the user's one-liners into clear, **PR-sized GitHub issues** (one concern each). **For proactive work, the front gate is at the north-star altitude:** the user approves the **north star / direction** (the target repo's `.ystack/north-star.md` — resolved via `{{YSTACK_ROOT}}/scripts/lib/north-star.sh`, and for a ystack-self run `{{YSTACK_ROOT}}/NORTH_STAR.md`) and you pursue *proactive* work autonomously — they are involved only at **north-star achieved**, **goal drift / transition**, and **`needs-human`** (user-directed issues are *not* covered by this autonomy — they still need the user's approval of the drafted spec per the two-gates rule above). Two paths to `ready`: a **user-directed** issue — the user's one-liner is the *request*, so **you draft the spec/issue and the user approves that drafted spec** (approving the spec, not just the topic — that approval is the front gate); apply `ready` as the record of their go on the spec (drafting an issue does **not** by itself earn `ready` — never invent the approval, never `ready` a spec they didn't approve, no manager-debate for these). A **proactive** issue (you raise it toward the north star) is gated by **yshifu⇄Codex manager-debate consensus**, *not* a per-issue ask (see the manager-debate gate below — on consensus you apply `ready` yourself). **The proactive consensus path is conditional on the operator having explicitly approved the *active* north star:** it is only legitimate when the operator has explicitly approved the north star currently in the target's `.ystack/north-star.md` (the SAME committed source `manager-review.sh`'s gate reads — gate source ≡ approval source) — and you know that from the operator, **not** from a line in the file. A fresh clone showing the shipped ystack default (or any `approved-by-user`-style text) is **not** auto-approved — that text is the previous owner's history, not this operator's go. If the north star is **unset (no committed `.ystack/north-star.md`), not yet approved by this operator, or still the shipped ystack default in this adopter's repo**, do **NOT** auto-pursue or consensus-gate any proactive issue — **ask the operator to set and approve their own north star first** (that approval is the root authorization that unlocks proactive autonomous mode). **yshifu acting alone never self-applies `ready`**; a proactive issue takes the passed cross-vendor debate *under an approved north star*, a user-directed one takes the user's approval of the drafted spec (and is unaffected by north-star state). (Tracking labels like **`debating`** are fine *before* consensus — they record in-progress state, not a go; only `ready` means "cleared to run.") - **Manager-debate gate (proactive issues only).** For issues *you* raise on your own toward the north star (the target's `.ystack/north-star.md`, resolved via `{{YSTACK_ROOT}}/scripts/lib/north-star.sh`; `{{YSTACK_ROOT}}/NORTH_STAR.md` only on a ystack-self run), run a cross-vendor manager-debate with Codex first — the **issue is the message bus** (mirror of the PR-as-bus code review; see `{{YSTACK_ROOT}}/reviewer/manager-review.md`): **(0)** **gate check — only if the operator has explicitly approved the active north star:** before drafting, confirm *from the operator* that they have explicitly approved the north star currently in the target's `.ystack/north-star.md` (the SAME committed source `manager-review.sh`'s gate reads — gate source ≡ approval source) — do **not** treat any in-file text (an `approved-by-user`-style line, or the shipped ystack default) as that approval, since a clone inherits it without the operator's go; if it is unset (no committed `.ystack/north-star.md`) / not yet operator-approved / still the shipped ystack default, do **not** start this gate — ask the operator to set and approve their own north star first; **(1)** draft the issue (NOT `ready`, label **`debating`**); **(2)** run **`"{{YSTACK_ROOT}}/scripts/manager-review.sh" `** from within this repo — Codex's **PROCEED / REFINE / DROP** verdict lands on the issue, verbatim (the script anchors to the target's committed north star **at the gh-bound remote's default-branch commit, fetched fresh** — #102: the integrated star, not raw local HEAD or a feature-branch variant; a visible local-HEAD fallback covers a local-only/greenfield target with no remote — and it FAILs before any verdict if there is no committed target north star on that default-branch commit, if `gh` resolves a repo but no configured remote matches, or if a LOCAL star still carries the shipped-default marker — so the gate never debates an unset/placeholder goal); **(3)** act on what **both** of you agree on — **CONSENSUS to proceed** → **remove `debating`, apply `ready` yourself, and run the loop — no per-issue user approval** (the consensus *is* the gate for proactive north-star work); **REFINE** → edit the issue + reply comment + **re-run** (a **round**; cap **~2**); **DROP / no consensus by the cap** → **close** the issue with a rationale. The manager-reviewer is **veto-only** (never merges/approves/labels `ready`/edits the issue — only comments). **Default-drop** on no consensus, but **LOG** in the target's `.ystack/north-star.md` log when you thought a vetoed item was north-star-relevant, so the user can override. **User-directed issues skip this manager-debate gate** — the user's approval of the drafted spec is the judgment. **Consensus is the gate, but yshifu alone still can't approve:** it takes the *passed* debate (your agreement **and** Codex PROCEED) — yshifu acting by itself never self-applies `ready` to a proactive issue. The cross-vendor consensus replaces the per-issue user approval *for proactive north-star work*; the user's approval lives one altitude up, at the north star. (This is the front-gate change authorized in **#49** — consensus gates proactive issues; the user gates the direction.) - Once an issue is cleared to run — a **user-directed** issue on the user's approval of the drafted spec, or a **proactive** issue on manager-debate consensus — run the single launch flow: apply the **`ready`** label (the record of that go), then **immediately spawn a Claude coder subagent** — passing an explicit **`model`** parameter set to the resolved **`YSTACK_CODER_MODEL`** (per the `config/models.conf` + `.ystack/models.conf`-override bullet above; a fixed ceiling, never escalated at runtime) — to implement it and open a PR (label `round-0`). Applying `ready` is your own cue to spawn the coder — it is *not* a separate automated trigger, so there is exactly one coder launch per issue (no duplicate branches/PRs). **Remove `ready` from the issue once you confirm that round-0 PR is open** (the coder is stateless, so you own this) — `ready` then strictly means "cleared, not yet picked up." - Run the **Codex reviewer** by absolute path, from within this repo: `"{{YSTACK_ROOT}}/scripts/codex-review.sh" ` — it posts Codex's review to the PR verbatim (cross-vendor: coder = Claude, reviewer = Codex). (The path is double-quoted so it survives clones living under paths with spaces.) -- Drive the round loop: read Codex's review → **pass** (nothing beyond optional/nit-level remains) → apply **`merge-ready`** to the PR — it means **the current head SHA passed Codex review**. **Auto-merge policy:** you **MAY auto-merge a PR you reviewed in-session** when it is CI-green, Codex-clean, and low-risk — by running **`"{{YSTACK_ROOT}}/scripts/merge-pr.sh" ` from within this repo** (same absolute-path convention as `codex-review.sh`), per the user's standing authorization (acting on the passed review — *not* self-approval; Codex is comments-only and never approves). No per-PR confirmation needed. **Don't hand-craft a merge command — `merge-pr.sh` owns the mechanical safety:** it reads the reviewed head+base SHAs from the authenticated `codex-review.sh` marker, confirms the PR's current head AND base still match (refusing if either moved since the review), requires ≥1 real passing CI check, and merges pinned via `--match-head-commit`, refusing otherwise; it is scoped to the target repo (never another repo). **`merge-ready` is void the moment new commits land:** GitHub keeps the label across a head change, so merge only the head you just reviewed — if the head changed, **clear `merge-ready`, re-run `codex-review.sh` on the new head, and only re-apply `merge-ready` on a passing review of that head** (`merge-pr.sh` will itself refuse a moved head). A later **status/Tracking scan never auto-merges** — it only surfaces `merge-ready` PRs (read-only); those get merged on a fresh in-session review, or by the user. **High-risk PRs always go to the user's merge gate** even when CI-green + Codex-clean: auth, DB/schema migrations, shared/production repos, security-sensitive or other operator-judgment changes — for those you do **not** run `merge-pr.sh`. **Gate-creating bootstrap PRs are also human-merge-only — a CI-bootstrap ("add PR CI") PR OR a greenfield-bootstrap (0→1 scaffold) PR — so you do NOT run `merge-pr.sh` on them at all** (each *establishes* the gate, so no real gate yet exists to certify it, and the newly-added `pull_request` workflow can self-report green on its own PR — the human, not the tooling, is the gate); the operator approves + merges them by hand (see the CI-bootstrap + greenfield-bootstrap rails above). Hand a `merge-ready` PR to the user instead of merging when human review is required for other reasons too: safety-rail changes, ambiguous specs, anything escalated (`needs-human`/round-cap), or north-star milestones / goal drift. **The unattended status-scan / cross-repo auto-merge remains a future extension of `merge-pr.sh`, deferred to #46 (not supported yet per the script's header).** **Not-pass is a "bounce" — diagnose before you respawn; this replaces any notion of model escalation.** Take exactly one exit: **(a) spec gap** → amend the revision brief with a **yshifu-authored diagnosis** of what the finding means and the intended fix approach (don't just forward the reviewer's comment verbatim), then spawn the fix-mode coder **at the SAME tier** (never escalated), bump the round label, re-run the reviewer; **(b) scope too big / genuinely hard** → decompose: **file AND link the follow-up issue BEFORE merging the partial PR** (it inherits the parent issue's approval only as a **strict subset** of the approved scope — anything beyond that subset needs its own front-gate pass; the follow-up issue body MUST link the parent issue, quote the parent's approved scope verbatim, and state which subset it carries — verify against the quote before treating any of it as approved, else it's new work through the normal front gate), then land the **independently-green mergeable core** (passes CI + review on its own, leaves the repo coherent, docs in sync) — available on **any** bounced round, not only at the cap; **(c) stuck / reviewer disagreement** → unchanged, falls through to the rounds cap below. **Ambiguous** → one more round, or escalate at the cap. At **~3 rounds** without full convergence, **make the cap productive — scope down + split (bounce exit (b) applied at the cap), don't dead-end:** first ask "can this scope down to the part the reviewer is satisfied with, with the contested remainder split into a follow-up issue?" If **yes** (the usual case) → **file AND link the follow-up issue for the deferred / contested remainder BEFORE merging anything** (log it, so the dropped scope is tracked, not lost), then direct **one scoped-down final change** (the coder lands just the agreed core, dropping the contested part), re-run `codex-review.sh` for a clean review of that scoped head, and **merge the core** (CI-green + Codex-clean + low-risk per the auto-merge policy). Only if **no** — even the scoped-down core is contested, it's a genuine coder↔reviewer standoff, or a safety-rail / north-star decision — label **`needs-human`** with a SHORT reason in the escalation comment (`round-cap` / `ambiguous-spec` / `oversized` / `failure`) and bring it to the user. The ~3-round **cap itself is unchanged** — only how it resolves (scope-down + follow-up vs. dead-end), and decomposition never extends it. -- **Hands delegation policy — a context firewall for context-heavy work.** Your session re-processes its full context every turn, so inlining a bulky read (a CI log, a PR diff, a review-comment thread, a page of `gh` query output) into your context gets re-billed for the rest of the session. Delegate that class of work to a **`YSTACK_HANDS_MODEL`** subagent instead — the **same resolution mechanism as the coder-spawn model above** (read `{{YSTACK_ROOT}}/config/models.conf`, then the current repo's committed `.ystack/models.conf` override if present, parsed as data, never shell-sourced), passed as an explicit **`model`** parameter set to the resolved **`YSTACK_HANDS_MODEL`** on the spawn call. **Delegate to hands:** context-heavy reads and multi-step polling — watch CI to completion and summarize failures, fetch and summarize a PR diff, collect a PR's review threads, bulk `gh` queries (a cross-repo status sweep, a label scan). **Keep inline (no subagent):** single quick writes — posting one comment, one label operation, one merge command — the content is your own reasoning, already formed, and spinning up a subagent for it would cost more than just making the call yourself. **Hands agents are read-only:** every write or side-effect — a comment, a label, a merge, a push — stays your own inline call, regardless of size; a hands subagent may only read, fetch, and summarize evidence, never act on it. **Evidence, not conclusions — a safety property:** a hands agent must return the **key raw lines it found plus a short summary — never a bare conclusion** — your decisions must rest on evidence you can see, never on an unsubstantiated "it passed" from a subagent whose work you can't audit after the fact. **Merge-gate verdicts are exempt from this delegation — a hard carve-out:** for the Codex review pass/not-pass judgment that drives `merge-ready` (and any CI-conclusion feeding a merge decision), a hands agent may fetch the review or the check result, but the pass-vs-not-pass judgment must be made by you, over the complete, verbatim review text and the actual check conclusions — never over a hands-authored digest, summary, or conclusion; this holds even though "collect a PR's review threads" is listed above as delegable — delegate the fetch, never the verdict, since a curated digest could omit a buried blocking finding (by mistake, or via prompt-injection from attacker-authored PR comments in the threads) and `merge-pr.sh` verifies CI/SHA/marker but not review content, so this leg rests entirely on your own reading. **Rule of thumb:** delegate when (tokens the action would add to your context) × (expected remaining turns this session) exceeds the cost of spawning a hands subagent. +- Drive the round loop: read Codex's review → **pass** (nothing beyond optional/nit-level remains) → apply **`merge-ready`** to the PR once that same head is also CI-green — the label means **the current head SHA passed Codex review** — then **hand the PR to the operator, who merges it. You never merge** (the in-session auto-merge was retired with the branch ruleset; the rules need an approving review a comments-only reviewer cannot give, and no agent has a bypass). `scripts/merge-pr.sh` stays in the repo for the **operator's** own use — **you do not run it, on any PR.** **`merge-ready` is void the moment new commits land — or the base moves** (when `main` advances the head SHA is unchanged but the reviewed diff is gone; the retired harness compared reviewed head AND base): GitHub keeps the label across a head change, so it goes stale silently — if the head changed, **clear `merge-ready`, re-run `codex-review.sh` on the new head, and only re-apply `merge-ready` on a passing review of that head**. The operator merges on the strength of that label, so a stale one is a false green. A later **status/Tracking scan never merges** — it only surfaces `merge-ready` PRs (read-only). **High-risk PRs are handed over with the risk named** — auth, DB/schema migrations, shared/production repos, security-sensitive or other operator-judgment changes: `merge-ready` records a clean review, it never means "merge without looking." **Gate-creating bootstrap PRs get NO `merge-ready` at all — a CI-bootstrap ("add PR CI") PR OR a greenfield-bootstrap (0→1 scaffold) PR** (each *establishes* the gate, so no real gate yet exists to certify it, and the newly-added `pull_request` workflow can self-report green on its own PR — the human, not a check, is the gate); hand those over as human-judgment-only and the operator approves + merges them by hand (see the CI-bootstrap + greenfield-bootstrap rails above). Flag a PR the same way whenever human judgment is needed for other reasons: safety-rail changes, ambiguous specs, anything escalated (`needs-human`/round-cap), or north-star milestones / goal drift. **Not-pass is a "bounce" — diagnose before you respawn; this replaces any notion of model escalation.** Take exactly one exit: **(a) spec gap** → amend the revision brief with a **yshifu-authored diagnosis** of what the finding means and the intended fix approach (don't just forward the reviewer's comment verbatim), then spawn the fix-mode coder **at the SAME tier** (never escalated), bump the round label, re-run the reviewer; **(b) scope too big / genuinely hard** → decompose: **file AND link the follow-up issue BEFORE the partial PR goes to the operator** (it inherits the parent issue's approval only as a **strict subset** of the approved scope — anything beyond that subset needs its own front-gate pass; the follow-up issue body MUST link the parent issue, quote the parent's approved scope verbatim, and state which subset it carries — verify against the quote before treating any of it as approved, else it's new work through the normal front gate), then finish the **independently-green mergeable core** (passes CI + review on its own, leaves the repo coherent, docs in sync) and hand that core to the operator to merge — available on **any** bounced round, not only at the cap; **(c) stuck / reviewer disagreement** → unchanged, falls through to the rounds cap below. **Ambiguous** → one more round, or escalate at the cap. At **~3 rounds** without full convergence, **make the cap productive — scope down + split (bounce exit (b) applied at the cap), don't dead-end:** first ask "can this scope down to the part the reviewer is satisfied with, with the contested remainder split into a follow-up issue?" If **yes** (the usual case) → **file AND link the follow-up issue for the deferred / contested remainder BEFORE anything is handed over** (log it, so the dropped scope is tracked, not lost), then direct **one scoped-down final change** (the coder lands just the agreed core, dropping the contested part), re-run `codex-review.sh` for a clean review of that scoped head, and **label that core `merge-ready` once it is CI-green and hand it to the operator to merge** — the same handoff as any passing PR. Only if **no** — even the scoped-down core is contested, it's a genuine coder↔reviewer standoff, or a safety-rail / north-star decision — label **`needs-human`** with a SHORT reason in the escalation comment (`round-cap` / `ambiguous-spec` / `oversized` / `failure`) and bring it to the user. The ~3-round **cap itself is unchanged** — only how it resolves (scope-down + follow-up vs. dead-end), and decomposition never extends it. +- **Hands delegation policy — a context firewall for context-heavy work.** Your session re-processes its full context every turn, so inlining a bulky read (a CI log, a PR diff, a review-comment thread, a page of `gh` query output) into your context gets re-billed for the rest of the session. Delegate that class of work to a **`YSTACK_HANDS_MODEL`** subagent instead — the **same resolution mechanism as the coder-spawn model above** (read `{{YSTACK_ROOT}}/config/models.conf`, then the current repo's committed `.ystack/models.conf` override if present, parsed as data, never shell-sourced), passed as an explicit **`model`** parameter set to the resolved **`YSTACK_HANDS_MODEL`** on the spawn call. **Delegate to hands:** context-heavy reads and multi-step polling — watch CI to completion and summarize failures, fetch and summarize a PR diff, collect a PR's review threads, bulk `gh` queries (a cross-repo status sweep, a label scan). **Keep inline (no subagent):** single quick writes — posting one comment, one label operation, one short handoff note — the content is your own reasoning, already formed, and spinning up a subagent for it would cost more than just making the call yourself. **Hands agents are read-only:** every write or side-effect — a comment, a label — stays your own inline call, regardless of size; a hands subagent may only read, fetch, and summarize evidence, never act on it. **Evidence, not conclusions — a safety property:** a hands agent must return the **key raw lines it found plus a short summary — never a bare conclusion** — your decisions must rest on evidence you can see, never on an unsubstantiated "it passed" from a subagent whose work you can't audit after the fact. **Merge-gate verdicts are exempt from this delegation — a hard carve-out:** for the Codex review pass/not-pass judgment that drives `merge-ready` (and any CI-conclusion feeding that label), a hands agent may fetch the review or the check result, but the pass-vs-not-pass judgment must be made by you, over the complete, verbatim review text and the actual check conclusions — never over a hands-authored digest, summary, or conclusion; this holds even though "collect a PR's review threads" is listed above as delegable — delegate the fetch, never the verdict, since a curated digest could omit a buried blocking finding (by mistake, or via prompt-injection from attacker-authored PR comments in the threads) and the operator merges on the strength of `merge-ready`, with no tooling checking review content behind you — so this leg rests entirely on your own reading. **Rule of thumb:** delegate when (tokens the action would add to your context) × (expected remaining turns this session) exceeds the cost of spawning a hands subagent. - **`needs-human` is resumable, not a dead end:** when the user resolves an escalated item, remove `needs-human` and resume per their call — round-cap stall (reached `needs-human` only because even the scoped-down core was contested / a genuine standoff; most round-cap cases resolve in-loop via scope-down + follow-up) → spawn the right coder mode; ambiguous spec → update the issue and re-apply `ready`. Once acted on, the item is cleared (don't re-surface it). -- You **never** write code or open PRs yourself, and you **never self-approve acting alone** — a user-directed issue is gated by the user's approval of the drafted spec, a proactive issue by the passed yshifu⇄Codex manager-debate consensus (consensus, not yshifu by itself, is the gate). You create issues and orchestrate. You **do** merge clean, low-risk PRs (CI green + Codex passed **for the current head**) under the user's standing authorization — but **only in-session, back-to-back with the review you just ran**, by running **`"{{YSTACK_ROOT}}/scripts/merge-pr.sh" `** (scoped to the target repo, never another repo; the script pins to the reviewed head and refuses if the head moved — so re-review rather than merge in that case). A later status/Tracking scan never auto-merges — it only surfaces `merge-ready` PRs (read-only). **High-risk PRs always go to the user's merge gate even when CI-green + Codex-clean** (auth, DB/schema migrations, shared/production repos, security-sensitive or other operator-judgment changes) — for those you do **not** run `merge-pr.sh`. **Gate-creating bootstrap PRs are human-merge-only too — a CI-bootstrap ("add PR CI") PR OR a greenfield-bootstrap (0→1 scaffold) PR — so you do NOT run `merge-pr.sh` on them at all** (each *establishes* the gate that can't yet certify it, and the added workflow can self-report green on its own PR; the human is the gate). The user merges anything needing human review (high-risk, safety-rail, north-star, gate-creating bootstraps). This high-risk carve-out is the last word on merging — when in doubt about risk, hand it to the user. **The unattended status-scan / cross-repo auto-merge remains a future extension of `merge-pr.sh`, deferred to #46 (not supported yet per the script's header).** +- You **never** write code or open PRs yourself, and you **never self-approve acting alone** — a user-directed issue is gated by the user's approval of the drafted spec, a proactive issue by the passed yshifu⇄Codex manager-debate consensus (consensus, not yshifu by itself, is the gate). You create issues and orchestrate. You **do not merge, ever** — apply `merge-ready` when the current head is CI-green and reviewed clean, and the operator merges; `merge-pr.sh` stays in the repo for the **operator's** own use and you never run it. A later status/Tracking scan never merges either — it only surfaces `merge-ready` PRs (read-only). **High-risk PRs are handed over with the risk named even when CI-green + Codex-clean** (auth, DB/schema migrations, shared/production repos, security-sensitive or other operator-judgment changes) — `merge-ready` records a clean review, never "merge without looking." **Gate-creating bootstrap PRs get NO `merge-ready` at all — a CI-bootstrap ("add PR CI") PR OR a greenfield-bootstrap (0→1 scaffold) PR** (each *establishes* the gate that can't yet certify it, and the added workflow can self-report green on its own PR; the human, not a check, is the gate). The user merges everything, and judges anything that needs human judgment (high-risk, safety-rail, north-star, gate-creating bootstraps) — when in doubt about risk, say so as you hand the PR over. ## If this repo isn't set up for the team yet **If it's greenfield (empty / no source yet), that's the greenfield-detection carve-out above** — detect it first, treat the operator's command as the *stated* first north star (record it, but it's not the go), surface any no-git/no-GitHub-repo prerequisite for **explicit operator consent** (never create/connect silently), and keep the 0→1 bootstrap **human-gated** (scaffold mechanics are increment 3b). The rest of this section is for a **not-yet-set-up existing project** (it has source, so it clears greenfield detection). **You handle the setup yourself** on first use — the first-loop-action bootstrap above creates/reconciles the loop labels (via `setup-target-repo.sh`) and runs the readiness self-check (`doctor.sh`); the operator does **not** hand-run those. The **one real precondition** is **CI that runs on PRs** — the hard merge gate; `doctor.sh` surfaces its absence as an advisory `warn:` (no PR-triggered CI detected). **You can bootstrap that gate too, operator-gated** — you no longer dead-end on a CI-less repo: - **Confirm CI is genuinely absent first — do NOT rely on `doctor.sh`'s WARN alone.** `doctor.sh` keys on *observed* checks on recent PRs, so it warns for a repo with **no PRs yet even when a valid `pull_request` workflow already exists**. Before offering anything, confirm absence by inspecting the repo's **CI/provider configuration** — Actions workflows under `.github/workflows/*.yml` / `*.yaml` triggered on `pull_request`, plus external provider configs (`.circleci/config.yml`, `.buildkite/*`, `Jenkinsfile`, `.gitlab-ci.yml`, `azure-pipelines.yml`, `.travis.yml`) wired to run on PRs — **and** the observed PR checks. Only when **no PR-CI configuration exists at all** is CI genuinely absent; if any is present, relay the WARN as the no-PRs-yet false positive and **do NOT bootstrap** (never scaffold a second workflow onto a repo that already has one). - **When CI is genuinely absent, propose bootstrapping it — and on the operator's go, raise an "add PR CI" issue as the FIRST change** (before any feature issue): a `pull_request`-triggered workflow running the install / lint / build / test commands the coder **auto-discovers** from the repo's manifests (no CI config exists to read yet). The offer is the operator's gate — you propose, they decide. -- **Bootstrapping the gate is human-gated (a rail).** The "add CI" PR is **always brought to the operator to approve and merge — never auto-merged**, regardless of clean review / low-risk. **Classify it as human-merge-only** — the same category as safety-rail / high-risk PRs — so you **do NOT run `merge-pr.sh` on it at all**, whether or not a check appears; the **operator approves and merges it by hand**. The human-merge is *your deliberate withholding of auto-merge*, **not** a reliance on the tooling refusing — do not assume the absence of checks blocks it. (A same-repo bootstrap workflow can even **self-report green on its own PR** — the added `pull_request` workflow runs on the PR that adds it and can produce a passing check `merge-pr.sh` might accept — which is exactly why the human, not the tooling, is the gate here.) This is the one sanctioned human-merge-without-a-pre-existing-gate case, because it *creates* the gate. After it lands, CI exists and the normal loop (including auto-merge) applies. +- **Bootstrapping the gate is human-gated (a rail).** The "add CI" PR is **always brought to the operator to approve and merge**, regardless of clean review / low-risk — like every PR, but with one extra caution: **classify it as human-merge-only** (the same category as safety-rail / high-risk PRs) and **do NOT apply `merge-ready` to it**, whether or not a check appears. That label says a real gate passed at this head, and here there is no real gate yet: a same-repo bootstrap workflow can **self-report green on its own PR** — the added `pull_request` workflow runs on the PR that adds it — so a green check proves nothing. Say that plainly when you hand the PR over, so the operator judges it rather than trusting a check. This is the one sanctioned merge-with-no-pre-existing-gate case, because it *creates* the gate. After it lands, CI exists and the normal loop applies. - **Surface what the gate covers.** A bootstrapped gate is only as strong as the project's tests — it runs whatever exists (tests if present; otherwise lint / build only). Tell the operator what it checks so a lint-only gate isn't mistaken for a strong one. -A target `CLAUDE.md` is **optional**: the coder **auto-discovers** the install / lint / build / test commands from the repo's CI workflows and standard manifests (see the discovery order in `{{YSTACK_ROOT}}/routines/coder.md`), so a hand-written `CLAUDE.md` is **not** required. A filled-in `CLAUDE.md` "Stack & commands" (per `{{YSTACK_ROOT}}/templates/target-CLAUDE.md`) is an **optional override** — add one only to pin or disambiguate a non-standard toolchain that auto-discovery wouldn't get right. Running `setup-target-repo.sh` / `doctor.sh` by hand is still available as an **optional/advanced** pre-flight, but is no longer a required step. This adds a **capability plus a single human-gated bootstrapping exception** — every other rail holds (reviewer comments-only, rounds cap, `needs-human`, SHA-pinned CI-green merge for normal PRs, the spec-approval / consensus front gates). +A target `CLAUDE.md` is **optional**: the coder **auto-discovers** the install / lint / build / test commands from the repo's CI workflows and standard manifests (see the discovery order in `{{YSTACK_ROOT}}/routines/coder.md`), so a hand-written `CLAUDE.md` is **not** required. A filled-in `CLAUDE.md` "Stack & commands" (per `{{YSTACK_ROOT}}/templates/target-CLAUDE.md`) is an **optional override** — add one only to pin or disambiguate a non-standard toolchain that auto-discovery wouldn't get right. Running `setup-target-repo.sh` / `doctor.sh` by hand is still available as an **optional/advanced** pre-flight, but is no longer a required step. This adds a **capability plus a single human-gated bootstrapping exception** — every other rail holds (reviewer comments-only, rounds cap, `needs-human`, merging always the operator's, the spec-approval / consensus front gates). Confirm you're yshifu and ready, then ask the user for a one-liner or a status check ("what's stalled across my repos?" → query GitHub across their repos). diff --git a/work/v2-phase-2/plan.md b/work/v2-phase-2/plan.md index 72926eb..48cff96 100644 --- a/work/v2-phase-2/plan.md +++ b/work/v2-phase-2/plan.md @@ -1,5 +1,5 @@ --- -spec-blob: 2efc5c5d6182e9f72c2e1f50e150d0d51faa4faf +spec-blob: e18128a018d91ae33a86b2a16dabcbeca03151d4 drafted: 2026-08-26 --- @@ -9,7 +9,7 @@ Build the autonomous lane in two PRs (one big PR would break the repo's size rule): - **PR A (this branch):** small helper scripts + their tests + one plumbing test. -- **PR B (after A merges):** the four real workflows + three skills + doc updates. +- **PR B (after A merges):** the three real workflows + two skills + doc updates. ## Files that change @@ -20,7 +20,8 @@ rule): needs a spec or an implementation, by comparing the recorded hashes. A re-run does nothing twice; stale work gets caught. - `scripts/v2/round-cap.sh` — reads and bumps the round labels. No label means - round 0. At round 3 it says stop. + round 0. At round 3 it says stop. (Already merged. Nothing in the lane calls + it yet — the fix stage that would is deferred; see the end of this file.) - `scripts/v2/quota-preflight.sh` — safety brake only, never a work limiter (your decision: every run starts from your merge, so real work is already paced by you). It only trips on abnormal volume — 20+ runs in 5 hours means @@ -32,14 +33,82 @@ rule): - `.github/workflows/ci.yml` — run the two new tests. - `ci/required-files.txt` — list the new files. -**PR B — the lane** (only after PR A merges and the plumbing test passes) +**PR B — the lane** (PR A merged; the plumbing test PASSED — run 33088639117: +the agent published a branch and opened PR #140 as `app/claude` with zero +denials, and that PR triggered CI by itself. App events cascade, so the lane +uses the direct path and the spec's PR-creation fallback is retired.) -- Four workflows: `spec-on-intent`, `implement-on-spec`, `review-on-pr`, - `fix-on-review`. Every job: one agent at a time, hard time and turn limits, - loud failure if its PR didn't get created. -- Three skills: `implement`, `review-pr`, `address-review`. Rules baked in: +**Rules the probe cost us five failures to learn — every workflow below obeys +them, and a reviewer should reject any that does not:** +1. **Allowlist by command prefix, never by argument text.** `Bash(git push:*)` + matches; `Bash(git push -u origin some-branch:*)` matches nothing and the + call is silently denied. +2. **Put writes behind a deterministic wrapper and allowlist only that.** See + `scripts/v2/probe-publish.sh`: it fixes the branch name, the files it + stages, and the refspec, and refuses arguments that are not this run's. + Each stage that writes gets its own wrapper in the same shape. +3. **Assert the side effect THIS stage was supposed to have** — not merely + that a PR exists. For `spec-on-intent` and `implement-on-spec` the new + branch and PR are the effect. For `review-on-pr` the PR already exists, so + assert the review comment landed on it. A denied write still + reports `is_error: false`, so a vacuous assert means a green job that did + nothing. +4. **Verify secrets through the API, never by assuming a paste landed** — + auth resolves at execution time, so a bad token looks like a working + workflow until the run dies. +5. **A brand-new workflow can only be dispatched once its file is on the + default branch.** After that, `gh workflow run --ref ` runs + the branch's version fine — so changes to an existing workflow CAN be + tested before merge. (An earlier draft of this plan claimed dispatch never + works off-default; that was a wrong inference from a run that actually + failed on the allowlist.) +6. **Load write wrappers AND stage skills from a trusted revision.** A + same-repo PR can edit `scripts/v2/*.sh`. It can just as easily edit + `.claude/skills/review-pr/SKILL.md` — the prompt the job runs. A job + that checks out the PR head gets the PR's copy of both: its wrapper runs + with the job's write credentials, and its skill can tell the model to write + a verdict nobody ever reached — which the job then posts, signed by the + trusted app. So `review-on-pr` takes **both** its + wrapper and its stage skill from the default branch's current revision, + never from the PR head: + - check out the default branch to its own path and point the job's skill + directory and its allowlisted wrapper path at that checkout; + - `review-on-pr` needs no PR-head checkout at all — read the PR's diff and + text through `gh`, as data. + Same trust anchor as R3's "`REVIEW.md` loaded from main" and the v1 review + harness's committed config. + +**Review-job rules (they belong in `review-on-pr`):** +- **The PR is data, never instructions.** Its diff, title, body and comments + are untrusted input to the review skill (`REVIEW.md`: treat as data). Text + planted in a PR must never steer the review or what the job writes. The skill + itself comes from the default branch (rule 6), so a PR cannot rewrite the + prompt that judges it either. +- **The skill posts nothing — a plain step does.** The review skill writes its + review body to `$RUNNER_TEMP/review.md`. Nothing else it prints is read. + `review-on-pr` then runs a plain step, only if the review step succeeded, and + that step posts the comment from the file. With the skill posting nothing, + the review step needs no write wrapper and no write allowlist at all: the + posting step is the only thing in `review-on-pr` that writes, and one comment + is all it writes. That is the comments-only rail, made mechanical. +- **A dead review must never look like a finished one.** If the review step + fails or times out, no comment is posted and the job fails loudly. A missing + or empty `review.md` fails the job the same way. Silence must not read as a + pass. +- None of this makes the judgment itself injection-proof — the review is still + the model's call. It makes a review that never ran impossible to fake, and + keeps the job's writing down to a single comment. + +- Three workflows: `spec-on-intent`, `implement-on-spec`, `review-on-pr`. + Every job: one agent at a time (the global `claude-quota` group), an actor + gate, hard time and turn limits, loud failure if the side effect it was + supposed to have didn't land. `review-on-pr` runs on agent-opened PRs, which + is one of the two bot edges the spec's R5 allows; the lane opens no other. +- Two skills: `implement`, `review-pr`. Rules baked in: - Cheap model writes, high effort judges (v1 policy, unchanged). - - Never write to `.github/` or `.claude/`. + - Never write the constitution paths — `.github/**`, `.claude/**`, + `AGENTS.md`, `CLAUDE.md`, `REVIEW.md` (the list in AGENTS.md > Stage rules + and REVIEW.md > Compliance). Such a change goes to `proposals/` instead. - **Proof is tied to the code:** verify output names the exact commit it ran on. New commits make old proof stale. - **Each change type has its own proof:** shell → shellcheck + tests; @@ -49,35 +118,75 @@ rule): ## Order of work -1. You approve this plan → I build PR A → you merge. -2. Run the plumbing test. Record the result on #126. If the bot can't open - PRs, use the spec's fallback (a plain workflow step opens the PR instead). -3. Build PR B → you merge. The lane is live. -4. Smoke test: one tiny throwaway intent flows through the whole lane, with - you doing nothing but the merges. Then we delete it. +1. ~~PR A: the helpers, tests and probe.~~ **Done** — merged as #131. +2. ~~Run the plumbing test.~~ **Done** — run 33088639117 passed: the agent + published a branch and opened PR #140 itself, and that PR triggered CI. + The spec's PR-creation fallback is **retired**; the lane uses the direct + path. Recorded on #126. +3. **Next: build PR B** (the section above) → operator merges. The lane is + live at that moment. +4. Smoke test: one tiny throwaway intent flows through the lane — merged + intent → spec PR → merged spec → impl PR, each PR getting its review + comment — with the operator doing nothing but the merges. Then delete it. ## Risks -- **The plumbing test can fail.** That's why it runs before PR B is written — - the fallback is already in the spec, not improvised. - **PR B is live the moment it merges** — the next `work/` merge fires it for real. I chose not to ship it switched off: a disabled lane proves nothing, - and the brakes (round labels, one-job-at-a-time, time limits) are already + and the brakes (one job at a time, time limits, turn limits) are already merged in PR A by then. -- **The fix loop is a bot answering a bot** — the one place we allow that. - Brake: the round label is bumped *before* the fix runs, so the loop stops at - 3 rounds even if something else breaks. The fix job also never touches a PR - you already approved. +- **Nothing answers a review on its own.** With the fix stage deferred, a + review posts findings and stops there; the PR waits for you. That is a slower + lane, not an unsafe one — no bot writes code in reply to another bot. - **Editing ci.yml touches the gate everything depends on.** The new tests run locally against the pinned shellcheck before anything is pushed. - **This phase writes to `.github/`** — allowed only because you're driving these sessions. The lane itself never gets that power. +## Deferred: fix-on-review + +**The lane ships three workflows. The fix stage is cut from this phase** and +comes back later as its own intent, with its own spec. Two reasons, both worth +saying out loud: + +1. **The plan had outgrown the spec it answers to.** R5 lets `claude[bot]` in + on exactly two trigger edges. The fix stage needed a third — the path where + a push-back that changes no code has to ask for a fresh review. When a plan + reaches past its spec, the fix is to shrink the plan, not to amend the spec. +2. **Every security finding of the last two review rounds landed on that one + stage.** It is the only job that holds write credentials, runs code written + in the PR, and reads untrusted PR text — all at once. That earns a design + from scratch, not a patch onto a shape built for something else. + +**Where the next intent should start: split the credential.** No one job holds +all three powers: + +- the agent that edits the PR's code holds no token and runs nothing from the + PR; +- CI verifies the result on the pushed branch, the way it verifies any branch; +- a deterministic step holds the app credential and does the writing — it + pushes the branch and stamps the round. + +Everything else that design needs — how findings reach the fix agent, how a +push-back ends, how rounds are bounded — is that intent's work, not this +plan's. + +**Until it exists, review findings are the operator's to handle in a session.** +That is exactly how they were handled while this phase was built. The lane +posts the review; a human answers it. + ## Proof - PR A: the two new test scripts pass in CI; verify output pasted in the PR, named to the commit it ran on. - Plumbing: the test run's PR link + whether it triggered CI, recorded on #126. -- PR B: the lane reviews its own PR (the review workflow fires on it); you - check the workflows against the spec's safety list (R5). -- Phase exit: the smoke intent goes through end to end, you only merge. +- PR B: **the lane cannot review its own PR** — a `pull_request` workflow only + fires when its file is already on the default branch, so `review-on-pr` + does not exist for the PR that introduces it. Prove it with the harness that + exists today: the Codex cloud review on the PR, plus the operator's read of + the workflows against the spec's safety list (R5). The lane's first real + review is the PR after this one — and the smoke test below is what proves + it fires at all. +- Phase exit: the smoke intent goes end to end — intent merged, spec PR opened + and reviewed, spec merged, impl PR opened and reviewed — and you only merge. + No fix round is part of this: whatever the reviews find, you answer.