From a3928bc64aa8b186dee27d0e11de28a8081c8b10 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 01:14:50 +0400 Subject: [PATCH 01/10] =?UTF-8?q?docs(openspec):=20change=20enforce-upstre?= =?UTF-8?q?am-loop=20=E2=80=94=20proposal=20=D0=B8=20design=20=D0=B3=D0=B5?= =?UTF-8?q?=D0=B9=D1=82=D0=BE=D0=B2=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../enforce-upstream-loop/.openspec.yaml | 2 + .../changes/enforce-upstream-loop/design.md | 84 +++++++++++++++++++ .../changes/enforce-upstream-loop/proposal.md | 63 ++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 openspec/changes/enforce-upstream-loop/.openspec.yaml create mode 100644 openspec/changes/enforce-upstream-loop/design.md create mode 100644 openspec/changes/enforce-upstream-loop/proposal.md diff --git a/openspec/changes/enforce-upstream-loop/.openspec.yaml b/openspec/changes/enforce-upstream-loop/.openspec.yaml new file mode 100644 index 0000000..7ccf7c3 --- /dev/null +++ b/openspec/changes/enforce-upstream-loop/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-25 diff --git a/openspec/changes/enforce-upstream-loop/design.md b/openspec/changes/enforce-upstream-loop/design.md new file mode 100644 index 0000000..2d57a7f --- /dev/null +++ b/openspec/changes/enforce-upstream-loop/design.md @@ -0,0 +1,84 @@ +# Design: enforce-upstream-loop — технические гейты цикла + +Issue: #59. Статус: **черновик до builder-check**; владелец 25.10.2026: +«оба гейта сразу», согласования отменены — решения ниже фиксируются в +«Decisions made without discussion» и действуют как решения владельца. + +## Контекст + +Сессия ce9187ca: модель пропустила builder-check, рационализировала +пропуск, план появился до утверждения дизайна. Цель — гейты, которые +нельзя пропустить текстом. + +## Артефакты и их писатели + +| Артефакт | Пишет | Когда | +|---|---|---| +| `openspec/changes//builder-check.md` | контроллер (отчёт свежего субагента, скопированный в change) | после builder-check, до передачи дизайна владельцу | +| `openspec/changes//approval.md` | только команда `/approve` | владелец утверждает design/plan | +| План `docs/superpowers/plans/*.md` | writing-plans-флоу | после design-утверждения (гейт A) | +| Диспатч implementer'а | sdd-флоу | после plan-утверждения (гейт B) | + +Формат `approval.md` (yaml): + +```yaml +design: + by: aep + at: 2026-10-25T10:00:00+04:00 +plan: + by: aep + at: 2026-10-25T11:30:00+04:00 +``` + +## Механика гейтов (в бандле, DSH-only) + +`ctx.tools.guard` уже зовёт bash-хуки (`lib/guards.js`). Новая +`lib/gates.js` добавляет **content-проверки** после хуков, в той же +цепочке (порядок: guard-main/require-plan → approval-gates; оба — deny с +причиной, монотонно): + +1. **Гейт A (план до approve дизайна).** Для Edit/Write/MultiEdit с + путём под `docs/superpowers/plans/`: из содержимого (`tool_input.content` + или `new_string`) извлечь `**Spec:** <путь>`; если путь указывает в + `openspec/changes//` и `approval.md` без секции `design:` — DENY + «design not approved; run /approve design». Содержимое без + Spec-ссылки на change — пропуск. +2. **Гейт B (sdd до approve плана).** Для `subagent`/`subagent_fork`: + в промпте найти ссылку на план — `docs/superpowers/plans/<имя>.md` + или `.superpowers/sdd/<план>/`; из плана взять `**Spec:**` → + change; `approval.md` без секции `plan:` — DENY «plan not approved; + run /approve plan». Нет ссылки на план — пропуск. +3. **`/approve`** (`command-approve.js`): `design` требует существование + proposal.md, design.md, builder-check.md; `plan` требует секцию + `design:` и существование ровно одного плана со Spec на этот change + (несколько — отказ с перечислением). Дописывает/обновляет секцию, + остальные секции не трогает. Владелец-агент различие: команда пишет + `by: owner`; вызовы не от владельца — отказ (как отличить: DSH не + даёт ролей; используем факт вызова из интерактивной сессии и + AskUserQuestion-подтверждение: `/approve` спрашивает подтверждение + перед записью, ответ «да» = владелец). + +## Decisions made without discussion + +| # | Решение | Обоснование | Цена ошибки | +|---|---|---|---| +| D-1 | `/approve` — единственный писатель approval.md | Маркер, который не может подделать модель (нет прямых записей в approval.md — гейт A защищает и его? нет: approval.md не в plans/; добавлю guard: любые Edit/Write в `**/approval.md` из сессии — DENY, кроме команды) | Обход остаётся через API-инструменты — принимаем как остаточный риск, фиксируем в спеке | +| D-2 | Гейты в бандле (DSH-only), CC-хуки не трогаем | CC-поток не должен меняться; один enforcement-слой на рантайм | Два мира правил (CC/DSH) — уже так, осознанно | +| D-3 | Презентация дизайна владельцу не гейтится напрямую; закрыта цепочкой артефактов | Гейтить сообщения модели технически нельзя; цепочка: builder-check → approve → план → sdd делает пропуск видимым на первом же BLOCKED | Модель может «показать» дизайн без builder-check.md — тогда `/approve` откажет, и это видно владельцу | +| D-4 | `/approve` требует подтверждения (вопрос владельцу) перед записью | Отделить владельца от модели в DSH (ролей нет) | Лишний клик на каждое утверждение — приемлемо | +| D-5 | builder-check.md пишет контроллер после завершения субагента | Единственная точка, где отчёт субагента гарантированно сохранён в change | Файл можно подделать вручную — принимаем: гейт против случайного пропуска, не против злого умысла | +| D-6 | Гейты не срабатывают для bounded-задач (нет change/плана) | Bounded-цикл не проходит лупа и не должен блокироваться | Никакого | + +## Тесты (идеи для плана) + +- parse approval.md / отказы / до-записи существующих секций; +- гейт A: write плана со Spec на неутверждённый change → DENY; без Spec → OK; утверждённый → OK; +- гейт B: subagent с промптом-ссылкой на план: без plan-секции → DENY, с ней → OK; промпт без ссылки → OK; +- `/approve design` без builder-check.md → отказ; полный флоу design→plan → OK; +- guard на записи в `**/approval.md` вне команды → DENY. + +## На потом (в план) + +- wiring `lib/gates.js` в `ctx.tools.guard` цепочку + конфиг `approvalGates: true`; +- `command-approve.js` + регистрация; sdd-skill текст; roles-заметка; +- спека-дельта capability `approval-gates`; README/PORT; тесты. diff --git a/openspec/changes/enforce-upstream-loop/proposal.md b/openspec/changes/enforce-upstream-loop/proposal.md new file mode 100644 index 0000000..d31bdb3 --- /dev/null +++ b/openspec/changes/enforce-upstream-loop/proposal.md @@ -0,0 +1,63 @@ +# Proposal: enforce-upstream-loop — технические гейты цикла + +Issue: #59. Источники: сессия ce9187ca 24.10.2026 (модель пропустила +builder-check и рационализировала пропуск; спека и план ушли владельцу одним +пакетом), решение владельца 25.10.2026 «оба гейта сразу», согласования по +этой фиче владельцем отменены (решения — в design.md). + +## Why + +Луп brainstorm→design→builder-check→approve→plan→approve→sdd существует в +superpowers 7.0.0 как инструкции. В DSH-сессии 24.10.2026 модель пропустила +builder-check, предложила его «вхолостую» и сняла предложение; план был +написан до того, как владелец утвердил дизайн. Инструкции не являются +механизмом: нужен технический гейт, который нельзя рационализировать. + +## What Changes + +1. **Артефакты**: `openspec/changes//builder-check.md` (отчёт свежего + builder-check субагента) и `openspec/changes//approval.md` (маркер + утверждения, yaml-секции `design:` и `plan:` с `by`/`at`). +2. **Команда `/approve design|plan`** в бандле — единственный + писатель `approval.md`: + - `design` — отказ, пока нет `proposal.md`+`design.md`+`builder-check.md`; + - `plan` — отказ, пока нет design-утверждения и файла плана в + `docs/superpowers/plans/`, чей `**Spec:**` указывает на этот change. +3. **Гейт A (вход)**: bundle-guard отклоняет Edit/Write/MultiEdit в + `docs/superpowers/plans/**`, если в содержимом есть строка `**Spec:**` + со ссылкой на change без design-утверждения — план физически не + создаётся до утверждения дизайна. «Уход к владельцу» закрывается цепочкой: + approve дизайна требует builder-check; план требует approve дизайна; + исполнение требует approve плана. +4. **Гейт B (исполнение)**: bundle-guard отклоняет диспатчи `subagent`/ + `subagent_fork`, чей промпт ссылается на план в + `docs/superpowers/plans/` или на `.superpowers/sdd//`, если + `approval.md` change'а не содержит `plan:` — sdd не стартует без + утверждённого плана. Причина отказа: «plan not approved; run + `/approve plan`». +5. **Тексты**: `dsh/skills/sdd/SKILL.md` — шаг «проверь approval-маркер до + первого диспатча, при BLOCKED не обходи»; `roles/dev.md` (DSH-заметка + `--roles`) — builder-check обязателен, пропуск — только явное решение + владельца. + +## Impact + +- Affected: `dsh/bundle/` (новая `lib/gates.js`, `command-approve.js`, + wiring в `index.js`), `dsh/skills/sdd/SKILL.md`, `dsh/tests/`, + `dsh/install.sh` (`--roles`-заметка), спека-дельта (новая capability + `approval-gates`), `dsh/README.md`/`dsh/PORT.md`. +- Не трогаем: CC-сторону (хуки, роли, скрипты) — гейты живут только в + DSH-бандле; bounded-задачи без change/плана не задеваются. +- Breaking: нет для существующих потоков с утверждёнными change/планами; + для неутверждённых — ожидаемый BLOCKED с подсказкой `/approve`. + +## Non-goals + +Свой UI апрува; переписывание sdd-протокола; гейт на сообщения модели +(презентация дизайна владельцу не перехватывается — закрыта цепочкой +артефактов); CC-сторона. + +## Открытые вопросы + +Нет: решения по enforcement-точкам приняты (design.md, «Decisions made +without discussion»), владелец согласования отменил до пробуждения. -- 2.47.2 From 4cbcff1c8fffe3d5a01471804e9e2173b06ad43b Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 01:22:00 +0400 Subject: [PATCH 02/10] =?UTF-8?q?docs:=20builder-check=20=D0=BE=D1=82?= =?UTF-8?q?=D1=87=D1=91=D1=82,=20=D0=BF=D0=BB=D0=B0=D0=BD=20enforce-upstre?= =?UTF-8?q?am-loop=20=E2=80=94=204=20=D0=B7=D0=B0=D0=B4=D0=B0=D1=87=D0=B8?= =?UTF-8?q?=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../plans/2026-10-25-enforce-upstream-loop.md | 182 ++++++++++++++++++ .../enforce-upstream-loop/builder-check.md | 19 ++ .../changes/enforce-upstream-loop/design.md | 7 + 3 files changed, 208 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md create mode 100644 openspec/changes/enforce-upstream-loop/builder-check.md diff --git a/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md b/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md new file mode 100644 index 0000000..16f3727 --- /dev/null +++ b/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md @@ -0,0 +1,182 @@ +# Enforce-upstream-loop Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use the `sdd` skill (DSH) to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Добавить в DSH-бандл технические гейты upstream-лупа: `/approve` (единственный писатель approval.md), гейт A (план не пишется без утверждённого дизайна), гейт B (sdd/диспатчи не стартуют без утверждённого плана), гейт C (approval.md не пишется инструментами), включая bash-обходы. + +**Architecture:** Чистое ядро `dsh/bundle/lib/gates.js` (decision-функции + io approval.md + разрешение план→change→approval + sha256) поверх существующего `ctx.tools.guard`; `/approve` — команда с подтверждением через `ctx.userQuestions.ask`; Bash-цели — через проверенный `hooks/harness_target.py targets`; wiring — в `index.js` после хуков (монотонная цепочка deny). CC-сторона не трогается. + +**Tech Stack:** Node builtins (без `@deepseek-ai/*`-импортов), python3 для `targets`, bash для тестовых песочниц. + +**Spec:** `openspec/changes/enforce-upstream-loop/` — [proposal.md](openspec/changes/enforce-upstream-loop/proposal.md) + [design.md](openspec/changes/enforce-upstream-loop/design.md) (D-1…D-13; план спорит от них). + +**Models (D-9 стоящее решение владельца):** Tasks 1, 2, 4 — `deepseek-official/deepseek-flash`; Task 3 — `deepseek-official/deepseek-v4-pro`; все ревью — `deepseek-official/deepseek-v4-pro`. + +## Global Constraints + +- Runtime-зависимости — только Node builtins; `package.test.mjs` enforcement обновляется при новых файлах. +- Гейты — DSH-only (бандл); CC-хуки/роли/скрипты не меняются. +- Гейты монотонны: отказ не отменяется последующим слушателем; ошибка разбора — пропуск (как в хуках). +- bounded-задачи (.bounded, нет change/плана) не блокируются (D-6). +- Маркеры находок и статусы — по `~/.dsh/roles/dev.md`; коммиты с `(#59)`. +- Язык субагентов — английский (R-8 от #69); маркеры/статусы/термины — без перевода; отчёты владельцу — по-русски. + +## Review Focus + +| # | Вход/условие | Ожидаемое поведение | +|---|---|---| +| RF1 | Write плана в `docs/superpowers/plans/` без `**Spec:**` (или legacy-путь) | DENY (D-9) | +| RF2 | Write плана со Spec на change без design-секции | DENY «design not approved» (гейт A) | +| RF3 | `subagent`/`workflow` с план-ссылкой при sha256-расхождении или без plan-секции | DENY «plan not approved / changed» (гейт B, D-10) | +| RF4 | Write в `**/approval.md` инструментами (в т.ч. bash heredoc) | DENY (гейт C, D-1/D-8) | +| RF5 | `/approve design` без builder-check.md; `/approve plan` без design-секции или до плана | Отказ с причиной (D-5/D-1) | + +--- + +### Task 1: Ядро gates.js (чистые decision-функции + approval io) + +**Files:** +- Create: `dsh/bundle/lib/gates.js` +- Create: `dsh/tests/gates.test.mjs` + +**Interfaces:** +- Consumes: ничего (чистые функции + fs только в io-функциях). +- Produces: + - `parseApproval(text) -> { design?: {by,at}, plan?: {by,at,sha256} }` (yaml-подмножество; толерантен к отсутствию файла → `{}`). + - `serializeApproval(parsed) -> string` (детерминированный вывод секций). + - `extractSpec(content) -> string|null` — строка `**Spec:** <путь>` из markdown (варианты: бэктики, трейлинг-слэш; нормализация). + - `resolveChangeFromSpec(specPath, repoRoot) -> changeId|null` — `openspec/changes//...` → id; иначе null (legacy `docs/superpowers/specs/` — null). + - `resolvePlanFromSdd(sddPath, repoRoot) -> planPath|null` — `.superpowers/sdd/<план>/...` → `/docs/superpowers/plans/<план>.md` если существует. + - `planApproved(approval, planPath) -> {ok:boolean, reason:string}` — есть `plan:` и `sha256 === hashPlan(planPath)`; `hashPlan(path) -> string` (sha256 содержимого). + - `checkPlanWrite({tool, filePath, content, repoRoot}) -> {deny?: string}` — гейт A: path под `docs/superpowers/plans/`; Spec извлечён → change → approval.md: нет design → deny «design not approved; run /approve design»; Spec нет/legacy → deny (D-9); иначе ok. + - `checkApprovalWrite({tool, filePath, repoRoot}) -> {deny?: string}` — гейт C: basename `approval.md` → deny «approval.md is written only by /approve». + - `checkDispatch({tool, prompt, script, repoRoot}) -> {deny?: string}` — гейт B: план-ссылки в prompt/script → план → Spec → change → `planApproved`; fail → deny «plan not approved; run /approve plan» / «plan changed after approval; re-approve». + - `checkBashTargets(targets, repoRoot)` — гейты A/C по списку файловых целей (из `harness_target.py targets`). + +- [ ] **Step 1: Падающие тесты** в `dsh/tests/gates.test.mjs` (tempDir-фикстуры: репозиторий с `docs/superpowers/plans/`, `openspec/changes/demo/`, approval.md): + +```js +test('extractSpec: backticks and trailing slash normalize', ...) // '**Spec:** `openspec/changes/demo/`' -> 'openspec/changes/demo' +test('checkPlanWrite: no Spec -> deny', ...) // RF1 +test('checkPlanWrite: legacy docs/superpowers/specs -> deny', ...) +test('checkPlanWrite: design approved -> ok', ...) +test('checkPlanWrite: design not approved -> deny with /approve hint', ...) // RF2 +test('checkApprovalWrite: approval.md via tool -> deny', ...) // RF4 +test('checkDispatch: prompt plan ref, plan approved -> ok', ...) +test('checkDispatch: no plan section -> deny', ...) // RF3 +test('checkDispatch: sha256 mismatch after edit -> deny re-approve', ...) // RF3 +test('checkDispatch: workflow script with plan ref -> deny/ok same as subagent', ...) +test('planApproved: happy / missing / mismatch', ...) +test('serialize/parse roundtrip preserves sections', ...) +``` + +- [ ] **Step 2: Прогони — FAIL** (`node --test dsh/tests/gates.test.mjs`, модуля нет). +- [ ] **Step 3: Реализуй `lib/gates.js`** по интерфейсам. Io-функции читают ФС только по переданным путям; никаких сетевых/глобальных сайд-эффектов. +- [ ] **Step 4: Прогони — PASS**; затем `bash dsh/tests/run.sh` (baseline 110/0). +- [ ] **Step 5: Коммит** + +```bash +git add dsh/bundle/lib/gates.js dsh/tests/gates.test.mjs +git commit -m "feat(dsh): gates — чистые decision-функции гейтов A/B/C и approval io (#59)" +``` + +### Task 2: Команда /approve + +**Files:** +- Create: `dsh/bundle/command-approve.js` +- Create: `dsh/tests/approve.test.mjs` + +**Interfaces:** +- Consumes: `parseApproval/serializeApproval` (Task 1), `ctx.commands.register` (object-form, как command-role.js), `ctx.userQuestions.ask({...}) -> Promise<{confirmed:boolean}>`-подобное (фейк в тестах). +- Produces: `registerApproveCommand({commands, userQuestions, repoRoot, logger})` — команда `/approve design|plan`: + - `design`: требует `/openspec/changes//proposal.md`, `design.md`, `builder-check.md`; отказ — текст с перечнем отсутствующего. + - `plan`: требует design-секцию в approval.md и ровно один план со Spec на change (несколько — отказ с перечнем); пишет `plan: {by:'owner', at:, sha256:hashPlan(plan)}`. + - Оба: перед записью — `userQuestions.ask` (текст «Approve ?»); нет подтверждения — отказ «not confirmed»; отсутствует userQuestions — отказ «userQuestions service unavailable». + - Запись: read→parse→обновить секцию→serialize→write; чужие секции не трогает. + +- [ ] **Step 1: Падающие тесты** (fake commands.register захватывает handler; fake userQuestions возвращает confirmed true/false; temp-репо): + +```js +test('approve design refuses without builder-check.md', ...) // RF5 +test('approve design writes design section (by owner, at ISO)', ...) +test('approve design refuses without confirmation', ...) +test('approve plan refuses without design section', ...) // RF5 +test('approve plan refuses when two plans reference the change', ...) +test('approve plan writes plan section with sha256 of the plan', ...) +test('approve refuses when userQuestions missing', ...) +``` + +- [ ] **Step 2: Прогони — FAIL.** +- [ ] **Step 3: Реализуй `command-approve.js`** по интерфейсам (парсинг аргументов ` `; неизвестный kind → текст usage). +- [ ] **Step 4: Прогони — PASS**; `bash dsh/tests/run.sh`. +- [ ] **Step 5: Коммит** + +```bash +git add dsh/bundle/command-approve.js dsh/tests/approve.test.mjs +git commit -m "feat(dsh): /approve — единственный писатель approval.md с подтверждением владельца (#59)" +``` + +### Task 3: Wiring в index.js + bash-цели + списки пакета + +**Files:** +- Modify: `dsh/bundle/index.js` +- Modify: `dsh/tests/selfcheck.test.mjs` (apply-тесты гейтов) +- Modify: `dsh/bundle/package.json` (`files` += `command-approve.js`) +- Modify: `dsh/tests/run.sh` (`node --check` += `command-approve.js`) +- Modify: `dsh/tests/package.test.mjs` (no-`@deepseek-ai`-список += новые файлы) + +**Interfaces:** +- Consumes: Task 1 (gates.js), Task 2 (command-approve.js), существующая цепочка `ctx.tools.guard` (lib/guards.js). +- Produces: в `apply()`: + - `DEFAULTS += { approvalGates: true }`; gatesOk — из selfcheck (tools.guard). + - В guard-цепочке после hook-проверок: Edit/Write/MultiEdit → `checkPlanWrite` + `checkApprovalWrite`; `subagent`/`subagent_fork`/`workflow` → `checkDispatch` (prompt/script из `exec.arguments`); `Bash` → если в команде грубая подстрока `docs/superpowers/plans` или `approval.md` — `spawnSync python3 hooks/harness_target.py targets '' `, цели → `checkBashTargets`. Любой deny → причина отказа (hook-стиль `BLOCKED [approval-gates]: …`). Ошибка/пропуск парсинга — allow (инвариант хуков). + - `/approve` регистрируется в том же `ctx.inject(['commands', 'userQuestions'])` (userQuestions может отсутствовать — команда сама откажет, D-12). + - `repoRoot` для гейтов — cwd сессии (`sessionCwd`), как в guards. + +- [ ] **Step 1: Падающие тесты** (в selfcheck.test.mjs, fake ctx с tools.guard-захватом: apply() → вызвать захваченный guard с fake exec; temp-репо-фикстуры): + +```js +test('apply: guard denies plan write without design approval', ...) +test('apply: guard allows plan write after /approve-like approval.md', ...) +test('apply: guard denies subagent dispatch referencing unapproved plan', ...) +test('apply: guard allows dispatch after plan approval (sha match)', ...) +test('apply: bash heredoc into approval.md denied', ...) // RF4 +test('apply: approvalGates:false disables all gates', ...) +``` + +- [ ] **Step 2: Прогони — FAIL.** +- [ ] **Step 3: Реализуй** wiring + списки пакета (файлы выше). Монотонность: порядок deny не отменяется; пропуск на ошибку разбора. +- [ ] **Step 4: Прогони — PASS**; `bash dsh/tests/run.sh` (110+новые) и `bash tests/run.sh` (CC 630/0 — хуки не трогаются). +- [ ] **Step 5: Коммит** + +```bash +git add dsh/bundle/index.js dsh/bundle/package.json dsh/tests/selfcheck.test.mjs dsh/tests/run.sh dsh/tests/package.test.mjs +git commit -m "feat(dsh): wiring гейтов A/B/C в guard-цепочку + bash-цели + /approve (#59)" +``` + +### Task 4: Скиллы, роли, спека-дельта, доки + +**Files:** +- Modify: `dsh/skills/sdd/SKILL.md` (шаг до первого диспатча: проверить approval-маркер плана; BLOCKED → не обходить, вернуть NEEDS_CONTEXT/владельцу) +- Modify: `dsh/install.sh` (`install_role_dsh_notes` — DSH-заметка в roles/dev.md: builder-check обязателен, `/approve` — маркер, гейты A/B/C и их DENY-тексты) +- Modify: `openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md` (новая capability: ADDED требования — артефакты, /approve-контракт, гейты A/B/C, bash-обход, bounded-исключение, deployment D-13) +- Modify: `dsh/README.md` (Configuration += `approvalGates`; Known limits — гейты активны только в сессиях с новым бандлом), `dsh/PORT.md` (§1/§7 — чем закрыт риск «инструкции ≠ enforcement») + +**Interfaces:** Consumes: поведение Tasks 1–3. Produces: спека-дельта (валидируется `openspec validate enforce-upstream-loop --strict`), тексты скилла/роли, доки. + +- [ ] **Step 1: Тесты** — в `dsh/tests/` добавить assert: `dsh/skills/sdd/SKILL.md` содержит строку «approval» / «/approve» (текстовый пин), `dsh/install.sh --roles` вставляет заметку с «/approve» (расширить существующий install-тест или добавить в install.test.mjs). +- [ ] **Step 2: Прогони — FAIL.** +- [ ] **Step 3: Реализуй** тексты и дельту; `openspec validate enforce-upstream-loop --strict` — valid. +- [ ] **Step 4: Прогони — PASS**: `bash dsh/tests/run.sh`, `bash tests/run.sh`, `openspec validate … --strict`. +- [ ] **Step 5: Коммит** + +```bash +git add dsh/skills/sdd/SKILL.md dsh/install.sh dsh/tests openspec/changes/enforce-upstream-loop/specs dsh/README.md dsh/PORT.md +git commit -m "docs(dsh): sdd-скилл, roles-заметка, спека approval-gates, доки (#59)" +``` + +## Предусловия окружения (контроллер проверяет один раз до Task 1) + +- `python3`, `git`, `node` в PATH; `bash dsh/tests/run.sh` зелёный на входной точке ветки `feat/59-loop-gates` (baseline 110/0). +- Сеть не нужна; тесты — temp-репозитории; реальный `~/.dsh` не трогается. +- Гейты в этой ветке ещё НЕ активны (D-13): исполнение идёт на старом рантайме, retro-утверждений не требуется; переустановка бандла после merge — шаг чеклиста закрытия. diff --git a/openspec/changes/enforce-upstream-loop/builder-check.md b/openspec/changes/enforce-upstream-loop/builder-check.md new file mode 100644 index 0000000..a64d506 --- /dev/null +++ b/openspec/changes/enforce-upstream-loop/builder-check.md @@ -0,0 +1,19 @@ +# Builder-check: enforce-upstream-loop + +Дата: 25.10.2026. Свежий субагент (v4-pro, сессия ff61ea11). Результат — +6 вопросов; все адьюдицированы контроллером (владелец делегировал +согласования до пробуждения) в design.md как D-7…D-13: + +1. На чём срабатывает гейт B → D-7: write в `.superpowers/sdd//**` + + диспатчи subagent/subagent_fork/workflow с план-ссылками. +2. Bash-обход в скоупе? → D-8: да, через `harness_target.py targets`. +3. Spec-less планы → D-9: DENY по умолчанию. +4. workflow гейтится → D-7: да, тот же канал. +5. Утверждение переживает правки плана → D-10: sha256 в `plan:`. +6. Бутстрап in-flight change → D-13: гейты активны только в сессиях с + новым бандлом; #59 исполняется на старом рантайме; retro-утверждений + не требуется. + +Подтверждено рантаймом: `ctx.tools.guard` видит subagent-инструменты +(name + arguments.prompt/script); `ctx.userQuestions.ask` — блокирующее +подтверждение владельца (DELEGATED_CALLER отклоняется). diff --git a/openspec/changes/enforce-upstream-loop/design.md b/openspec/changes/enforce-upstream-loop/design.md index 2d57a7f..9fe0db7 100644 --- a/openspec/changes/enforce-upstream-loop/design.md +++ b/openspec/changes/enforce-upstream-loop/design.md @@ -68,6 +68,13 @@ plan: | D-4 | `/approve` требует подтверждения (вопрос владельцу) перед записью | Отделить владельца от модели в DSH (ролей нет) | Лишний клик на каждое утверждение — приемлемо | | D-5 | builder-check.md пишет контроллер после завершения субагента | Единственная точка, где отчёт субагента гарантированно сохранён в change | Файл можно подделать вручную — принимаем: гейт против случайного пропуска, не против злого умысла | | D-6 | Гейты не срабатывают для bounded-задач (нет change/плана) | Bounded-цикл не проходит лупа и не должен блокироваться | Никакого | +| D-7 | Гейт B = два механизма: (a) write в `.superpowers/sdd//**` (ledger — старт sdd в Mode 1) → план → Spec → change → plan-утверждение; (b) диспатчи `subagent`/`subagent_fork`/`workflow` с план-ссылкой в промпте/скрипте → та же проверка | Mode 1 передаёт брифы файлами, промпт короткий без пути плана; ledger пишет контроллер — write-гейт реально ловит старт sdd | Ложный блок при смене имён каталогов — митигируется нормализацией путей | +| D-8 | Bash-обход закрыт: для Bash-вызовов gates.js прогоняет `python3 hooks/harness_target.py targets '' ` и применяет гейты A/C к целям под `docs/superpowers/plans/` и `**/approval.md` | Тот же парсер, что в хуках (heredoc-aware), без дублирования логики | Стоимость — subprocess на каждый Bash-вызов с подозрительным текстом; фильтруем до прогона по грубой подстроке | +| D-9 | План без `**Spec:**` в `openspec/changes/` — DENY по умолчанию (включая legacy `docs/superpowers/specs/`) | Реальные планы всегда несут Spec; .bounded-поток планов не пишет — проверено по репозиторию | Ложный блок легитимного Spec-less плана — такого флоу нет | +| D-10 | `plan:` в approval.md несёт `sha256` файла плана; гейт B сверяет хэш — план изменён после утверждения → DENY «plan changed after approval; re-approve» | Утверждение привязывается к содержимому исполняемого объекта | Правки плана после approve требуют re-approve — осознанно | +| D-11 | `design:` — только существование секции (без хэша): design.md дополняется решениями по ходу | Контент дизайна не является исполняемым объектом | Правка дизайна после approve не отзывается — принято | +| D-12 | Подтверждение владельца в `/approve` — через `ctx.userQuestions.ask` (блокирующий вопрос); сервис добавляется в inject; отсутствует — `/approve` отказывает с сообщением | Проверенный примитив рантайма (DELEGATED_CALLER не пройдёт) вместо эвристики «интерактивная сессия» | Нет userQuestions в профиле — /approve недоступен, гейты всё равно работают на блокировку | +| D-13 | Бутстрап: гейты активны только в сессиях с новым бандлом (после установки/рестарта); сама фича #59 исполняется на старом рантайме — ретро-утверждений не требует; для будущих change контроллер кладёт builder-check.md штатно | Развёртывание разводит enforcement и исполняемую фичу во времени | Окно до переустановки бандла — гейтов нет; фиксируем шаг переустановки в чеклисте закрытия | ## Тесты (идеи для плана) -- 2.47.2 From 99d7238d90c741d0ec12befa06f0d2b8459533a3 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 01:28:30 +0400 Subject: [PATCH 03/10] =?UTF-8?q?feat(dsh):=20gates=20=E2=80=94=20=D1=87?= =?UTF-8?q?=D0=B8=D1=81=D1=82=D1=8B=D0=B5=20decision-=D1=84=D1=83=D0=BD?= =?UTF-8?q?=D0=BA=D1=86=D0=B8=D0=B8=20=D0=B3=D0=B5=D0=B9=D1=82=D0=BE=D0=B2?= =?UTF-8?q?=20A/B/C=20=D0=B8=20approval=20io=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/bundle/lib/gates.js | 388 +++++++++++++++++++++++++++++++++++++++ dsh/tests/gates.test.mjs | 369 +++++++++++++++++++++++++++++++++++++ 2 files changed, 757 insertions(+) create mode 100644 dsh/bundle/lib/gates.js create mode 100644 dsh/tests/gates.test.mjs diff --git a/dsh/bundle/lib/gates.js b/dsh/bundle/lib/gates.js new file mode 100644 index 0000000..b3deb48 --- /dev/null +++ b/dsh/bundle/lib/gates.js @@ -0,0 +1,388 @@ +/** + * Approval gates A/B/C of the upstream loop: the pure decision core plus the + * approval.md / plan io it needs. + * + * No DSH runtime is imported here. Every decision function takes plain values; + * where it must touch the filesystem it does so only through an explicit + * repository root and only synchronously (`readFileSync` / `existsSync`). + * + * Failure policy mirrors the bash hooks: a call that references no plan is + * never blocked, while a *referenced* plan that cannot be read or tied to an + * approved change is denied (fail closed). Parsing never throws: malformed + * YAML or markdown degrades to "missing", which the gates reject. + */ + +import { createHash } from 'node:crypto' +import { readFileSync, statSync } from 'node:fs' +import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' + +export const APPROVAL_FILE = 'approval.md' +export const PLANS_DIR = join('docs', 'superpowers', 'plans') +export const CHANGE_DIR = join('openspec', 'changes') + +const SPEC_DENY = 'plan must carry **Spec:** pointing to openspec/changes/' +const APPROVAL_DENY = 'approval.md is written only by /approve' +const PLAN_NO_APPROVAL = 'no plan approval' +const PLAN_CHANGED = 'plan changed after approval' +const PLAN_CHANGED_DENY = 'plan changed after approval; re-approve' + +/** Scalar keys of each approval.md section, in serialization order. */ +const SECTION_FIELDS = { + design: ['by', 'at'], + plan: ['by', 'at', 'sha256'], +} + +/** `openspec/changes/` anywhere in a path (also inside an absolute path). */ +const CHANGE_PATH = /(?:^|\/)openspec\/changes\/([^/\s]+)/u +/** `.superpowers/sdd/` anywhere in a path or a prompt. */ +const SDD_PATH = /(?:^|\/)\.superpowers\/sdd\/([^/\s]+)/u +/** A direct plan reference; the match ends at `.md`, prose may follow it. */ +const DIRECT_PLAN = /^docs\/superpowers\/plans\/([^/\s"'`()<>[\]{}]+\.md)/u + +const DIRECT_PREFIX = 'docs/superpowers/plans/' +const SDD_PREFIX = '.superpowers/sdd/' + +/** + * Characters that end a path token when scanning a prompt backwards. `=` + * separates `--flag=` and `:` ends a prose label like `Plan:`. + */ +const TOKEN_STOP = new Set([ + ' ', + '\t', + '\n', + '\r', + '"', + "'", + '`', + '(', + ')', + '[', + ']', + '{', + '}', + '<', + '>', + ',', + ';', + ':', + '=', + '&', + '|', +]) + +function isFile(path) { + try { + return statSync(path).isFile() + } catch { + return false + } +} + +function readText(path) { + try { + return readFileSync(path, 'utf8') + } catch { + return null + } +} + +/** Strip a comment / quotes / backticks from one yaml scalar. */ +function scalar(raw) { + let value = String(raw ?? '').trim() + const comment = value.search(/\s#/u) + if (comment !== -1 && !/^['"`]/u.test(value)) value = value.slice(0, comment).trim() + if ( + (value.startsWith('`') && value.endsWith('`')) || + (value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'")) + ) { + value = value.slice(1, -1).trim() + } + return value +} + +/** `openspec/changes/` when the value points into a change, else null. */ +function changeSpec(value) { + const match = CHANGE_PATH.exec(value) + if (!match) return null + const id = match[1] + if (id === '.' || id === '..') return null + return `openspec/changes/${id}` +} + +function isInside(parent, child) { + const rel = relative(parent, child) + return rel !== '' && rel !== '..' && !rel.startsWith(`..${sep}`) && !isAbsolute(rel) +} + +/** + * Parse the tolerant yaml subset used by approval.md: top-level `design:` and + * `plan:` blocks with scalar `by` / `at` / `sha256` children. A section key is + * kept even when its block is empty; unknown sections and malformed lines are + * ignored. Any non-string input yields `{}` — the caller passes null when the + * file is missing. + */ +export function parseApproval(text) { + const parsed = {} + if (typeof text !== 'string' || text === '') return parsed + + let section = null + for (const rawLine of text.split(/\r?\n/u)) { + const line = rawLine.replace(/[ \t]+$/u, '') + if (line.trim() === '' || /^\s*#/u.test(line)) continue + + const top = /^([A-Za-z0-9_-]+)\s*:\s*(.*)$/u.exec(line) + if (top) { + // Own-key check, not `in`: a crafted `__proto__:`/`constructor:` line + // must be ignored instead of touching the prototype. + const known = Object.prototype.hasOwnProperty.call(SECTION_FIELDS, top[1]) + section = known ? {} : null + if (section) parsed[top[1]] = section + continue + } + + if (section === null) continue + const child = /^\s+([A-Za-z0-9_-]+)\s*:\s*(.*)$/u.exec(line) + if (!child) continue + const value = scalar(child[2]) + if (value !== '') section[child[1]] = value + } + return parsed +} + +/** + * Deterministic approval.md text: sections in `design`, `plan` order, fields + * in a fixed order, one blank line between sections, no trailing spaces, and + * a final newline. An absent or non-object section is skipped. + */ +export function serializeApproval(parsed) { + const sections = [] + for (const [section, fields] of Object.entries(SECTION_FIELDS)) { + const source = parsed?.[section] + if (!source || typeof source !== 'object') continue + const lines = [`${section}:`] + for (const field of fields) { + const value = source[field] + if (value === undefined || value === null) continue + const text = String(value).trim() + if (text === '') continue + lines.push(` ${field}: ${text}`) + } + sections.push(lines.join('\n')) + } + return sections.length === 0 ? '' : `${sections.join('\n\n')}\n` +} + +/** + * Normalized spec path named by the plan's `**Spec:**` line, or null. + * + * Backticks, a markdown link target and a trailing slash are stripped. A path + * into `openspec/changes//...` is normalized to `openspec/changes/`; + * any other path (legacy `docs/superpowers/specs/...`) is returned as written + * so the caller can reject it explicitly (D-9). + */ +export function extractSpec(content) { + if (typeof content !== 'string' || content === '') return null + const match = /^[ \t]*\*\*Spec:\*\*[ \t]*(.+?)[ \t]*$/mu.exec(content) + if (!match) return null + + let value = match[1].trim() + const backtick = /^`([^`]+)`/u.exec(value) + const link = /^\[[^\]]*\]\(([^)]+)\)/u.exec(value) + if (backtick) value = backtick[1] + else if (link) value = link[1] + else value = value.split(/\s+/u)[0] + + value = value + .trim() + .replace(/^[`"']+/u, '') + .replace(/[`"']+$/u, '') + .replace(/^\.\//u, '') + .replace(/[/.,;:]+$/u, '') + if (value === '') return null + return changeSpec(value) ?? value +} + +/** + * Change id of a spec path: `openspec/changes/(/...)` -> ``. + * Legacy `docs/superpowers/specs/...` and everything else -> null. + */ +export function resolveChangeFromSpec(specPath) { + if (typeof specPath !== 'string' || specPath === '') return null + const change = changeSpec(specPath) + return change === null ? null : change.slice('openspec/changes/'.length) +} + +/** + * Plan file behind an sdd ledger path: `.superpowers/sdd/(/...)` -> + * `/docs/superpowers/plans/.md`, when that file exists. + */ +export function resolvePlanFromSdd(sddPath, repoRoot) { + if (typeof sddPath !== 'string' || sddPath === '' || !repoRoot) return null + const match = SDD_PATH.exec(sddPath) + if (!match) return null + const name = match[1].replace(/[/.,;:]+$/u, '') + if (name === '' || name === '.' || name === '..') return null + const planPath = join(resolve(repoRoot), PLANS_DIR, `${name}.md`) + return isFile(planPath) ? planPath : null +} + +/** sha256 hex of the plan content; '' when the file cannot be read. */ +export function hashPlan(planPath) { + try { + return createHash('sha256').update(readFileSync(planPath)).digest('hex') + } catch { + return '' + } +} + +/** + * Is the plan approved for its current content (D-10)? + * Fails with `no plan approval` when the section or its sha256 is missing and + * with `plan changed after approval` when the hash does not match (a missing + * plan file is a hash mismatch by construction). + */ +export function planApproved(approval, planPath) { + const plan = approval?.plan + if (!plan || typeof plan !== 'object') return { ok: false, reason: PLAN_NO_APPROVAL } + const expected = typeof plan.sha256 === 'string' ? plan.sha256.trim().toLowerCase() : '' + if (expected === '') return { ok: false, reason: PLAN_NO_APPROVAL } + const actual = hashPlan(planPath) + if (actual === '' || actual !== expected) return { ok: false, reason: PLAN_CHANGED } + return { ok: true, reason: '' } +} + +function readApproval(repoRoot, changeId) { + return parseApproval(readText(join(repoRoot, CHANGE_DIR, changeId, APPROVAL_FILE))) +} + +/** + * Gate A: a plan may only be written for a change whose design is approved. + * The `**Spec:**` reference is mandatory and must resolve to an openspec + * change (D-9); a plan written elsewhere is not gated. + */ +export function checkPlanWrite({ filePath, content, repoRoot } = {}) { + if (typeof filePath !== 'string' || filePath === '') return {} + const root = resolve(repoRoot || '.') + const target = resolve(root, filePath) + if (!isInside(join(root, PLANS_DIR), target)) return {} + + const changeId = resolveChangeFromSpec(extractSpec(content)) + if (changeId === null) return { deny: SPEC_DENY } + if (!readApproval(root, changeId).design) { + return { deny: `design not approved; run /approve ${changeId} design` } + } + return {} +} + +/** + * Gate C: `approval.md` is written only by `/approve`, never by a tool (D-1). + * Applies wherever the file lives, not only inside a change directory. + */ +export function checkApprovalWrite({ filePath, repoRoot } = {}) { + if (typeof filePath !== 'string' || filePath === '') return {} + const target = resolve(resolve(repoRoot || '.'), filePath) + if (basename(target) === APPROVAL_FILE) return { deny: APPROVAL_DENY } + return {} +} + +function tokenStart(text, index) { + let start = index + while (start > 0 && !TOKEN_STOP.has(text[start - 1])) start -= 1 + return start +} + +/** + * First plan reference in one string: a direct `docs/superpowers/plans/x.md` + * path (absolute or repository-relative) or an `.superpowers/sdd/x/` ledger + * path. Returns null when the string references no plan. + */ +function planReference(text) { + if (typeof text !== 'string' || text === '') return null + const found = [] + + const directAt = text.indexOf(DIRECT_PREFIX) + if (directAt !== -1) { + const match = DIRECT_PLAN.exec(text.slice(directAt)) + if (match) { + const start = tokenStart(text, directAt) + found.push({ index: directAt, path: text.slice(start, directAt + match[0].length) }) + } + } + + const sddAt = text.indexOf(SDD_PREFIX) + if (sddAt !== -1) { + const match = SDD_PATH.exec(text.slice(sddAt)) + if (match) { + const name = match[1].replace(/[/.,;:]+$/u, '') + if (name !== '' && name !== '.' && name !== '..') { + found.push({ index: sddAt, name }) + } + } + } + + if (found.length === 0) return null + found.sort((left, right) => left.index - right.index) + return found[0] +} + +/** Deny text for an unapproved plan; `` stays literal when unknown. */ +function denyPlanNotApproved(changeId) { + return { deny: `plan not approved; run /approve ${changeId ?? ''} plan` } +} + +function checkReferencedPlan(repoRoot, planPath) { + const content = readText(planPath) + if (content === null) return denyPlanNotApproved() + const changeId = resolveChangeFromSpec(extractSpec(content)) + if (changeId === null) return denyPlanNotApproved() + + const verdict = planApproved(readApproval(repoRoot, changeId), planPath) + if (verdict.ok) return {} + if (verdict.reason === PLAN_NO_APPROVAL) return denyPlanNotApproved(changeId) + return { deny: PLAN_CHANGED_DENY } +} + +/** + * Gate B: a dispatch may only start when the plan it references is approved + * for its current content (D-10). The check covers the prompt and, when + * given, the workflow script; no plan reference means no gate. A reference to + * a missing or spec-less plan fails closed. + */ +export function checkDispatch({ prompt, script, repoRoot } = {}) { + const root = resolve(repoRoot || '.') + const reference = planReference(prompt) ?? planReference(script) + if (reference === null) return {} + + let planPath + if (typeof reference.name === 'string') { + planPath = + resolvePlanFromSdd(`${SDD_PREFIX}${reference.name}`, root) ?? + join(root, PLANS_DIR, `${reference.name}.md`) + } else { + planPath = isAbsolute(reference.path) + ? resolve(reference.path) + : resolve(root, reference.path) + } + return checkReferencedPlan(root, planPath) +} + +/** + * Gates A and C for a list of absolute write targets (from + * `harness_target.py targets`). First denial wins. A target under plans/ has + * no content here, so it can never carry a `**Spec:**` line and is denied — + * exactly the bash-heredoc case the tool-level gates would otherwise miss. + */ +export function checkBashTargets(targets, repoRoot) { + if (!Array.isArray(targets)) return {} + const root = resolve(repoRoot || '.') + for (const raw of targets) { + if (typeof raw !== 'string' || raw === '') continue + const target = resolve(root, raw) + const approval = checkApprovalWrite({ filePath: target, repoRoot: root }) + if (approval.deny !== undefined) return approval + const plan = checkPlanWrite({ filePath: target, content: null, repoRoot: root }) + if (plan.deny !== undefined) return plan + } + return {} +} diff --git a/dsh/tests/gates.test.mjs b/dsh/tests/gates.test.mjs new file mode 100644 index 0000000..e208ab7 --- /dev/null +++ b/dsh/tests/gates.test.mjs @@ -0,0 +1,369 @@ +import assert from 'node:assert/strict' +import { createHash } from 'node:crypto' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import test from 'node:test' +import { + checkApprovalWrite, + checkBashTargets, + checkDispatch, + checkPlanWrite, + extractSpec, + hashPlan, + parseApproval, + planApproved, + resolveChangeFromSpec, + resolvePlanFromSdd, + serializeApproval, +} from '../bundle/lib/gates.js' +import { cleanup, tempDir, write } from './helpers.mjs' + +const PLAN_TEXT = [ + '# Plan X', + '', + '**Spec:** `openspec/changes/demo/`', + '', + '## Task 1', + '', + 'Do the work.', + '', +].join('\n') + +const DESIGN = { by: 'aep', at: '2026-10-25T10:00:00+04:00' } +const PLAN = { by: 'aep', at: '2026-10-25T11:30:00+04:00' } + +/** A throwaway repository with the plan/change artifacts the gates read. */ +function repoFixture() { + const tmp = tempDir('dsh-gates-') + const repo = join(tmp, 'repo') + write(join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), '# Proposal\n') + write(join(repo, 'openspec', 'changes', 'demo', 'design.md'), '# Design\n') + const planPath = join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md') + write(planPath, PLAN_TEXT) + return { tmp, repo, planPath } +} + +function approvalPath(repo) { + return join(repo, 'openspec', 'changes', 'demo', 'approval.md') +} + +/** Independent sha256, so the tests do not trust the implementation's own io. */ +function shaOf(path) { + return createHash('sha256').update(readFileSync(path)).digest('hex') +} + +/** + * Write approval.md. `design` false omits the design section; leaving `planSha` + * undefined omits the plan section; `null` writes an empty plan section. + */ +function writeApproval(repo, { design = true, planSha } = {}) { + const sections = [] + if (design) { + sections.push(['design:', [` by: ${DESIGN.by}`, ` at: ${DESIGN.at}`]]) + } + if (planSha !== undefined) { + const lines = [` by: ${PLAN.by}`, ` at: ${PLAN.at}`] + if (planSha !== null) lines.push(` sha256: ${planSha}`) + sections.push(['plan:', lines]) + } + const text = `${sections.map(([head, lines]) => [head, ...lines].join('\n')).join('\n\n')}\n` + write(approvalPath(repo), text) + return text +} + +test('extractSpec: backticks and trailing slash normalize', () => { + assert.equal(extractSpec('**Spec:** `openspec/changes/demo/`'), 'openspec/changes/demo') + assert.equal( + extractSpec( + 'intro\n\n**Spec:** `openspec/changes/demo/` — [proposal.md](openspec/changes/demo/proposal.md)\n', + ), + 'openspec/changes/demo', + ) + assert.equal(extractSpec('**Spec:** openspec/changes/demo/proposal.md'), 'openspec/changes/demo') + assert.equal(extractSpec('no spec line here'), null) + assert.equal(extractSpec(null), null) +}) + +test('checkPlanWrite: no Spec -> deny', () => { + const { tmp, repo } = repoFixture() + try { + const result = checkPlanWrite({ + filePath: join(repo, 'docs', 'superpowers', 'plans', 'plan-new.md'), + content: '# Plan\n\nNo spec reference.\n', + repoRoot: repo, + }) + assert.equal(result.deny, 'plan must carry **Spec:** pointing to openspec/changes/') + } finally { + cleanup(tmp) + } +}) + +test('checkPlanWrite: legacy docs/superpowers/specs -> deny', () => { + const { tmp, repo } = repoFixture() + try { + writeApproval(repo) + const result = checkPlanWrite({ + filePath: join(repo, 'docs', 'superpowers', 'plans', 'plan-legacy.md'), + content: '# Plan\n\n**Spec:** docs/superpowers/specs/old/spec.md\n', + repoRoot: repo, + }) + assert.equal(result.deny, 'plan must carry **Spec:** pointing to openspec/changes/') + } finally { + cleanup(tmp) + } +}) + +test('checkPlanWrite: design approved -> ok', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo) + const result = checkPlanWrite({ filePath: planPath, content: PLAN_TEXT, repoRoot: repo }) + assert.equal(result.deny, undefined) + } finally { + cleanup(tmp) + } +}) + +test('checkPlanWrite: design not approved -> deny with /approve hint', () => { + const { tmp, repo } = repoFixture() + try { + const filePath = join(repo, 'docs', 'superpowers', 'plans', 'plan-new.md') + const missing = checkPlanWrite({ filePath, content: PLAN_TEXT, repoRoot: repo }) + assert.equal(missing.deny, 'design not approved; run /approve demo design') + + writeApproval(repo, { design: false, planSha: 'deadbeef' }) + const planOnly = checkPlanWrite({ filePath, content: PLAN_TEXT, repoRoot: repo }) + assert.equal(planOnly.deny, 'design not approved; run /approve demo design') + } finally { + cleanup(tmp) + } +}) + +test('checkPlanWrite: files outside plans/ are not gated', () => { + const { tmp, repo } = repoFixture() + try { + const result = checkPlanWrite({ + filePath: join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), + content: '# Proposal\n', + repoRoot: repo, + }) + assert.equal(result.deny, undefined) + } finally { + cleanup(tmp) + } +}) + +test('checkApprovalWrite: approval.md via tool -> deny', () => { + const { tmp, repo } = repoFixture() + try { + const absolute = checkApprovalWrite({ filePath: approvalPath(repo), repoRoot: repo }) + assert.equal(absolute.deny, 'approval.md is written only by /approve') + + const relative = checkApprovalWrite({ + filePath: 'openspec/changes/demo/approval.md', + repoRoot: repo, + }) + assert.equal(relative.deny, 'approval.md is written only by /approve') + + const other = checkApprovalWrite({ + filePath: join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), + repoRoot: repo, + }) + assert.equal(other.deny, undefined) + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: prompt plan ref, plan approved -> ok', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo, { planSha: shaOf(planPath) }) + const result = checkDispatch({ + prompt: `Implement task 1. Read ${planPath} before you start.`, + repoRoot: repo, + }) + assert.equal(result.deny, undefined) + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: no plan section -> deny', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo) + const result = checkDispatch({ prompt: `Read ${planPath}.`, repoRoot: repo }) + assert.equal(result.deny, 'plan not approved; run /approve demo plan') + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: sha256 mismatch after edit -> deny re-approve', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo, { planSha: shaOf(planPath) }) + write(planPath, `${PLAN_TEXT}\nadded after approval\n`) + const result = checkDispatch({ prompt: `Read ${planPath}.`, repoRoot: repo }) + assert.equal(result.deny, 'plan changed after approval; re-approve') + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: workflow script with plan ref -> deny/ok same as subagent', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo, { planSha: shaOf(planPath) }) + const direct = checkDispatch({ + prompt: 'Run the workflow.', + script: `const plan = ${JSON.stringify(planPath)}\n`, + repoRoot: repo, + }) + assert.equal(direct.deny, undefined) + + writeApproval(repo) + const sdd = checkDispatch({ + prompt: 'Run the workflow.', + script: 'cat .superpowers/sdd/plan-x/progress.md', + repoRoot: repo, + }) + assert.equal(sdd.deny, 'plan not approved; run /approve demo plan') + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: no plan reference -> ok', () => { + const { tmp, repo } = repoFixture() + try { + const result = checkDispatch({ prompt: 'Answer the question about the weather.', repoRoot: repo }) + assert.equal(result.deny, undefined) + assert.equal(checkDispatch({ prompt: '', script: '', repoRoot: repo }).deny, undefined) + } finally { + cleanup(tmp) + } +}) + +test('resolveChangeFromSpec: only openspec/changes resolve', () => { + assert.equal(resolveChangeFromSpec('openspec/changes/demo'), 'demo') + assert.equal(resolveChangeFromSpec('openspec/changes/demo/proposal.md'), 'demo') + assert.equal(resolveChangeFromSpec('docs/superpowers/specs/demo/spec.md'), null) + assert.equal(resolveChangeFromSpec('openspec/specs/hook-guards/spec.md'), null) + assert.equal(resolveChangeFromSpec(null), null) +}) + +test('resolvePlanFromSdd: .superpowers/sdd -> existing plan file', () => { + const { tmp, repo, planPath } = repoFixture() + try { + assert.equal(resolvePlanFromSdd('.superpowers/sdd/plan-x/progress.md', repo), planPath) + assert.equal( + resolvePlanFromSdd('/abs/repo/.superpowers/sdd/plan-x/briefs/task-1.md', repo), + planPath, + ) + assert.equal(resolvePlanFromSdd('.superpowers/sdd/missing/progress.md', repo), null) + assert.equal(resolvePlanFromSdd('docs/superpowers/plans/plan-x.md', repo), null) + } finally { + cleanup(tmp) + } +}) + +test('planApproved: happy / missing / mismatch', () => { + const { tmp, repo, planPath } = repoFixture() + try { + const approved = { + design: DESIGN, + plan: { ...PLAN, sha256: shaOf(planPath) }, + } + assert.equal(hashPlan(planPath), shaOf(planPath)) + assert.equal(planApproved(approved, planPath).ok, true) + + assert.deepEqual(planApproved({}, planPath), { ok: false, reason: 'no plan approval' }) + assert.deepEqual(planApproved({ design: DESIGN }, planPath), { + ok: false, + reason: 'no plan approval', + }) + assert.deepEqual( + planApproved({ plan: { ...PLAN, sha256: 'a'.repeat(64) } }, planPath), + { ok: false, reason: 'plan changed after approval' }, + ) + } finally { + cleanup(tmp) + } +}) + +test('serialize/parse roundtrip preserves sections', () => { + const approval = { design: DESIGN, plan: { ...PLAN, sha256: 'b'.repeat(64) } } + const text = serializeApproval(approval) + assert.equal( + text, + [ + 'design:', + ` by: ${DESIGN.by}`, + ` at: ${DESIGN.at}`, + '', + 'plan:', + ` by: ${PLAN.by}`, + ` at: ${PLAN.at}`, + ` sha256: ${'b'.repeat(64)}`, + '', + ].join('\n'), + ) + assert.deepEqual(parseApproval(text), approval) + assert.equal(serializeApproval(parseApproval(text)), text) + assert.doesNotMatch(text, /[ \t]+\n/) + + const designOnly = serializeApproval({ design: DESIGN }) + assert.doesNotMatch(designOnly, /plan:/) + assert.deepEqual(parseApproval(designOnly), { design: DESIGN }) +}) + +test('parseApproval tolerates malformed and missing input', () => { + assert.deepEqual(parseApproval(null), {}) + assert.deepEqual(parseApproval(undefined), {}) + assert.deepEqual(parseApproval(''), {}) + assert.deepEqual(parseApproval('design:\n by: aep\n at: x\n'), { + design: { by: 'aep', at: 'x' }, + }) + assert.deepEqual(parseApproval('design:\n\nplan:\n sha256: abc\n'), { + design: {}, + plan: { sha256: 'abc' }, + }) + assert.deepEqual(parseApproval('garbage [not yaml\n stray: value\n'), {}) + assert.deepEqual(parseApproval('design:\r\n by: aep\r\n'), { design: { by: 'aep' } }) +}) + +test('checkBashTargets: heredoc-equivalent targets deny', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo) + assert.equal(checkBashTargets([join(repo, 'a.txt')], repo).deny, undefined) + assert.equal(checkBashTargets([], repo).deny, undefined) + assert.equal(checkBashTargets(undefined, repo).deny, undefined) + + assert.equal( + checkBashTargets([approvalPath(repo)], repo).deny, + 'approval.md is written only by /approve', + ) + assert.equal( + checkBashTargets([planPath], repo).deny, + 'plan must carry **Spec:** pointing to openspec/changes/', + ) + assert.equal( + checkBashTargets(['openspec/changes/demo/approval.md'], repo).deny, + 'approval.md is written only by /approve', + ) + // First deny wins, in target order. + assert.equal( + checkBashTargets([approvalPath(repo), planPath], repo).deny, + 'approval.md is written only by /approve', + ) + assert.equal( + checkBashTargets([planPath, approvalPath(repo)], repo).deny, + 'plan must carry **Spec:** pointing to openspec/changes/', + ) + } finally { + cleanup(tmp) + } +}) -- 2.47.2 From 874457527111baa9cf57e764e8d043721cec06c9 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 01:36:12 +0400 Subject: [PATCH 04/10] =?UTF-8?q?fix(dsh):=20gates=20=E2=80=94=20write-?= =?UTF-8?q?=D0=B3=D0=B5=D0=B9=D1=82=20.superpowers/sdd=20=D0=B8=20=D0=BF?= =?UTF-8?q?=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=BA=D0=B0=20=D0=B2=D1=81=D0=B5?= =?UTF-8?q?=D1=85=20=D0=BF=D0=BB=D0=B0=D0=BD-=D1=81=D1=81=D1=8B=D0=BB?= =?UTF-8?q?=D0=BE=D0=BA=20(T1.1-T1.2,=20#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/bundle/lib/gates.js | 124 ++++++++++++++++++++++++++------------- dsh/tests/gates.test.mjs | 99 +++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 42 deletions(-) diff --git a/dsh/bundle/lib/gates.js b/dsh/bundle/lib/gates.js index b3deb48..8500aa6 100644 --- a/dsh/bundle/lib/gates.js +++ b/dsh/bundle/lib/gates.js @@ -19,6 +19,7 @@ import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' export const APPROVAL_FILE = 'approval.md' export const PLANS_DIR = join('docs', 'superpowers', 'plans') export const CHANGE_DIR = join('openspec', 'changes') +export const SDD_DIR = join('.superpowers', 'sdd') const SPEC_DENY = 'plan must carry **Spec:** pointing to openspec/changes/' const APPROVAL_DENY = 'approval.md is written only by /approve' @@ -256,6 +257,22 @@ function readApproval(repoRoot, changeId) { return parseApproval(readText(join(repoRoot, CHANGE_DIR, changeId, APPROVAL_FILE))) } +/** + * Does approval.md carry a real design approval? An empty `design:` block is + * treated as no approval (R-4/T1.6f): the section must carry the `by` and `at` + * scalars `/approve` writes, so a bare header cannot unlock the plan gate. + */ +export function hasDesignApproval(approval) { + const design = approval?.design + if (!design || typeof design !== 'object') return false + return ( + typeof design.by === 'string' && + design.by !== '' && + typeof design.at === 'string' && + design.at !== '' + ) +} + /** * Gate A: a plan may only be written for a change whose design is approved. * The `**Spec:**` reference is mandatory and must resolve to an openspec @@ -269,7 +286,7 @@ export function checkPlanWrite({ filePath, content, repoRoot } = {}) { const changeId = resolveChangeFromSpec(extractSpec(content)) if (changeId === null) return { deny: SPEC_DENY } - if (!readApproval(root, changeId).design) { + if (!hasDesignApproval(readApproval(root, changeId))) { return { deny: `design not approved; run /approve ${changeId} design` } } return {} @@ -293,37 +310,29 @@ function tokenStart(text, index) { } /** - * First plan reference in one string: a direct `docs/superpowers/plans/x.md` - * path (absolute or repository-relative) or an `.superpowers/sdd/x/` ledger - * path. Returns null when the string references no plan. + * All plan references in one string, in textual order: a direct + * `docs/superpowers/plans/x.md` path (absolute or repository-relative) or an + * `.superpowers/sdd/x/` ledger path. Empty when the string references none. */ -function planReference(text) { - if (typeof text !== 'string' || text === '') return null +function planReferences(text) { + if (typeof text !== 'string' || text === '') return [] const found = [] - const directAt = text.indexOf(DIRECT_PREFIX) - if (directAt !== -1) { - const match = DIRECT_PLAN.exec(text.slice(directAt)) - if (match) { - const start = tokenStart(text, directAt) - found.push({ index: directAt, path: text.slice(start, directAt + match[0].length) }) - } + for (let at = text.indexOf(DIRECT_PREFIX); at !== -1; at = text.indexOf(DIRECT_PREFIX, at + 1)) { + const match = DIRECT_PLAN.exec(text.slice(at)) + if (!match) continue + const start = tokenStart(text, at) + found.push({ index: at, path: text.slice(start, at + match[0].length) }) } - const sddAt = text.indexOf(SDD_PREFIX) - if (sddAt !== -1) { - const match = SDD_PATH.exec(text.slice(sddAt)) - if (match) { - const name = match[1].replace(/[/.,;:]+$/u, '') - if (name !== '' && name !== '.' && name !== '..') { - found.push({ index: sddAt, name }) - } - } + for (let at = text.indexOf(SDD_PREFIX); at !== -1; at = text.indexOf(SDD_PREFIX, at + 1)) { + const match = SDD_PATH.exec(text.slice(at)) + if (!match) continue + const name = match[1].replace(/[/.,;:]+$/u, '') + if (name !== '' && name !== '.' && name !== '..') found.push({ index: at, name }) } - if (found.length === 0) return null - found.sort((left, right) => left.index - right.index) - return found[0] + return found.sort((left, right) => left.index - right.index) } /** Deny text for an unapproved plan; `` stays literal when unknown. */ @@ -331,7 +340,23 @@ function denyPlanNotApproved(changeId) { return { deny: `plan not approved; run /approve ${changeId ?? ''} plan` } } -function checkReferencedPlan(repoRoot, planPath) { +/** Plan path a parsed reference points to (the file may be missing). */ +function referencePlanPath(reference, repoRoot) { + if (typeof reference.name === 'string') { + return ( + resolvePlanFromSdd(`${SDD_PREFIX}${reference.name}`, repoRoot) ?? + join(repoRoot, PLANS_DIR, `${reference.name}.md`) + ) + } + return isAbsolute(reference.path) ? resolve(reference.path) : resolve(repoRoot, reference.path) +} + +/** + * Full gate-B chain for one referenced plan: read it, take its `**Spec:**`, + * resolve the change, verify the plan approval against the file content. + * Missing/spec-less plans fail closed. + */ +function planVerdict(repoRoot, planPath) { const content = readText(planPath) if (content === null) return denyPlanNotApproved() const changeId = resolveChangeFromSpec(extractSpec(content)) @@ -344,27 +369,42 @@ function checkReferencedPlan(repoRoot, planPath) { } /** - * Gate B: a dispatch may only start when the plan it references is approved + * Gate B: a dispatch may only start when every plan it references is approved * for its current content (D-10). The check covers the prompt and, when - * given, the workflow script; no plan reference means no gate. A reference to - * a missing or spec-less plan fails closed. + * given, the workflow script; the first failing reference denies, so an + * approved plan cannot mask a later unapproved one. No plan reference at all + * means no gate. */ export function checkDispatch({ prompt, script, repoRoot } = {}) { const root = resolve(repoRoot || '.') - const reference = planReference(prompt) ?? planReference(script) - if (reference === null) return {} - - let planPath - if (typeof reference.name === 'string') { - planPath = - resolvePlanFromSdd(`${SDD_PREFIX}${reference.name}`, root) ?? - join(root, PLANS_DIR, `${reference.name}.md`) - } else { - planPath = isAbsolute(reference.path) - ? resolve(reference.path) - : resolve(root, reference.path) + const references = [...planReferences(prompt), ...planReferences(script)] + for (const reference of references) { + const verdict = planVerdict(root, referencePlanPath(reference, root)) + if (verdict.deny !== undefined) return verdict } - return checkReferencedPlan(root, planPath) + return {} +} + +/** + * Gate B for Mode 1 starts: a write under `.superpowers/sdd//` + * (ledger, briefs) is allowed only when that plan is approved for its current + * content (D-7). A missing plan file fails closed with `plan not found`. + */ +export function checkSddWrite({ filePath, repoRoot } = {}) { + if (typeof filePath !== 'string' || filePath === '') return {} + const root = resolve(repoRoot || '.') + const target = resolve(root, filePath) + const sddDir = join(root, SDD_DIR) + if (!isInside(sddDir, target)) return {} + + const segments = relative(sddDir, target).split(sep) + if (segments.length < 2) return {} + const name = segments[0] + if (name === '' || name === '.' || name === '..') return {} + + const planPath = resolvePlanFromSdd(`${SDD_PREFIX}${name}`, root) + if (planPath === null) return { deny: `plan not found: ${name}` } + return planVerdict(root, planPath) } /** diff --git a/dsh/tests/gates.test.mjs b/dsh/tests/gates.test.mjs index e208ab7..8ed1e26 100644 --- a/dsh/tests/gates.test.mjs +++ b/dsh/tests/gates.test.mjs @@ -8,6 +8,7 @@ import { checkBashTargets, checkDispatch, checkPlanWrite, + checkSddWrite, extractSpec, hashPlan, parseApproval, @@ -367,3 +368,101 @@ test('checkBashTargets: heredoc-equivalent targets deny', () => { cleanup(tmp) } }) + +test('checkSddWrite: plan not approved -> deny', () => { + const { tmp, repo } = repoFixture() + try { + writeApproval(repo) + const result = checkSddWrite({ + filePath: join(repo, '.superpowers', 'sdd', 'plan-x', 'progress.md'), + repoRoot: repo, + }) + assert.equal(result.deny, 'plan not approved; run /approve demo plan') + } finally { + cleanup(tmp) + } +}) + +test('checkSddWrite: approved plan -> ok, files elsewhere are not gated', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo, { planSha: shaOf(planPath) }) + const ledger = checkSddWrite({ + filePath: join(repo, '.superpowers', 'sdd', 'plan-x', 'briefs', 'task-1.md'), + repoRoot: repo, + }) + assert.equal(ledger.deny, undefined) + + assert.equal( + checkSddWrite({ filePath: join(repo, 'README.md'), repoRoot: repo }).deny, + undefined, + ) + assert.equal( + checkSddWrite({ + filePath: join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md'), + repoRoot: repo, + }).deny, + undefined, + ) + } finally { + cleanup(tmp) + } +}) + +test('checkSddWrite: missing plan -> fail-closed deny', () => { + const { tmp, repo } = repoFixture() + try { + writeApproval(repo) + const result = checkSddWrite({ + filePath: join(repo, '.superpowers', 'sdd', 'ghost-plan', 'progress.md'), + repoRoot: repo, + }) + assert.equal(result.deny, 'plan not found: ghost-plan') + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: all plan references are checked, not only the first', () => { + const { tmp, repo, planPath } = repoFixture() + try { + const otherPlan = join(repo, 'docs', 'superpowers', 'plans', 'plan-y.md') + write(otherPlan, '# Plan Y\n\n**Spec:** `openspec/changes/demo2/`\n') + writeApproval(repo, { planSha: shaOf(planPath) }) + + // The first reference is approved; the second one is not. A first-only + // check would pass, so this pin proves every reference is checked. + const result = checkDispatch({ + prompt: `Read ${planPath} first, then ${otherPlan}.`, + repoRoot: repo, + }) + assert.equal(result.deny, 'plan not approved; run /approve demo2 plan') + } finally { + cleanup(tmp) + } +}) + +test('checkDispatch: sdd reference with an approved plan -> ok', () => { + const { tmp, repo, planPath } = repoFixture() + try { + writeApproval(repo, { planSha: shaOf(planPath) }) + const result = checkDispatch({ + prompt: 'Start Mode 1: write .superpowers/sdd/plan-x/progress.md, then the briefs.', + repoRoot: repo, + }) + assert.equal(result.deny, undefined) + } finally { + cleanup(tmp) + } +}) + +test('checkPlanWrite: empty design section counts as not approved', () => { + const { tmp, repo, planPath } = repoFixture() + try { + write(join(repo, 'openspec', 'changes', 'demo', 'approval.md'), 'design:\n') + const result = checkPlanWrite({ filePath: planPath, content: PLAN_TEXT, repoRoot: repo }) + assert.equal(result.deny, 'design not approved; run /approve demo design') + } finally { + cleanup(tmp) + } +}) -- 2.47.2 From c255a24b94545b4124b83da74ef1d4352a968a42 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 01:42:51 +0400 Subject: [PATCH 05/10] =?UTF-8?q?feat(dsh):=20/approve=20=E2=80=94=20?= =?UTF-8?q?=D0=B5=D0=B4=D0=B8=D0=BD=D1=81=D1=82=D0=B2=D0=B5=D0=BD=D0=BD?= =?UTF-8?q?=D1=8B=D0=B9=20=D0=BF=D0=B8=D1=81=D0=B0=D1=82=D0=B5=D0=BB=D1=8C?= =?UTF-8?q?=20approval.md=20=D1=81=20=D0=BF=D0=BE=D0=B4=D1=82=D0=B2=D0=B5?= =?UTF-8?q?=D1=80=D0=B6=D0=B4=D0=B5=D0=BD=D0=B8=D0=B5=D0=BC=20=D0=B2=D0=BB?= =?UTF-8?q?=D0=B0=D0=B4=D0=B5=D0=BB=D1=8C=D1=86=D0=B0=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/bundle/command-approve.js | 264 ++++++++++++++++++++++++++++++++++ dsh/tests/approve.test.mjs | 223 ++++++++++++++++++++++++++++ 2 files changed, 487 insertions(+) create mode 100644 dsh/bundle/command-approve.js create mode 100644 dsh/tests/approve.test.mjs diff --git a/dsh/bundle/command-approve.js b/dsh/bundle/command-approve.js new file mode 100644 index 0000000..56aa63f --- /dev/null +++ b/dsh/bundle/command-approve.js @@ -0,0 +1,264 @@ +/** + * Human-facing `/approve` command for DeepSeek Harness. + * + * `/approve design|plan` is the only writer of + * `openspec/changes//approval.md` (D-1). The command checks the upstream + * artifacts, asks the owner through `userQuestions.ask` (D-12), and only then + * merges the new section into the existing file. A missing user-questions + * service refuses instead of writing, so no model tool can approve itself. + * + * Mirrors `command-role.js`: object-form `commands.register`, the same + * `{ kind, text }` result shape. + */ + +import { mkdirSync, readFileSync, readdirSync, statSync, writeFileSync } from 'node:fs' +import { dirname, join, relative, resolve } from 'node:path' +import { + APPROVAL_FILE, + CHANGE_DIR, + PLANS_DIR, + extractSpec, + hasDesignApproval, + hashPlan, + parseApproval, + resolveChangeFromSpec, + serializeApproval, +} from './lib/gates.js' + +const USAGE = [ + 'Usage: /approve design|plan', + ' design — record the owner approval of the change design', + ' plan — record the owner approval of the plan whose **Spec:** is ', +].join('\n') + +const DESIGN_ARTIFACTS = ['proposal.md', 'design.md', 'builder-check.md'] +const APPROVE_LABEL = 'Approve' +const CANCEL_LABEL = 'Cancel' +/** Change ids are directory names: keep them one safe path segment. */ +const CHANGE_ID = /^[A-Za-z0-9][A-Za-z0-9._-]*$/u + +function isFile(path) { + try { + return statSync(path).isFile() + } catch { + return false + } +} + +function readText(path) { + try { + return readFileSync(path, 'utf8') + } catch { + return null + } +} + +function refusal(text) { + return { kind: 'error', text: `Refused: ${text}` } +} + +function usage(text) { + return { kind: 'error', text: `${text}\n${USAGE}` } +} + +/** + * Read the answer of `userQuestions.ask`. + * + * The DSH UserQuestionService resolves to `{ answers: [{ id, selected }] }`; + * the plan/brief contract also allows `{ approved }` / `{ confirmed }` (and a + * bare `true`) so the command stays testable without the runtime. Anything + * else — a skipped question, a cancel, a timeout, null — is not an approval. + */ +function isApproved(answer) { + if (answer === true) return true + if (answer === null || typeof answer !== 'object') return false + + const flag = answer.approved ?? answer.confirmed + if (flag !== undefined) return Boolean(flag) + + const items = Array.isArray(answer.answers) ? answer.answers : [] + return items.some( + (item) => item !== null && typeof item === 'object' && Array.isArray(item.selected) && item.selected.includes(APPROVE_LABEL), + ) +} + +/** + * Ask the owner and return `{ ok: true }` or a refusal `{ ok: false, text }`. + * The detail shows exactly what is being approved; the answer is never + * inferred from the absence of an error. + */ +async function askOwner(userQuestions, { changeId, kind, detail, agent, signal }) { + if (typeof userQuestions?.ask !== 'function') { + return { ok: false, text: 'userQuestions service unavailable; /approve requires it' } + } + + const request = { + questions: [ + { + id: `approve-${kind}`, + header: 'Approve', + question: `Approve ${changeId} ${kind}?`, + detail, + options: [ + { + label: APPROVE_LABEL, + description: `Write the ${kind} approval to openspec/changes/${changeId}/approval.md.`, + }, + { label: CANCEL_LABEL, description: 'Write nothing.' }, + ], + }, + ], + } + if (agent !== undefined) request.agent = agent + if (signal !== undefined) request.signal = signal + + let answer + try { + answer = await userQuestions.ask(request) + } catch (error) { + return { ok: false, text: `userQuestions failed: ${String(error?.message ?? error)}` } + } + + return isApproved(answer) ? { ok: true } : { ok: false, text: 'not confirmed' } +} + +/** Merge one section into approval.md, leaving every other section as is. */ +function writeApproval(root, changeId, mutate) { + const path = join(root, CHANGE_DIR, changeId, APPROVAL_FILE) + const next = mutate(parseApproval(readText(path))) + mkdirSync(dirname(path), { recursive: true }) + writeFileSync(path, serializeApproval(next), 'utf8') + return path +} + +/** Existing plans under docs/superpowers/plans/ whose **Spec:** is ``. */ +function plansForChange(root, changeId) { + const plansDir = join(root, PLANS_DIR) + let names + try { + names = readdirSync(plansDir) + } catch { + return [] + } + + const found = [] + for (const name of [...names].sort()) { + if (!name.endsWith('.md')) continue + const path = join(plansDir, name) + if (!isFile(path)) continue + const content = readText(path) + if (content === null) continue + if (resolveChangeFromSpec(extractSpec(content)) === changeId) found.push(path) + } + return found +} + +function approvalText(root, path) { + return relative(root, path).split('\\').join('/') +} + +async function approveDesign({ root, changeId, userQuestions, agent, signal }) { + const changeDir = join(root, CHANGE_DIR, changeId) + const missing = DESIGN_ARTIFACTS.filter((name) => !isFile(join(changeDir, name))) + if (missing.length > 0) { + const listed = missing.map((name) => `openspec/changes/${changeId}/${name}`).join(', ') + return refusal(`design approval requires the change artifacts; missing: ${listed}.`) + } + + const asked = await askOwner(userQuestions, { + changeId, + kind: 'design', + detail: `Change: openspec/changes/${changeId} (proposal.md, design.md, builder-check.md).`, + agent, + signal, + }) + if (!asked.ok) return refusal(asked.text) + + const at = new Date().toISOString() + const path = writeApproval(root, changeId, (approval) => ({ + ...approval, + design: { by: 'owner', at }, + })) + return { + kind: 'success', + text: `Approved ${changeId} design at ${at}; wrote ${approvalText(root, path)}.`, + } +} + +async function approvePlan({ root, changeId, userQuestions, agent, signal }) { + const approval = parseApproval(readText(join(root, CHANGE_DIR, changeId, APPROVAL_FILE))) + if (!hasDesignApproval(approval)) { + return refusal(`design not approved; run /approve ${changeId} design`) + } + + const plans = plansForChange(root, changeId) + if (plans.length === 0) { + return refusal( + `no plan for ${changeId} found under ${PLANS_DIR.split('\\').join('/')}/ with **Spec:** openspec/changes/${changeId}.`, + ) + } + if (plans.length > 1) { + const listed = plans.map((path) => approvalText(root, path)).join(', ') + return refusal(`several plans reference ${changeId}: ${listed}. Keep exactly one.`) + } + + const [planPath] = plans + const sha256 = hashPlan(planPath) + if (sha256 === '') { + return refusal(`cannot read plan ${approvalText(root, planPath)}.`) + } + + const asked = await askOwner(userQuestions, { + changeId, + kind: 'plan', + detail: `Plan: ${approvalText(root, planPath)} (sha256 ${sha256}).`, + agent, + signal, + }) + if (!asked.ok) return refusal(asked.text) + + const at = new Date().toISOString() + const path = writeApproval(root, changeId, (current) => ({ + ...current, + plan: { by: 'owner', at, sha256 }, + })) + return { + kind: 'success', + text: `Approved ${changeId} plan at ${at} (sha256 ${sha256}); wrote ${approvalText(root, path)}.`, + } +} + +export function registerApproveCommand({ commands, userQuestions, repoRoot, logger }) { + commands.register({ + name: 'approve', + description: 'Утвердить design или plan change (единственный писатель approval.md)', + input: { hint: ' design|plan' }, + handler: async ({ agent, rawInput, signal } = {}) => { + const parts = + typeof rawInput === 'string' ? rawInput.trim().split(/\s+/u).filter((part) => part !== '') : [] + + if (parts.length !== 2) { + return usage('Missing arguments: expected design|plan.') + } + + const [changeId, kind] = parts + if (!CHANGE_ID.test(changeId)) { + return usage(`Invalid change id "${changeId}".`) + } + if (kind !== 'design' && kind !== 'plan') { + return usage(`Unknown kind "${kind}".`) + } + + const root = resolve(repoRoot || process.cwd()) + try { + if (kind === 'design') { + return await approveDesign({ root, changeId, userQuestions, agent, signal }) + } + return await approvePlan({ root, changeId, userQuestions, agent, signal }) + } catch (error) { + logger?.warn?.('[dsh-harness] /approve failed: %s', error?.message ?? error) + return refusal(`/approve failed: ${String(error?.message ?? error)}`) + } + }, + }) +} diff --git a/dsh/tests/approve.test.mjs b/dsh/tests/approve.test.mjs new file mode 100644 index 0000000..8f2016d --- /dev/null +++ b/dsh/tests/approve.test.mjs @@ -0,0 +1,223 @@ +import assert from 'node:assert/strict' +import { createHash } from 'node:crypto' +import { existsSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import test from 'node:test' +import { registerApproveCommand } from '../bundle/command-approve.js' +import { hashPlan, parseApproval, serializeApproval } from '../bundle/lib/gates.js' +import { cleanup, tempDir, write } from './helpers.mjs' + +const PLAN_TEXT = [ + '# Plan X', + '', + '**Spec:** `openspec/changes/demo/`', + '', + '## Task 1', + '', + 'Do the work.', + '', +].join('\n') + +const ISO = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/u +const APPROVE = 'Approve' + +/** A throwaway repository with the design artifacts and one demo plan. */ +function repoFixture({ builderCheck = true } = {}) { + const tmp = tempDir('dsh-approve-') + const repo = join(tmp, 'repo') + write(join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), '# Proposal\n') + write(join(repo, 'openspec', 'changes', 'demo', 'design.md'), '# Design\n') + if (builderCheck) write(join(repo, 'openspec', 'changes', 'demo', 'builder-check.md'), '# Check\n') + const planPath = join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md') + write(planPath, PLAN_TEXT) + return { tmp, repo, planPath } +} + +function approvalPath(repo) { + return join(repo, 'openspec', 'changes', 'demo', 'approval.md') +} + +/** An ask() answer batch shaped like the DSH UserQuestionService result. */ +function approveAnswer(kind = 'design') { + return { answers: [{ id: `approve-${kind}`, selected: [APPROVE], custom: undefined }] } +} + +/** + * Wire the command against a fake `commands.register` that captures the + * registered definition, and a scripted `userQuestions.ask`. + */ +function setup(repo, { answer, missingService = false } = {}) { + const captured = {} + const commands = { + register(definition) { + Object.assign(captured, definition) + }, + } + const calls = [] + let userQuestions + if (!missingService) { + userQuestions = { + async ask(request) { + calls.push(request) + return answer + }, + } + } + registerApproveCommand({ + commands, + userQuestions, + repoRoot: repo, + logger: { warn() {} }, + }) + return { captured, calls, run: (rawInput) => captured.handler({ rawInput }) } +} + +function shaOf(path) { + return createHash('sha256').update(readFileSync(path)).digest('hex') +} + +test('approve design refuses without builder-check.md', async () => { + const { tmp, repo } = repoFixture({ builderCheck: false }) + try { + const harness = setup(repo, { answer: approveAnswer('design') }) + const result = await harness.run('demo design') + + assert.match(result.text, /builder-check\.md/u) + assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written') + assert.equal(harness.calls.length, 0, 'the owner is not asked when artifacts are missing') + } finally { + cleanup(tmp) + } +}) + +test('approve design writes design section (by owner, at ISO)', async () => { + const { tmp, repo } = repoFixture() + try { + const existingPlan = { by: 'aep', at: '2026-10-25T11:30:00+04:00', sha256: 'a'.repeat(64) } + write(approvalPath(repo), serializeApproval({ plan: existingPlan })) + + const harness = setup(repo, { answer: approveAnswer('design') }) + const result = await harness.run('demo design') + + assert.equal(result.kind, 'success') + const request = harness.calls[0].questions[0] + assert.equal(request.question, 'Approve demo design?') + + const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8')) + assert.equal(parsed.design.by, 'owner') + assert.match(parsed.design.at, ISO) + assert.equal(Number.isNaN(Date.parse(parsed.design.at)), false) + assert.deepEqual(parsed.plan, existingPlan, 'the pre-existing plan section is untouched') + } finally { + cleanup(tmp) + } +}) + +test('approve design refuses without confirmation', async () => { + const { tmp, repo } = repoFixture() + try { + const harness = setup(repo, { + answer: { answers: [{ id: 'approve-design', selected: ['Cancel'] }] }, + }) + const result = await harness.run('demo design') + + assert.match(result.text, /not confirmed/u) + assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written') + } finally { + cleanup(tmp) + } +}) + +test('approve plan refuses without design section', async () => { + const { tmp, repo } = repoFixture() + try { + const harness = setup(repo, { answer: approveAnswer('plan') }) + const result = await harness.run('demo plan') + + assert.match(result.text, /design not approved/u) + assert.match(result.text, /\/approve demo design/u) + assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written') + assert.equal(harness.calls.length, 0, 'the owner is not asked before the design gate passes') + } finally { + cleanup(tmp) + } +}) + +test('approve plan refuses when two plans reference the change', async () => { + const { tmp, repo } = repoFixture() + try { + write(join(repo, 'docs', 'superpowers', 'plans', 'plan-y.md'), PLAN_TEXT) + write( + approvalPath(repo), + serializeApproval({ design: { by: 'aep', at: '2026-10-25T10:00:00+04:00' } }), + ) + const before = readFileSync(approvalPath(repo), 'utf8') + + const harness = setup(repo, { answer: approveAnswer('plan') }) + const result = await harness.run('demo plan') + + assert.match(result.text, /plan-x\.md/u) + assert.match(result.text, /plan-y\.md/u) + assert.equal(readFileSync(approvalPath(repo), 'utf8'), before, 'approval.md is untouched') + assert.equal(harness.calls.length, 0, 'the owner is not asked when the plan is ambiguous') + } finally { + cleanup(tmp) + } +}) + +test('approve plan writes plan section with sha256 of the plan', async () => { + const { tmp, repo, planPath } = repoFixture() + try { + const design = { by: 'aep', at: '2026-10-25T10:00:00+04:00' } + write(approvalPath(repo), serializeApproval({ design })) + + const harness = setup(repo, { answer: approveAnswer('plan') }) + const result = await harness.run('demo plan') + + assert.equal(result.kind, 'success') + const request = harness.calls[0].questions[0] + assert.equal(request.question, 'Approve demo plan?') + + const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8')) + assert.equal(parsed.plan.by, 'owner') + assert.match(parsed.plan.at, ISO) + assert.equal(parsed.plan.sha256, shaOf(planPath)) + assert.equal(parsed.plan.sha256, hashPlan(planPath)) + assert.deepEqual(parsed.design, design, 'the design section is untouched') + } finally { + cleanup(tmp) + } +}) + +test('approve refuses when userQuestions is missing', async () => { + const { tmp, repo } = repoFixture() + try { + const harness = setup(repo, { missingService: true }) + const result = await harness.run('demo design') + + assert.match(result.text, /userQuestions service unavailable/u) + assert.match(result.text, /\/approve requires it/u) + assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written') + } finally { + cleanup(tmp) + } +}) + +test('approve with unknown kind returns usage', async () => { + const { tmp, repo } = repoFixture() + try { + const harness = setup(repo, { answer: approveAnswer() }) + + const unknown = await harness.run('demo review') + assert.match(unknown.text, /Usage: \/approve design\|plan/u) + assert.equal(unknown.kind, 'error') + + const missing = await harness.run('demo') + assert.match(missing.text, /Usage: \/approve design\|plan/u) + + assert.equal(harness.calls.length, 0, 'nothing is asked') + assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written') + } finally { + cleanup(tmp) + } +}) -- 2.47.2 From bb9b6de81a68b441b6bfad3a27b310d41c70cd07 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 01:43:25 +0400 Subject: [PATCH 06/10] =?UTF-8?q?docs:=20=D0=BF=D0=BB=D0=B0=D0=BD=20?= =?UTF-8?q?=E2=80=94=20Task=203=20=D0=BF=D0=BE=D0=B4=D0=BA=D0=BB=D1=8E?= =?UTF-8?q?=D1=87=D0=B0=D0=B5=D1=82=20checkSddWrite=20(=D0=B2=D0=BA=D0=BB.?= =?UTF-8?q?=20bash)=20+=20=D0=BF=D0=B8=D0=BD=D1=8B=20T1.7=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md b/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md index 16f3727..03301de 100644 --- a/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md +++ b/docs/superpowers/plans/2026-10-25-enforce-upstream-loop.md @@ -129,7 +129,7 @@ git commit -m "feat(dsh): /approve — единственный писатель - Consumes: Task 1 (gates.js), Task 2 (command-approve.js), существующая цепочка `ctx.tools.guard` (lib/guards.js). - Produces: в `apply()`: - `DEFAULTS += { approvalGates: true }`; gatesOk — из selfcheck (tools.guard). - - В guard-цепочке после hook-проверок: Edit/Write/MultiEdit → `checkPlanWrite` + `checkApprovalWrite`; `subagent`/`subagent_fork`/`workflow` → `checkDispatch` (prompt/script из `exec.arguments`); `Bash` → если в команде грубая подстрока `docs/superpowers/plans` или `approval.md` — `spawnSync python3 hooks/harness_target.py targets '' `, цели → `checkBashTargets`. Любой deny → причина отказа (hook-стиль `BLOCKED [approval-gates]: …`). Ошибка/пропуск парсинга — allow (инвариант хуков). + - В guard-цепочке после hook-проверок: Edit/Write/MultiEdit → `checkPlanWrite` + `checkApprovalWrite` + `checkSddWrite`; `subagent`/`subagent_fork`/`workflow` → `checkDispatch` (prompt/script из `exec.arguments`); `Bash` → если в команде грубая подстрока `docs/superpowers/plans`, `approval.md` или `.superpowers/sdd` — `spawnSync python3 hooks/harness_target.py targets '' `, цели → `checkBashTargets` + `checkSddWrite` для путей под `.superpowers/sdd/`. Любой deny → причина отказа (hook-стиль `BLOCKED [approval-gates]: …`). Ошибка/пропуск парсинга — allow (инвариант хуков). - `/approve` регистрируется в том же `ctx.inject(['commands', 'userQuestions'])` (userQuestions может отсутствовать — команда сама откажет, D-12). - `repoRoot` для гейтов — cwd сессии (`sessionCwd`), как в guards. @@ -141,6 +141,10 @@ test('apply: guard allows plan write after /approve-like approval.md', ...) test('apply: guard denies subagent dispatch referencing unapproved plan', ...) test('apply: guard allows dispatch after plan approval (sha match)', ...) test('apply: bash heredoc into approval.md denied', ...) // RF4 +test('apply: bash write under .superpowers/sdd// denied without approval', ...) // checkSddWrite bash-ветка +test('apply: bash write under .superpowers/sdd// allowed with approval', ...) +test('gates: checkSddWrite sha-mismatch deny', ...) // T1.7 pin +test('gates: bare .superpowers/sdd/ dir ungated', ...) // T1.7 pin test('apply: approvalGates:false disables all gates', ...) ``` -- 2.47.2 From a4cda5be8dfdd273065ee5a608a57a85cd9a750b Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 02:03:07 +0400 Subject: [PATCH 07/10] =?UTF-8?q?feat(dsh):=20wiring=20=D0=B3=D0=B5=D0=B9?= =?UTF-8?q?=D1=82=D0=BE=D0=B2=20A/B/C=20=D0=B2=20guard-=D1=86=D0=B5=D0=BF?= =?UTF-8?q?=D0=BE=D1=87=D0=BA=D1=83=20+=20bash-=D1=86=D0=B5=D0=BB=D0=B8=20?= =?UTF-8?q?+=20/approve=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/bundle/index.js | 127 ++++++++++++++++++- dsh/bundle/package.json | 1 + dsh/tests/approve.test.mjs | 65 +++++++++- dsh/tests/package.test.mjs | 3 + dsh/tests/run.sh | 2 +- dsh/tests/selfcheck.test.mjs | 239 +++++++++++++++++++++++++++++++++++ 6 files changed, 430 insertions(+), 7 deletions(-) diff --git a/dsh/bundle/index.js b/dsh/bundle/index.js index af25189..76c9774 100644 --- a/dsh/bundle/index.js +++ b/dsh/bundle/index.js @@ -17,13 +17,16 @@ * installation, so only Node builtins and files in this package are safe. */ +import { spawnSync } from 'node:child_process' import { existsSync } from 'node:fs' import { homedir } from 'node:os' -import { dirname, join } from 'node:path' +import { dirname, isAbsolute, join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { registerRoleCommand } from './command-role.js' import { registerCheckCommand } from './command-check.js' +import { registerApproveCommand } from './command-approve.js' import { createGuard } from './lib/guards.js' +import { checkApprovalWrite, checkDispatch, checkPlanWrite, checkSddWrite } from './lib/gates.js' import { isBootstrapVisible, prepareBootstrap } from './lib/bootstrap.js' import { isLegacyClaudePath, warnLegacyOnce } from './lib/legacy.js' import { createUserMessage } from './lib/message.js' @@ -73,9 +76,109 @@ function sessionCwd(workingDirectory, agent) { return process.cwd() } +/** DSH tools that write file content (the Edit/Write/MultiEdit family). */ +const WRITE_TOOLS = new Set(['write', 'edit', 'multi_edit', 'str_replace_editor']) + +/** Coarse substrings that make a Bash command worth target-parsing (D-8). */ +const BASH_GATE_HINTS = ['docs/superpowers/plans', 'approval.md', '.superpowers/sdd'] + +/** + * Write targets of one Bash command, parsed by the same parser the hooks use + * (`harness_target.py targets`). `null` on any spawn/parse failure — the gate + * fails open exactly like the hooks do. + */ +function bashWriteTargets(hooksDir, command, cwd, logger) { + const parser = join(hooksDir, 'harness_target.py') + let result + try { + result = spawnSync('python3', [parser, 'targets', command, cwd], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 15_000, + }) + } catch (error) { + logger?.warn?.('[dsh-harness] approval gates: bash target parse failed open: %s', error?.message ?? String(error)) + return null + } + if (result.error || result.status !== 0) { + logger?.warn?.( + '[dsh-harness] approval gates: harness_target.py failed open (%s)', + result.error?.message ?? `exit ${result.status}`, + ) + return null + } + const targets = [] + for (const line of String(result.stdout ?? '').split(/\r?\n/u)) { + if (!line.startsWith('file ')) continue + const path = line.slice('file '.length).trim().split(/\s+/u)[0] + if (path !== '') targets.push(path) + } + return targets +} + +/** + * Approval gates A/B/C for one tool call, run after the hook guards. + * Returns the first deny reason or undefined. All failures fail open: a call + * that names no plan/path is never blocked, and a broken bash parser pass is + * allowed, mirroring the hook invariant. + */ +function approvalGateDeny({ exec, repoRoot, hooksDir, logger }) { + const name = exec?.name + const args = exec?.arguments ?? {} + + if (WRITE_TOOLS.has(name)) { + const rawPath = args.file_path ?? args.path + if (typeof rawPath !== 'string' || rawPath === '') return undefined + const filePath = isAbsolute(rawPath) ? resolve(rawPath) : resolve(repoRoot, rawPath) + const content = + typeof args.content === 'string' + ? args.content + : typeof args.new_string === 'string' + ? args.new_string + : null + for (const verdict of [ + checkApprovalWrite({ filePath, repoRoot }), + checkPlanWrite({ filePath, content, repoRoot }), + checkSddWrite({ filePath, repoRoot }), + ]) { + if (verdict.deny !== undefined) return verdict.deny + } + return undefined + } + + if (name === 'subagent' || name === 'subagent_fork' || name === 'workflow') { + return checkDispatch({ + prompt: typeof args.prompt === 'string' ? args.prompt : undefined, + script: typeof args.script === 'string' ? args.script : undefined, + repoRoot, + }).deny + } + + if (name === 'bash') { + const command = args.command + if (typeof command !== 'string' || command === '') return undefined + if (!BASH_GATE_HINTS.some((hint) => command.includes(hint))) return undefined + const targets = bashWriteTargets(hooksDir, command, repoRoot, logger) + if (targets === null) return undefined + for (const target of targets) { + for (const verdict of [ + checkApprovalWrite({ filePath: target, repoRoot }), + checkPlanWrite({ filePath: target, content: null, repoRoot }), + checkSddWrite({ filePath: target, repoRoot }), + ]) { + if (verdict.deny !== undefined) return verdict.deny + } + } + return undefined + } + + return undefined +} + const DEFAULTS = { enabled: true, guards: true, + approvalGates: true, bootstrap: true, bootstrapSubagents: false, roleCommand: true, @@ -130,11 +233,21 @@ export function apply(ctx, rawConfig = {}) { // `tools/pre-execute` waterfall, so no other listener can turn a denial // back into permission. ctx.tools.guard((exec) => { - const reason = guard.check(exec, sessionCwd(ctx.workingDirectory, exec.agent)) + const repoRoot = sessionCwd(ctx.workingDirectory, exec.agent) + const reason = guard.check(exec, repoRoot) if (reason !== undefined) { ctx.logger?.info?.('[dsh-harness] blocked %s: %s', exec.name, reason.split('\n')[0]) + return reason } - return reason + if (config.enabled && config.approvalGates) { + const gateDeny = approvalGateDeny({ exec, repoRoot, hooksDir, logger: ctx.logger }) + if (gateDeny !== undefined) { + const gateReason = `BLOCKED [approval-gates]: ${gateDeny}` + ctx.logger?.info?.('[dsh-harness] blocked %s: %s', exec.name, gateReason.split('\n')[0]) + return gateReason + } + } + return undefined }) } @@ -185,6 +298,14 @@ export function apply(ctx, rawConfig = {}) { logger: ctx.logger, }) } + registerApproveCommand({ + commands: commandCtx.commands, + userQuestions: typeof ctx.get === 'function' ? ctx.get('userQuestions') : undefined, + // The command-injection scope may carry the owning agent/session; + // when it does not, sessionCwd falls back to the process directory. + repoRoot: sessionCwd(ctx.workingDirectory, commandCtx.agent ?? commandCtx), + logger: ctx.logger, + }) }) } diff --git a/dsh/bundle/package.json b/dsh/bundle/package.json index 00b2214..2a4d524 100644 --- a/dsh/bundle/package.json +++ b/dsh/bundle/package.json @@ -13,6 +13,7 @@ "index.js", "command-role.js", "command-check.js", + "command-approve.js", "lib", "dsh-tools.md", "cordis.patch.yml", diff --git a/dsh/tests/approve.test.mjs b/dsh/tests/approve.test.mjs index 8f2016d..2394b33 100644 --- a/dsh/tests/approve.test.mjs +++ b/dsh/tests/approve.test.mjs @@ -22,14 +22,17 @@ const ISO = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/u const APPROVE = 'Approve' /** A throwaway repository with the design artifacts and one demo plan. */ -function repoFixture({ builderCheck = true } = {}) { +function repoFixture({ builderCheck = true, withPlan = true } = {}) { const tmp = tempDir('dsh-approve-') const repo = join(tmp, 'repo') write(join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), '# Proposal\n') write(join(repo, 'openspec', 'changes', 'demo', 'design.md'), '# Design\n') if (builderCheck) write(join(repo, 'openspec', 'changes', 'demo', 'builder-check.md'), '# Check\n') - const planPath = join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md') - write(planPath, PLAN_TEXT) + let planPath = null + if (withPlan) { + planPath = join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md') + write(planPath, PLAN_TEXT) + } return { tmp, repo, planPath } } @@ -221,3 +224,59 @@ test('approve with unknown kind returns usage', async () => { cleanup(tmp) } }) + +// Pins for the Task 2 review findings (R-5): branches without a first test. + +test('approve plan refuses when no plan references the change', async () => { + const { tmp, repo } = repoFixture({ withPlan: false }) + try { + write( + approvalPath(repo), + serializeApproval({ design: { by: 'aep', at: '2026-10-25T10:00:00+04:00' } }), + ) + const before = readFileSync(approvalPath(repo), 'utf8') + + const harness = setup(repo, { answer: approveAnswer('plan') }) + const result = await harness.run('demo plan') + + assert.equal(result.kind, 'error') + assert.match(result.text, /no plan for demo found/u) + assert.match(result.text, /docs\/superpowers\/plans\//u) + assert.equal(readFileSync(approvalPath(repo), 'utf8'), before, 'approval.md is untouched') + assert.equal(harness.calls.length, 0, 'the owner is not asked without a plan') + } finally { + cleanup(tmp) + } +}) + +test('approve refuses an unsafe change id', async () => { + const { tmp, repo } = repoFixture() + try { + const harness = setup(repo, { answer: approveAnswer('design') }) + + const result = await harness.run('../x design') + assert.match(result.text, /Invalid change id "\.\.\/x"\./u) + assert.match(result.text, /Usage: \/approve design\|plan/u) + assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written') + assert.equal(harness.calls.length, 0, 'the owner is not asked for an invalid id') + } finally { + cleanup(tmp) + } +}) + +test('approve accepts the fallback answer shapes {approved}/{confirmed}/true', async () => { + const { tmp, repo } = repoFixture() + try { + const shapes = [{ approved: true }, { confirmed: true }, true] + for (const answer of shapes) { + const harness = setup(repo, { answer }) + const result = await harness.run('demo design') + assert.equal(result.kind, 'success', `answer shape ${JSON.stringify(answer)} approves`) + const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8')) + assert.equal(parsed.design.by, 'owner') + assert.match(parsed.design.at, ISO) + } + } finally { + cleanup(tmp) + } +}) diff --git a/dsh/tests/package.test.mjs b/dsh/tests/package.test.mjs index acb7a5d..010625d 100644 --- a/dsh/tests/package.test.mjs +++ b/dsh/tests/package.test.mjs @@ -14,6 +14,7 @@ test('package declares a DSH bundle patch and the runtime files', () => { assert.equal(pkg.dsh?.bundle?.patch, './cordis.patch.yml') assert.equal(pkg.exports?.['.'], './index.js') assert.ok(pkg.files.includes('index.js')) + assert.ok(pkg.files.includes('command-approve.js')) assert.ok(pkg.files.includes('lib')) assert.ok(pkg.files.includes('hooks')) assert.ok(existsSync(join(bundleDir, 'cordis.patch.yml'))) @@ -56,8 +57,10 @@ test('runtime files have no external @deepseek-ai imports', () => { 'index.js', 'command-role.js', 'command-check.js', + 'command-approve.js', 'lib/message.js', 'lib/guards.js', + 'lib/gates.js', 'lib/bootstrap.js', 'lib/role.js', 'lib/legacy.js', diff --git a/dsh/tests/run.sh b/dsh/tests/run.sh index 6ce8e30..6130c4b 100755 --- a/dsh/tests/run.sh +++ b/dsh/tests/run.sh @@ -23,7 +23,7 @@ ensure_bundle_hooks() { ensure_bundle_hooks -for f in dsh/bundle/index.js dsh/bundle/command-role.js dsh/bundle/command-check.js dsh/bundle/lib/*.js dsh/tests/*.mjs; do +for f in dsh/bundle/index.js dsh/bundle/command-role.js dsh/bundle/command-check.js dsh/bundle/command-approve.js dsh/bundle/lib/*.js dsh/tests/*.mjs; do node --check "$f" done diff --git a/dsh/tests/selfcheck.test.mjs b/dsh/tests/selfcheck.test.mjs index d3d6e9e..d32a086 100644 --- a/dsh/tests/selfcheck.test.mjs +++ b/dsh/tests/selfcheck.test.mjs @@ -1,8 +1,12 @@ import assert from 'node:assert/strict' +import { createHash } from 'node:crypto' +import { join } from 'node:path' import test from 'node:test' import { apply } from '../bundle/index.js' import { collectSelfCheck } from '../bundle/lib/selfcheck.js' +import { checkSddWrite } from '../bundle/lib/gates.js' import { registerCheckCommand } from '../bundle/command-check.js' +import { cleanup, git, makeRepo, write } from './helpers.mjs' const FEATURES = ['guards', 'bootstrap', 'commands', 'workingDirectory'] @@ -167,3 +171,238 @@ test('apply wires guards, bootstrap, and commands with a full ctx', () => { assert.equal(calls.on, 1, 'bootstrap wired') assert.equal(calls.inject, 1, 'commands wired') }) + +// --------------------------------------------------------------------------- +// Approval gates (Task 3): apply() wires the A/B/C gates after the hooks. +// --------------------------------------------------------------------------- + +const PLAN_TEXT = [ + '# Plan X', + '', + '**Spec:** `openspec/changes/demo/`', + '', + '## Task 1', + '', + 'Do the work.', + '', +].join('\n') + +/** + * A throwaway repository on a feature branch (the hooks must pass) with the + * change artifacts and one demo plan. `design` writes a design-approved + * approval.md; `planSha` additionally writes the plan section (default: the + * real sha256 of the fixture plan). + */ +function gateFixture({ design = false, planSha } = {}) { + const repo = makeRepo() + git(repo, ['checkout', '-b', 'feat/x']) + write(join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), '# Proposal\n') + write(join(repo, 'openspec', 'changes', 'demo', 'design.md'), '# Design\n') + const planPath = join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md') + write(planPath, PLAN_TEXT) + if (design || planSha !== undefined) { + const sections = [] + if (design) { + sections.push('design:\n by: owner\n at: 2026-10-25T10:00:00.000Z') + } + if (planSha !== undefined) { + sections.push(`plan:\n by: owner\n at: 2026-10-25T11:00:00.000Z\n sha256: ${planSha}`) + } + write(join(repo, 'openspec', 'changes', 'demo', 'approval.md'), `${sections.join('\n\n')}\n`) + } + return { repo, planPath } +} + +function planShaOf(planText) { + return createHash('sha256').update(planText).digest('hex') +} + +/** apply() against a fake ctx: captured guard + inject, session cwd = repo. */ +function gateHarness(repo, config = {}) { + const guardFns = [] + const injected = [] + const ctx = { + tools: { guard(fn) { guardFns.push(fn) } }, + on() {}, + inject(names, callback) { injected.push({ names, callback }) }, + workingDirectory: { get() { return repo } }, + get() { return undefined }, + logger: { info() {}, warn() {}, error() {} }, + } + apply(ctx, config) + return { + guard: (exec) => guardFns[0]({ agent: { session: 's1' }, ...exec }), + injected, + } +} + +function call(name, args) { + return { name, arguments: args } +} + +test('apply: guard denies plan write without design approval', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('write', { + file_path: join(repo, 'docs', 'superpowers', 'plans', 'plan-new.md'), + content: PLAN_TEXT, + })) + assert.match(reason ?? '', /BLOCKED \[approval-gates\]: design not approved; run \/approve demo design/) + } finally { + cleanup(repo) + } +}) + +test('apply: guard allows plan write after approval.md with design', () => { + const { repo } = gateFixture({ design: true }) + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('write', { + file_path: join(repo, 'docs', 'superpowers', 'plans', 'plan-new.md'), + content: PLAN_TEXT, + })) + assert.equal(reason, undefined) + } finally { + cleanup(repo) + } +}) + +test('apply: guard denies subagent dispatch referencing unapproved plan', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('subagent', { + prompt: 'Implement docs/superpowers/plans/plan-x.md Task 1.', + })) + assert.match(reason ?? '', /BLOCKED \[approval-gates\]: plan not approved; run \/approve demo plan/) + } finally { + cleanup(repo) + } +}) + +test('apply: guard allows dispatch after plan approval (sha match)', () => { + const { repo } = gateFixture({ design: true, planSha: planShaOf(PLAN_TEXT) }) + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('subagent', { + prompt: 'Implement docs/superpowers/plans/plan-x.md Task 1.', + })) + assert.equal(reason, undefined) + } finally { + cleanup(repo) + } +}) + +test('apply: bash heredoc into approval.md denied', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('bash', { + command: "cat > openspec/changes/demo/approval.md <<'EOF'\nby: me\nEOF\n", + })) + assert.match(reason ?? '', /BLOCKED \[approval-gates\]: approval\.md is written only by \/approve/) + } finally { + cleanup(repo) + } +}) + +test('apply: bash write under .superpowers/sdd// denied without approval', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('bash', { + command: 'mkdir -p .superpowers/sdd/plan-x && printf "x" > .superpowers/sdd/plan-x/progress.md', + })) + assert.match(reason ?? '', /BLOCKED \[approval-gates\]: plan not approved; run \/approve demo plan/) + } finally { + cleanup(repo) + } +}) + +test('apply: bash write under .superpowers/sdd// allowed with approval', () => { + const { repo } = gateFixture({ design: true, planSha: planShaOf(PLAN_TEXT) }) + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('bash', { + command: 'mkdir -p .superpowers/sdd/plan-x && printf "x" > .superpowers/sdd/plan-x/progress.md', + })) + assert.equal(reason, undefined) + } finally { + cleanup(repo) + } +}) + +test('gates: checkSddWrite sha-mismatch deny', () => { + const { repo } = gateFixture({ design: true, planSha: 'a'.repeat(64) }) + try { + const result = checkSddWrite({ + filePath: join(repo, '.superpowers', 'sdd', 'plan-x', 'progress.md'), + repoRoot: repo, + }) + assert.equal(result.deny, 'plan changed after approval; re-approve') + } finally { + cleanup(repo) + } +}) + +test('gates: bare .superpowers/sdd/ dir ungated', () => { + const { repo } = gateFixture() + try { + const result = checkSddWrite({ + filePath: join(repo, '.superpowers', 'sdd', 'plan-x'), + repoRoot: repo, + }) + assert.equal(result.deny, undefined) + } finally { + cleanup(repo) + } +}) + +test('apply: approvalGates:false disables all gates', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo, { approvalGates: false }) + assert.equal( + harness.guard(call('write', { + file_path: join(repo, 'docs', 'superpowers', 'plans', 'plan-new.md'), + content: PLAN_TEXT, + })), + undefined, + ) + assert.equal( + harness.guard(call('subagent', { + prompt: 'Implement docs/superpowers/plans/plan-x.md Task 1.', + })), + undefined, + ) + assert.equal( + harness.guard(call('bash', { + command: "cat > openspec/changes/demo/approval.md <<'EOF'\nby: me\nEOF\n", + })), + undefined, + ) + } finally { + cleanup(repo) + } +}) + +test('apply: /approve registered when commands present; absent userQuestions still registers', () => { + const { repo } = gateFixture({ design: true }) + try { + const harness = gateHarness(repo) + assert.equal(harness.injected.length, 1, 'the commands inject block ran') + + const registered = [] + harness.injected[0].callback({ + commands: { register(definition) { registered.push(definition) } }, + }) + + const approve = registered.find((definition) => definition.name === 'approve') + assert.ok(approve, '/approve is registered') + assert.equal(typeof approve.handler, 'function') + assert.equal(approve.input.hint, ' design|plan') + } finally { + cleanup(repo) + } +}) -- 2.47.2 From 653d1df30f8ff678c814c28ff5f40aa1764a7cda Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 02:20:27 +0400 Subject: [PATCH 08/10] =?UTF-8?q?fix(dsh):=20/approve=20=E2=80=94=20per-in?= =?UTF-8?q?vocation=20repoRoot,=20=D1=80=D0=B5=D0=B3=D0=B8=D1=81=D1=82?= =?UTF-8?q?=D1=80=D0=B0=D1=86=D0=B8=D1=8F=20=D0=BF=D0=BE=20approvalGates,?= =?UTF-8?q?=20checkBashTargets=20(T3.1-T3.2,T3.7,=20#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/bundle/command-approve.js | 20 +++++++++- dsh/bundle/index.js | 35 +++++++++++----- dsh/tests/approve.test.mjs | 39 +++++++++++++++++- dsh/tests/selfcheck.test.mjs | 75 ++++++++++++++++++++++++++++++++--- 4 files changed, 149 insertions(+), 20 deletions(-) diff --git a/dsh/bundle/command-approve.js b/dsh/bundle/command-approve.js index 56aa63f..bfece3a 100644 --- a/dsh/bundle/command-approve.js +++ b/dsh/bundle/command-approve.js @@ -7,6 +7,11 @@ * merges the new section into the existing file. A missing user-questions * service refuses instead of writing, so no model tool can approve itself. * + * The repository root is resolved per invocation from the receiving agent's + * session (`workingDirectory.get(agent.session)`, T3.1) — the registration + * itself carries no root, because the command runs against whatever agent + * invoked it. + * * Mirrors `command-role.js`: object-form `commands.register`, the same * `{ kind, text }` result shape. */ @@ -53,6 +58,17 @@ function readText(path) { } } +/** Session working directory of the invoking agent; process.cwd() fallback. */ +function currentDirectory(workingDirectory, agent) { + try { + const cwd = workingDirectory?.get?.(agent?.session) + if (typeof cwd === 'string' && cwd !== '') return cwd + } catch { + // Fall through to the process directory. + } + return process.cwd() +} + function refusal(text) { return { kind: 'error', text: `Refused: ${text}` } } @@ -228,7 +244,7 @@ async function approvePlan({ root, changeId, userQuestions, agent, signal }) { } } -export function registerApproveCommand({ commands, userQuestions, repoRoot, logger }) { +export function registerApproveCommand({ commands, userQuestions, workingDirectory, logger }) { commands.register({ name: 'approve', description: 'Утвердить design или plan change (единственный писатель approval.md)', @@ -249,7 +265,7 @@ export function registerApproveCommand({ commands, userQuestions, repoRoot, logg return usage(`Unknown kind "${kind}".`) } - const root = resolve(repoRoot || process.cwd()) + const root = resolve(currentDirectory(workingDirectory, agent)) try { if (kind === 'design') { return await approveDesign({ root, changeId, userQuestions, agent, signal }) diff --git a/dsh/bundle/index.js b/dsh/bundle/index.js index 76c9774..761578e 100644 --- a/dsh/bundle/index.js +++ b/dsh/bundle/index.js @@ -26,7 +26,13 @@ import { registerRoleCommand } from './command-role.js' import { registerCheckCommand } from './command-check.js' import { registerApproveCommand } from './command-approve.js' import { createGuard } from './lib/guards.js' -import { checkApprovalWrite, checkDispatch, checkPlanWrite, checkSddWrite } from './lib/gates.js' +import { + checkApprovalWrite, + checkBashTargets, + checkDispatch, + checkPlanWrite, + checkSddWrite, +} from './lib/gates.js' import { isBootstrapVisible, prepareBootstrap } from './lib/bootstrap.js' import { isLegacyClaudePath, warnLegacyOnce } from './lib/legacy.js' import { createUserMessage } from './lib/message.js' @@ -160,14 +166,13 @@ function approvalGateDeny({ exec, repoRoot, hooksDir, logger }) { if (!BASH_GATE_HINTS.some((hint) => command.includes(hint))) return undefined const targets = bashWriteTargets(hooksDir, command, repoRoot, logger) if (targets === null) return undefined + // Plans/approval.md through the shared target check (T3.7), then the + // additional sdd pass for `.superpowers/sdd/` targets. + const bashVerdict = checkBashTargets(targets, repoRoot) + if (bashVerdict.deny !== undefined) return bashVerdict.deny for (const target of targets) { - for (const verdict of [ - checkApprovalWrite({ filePath: target, repoRoot }), - checkPlanWrite({ filePath: target, content: null, repoRoot }), - checkSddWrite({ filePath: target, repoRoot }), - ]) { - if (verdict.deny !== undefined) return verdict.deny - } + const sdd = checkSddWrite({ filePath: target, repoRoot }) + if (sdd.deny !== undefined) return sdd.deny } return undefined } @@ -298,12 +303,20 @@ export function apply(ctx, rawConfig = {}) { logger: ctx.logger, }) } + }) + } + + // /approve has its own activation switch (T3.2), independent of /role and + // /dsh-harness-check: the command resolves its repoRoot per invocation from + // the receiving agent, so the wiring passes the service, not a root. The + // commands availability is checked directly — the self-check skips the + // `commands` feature when both roleCommand and checkCommand are off. + if (config.enabled && config.approvalGates && typeof ctx.inject === 'function') { + ctx.inject(['commands'], (commandCtx) => { registerApproveCommand({ commands: commandCtx.commands, userQuestions: typeof ctx.get === 'function' ? ctx.get('userQuestions') : undefined, - // The command-injection scope may carry the owning agent/session; - // when it does not, sessionCwd falls back to the process directory. - repoRoot: sessionCwd(ctx.workingDirectory, commandCtx.agent ?? commandCtx), + workingDirectory: ctx.workingDirectory, logger: ctx.logger, }) }) diff --git a/dsh/tests/approve.test.mjs b/dsh/tests/approve.test.mjs index 2394b33..f993060 100644 --- a/dsh/tests/approve.test.mjs +++ b/dsh/tests/approve.test.mjs @@ -47,7 +47,8 @@ function approveAnswer(kind = 'design') { /** * Wire the command against a fake `commands.register` that captures the - * registered definition, and a scripted `userQuestions.ask`. + * registered definition, and a scripted `userQuestions.ask`. The fake + * `workingDirectory` resolves every session to the fixture repo. */ function setup(repo, { answer, missingService = false } = {}) { const captured = {} @@ -69,7 +70,7 @@ function setup(repo, { answer, missingService = false } = {}) { registerApproveCommand({ commands, userQuestions, - repoRoot: repo, + workingDirectory: { get() { return repo } }, logger: { warn() {} }, }) return { captured, calls, run: (rawInput) => captured.handler({ rawInput }) } @@ -280,3 +281,37 @@ test('approve accepts the fallback answer shapes {approved}/{confirmed}/true', a cleanup(tmp) } }) + +// T3.1 regression: the repo root is resolved per handler invocation from the +// invocation agent's session, not fixed at registration. + +test('approve resolves the session cwd per invocation (T3.1)', async () => { + const first = repoFixture() + const second = repoFixture() + try { + const roots = new Map([ + ['s1', first.repo], + ['s2', second.repo], + ]) + const captured = {} + const commands = { register(definition) { Object.assign(captured, definition) } } + registerApproveCommand({ + commands, + userQuestions: { async ask() { return approveAnswer('design') } }, + workingDirectory: { get(session) { return roots.get(session) } }, + logger: { warn() {} }, + }) + + const firstResult = await captured.handler({ agent: { session: 's1' }, rawInput: 'demo design' }) + assert.equal(firstResult.kind, 'success') + assert.equal(existsSync(approvalPath(first.repo)), true, 'approval.md written in the s1 repo') + assert.equal(existsSync(approvalPath(second.repo)), false, 'the s2 repo is untouched') + + const secondResult = await captured.handler({ agent: { session: 's2' }, rawInput: 'demo design' }) + assert.equal(secondResult.kind, 'success') + assert.equal(existsSync(approvalPath(second.repo)), true, 'approval.md written in the s2 repo') + } finally { + cleanup(first.tmp) + cleanup(second.tmp) + } +}) diff --git a/dsh/tests/selfcheck.test.mjs b/dsh/tests/selfcheck.test.mjs index d32a086..093fb16 100644 --- a/dsh/tests/selfcheck.test.mjs +++ b/dsh/tests/selfcheck.test.mjs @@ -169,7 +169,7 @@ test('apply wires guards, bootstrap, and commands with a full ctx', () => { assert.doesNotThrow(() => apply(ctx)) assert.equal(calls.guard, 1, 'guards wired') assert.equal(calls.on, 1, 'bootstrap wired') - assert.equal(calls.inject, 1, 'commands wired') + assert.equal(calls.inject, 2, 'commands wired (role/check + /approve)') }) // --------------------------------------------------------------------------- @@ -391,12 +391,23 @@ test('apply: /approve registered when commands present; absent userQuestions sti const { repo } = gateFixture({ design: true }) try { const harness = gateHarness(repo) - assert.equal(harness.injected.length, 1, 'the commands inject block ran') + assert.equal(harness.injected.length, 2, 'role/check and /approve inject blocks ran') + // A cordis-like command context: declared services resolve, any other key + // (e.g. `agent`) throws — the wiring must not read it (T3.1). const registered = [] - harness.injected[0].callback({ - commands: { register(definition) { registered.push(definition) } }, - }) + const commandCtx = new Proxy( + { commands: { register(definition) { registered.push(definition) } } }, + { + get(target, prop) { + if (prop in target) return target[prop] + throw new Error(`unknown service "${String(prop)}"`) + }, + }, + ) + for (const { callback } of harness.injected) { + assert.doesNotThrow(() => callback(commandCtx), 'the inject callback must not touch ctx.agent') + } const approve = registered.find((definition) => definition.name === 'approve') assert.ok(approve, '/approve is registered') @@ -406,3 +417,57 @@ test('apply: /approve registered when commands present; absent userQuestions sti cleanup(repo) } }) + +test('apply: /approve registers on approvalGates alone (role/check commands off)', () => { + const { repo } = gateFixture({ design: true }) + try { + const harness = gateHarness(repo, { roleCommand: false, checkCommand: false }) + assert.equal(harness.injected.length, 1, 'only the /approve inject block runs') + + const registered = [] + harness.injected[0].callback({ + commands: { register(definition) { registered.push(definition) } }, + }) + assert.ok( + registered.some((definition) => definition.name === 'approve'), + '/approve is registered without role/check commands', + ) + assert.ok( + !registered.some((definition) => definition.name === 'role'), + '/role is not registered', + ) + } finally { + cleanup(repo) + } +}) + +test('apply: approvalGates:false leaves /approve unregistered', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo, { approvalGates: false }) + + const registered = [] + for (const { callback } of harness.injected) { + callback({ commands: { register(definition) { registered.push(definition) } } }) + } + assert.ok( + !registered.some((definition) => definition.name === 'approve'), + '/approve is not registered when approvalGates is off', + ) + } finally { + cleanup(repo) + } +}) + +test('apply: bash targets prefer approval/plan denies before sdd denies', () => { + const { repo } = gateFixture() + try { + const harness = gateHarness(repo) + const reason = harness.guard(call('bash', { + command: 'printf "y" > .superpowers/sdd/plan-x/progress.md && printf "x" > openspec/changes/demo/approval.md', + })) + assert.match(reason ?? '', /BLOCKED \[approval-gates\]: approval\.md is written only by \/approve/) + } finally { + cleanup(repo) + } +}) -- 2.47.2 From 734c80899dc32f611321e04659d9a53642278faf Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 02:30:36 +0400 Subject: [PATCH 09/10] =?UTF-8?q?docs(dsh):=20sdd-=D1=81=D0=BA=D0=B8=D0=BB?= =?UTF-8?q?=D0=BB,=20roles-=D0=B7=D0=B0=D0=BC=D0=B5=D1=82=D0=BA=D0=B0,=20?= =?UTF-8?q?=D1=81=D0=BF=D0=B5=D0=BA=D0=B0=20approval-gates,=20=D0=B4=D0=BE?= =?UTF-8?q?=D0=BA=D0=B8=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/PORT.md | 15 +- dsh/README.md | 19 ++ dsh/install.sh | 6 +- dsh/skills/sdd/SKILL.md | 6 + dsh/tests/approve.test.mjs | 32 ++ dsh/tests/install.test.mjs | 8 +- .../changes/enforce-upstream-loop/design.md | 32 +- .../specs/approval-gates/spec.md | 287 ++++++++++++++++++ 8 files changed, 393 insertions(+), 12 deletions(-) create mode 100644 openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md diff --git a/dsh/PORT.md b/dsh/PORT.md index e7ac230..e89c821 100644 --- a/dsh/PORT.md +++ b/dsh/PORT.md @@ -15,7 +15,7 @@ This is a port, not a copy. Three layers map differently: | Instructions | `~/.claude/CLAUDE.md`, project `CLAUDE.md` | `$DSH_HOME/AGENTS.md`, project `CLAUDE.md`/`AGENTS.md`, nested | almost 1:1 | | Skills | `~/.claude/skills`, `.claude/skills` | `~/.dsh/skills`, `.agents/skills`, `customSkillDirs` | 1:1 format | | Roles | `roles/`, `/role` skill | same files + `/role` command + model-invocable `role` skill | close | -| Hard rules | `PreToolUse` shell hooks | `ctx.tools.guard` Cordis guard | logic reused, wrapper new | +| Hard rules | `PreToolUse` shell hooks | `ctx.tools.guard` Cordis guard + approval gates A/B/C | logic reused, wrapper new | | Superpowers | plugin + `SessionStart` hook | pinned sync of tag `v7.0.1` (`DSH_SUPERPOWERS_REF`) into `~/.dsh/skills` + `agent/pre-step` bootstrap | content adapted by sync, bootstrap new | | SDD | `Task` (synchronous) | `subagent` (asynchronous) / `workflow` | semantics differ | | Gitea | CLI + skills | `bash` + skills | 1:1 | @@ -172,6 +172,7 @@ for the repository layout: config: enabled: true guards: true + approvalGates: true # gates A/B/C; active only inside the guard chain bootstrap: true roleCommand: true selfCheck: true # activation check of the DSH contract; not-ok is logged @@ -354,6 +355,18 @@ profile was removed; the real `desktop` profile was not touched. ## 7. Risks and open questions +- **Instructions are not enforcement — closed by gates A/B/C.** The upstream loop + (builder-check → `/approve design` → plan → `/approve plan` → sdd) + exists in superpowers as text, and the 24.10.2026 session skipped builder-check + while the plan was written before design approval. Change + `enforce-upstream-loop` adds `lib/gates.js` + `command-approve.js`: gate A + denies a plan for an unapproved design, gate B denies an sdd start (a dispatch + or a `.superpowers/sdd/**` write) for an unapproved or changed plan (sha256), + gate C keeps `approval.md` `/approve`-only, and bash write targets are parsed + by `harness_target.py` and gated too. Residual risk: only a session with the + reinstalled bundle is gated (D-13), and `terminal_*` tools stay outside the + chain (T3.6); the contract is pinned in + `openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md`. - **DSH is 0.2.1-rc.3.** The plugin/event API is not frozen; pin the version and re-run the tests after upgrades. - **Contract self-check.** The plugin/event API drift is now caught from inside diff --git a/dsh/README.md b/dsh/README.md index 3c80e68..6d97d92 100644 --- a/dsh/README.md +++ b/dsh/README.md @@ -9,6 +9,10 @@ separate Cordis bundle plus a thin install helper. - `guard-main` / `require-plan` — the tested bash/Python hooks run through `ctx.tools.guard`; the agent gets `BLOCKED` with the same messages as in Claude Code; +- approval gates A/B/C — `lib/gates.js` + `command-approve.js`: a plan cannot be + written before its change design is approved, sdd cannot start before the plan + is approved (sha256-pinned), and `approval.md` is written only by + `/approve design|plan`; - superpowers bootstrap — `using-superpowers` is injected once per session from `agent/pre-step` and re-injected after compaction; - roles — the model loads them with the `role` skill (`~/.dsh/roles`, then @@ -145,7 +149,9 @@ dsh/ index.js ctx.tools.guard + agent/pre-step + /role command-role.js /role command glue command-check.js dsh-harness-check command glue + command-approve.js /approve command: the only writer of approval.md lib/guards.js DSH tool call -> claude-harness hook JSON + lib/gates.js approval gates A/B/C and approval.md/plan io (pure) lib/bootstrap.js superpowers skill resolution, marker, dedupe lib/role.js role layers and text (pure) lib/selfcheck.js bundle contract self-check (pure) @@ -178,6 +184,7 @@ repository layout: - id: dsh-harness config: guards: true + approvalGates: true # gates A/B/C; effective only inside the guard chain bootstrap: true roleCommand: true selfCheck: true # activation check of the DSH contract; not-ok is logged @@ -196,6 +203,18 @@ repository layout: - The guard adapter uses the repository's bash/Python hooks; if `python3` is missing, the hooks fail open by design. +- Approval gates (change `enforce-upstream-loop`) live inside the guard chain: + they need `enabled: true`, `guards: true`, `approvalGates: true` **and** a + passing `guards` self-check (`guardsOk`); `approvalGates: true` alone does not + enable them. They are active only in a session with the reinstalled bundle + (D-13): an old install keeps the text-only loop, and the `sdd` skill checks the + plan approval by hand. +- Plan files are always rewritten in full: an `edit` fragment cannot carry the + `**Spec:**` line, so gate A denies it — as it denies bash plan targets and + `str_replace_editor` creates, whose content it cannot see. The deny text then + names the missing `**Spec:**`, not the real reason. +- `terminal_open` / `terminal_send` write files outside both the gates and the + bash hooks — a residual risk not covered by the port. - `bundle/hooks` is a symlink for local-directory installation. A published bundle needs a real copy of `hooks/` (already listed in `package.json` `files`). diff --git a/dsh/install.sh b/dsh/install.sh index d95fd5c..5ce3d81 100755 --- a/dsh/install.sh +++ b/dsh/install.sh @@ -617,7 +617,11 @@ note = ( "Режим контроллера — по числу параллельных планов: один план — главная сессия, " "несколько — фоновый контроллер (нужен `maxDepth: 2` из " "`dsh/subagent-models.yml`). В Claude Code строка не действует: там исходный " - "`subagent-driven-development`." + "`subagent-driven-development`. " + "Бандл гейтит цикл: builder-check обязателен до передачи дизайна владельцу " + "(иначе `/approve design` откажет); план пишется только после `/approve design`; " + "sdd стартует только после `/approve plan` (гейты A/B/C, " + "`BLOCKED [approval-gates]`). Пропуск — только явное решение владельца." ) anchor = "- Режим по умолчанию: subagent-driven-development." lines = text.splitlines(keepends=True) diff --git a/dsh/skills/sdd/SKILL.md b/dsh/skills/sdd/SKILL.md index dc9d1e2..e9fefab 100644 --- a/dsh/skills/sdd/SKILL.md +++ b/dsh/skills/sdd/SKILL.md @@ -39,6 +39,12 @@ overlay ребёнок наследует модель сессии. ## Протокол диспатча и отчёта +**Проверка approval-маркера до первого диспатча.** Найди change по строке +`**Spec:**` плана и проверь `openspec/changes//approval.md`: секция `plan:` +есть — можно начинать; нет — не диспатчить, вернуть владельцу `NEEDS_CONTEXT` +с «run /approve plan». Гейты активны только с переустановленным бандлом +(D-13): до него это ручная проверка, после — гейт B (`BLOCKED [approval-gates]`). + 1. Первым делом — ledger (см. ниже) и скан плана на конфликты; рулинги до Task 1. 2. Бриф implementer'а — **файлом** `/briefs/task-.md`: контекст, diff --git a/dsh/tests/approve.test.mjs b/dsh/tests/approve.test.mjs index f993060..8bf52c2 100644 --- a/dsh/tests/approve.test.mjs +++ b/dsh/tests/approve.test.mjs @@ -315,3 +315,35 @@ test('approve resolves the session cwd per invocation (T3.1)', async () => { cleanup(second.tmp) } }) + +// T3.9 pin: the process.cwd() fallback of currentDirectory is part of the +// T3.1 contract — a workingDirectory that throws must not break /approve. +// The test changes the process directory (each test file runs in its own +// process) so the write lands in an assertable place, then restores it. + +test('approve falls back to process.cwd() when the session cwd lookup throws (T3.9)', async () => { + const { tmp, repo } = repoFixture() + const previous = process.cwd() + try { + process.chdir(repo) + const captured = {} + const commands = { register(definition) { Object.assign(captured, definition) } } + registerApproveCommand({ + commands, + userQuestions: { async ask() { return approveAnswer('design') } }, + workingDirectory: { + get() { + throw new Error('cannot get property "session" without inject') + }, + }, + logger: { warn() {} }, + }) + + const result = await captured.handler({ agent: { session: 's1' }, rawInput: 'demo design' }) + assert.equal(result.kind, 'success') + assert.equal(existsSync(approvalPath(repo)), true, 'approval.md is written into process.cwd()') + } finally { + process.chdir(previous) + cleanup(tmp) + } +}) diff --git a/dsh/tests/install.test.mjs b/dsh/tests/install.test.mjs index 3f2d1ed..373817f 100644 --- a/dsh/tests/install.test.mjs +++ b/dsh/tests/install.test.mjs @@ -50,7 +50,10 @@ test('install.sh copies skills and roles instead of linking into ~/.claude', () const sddSkill = join(dshHome, 'skills', 'sdd', 'SKILL.md') assert.ok(existsSync(sddSkill), 'sdd skill was not installed') - assert.match(readFileSync(sddSkill, 'utf8'), /name: sdd/) + const sddText = readFileSync(sddSkill, 'utf8') + assert.match(sddText, /name: sdd/) + assert.match(sddText, /\/approve/, 'the sdd skill must check the plan approval marker before dispatch') + assert.match(sddText, /approval\.md/, 'the sdd skill must name the approval marker file') for (const role of ['dev', 'admin', 'analyst', 'qa']) { assert.ok(existsSync(join(dshHome, 'roles', `${role}.md`)), `missing ${role}.md`) @@ -60,6 +63,9 @@ test('install.sh copies skills and roles instead of linking into ~/.claude', () const devText = readFileSync(devRole, 'utf8') assert.match(devText, /В DSH-сессии исполнение плана идёт по skill `sdd`/) assert.match(devText, /`maxDepth: 2`/) + assert.match(devText, /Бандл гейтит цикл/) + assert.match(devText, /\/approve/, 'the DSH note must point at /approve') + assert.match(devText, /approval-gates/, 'the DSH note must name the approval-gates deny prefix') assert.match(first.stdout, /role dsh notes/) assert.equal(countNotes(devText), 1, 'note must be inserted once') diff --git a/openspec/changes/enforce-upstream-loop/design.md b/openspec/changes/enforce-upstream-loop/design.md index 9fe0db7..ba345ba 100644 --- a/openspec/changes/enforce-upstream-loop/design.md +++ b/openspec/changes/enforce-upstream-loop/design.md @@ -28,6 +28,7 @@ design: plan: by: aep at: 2026-10-25T11:30:00+04:00 + sha256: 3f2c9a… (hex, 64 знака) ``` ## Механика гейтов (в бандле, DSH-only) @@ -42,22 +43,35 @@ plan: или `new_string`) извлечь `**Spec:** <путь>`; если путь указывает в `openspec/changes//` и `approval.md` без секции `design:` — DENY «design not approved; run /approve design». Содержимое без - Spec-ссылки на change — пропуск. -2. **Гейт B (sdd до approve плана).** Для `subagent`/`subagent_fork`: - в промпте найти ссылку на план — `docs/superpowers/plans/<имя>.md` - или `.superpowers/sdd/<план>/`; из плана взять `**Spec:**` → - change; `approval.md` без секции `plan:` — DENY «plan not approved; - run /approve plan». Нет ссылки на план — пропуск. + `**Spec:**` или со Spec-ссылкой вне `openspec/changes/` (legacy + `docs/superpowers/specs/`) — DENY «plan must carry **Spec:** pointing to + openspec/changes/» (D-9); правка без содержимого (bash-цель, + edit-фрагмент) тоже DENY — гейт не видит содержимого. +2. **Гейт B (sdd до approve плана).** Два входа (D-7): (a) запись в + `.superpowers/sdd/<план>/**` (ledger, брифы — старт sdd в Режиме 1) и + (b) диспатч `subagent`/`subagent_fork`/`workflow`, в промпте или + скрипте которого есть ссылка на план — `docs/superpowers/plans/<имя>.md` + или `.superpowers/sdd/<план>/`. Из плана взять `**Spec:**` → change; + `approval.md` без секции `plan:` или с чужим sha256 — DENY «plan not + approved; run /approve plan» / «plan changed after approval; + re-approve». Проверяются все план-ссылки, а не первая; ссылка без + читаемого плана и план без Spec — fail closed. 3. **`/approve`** (`command-approve.js`): `design` требует существование proposal.md, design.md, builder-check.md; `plan` требует секцию - `design:` и существование ровно одного плана со Spec на этот change - (несколько — отказ с перечислением). Дописывает/обновляет секцию, + `design:` с непустыми `by`/`at` и существование ровно одного плана со + Spec на этот change (несколько — отказ с перечислением). Дописывает/обновляет секцию, остальные секции не трогает. Владелец-агент различие: команда пишет `by: owner`; вызовы не от владельца — отказ (как отличить: DSH не даёт ролей; используем факт вызова из интерактивной сессии и AskUserQuestion-подтверждение: `/approve` спрашивает подтверждение перед записью, ответ «да» = владелец). +Исторические планы: четыре файла — `2026-10-11-roles-and-memory`, +`2026-10-24-dsh-decouple`, `2026-10-24-dsh-port-spike`, +`2026-10-24-dsh-role-loader` — написаны до лупа и не несут `**Spec:**` +(T1.3). По D-9 любая их правка — DENY: они заморожены, новая работа идёт +отдельным планом. Это ожидаемое поведение, а не регресс. + ## Decisions made without discussion | # | Решение | Обоснование | Цена ошибки | @@ -72,7 +86,7 @@ plan: | D-8 | Bash-обход закрыт: для Bash-вызовов gates.js прогоняет `python3 hooks/harness_target.py targets '' ` и применяет гейты A/C к целям под `docs/superpowers/plans/` и `**/approval.md` | Тот же парсер, что в хуках (heredoc-aware), без дублирования логики | Стоимость — subprocess на каждый Bash-вызов с подозрительным текстом; фильтруем до прогона по грубой подстроке | | D-9 | План без `**Spec:**` в `openspec/changes/` — DENY по умолчанию (включая legacy `docs/superpowers/specs/`) | Реальные планы всегда несут Spec; .bounded-поток планов не пишет — проверено по репозиторию | Ложный блок легитимного Spec-less плана — такого флоу нет | | D-10 | `plan:` в approval.md несёт `sha256` файла плана; гейт B сверяет хэш — план изменён после утверждения → DENY «plan changed after approval; re-approve» | Утверждение привязывается к содержимому исполняемого объекта | Правки плана после approve требуют re-approve — осознанно | -| D-11 | `design:` — только существование секции (без хэша): design.md дополняется решениями по ходу | Контент дизайна не является исполняемым объектом | Правка дизайна после approve не отзывается — принято | +| D-11 | `design:` — только непустые `by` и `at` (без хэша): design.md дополняется решениями по ходу | Контент дизайна не является исполняемым объектом; пустая секция `design:` утверждением не считается, `by`/`at` должны быть непустыми (T1.8) | Правка дизайна после approve не отзывается — принято | | D-12 | Подтверждение владельца в `/approve` — через `ctx.userQuestions.ask` (блокирующий вопрос); сервис добавляется в inject; отсутствует — `/approve` отказывает с сообщением | Проверенный примитив рантайма (DELEGATED_CALLER не пройдёт) вместо эвристики «интерактивная сессия» | Нет userQuestions в профиле — /approve недоступен, гейты всё равно работают на блокировку | | D-13 | Бутстрап: гейты активны только в сессиях с новым бандлом (после установки/рестарта); сама фича #59 исполняется на старом рантайме — ретро-утверждений не требует; для будущих change контроллер кладёт builder-check.md штатно | Развёртывание разводит enforcement и исполняемую фичу во времени | Окно до переустановки бандла — гейтов нет; фиксируем шаг переустановки в чеклисте закрытия | diff --git a/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md b/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md new file mode 100644 index 0000000..e9de04a --- /dev/null +++ b/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md @@ -0,0 +1,287 @@ +# Spec Delta: approval-gates + +## Purpose + +Технические гейты цикла разработки в DSH-бандле: builder-check → утверждение +дизайна → план → утверждение плана → sdd. Инструкции superpowers сами по себе не +являются механизмом (сессия ce9187ca 24.10.2026 пропустила builder-check, а план +появился до утверждения дизайна), поэтому бандл проверяет артефакты утверждения на +каждом переходе и отказывает с `BLOCKED [approval-gates]: <причина>`. Гейты живут +только в DSH-бандле (D-2); Claude Code-хуки и поток не меняются. + +## ADDED Requirements + +### Requirement: Артефакты утверждения + +`openspec/changes//builder-check.md` SHALL содержать отчёт свежего +builder-check субагента, скопированный в change контроллером; он MUST быть записан +после builder-check и до передачи дизайна владельцу. + +`openspec/changes//approval.md` SHALL быть маркером утверждения владельцем; +его единственный писатель — команда `/approve` (D-1), инструменты модели писать +его MUST NOT (гейт C ниже). + +Формат `approval.md` — yaml с секциями `design:` (поля `by`, `at`) и `plan:` +(поля `by`, `at`, `sha256`); `sha256` — hex-хэш sha256 содержимого файла плана на +момент утверждения (D-10). Запись MUST опускать пустые поля, разбор MUST быть +терпимым: неизвестные секции и строки игнорируются, битый файл деградирует в +«секции нет», и гейт отказывает. + +Секция `design:` SHALL считаться утверждением только при непустых `by` и `at`: +пустая секция — не утверждение (T1.8). Запись одной секции MUST NOT менять +содержимое другой. + +#### Scenario: Пустая секция design не разблокирует план + +- **WHEN** `approval.md` содержит заголовок `design:` без полей `by` и `at` +- **THEN** гейт A отказывает с `design not approved`, как будто секции нет + +#### Scenario: Утверждение плана не теряет утверждение дизайна + +- **WHEN** `/approve plan` записывает plan-секцию в существующий `approval.md` +- **THEN** design-секция остаётся дословно прежней + +### Requirement: Команда /approve + +`/approve design|plan` SHALL быть единственным писателем +`approval.md` и MUST регистрироваться при `enabled` и `approvalGates: true`, +независимо от флагов `roleCommand` и `checkCommand`: иначе гейты остаются без +команды-разблокировки (T3.2). `change-id` MUST быть одним безопасным сегментом +пути; небезопасный id или неизвестный вид — usage-отказ без записи. + +Вид `design` SHALL требовать существования `proposal.md`, `design.md` и +`builder-check.md` в `openspec/changes//`; при отсутствии файла команда +MUST отказать с перечислением отсутствующих и MUST NOT спрашивать владельца. + +Вид `plan` SHALL требовать design-секцию с непустыми `by` и `at` (иначе отказ +`design not approved; run /approve design`) и ровно одного плана в +`docs/superpowers/plans/`, чей `**Spec:**` указывает на ``; ноль планов — +отказ, несколько — отказ с перечислением файлов. Утверждаемый план MUST получать +`sha256` своего текущего содержимого. + +Подтверждение владельца MUST идти через `ctx.userQuestions.ask` (D-12): вопрос с +вариантами Approve и Cancel; без сервиса команда MUST отказать, а не писать. При +подтверждении пишутся `by: owner` и `at` в ISO 8601 UTC; при отмене или +неподтверждении — отказ `not confirmed`, файл не меняется. Команда MUST NOT +создавать план и не трогать `proposal.md`, `design.md`, `builder-check.md`. + +#### Scenario: /approve design без builder-check + +- **WHEN** в change есть proposal и design, но нет `builder-check.md` +- **THEN** команда отказывает с перечислением `builder-check.md`, владелец не + спрашивается, `approval.md` не создаётся + +#### Scenario: /approve plan без утверждения дизайна + +- **WHEN** `approval.md` не содержит непустой design-секции +- **THEN** команда отказывает с `design not approved; run /approve design` и + не спрашивает владельца + +#### Scenario: /approve plan при двух планах на change + +- **WHEN** два файла в `docs/superpowers/plans/` несут `**Spec:**` на один change +- **THEN** команда отказывает, перечисляя оба файла, и `approval.md` не меняется + +#### Scenario: Утверждение плана фиксирует sha256 + +- **WHEN** владелец подтвердил `/approve plan` при единственном плане +- **THEN** plan-секция содержит `by: owner`, `at` в ISO 8601 и sha256 содержимого + плана + +#### Scenario: /approve без сервиса вопросов + +- **WHEN** `ctx.userQuestions` отсутствует +- **THEN** команда отказывает с `userQuestions service unavailable; /approve + requires it` и `approval.md` не пишется + +### Requirement: Гейт A — план до утверждения дизайна + +Для write/edit/multi_edit/`str_replace_editor` с целевым путём под +`docs/superpowers/plans/` гейт SHALL извлекать `**Spec:**` из содержимого +(`content` или `new_string`). Ссылка MUST указывать в `openspec/changes/`; +план без `**Spec:**` и план с legacy-ссылкой в `docs/superpowers/specs/**` MUST +получать отказ `plan must carry **Spec:** pointing to openspec/changes/` +(D-9). Правка без содержимого (bash-цель, edit-фрагмент, `str_replace_editor` +create) MUST получать тот же отказ: гейт не видит содержимого и не может +подтвердить Spec — fail closed. + +Если change определён, а его design-секция не является утверждением, гейт MUST +отказать `design not approved; run /approve design`. Утверждённый дизайн +разрешает запись плана; запись вне `docs/superpowers/plans/` гейтом A не +проверяется. + +Исторические планы, написанные до лупа и не несущие `**Spec:**`, заморожены — +четыре файла: `2026-10-11-roles-and-memory`, `2026-10-24-dsh-decouple`, +`2026-10-24-dsh-port-spike`, `2026-10-24-dsh-role-loader` (T1.3). Любая их правка +MUST получать отказ по D-9; это ожидаемое поведение, а не дефект. + +#### Scenario: План со Spec на неутверждённый дизайн + +- **WHEN** write в `docs/superpowers/plans/x.md` с `**Spec:** + openspec/changes//`, а `approval.md` без design-секции +- **THEN** отказ `BLOCKED [approval-gates]: design not approved; run /approve + design` + +#### Scenario: План без Spec + +- **WHEN** write в `docs/superpowers/plans/x.md` без строки `**Spec:**` +- **THEN** отказ `plan must carry **Spec:** pointing to openspec/changes/` + +#### Scenario: План после утверждения дизайна + +- **WHEN** design-секция содержит непустые `by` и `at`, а план несёт `**Spec:**` + на этот change +- **THEN** запись плана разрешена + +#### Scenario: Правка плана фрагментом + +- **WHEN** edit плана передаёт `new_string` без `**Spec:**` (фрагмент правки) +- **THEN** отказ: план пишется только full-file write + +#### Scenario: Правка исторического плана + +- **WHEN** write в `docs/superpowers/plans/2026-10-24-dsh-decouple.md` +- **THEN** отказ `plan must carry **Spec:** …` по D-9 + +### Requirement: Гейт B — sdd до утверждения плана + +Гейт SHALL срабатывать на двух входах (D-7): (a) write в +`.superpowers/sdd//**` (ledger, брифы) — старт sdd в Режиме 1; +(b) диспатч `subagent`, `subagent_fork` или `workflow`, в `prompt` или `script` +которого есть ссылка на план — `docs/superpowers/plans/<имя>.md` или +`.superpowers/sdd/<имя>/`. + +Для каждого входа гейт MUST определить файл +`docs/superpowers/plans/.md`, взять из него `**Spec:**` → change и +потребовать plan-секцию `approval.md` с sha256, равным sha256 текущего +содержимого плана. Нет секции или sha256 — отказ `plan not approved; run +/approve plan`; несовпадение хэша или нечитаемый план — отказ `plan changed +after approval; re-approve`; нет файла плана для sdd-каталога — отказ `plan not +found: `; план без `**Spec:**` — отказ `plan not approved` (id +неизвестен). Проверяться MUST все план-ссылки, а не первая; возвращается первый +отказ, чтобы утверждённый план не маскировал неутверждённый. + +Вызов без план-ссылки MUST NOT гейтиться. Разбор промпта — эвристика по +подстрокам, поэтому разбор fail open: вызов, в котором ссылка не распознана, не +блокируется. Диспатч-гейт резолвит ссылки против cwd сессии и игнорирует +`arguments.cwd`: диспатч с cwd в другом worktree и ссылкой на план этого worktree +получает отказ (fail closed, over-blocking, T3.8) — план должен быть утверждён в +репозитории сессии. + +#### Scenario: Ledger до утверждения плана + +- **WHEN** write в `.superpowers/sdd//progress.md`, а plan-секции нет +- **THEN** отказ `BLOCKED [approval-gates]: plan not approved; run /approve + plan` + +#### Scenario: Диспатч с утверждённым планом + +- **WHEN** `subagent` получает промпт со ссылкой на план, чей sha256 совпадает с + plan-секцией +- **THEN** диспатч разрешён + +#### Scenario: Один из двух планов не утверждён + +- **WHEN** промпт `subagent` ссылается на два плана, и второй не утверждён +- **THEN** диспатч отклонён по второму плану, даже если первый утверждён + +#### Scenario: План изменён после утверждения + +- **WHEN** содержимое плана изменилось после `/approve plan` +- **THEN** отказ `plan changed after approval; re-approve`; снимает его только + повторное утверждение + +#### Scenario: Диспатч без ссылки на план + +- **WHEN** в промпте `subagent` нет ни пути плана, ни sdd-каталога +- **THEN** гейт B не применяется + +### Requirement: Гейт C и bash-обход + +`approval.md` MUST писаться только командой `/approve`: любой write/edit с целью, +чей basename — `approval.md`, и любая bash-цель с таким именем MUST получать +отказ `approval.md is written only by /approve` (D-1). + +Bash-обход MUST быть закрыт (D-8): если команда содержит грубую подстроку +`docs/superpowers/plans`, `approval.md` или `.superpowers/sdd`, её файловые цели +разбираются тем же парсером, что у хуков, — `python3 /harness_target.py +targets '' `. К целям применяются гейт A (без содержимого), гейт C и +write-гейт B; возвращается первый отказ. Сбой запуска, разбора или таймаут +парсера MUST fail open, как в хуках: сбой разбора входа не блокирует. + +Команда без плановых подстрок и целей MUST NOT проверяться гейтами. + +#### Scenario: Heredoc в план без Spec + +- **WHEN** bash-команда `cat > docs/superpowers/plans/x.md <<'EOF' … EOF` без + `**Spec:**` +- **THEN** отказ `plan must carry **Spec:** …` (содержимое bash-цели гейту + недоступно) + +#### Scenario: Запись approval.md через bash + +- **WHEN** bash-команда `echo x > openspec/changes//approval.md` +- **THEN** отказ `approval.md is written only by /approve` + +#### Scenario: Запись в sdd-каталог через bash + +- **WHEN** bash-цель — файл в `.superpowers/sdd//`, а план не утверждён +- **THEN** отказ `plan not approved` + +#### Scenario: Обычная bash-команда + +- **WHEN** в команде нет плановых подстрок +- **THEN** гейты не запускаются + +### Requirement: Исключения и известные пределы + +Bounded-задача без change и плана MUST NOT гейтиться (D-6): у неё нет план-ссылок, +записей в `docs/superpowers/plans/` и sdd-каталога. + +Гейты SHALL жить в guard-цепочке и быть активными только при `enabled: true`, +`guards: true`, успешном self-check `guards` (`guardsOk`) и `approvalGates: true`; +`approvalGates: true` при выключенных guards или проваленном self-check MUST NOT их +включать (T3.3). Гейты MUST быть активны только в сессии с переустановленным +бандлом (D-13): до переустановки их нет, и sdd-скилл делает ручную проверку +маркера; после — её дублирует enforcement. + +Правка плана фрагментом (`edit`) MUST всегда получать отказ, план пишется +full-file write (T3.5). Bash-цели и `str_replace_editor` create с невидимым +содержимым тоже отказывают; deny-текст про отсутствующий `**Spec:**` для них +вводит в заблуждение — реальная причина в невидимом содержимом (T3.4). + +`terminal_open` / `terminal_send` MUST оставаться вне гейтов: они пишут файлы в +обход и гейтов, и bash-хуков — остаточный риск (T3.6). Симлинк вне +`docs/superpowers/plans/`, указывающий на план внутри, лексически обходит гейт A — +класс остаточного риска D-1; гейт защищает от случайного пропуска, а не от злого +умысла (T1.5). Гейты не перехватывают сообщения модели: презентация дизайна +владельцу не гейтится, пропуск становится виден на цепочке builder-check → +`/approve` → план → sdd (D-3). + +`/approve` SHALL считать подтверждением ответ +`{answers: [{selected: [Approve]}]}`, а также тестовые формы `{approved}`, +`{confirmed}` и `true`; в рантайме подтверждение — блокирующий вопрос владельцу, а +не эвристика по вызывающему (D-12, T2.3). Тестовые формы MUST NOT использоваться +моделью для самоутверждения: команда вызывается только владельцем. + +#### Scenario: Bounded-правка + +- **WHEN** в репозитории нет change и плана, а правка не трогает + `docs/superpowers/plans/`, `approval.md` и `.superpowers/sdd/` +- **THEN** гейты не применяются + +#### Scenario: approvalGates без guards + +- **WHEN** профиль задаёт `guards: false` и `approvalGates: true` +- **THEN** гейты не работают: они часть guard-цепочки + +#### Scenario: terminal_send пишет план + +- **WHEN** `terminal_send` выполняет команду, пишущую в + `docs/superpowers/plans/` +- **THEN** гейты не срабатывают — документированный предел, не регресс + +#### Scenario: Правка дизайна после утверждения + +- **WHEN** `design.md` дополняется решением после `/approve design` +- **THEN** утверждение не отзывается (D-11): хэш фиксируется только для плана -- 2.47.2 From 7e3b86b59aade19d42ae0e62c826ddc149706090 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 02:40:39 +0400 Subject: [PATCH 10/10] =?UTF-8?q?docs:=20=D1=81=D0=BF=D0=B5=D0=BA=D0=B0=20?= =?UTF-8?q?approval-gates=20=E2=80=94=20=D1=82=D0=BE=D1=87=D0=BD=D1=8B?= =?UTF-8?q?=D0=B5=20=D1=84=D0=BE=D1=80=D0=BC=D1=83=D0=BB=D0=B8=D1=80=D0=BE?= =?UTF-8?q?=D0=B2=D0=BA=D0=B8=20=D0=B3=D0=B5=D0=B9=D1=82=D0=BE=D0=B2=20A/B?= =?UTF-8?q?=20=D0=B8=20frozen-=D0=BF=D0=BB=D0=B0=D0=BD=D0=BE=D0=B2=20(T4.1?= =?UTF-8?q?-T4.3,=20#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/README.md | 10 ++- .../changes/enforce-upstream-loop/design.md | 12 +-- .../specs/approval-gates/spec.md | 74 ++++++++++++++----- 3 files changed, 68 insertions(+), 28 deletions(-) diff --git a/dsh/README.md b/dsh/README.md index 6d97d92..84ea039 100644 --- a/dsh/README.md +++ b/dsh/README.md @@ -209,10 +209,12 @@ repository layout: enable them. They are active only in a session with the reinstalled bundle (D-13): an old install keeps the text-only loop, and the `sdd` skill checks the plan approval by hand. -- Plan files are always rewritten in full: an `edit` fragment cannot carry the - `**Spec:**` line, so gate A denies it — as it denies bash plan targets and - `str_replace_editor` creates, whose content it cannot see. The deny text then - names the missing `**Spec:**`, not the real reason. +- Gate A is content-based: it reads `**Spec:**` from `content` / `new_string`. + An `edit` fragment is checked against its own text, so a fragment carrying a + `**Spec:**` line for an approved change passes, while the usual Spec-less + fragment is denied (rewrite the plan file in full). Bash plan targets and + `str_replace_editor` creates are denied because the gate cannot see their + content; the deny text then names the missing `**Spec:**`, not the real reason. - `terminal_open` / `terminal_send` write files outside both the gates and the bash hooks — a residual risk not covered by the port. - `bundle/hooks` is a symlink for local-directory installation. A published diff --git a/openspec/changes/enforce-upstream-loop/design.md b/openspec/changes/enforce-upstream-loop/design.md index ba345ba..5ed170d 100644 --- a/openspec/changes/enforce-upstream-loop/design.md +++ b/openspec/changes/enforce-upstream-loop/design.md @@ -46,7 +46,8 @@ plan: `**Spec:**` или со Spec-ссылкой вне `openspec/changes/` (legacy `docs/superpowers/specs/`) — DENY «plan must carry **Spec:** pointing to openspec/changes/» (D-9); правка без содержимого (bash-цель, - edit-фрагмент) тоже DENY — гейт не видит содержимого. + `str_replace_editor` create) и фрагмент `edit` без `**Spec:**` тоже DENY — + гейт проверяет только то содержимое, которое видит. 2. **Гейт B (sdd до approve плана).** Два входа (D-7): (a) запись в `.superpowers/sdd/<план>/**` (ledger, брифы — старт sdd в Режиме 1) и (b) диспатч `subagent`/`subagent_fork`/`workflow`, в промпте или @@ -66,10 +67,11 @@ plan: AskUserQuestion-подтверждение: `/approve` спрашивает подтверждение перед записью, ответ «да» = владелец). -Исторические планы: четыре файла — `2026-10-11-roles-and-memory`, -`2026-10-24-dsh-decouple`, `2026-10-24-dsh-port-spike`, -`2026-10-24-dsh-role-loader` — написаны до лупа и не несут `**Spec:**` -(T1.3). По D-9 любая их правка — DENY: они заморожены, новая работа идёт +Исторические планы: четыре файла написаны до лупа (T1.3, T4.3) — +`2026-10-11-roles-and-memory` несёт legacy-`**Spec:**` в +`docs/superpowers/specs/`, а `2026-10-24-dsh-decouple`, +`2026-10-24-dsh-port-spike` и `2026-10-24-dsh-role-loader` — без `**Spec:**`. +По D-9 запись в любой из них — DENY: они заморожены, новая работа идёт отдельным планом. Это ожидаемое поведение, а не регресс. ## Decisions made without discussion diff --git a/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md b/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md index e9de04a..9bf45b0 100644 --- a/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md +++ b/openspec/changes/enforce-upstream-loop/specs/approval-gates/spec.md @@ -101,19 +101,22 @@ MUST отказать с перечислением отсутствующих (`content` или `new_string`). Ссылка MUST указывать в `openspec/changes/`; план без `**Spec:**` и план с legacy-ссылкой в `docs/superpowers/specs/**` MUST получать отказ `plan must carry **Spec:** pointing to openspec/changes/` -(D-9). Правка без содержимого (bash-цель, edit-фрагмент, `str_replace_editor` -create) MUST получать тот же отказ: гейт не видит содержимого и не может -подтвердить Spec — fail closed. +(D-9). Правка без содержимого (bash-цель, `str_replace_editor` create) MUST +получать тот же отказ: гейт не видит содержимого и не может подтвердить Spec — +fail closed. Фрагмент `edit` проверяется по своему `new_string` (content-based): +фрагмент, который сам несёт `**Spec:**` на утверждённый change, проходит; фрагмент +без Spec, с legacy-ссылкой или с неутверждённым change — DENY. Если change определён, а его design-секция не является утверждением, гейт MUST отказать `design not approved; run /approve design`. Утверждённый дизайн разрешает запись плана; запись вне `docs/superpowers/plans/` гейтом A не проверяется. -Исторические планы, написанные до лупа и не несущие `**Spec:**`, заморожены — -четыре файла: `2026-10-11-roles-and-memory`, `2026-10-24-dsh-decouple`, -`2026-10-24-dsh-port-spike`, `2026-10-24-dsh-role-loader` (T1.3). Любая их правка -MUST получать отказ по D-9; это ожидаемое поведение, а не дефект. +Исторические планы заморожены — все написаны до лупа: четыре файла. +`2026-10-11-roles-and-memory` несёт legacy-`**Spec:**` в +`docs/superpowers/specs/`; `2026-10-24-dsh-decouple`, `2026-10-24-dsh-port-spike` +и `2026-10-24-dsh-role-loader` — без `**Spec:**` (T1.3, T4.3). Любая запись в +любой из них MUST получать отказ по D-9; это ожидаемое поведение, а не дефект. #### Scenario: План со Spec на неутверждённый дизайн @@ -133,16 +136,30 @@ MUST получать отказ по D-9; это ожидаемое повед на этот change - **THEN** запись плана разрешена -#### Scenario: Правка плана фрагментом +#### Scenario: Правка плана фрагментом без Spec -- **WHEN** edit плана передаёт `new_string` без `**Spec:**` (фрагмент правки) -- **THEN** отказ: план пишется только full-file write +- **WHEN** edit плана передаёт `new_string` без `**Spec:**` (обычный фрагмент + правки) +- **THEN** отказ `plan must carry **Spec:** …`: гейт видит только фрагмент, план + целиком пишется full-file write + +#### Scenario: Фрагмент edit со Spec на утверждённый change + +- **WHEN** edit плана передаёт `new_string`, в котором есть `**Spec:**` на change + с непустой design-секцией +- **THEN** запись разрешена: гейт проверяет содержимое, а не инструмент #### Scenario: Правка исторического плана - **WHEN** write в `docs/superpowers/plans/2026-10-24-dsh-decouple.md` - **THEN** отказ `plan must carry **Spec:** …` по D-9 +#### Scenario: Правка плана с legacy-Spec + +- **WHEN** write в `docs/superpowers/plans/2026-10-11-roles-and-memory.md`, чей + `**Spec:**` указывает в `docs/superpowers/specs/` +- **THEN** отказ `plan must carry **Spec:** …` по D-9 + ### Requirement: Гейт B — sdd до утверждения плана Гейт SHALL срабатывать на двух входах (D-7): (a) write в @@ -154,12 +171,22 @@ MUST получать отказ по D-9; это ожидаемое повед Для каждого входа гейт MUST определить файл `docs/superpowers/plans/.md`, взять из него `**Spec:**` → change и потребовать plan-секцию `approval.md` с sha256, равным sha256 текущего -содержимого плана. Нет секции или sha256 — отказ `plan not approved; run -/approve plan`; несовпадение хэша или нечитаемый план — отказ `plan changed -after approval; re-approve`; нет файла плана для sdd-каталога — отказ `plan not -found: `; план без `**Spec:**` — отказ `plan not approved` (id -неизвестен). Проверяться MUST все план-ссылки, а не первая; возвращается первый -отказ, чтобы утверждённый план не маскировал неутверждённый. +содержимого плана. Отказы MUST различаться по причине: + +- (a) файл плана по ссылке отсутствует или не читается — `plan not approved; run + /approve plan` (fail closed; id остаётся литеральным ``, если план не + прочитан); +- (b) план читается, но без `**Spec:**` или с legacy-ссылкой вне + `openspec/changes/` — тот же `plan not approved; run /approve plan`; +- (c) план и change определены, но plan-секции нет или в ней нет sha256 — тот же + `plan not approved; run /approve plan`, уже с id change; +- (d) plan-секция есть, но sha256 не совпадает с текущим содержимым плана — + `plan changed after approval; re-approve`. + +Для входа через `.superpowers/sdd//` без файла +`docs/superpowers/plans/.md` отказ — `plan not found: `. +Проверяться MUST все план-ссылки, а не первая; возвращается первый отказ, чтобы +утверждённый план не маскировал неутверждённый. Вызов без план-ссылки MUST NOT гейтиться. Разбор промпта — эвристика по подстрокам, поэтому разбор fail open: вызов, в котором ссылка не распознана, не @@ -185,6 +212,13 @@ found: `; план без `**Spec:**` — отказ `plan not approv - **WHEN** промпт `subagent` ссылается на два плана, и второй не утверждён - **THEN** диспатч отклонён по второму плану, даже если первый утверждён +#### Scenario: План по ссылке не найден + +- **WHEN** `subagent` получает промпт со ссылкой на + `docs/superpowers/plans/missing.md`, которого нет +- **THEN** отказ `plan not approved; run /approve plan` (fail closed), а не + `plan changed after approval` + #### Scenario: План изменён после утверждения - **WHEN** содержимое плана изменилось после `/approve plan` @@ -245,9 +279,11 @@ Bounded-задача без change и плана MUST NOT гейтиться (D- бандлом (D-13): до переустановки их нет, и sdd-скилл делает ручную проверку маркера; после — её дублирует enforcement. -Правка плана фрагментом (`edit`) MUST всегда получать отказ, план пишется -full-file write (T3.5). Bash-цели и `str_replace_editor` create с невидимым -содержимым тоже отказывают; deny-текст про отсутствующий `**Spec:**` для них +Правка плана фрагментом (`edit`) обычно получает отказ: гейт проверяет только +`new_string`, а обычный фрагмент не несёт `**Spec:**`, поэтому план целиком пишется +full-file write; фрагмент со своей строкой `**Spec:**` на утверждённый change +проходит (T3.5, T4.1). Bash-цели и `str_replace_editor` create с невидимым +содержимым отказывают всегда; deny-текст про отсутствующий `**Spec:**` для них вводит в заблуждение — реальная причина в невидимом содержимом (T3.4). `terminal_open` / `terminal_send` MUST оставаться вне гейтов: они пишут файлы в -- 2.47.2