diff --git a/BACKLOG.md b/BACKLOG.md index 92027e5a..7c3d029a 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -31,7 +31,7 @@ CC-001/CC-002 were consumed by PR #24 fix bundle inline, with no standalone entr | CC-509 | ✅ closed 2026-07-22 | detached gate launch liveness:對 sandbox parent-death 早期死亡 fail-loud,提供 supervisor readiness/identity evidence | arch/gate | 2026-07-22 | pr:#440 | P2 | hygiene | | CC-510 | ✅ closed 2026-07-23 | Codex detached dispatch continuation:App Server callback、authenticated completion envelope 與 foreground fallback | arch/DX | 2026-07-23 | pr:#443 | P2 | design | | CC-511 | ⚠️ partial 2026-07-24 | ship publish authorization:Phase A current-tree authoritative full-suite 已交付;Phase B review-closure evidence 仍待 CC-515/CC-517 | release/gate | 2026-07-23 | pr:#446 | P1 | design | -| CC-512 | 🔵 active | gate tier、execution mode、reviewer coverage 與 independence assurance 正交化 | ops/gate | 2026-07-23 | — | P1 | design | +| CC-512 | ⚠️ partial 2026-07-27 | Slice A 已交付 coordinate sources/CLI resolution;machine envelope、evidence capture 與 shared verifier 仍待 Slice B/C | ops/gate | 2026-07-23 | — | P1 | design | | CC-513 | 🔵 active | canonical gate policy resolver:minimum tier、required reviewers、mode recommendation 與 downgrade audit | security/gate | 2026-07-23 | — | P1 | design | | CC-514 | 🔵 active | orthogonal delivery assurance map、machine-derived tables 與 feature/docs/high-risk recipes | docs/process | 2026-07-23 | — | P2 | design | | CC-515 | 🔵 active | gate artifact immutable subject、freshness 與 consumer applicability shared verifier | arch/gate | 2026-07-23 | — | P1 | design | @@ -1616,47 +1616,114 @@ authorization。 --- -## CC-512 — tier/mode/coverage/independence assurance 正交化 🔵 active - -**Problem**: runtime 原本就將 tier detection、reviewer selection 與 -`SEQUENTIAL=true|false` 分開,但文件曾把 `full` 描述成 parallel cross-context + -五個 reviewer,造成 tier、execution topology、實際 coverage 與 independence -assurance 相互暗示。`full + sequential` 與 `express + parallel` 本應合法,artifact -卻沒有足夠 metadata 防止錯誤宣稱。 +## CC-512 — tier/mode/pass/coverage/independence assurance 正交化 ⚠️ partial 2026-07-27 + +**Problem**: runtime 雖已將 tier detection、reviewer selection 與 +`SEQUENTIAL=true|false` 分開,但目前仍有三個 truth gap: + +1. `--reviewers` 在沒有 explicit tier 時會把 `TIER` 改成 `targeted`,把 remediation + delta/review scope 誤分類成 rigor tier;`--targeted` 又只是 `--reviewers` alias, + 無法引用 initial review。 +2. `express|standard|full` 實際主要選擇預設 reviewer 清單,文件卻把 `full` 寫成 + parallel + 五 reviewer + 較高 assurance。requested intent、resolved defaults 與 + actual evidence 沒有分開。 +3. final Markdown frontmatter 由 reviewer/synthesis session 產生; + `gate_result_verify` 只驗 `Final:` 唯一性與 frontmatter/body parity,不能證明實際 + mode、selected reviewers、skipped reviewers 或 session independence。 + +**Assurance coordinates**: + +1. **Tier** 是 rigor intent/preset,closed enum 僅 + `express|standard|full`。記錄 `tier.requested: auto|` 與 + `tier.resolved: `;tier table 可提供 default reviewers/evidence floor, + 但不得把 default 當 actual coverage 或 policy authorization。 +2. **Mode** 只描述 execution topology。記錄 + `mode.requested: default|sequential|parallel` 與 + `mode.resolved: sequential|parallel`;sequential = + `combined-session`,parallel = `per-reviewer-sessions` + synthesis。 +3. **Pass kind** 獨立為 `initial|targeted`。Targeted 是 remediation delta review, + 不是 tier;必須記錄 initial gate result reference 與 inherited/requested + coverage basis,不能冒充 initial comprehensive review。 +4. **Coverage** 分開記錄 requested、selected、skipped reviewer sets;tier defaults、 + CLI override 與實際 dispatch 結果不得互相替代。 +5. **Independence** 分開記錄 implementation-context isolation、 + reviewer topology、`per_reviewer_independent` 與 machine-captured session + evidence/status。缺 evidence 時只能標 `unverified|unavailable`,不能宣稱 verified。 **Requirement**: -1. 建立分離的 machine-readable policy tables: - - tier table:express/standard/full/targeted 的 default reviewers 與 evidence - floor; - - mode table:sequential combined session、parallel per-reviewer sessions + - synthesis 的 execution topology。 - 不得建立 `full => parallel` 的合併 table。 -2. requested/resolved 值分開記錄: - `tier.requested/resolved`、`mode.requested/resolved`。Tier 不改 mode,parallel - 不提升 tier;express/standard/full/targeted × sequential/parallel 都是合法組合, - subject/policy constraints 另由 [[CC-513]] 判斷。 -3. artifact 明列實際 `coverage.reviewers`/`coverage.skipped`,以及 independence: - implementation-context isolation、`combined-session|per-reviewer-sessions`、 - `per_reviewer_independent: true|false` 與可驗證 session evidence。不能由 tier、 - mode 名稱或 reviewer 數量推論另一維度。 -4. targeted 結果必須標示 delta-specific 與引用的 initial coverage;不能呈現為完整 - initial review。Sequential review 仍可證明與 implementation session 隔離,只是 - reviewer 彼此共享 combined session。 -5. validator 拒絕/標 invalid:宣稱 parallel 卻無個別 session evidence、宣稱 - per-reviewer independence 但實際 combined session、express+parallel 被呈現成 - full coverage、full+sequential 被呈現成 parallel、targeted 被呈現成 initial full。 -6. CLI help、result frontmatter、review-model、commands、skills 與測試都由同一 - machine source 派生或引用 bounded generated markers;README 只提供 pointer。 - -**Done-when**: 任一 gate artifact 都能獨立回答「要求的深度、實際拓撲、實際 reviewer -coverage、取得何種 independence」,所有合法組合 round-trip,錯誤 assurance claims -被 validator 擋下。 - -**Non-goals**: 不讓 full 自動 parallel;不讓 parallel 自動 full;不決定 risk-based -minimum floor(見 [[CC-513]]);不新增另一種 gate。 - -**Cross-link**: [[CC-515]]、[[CC-518]]、[[CC-519]]、`docs/review-model.md`。 +1. 建立分離的 portable machine sources(預定 + `core/policy/gate-tiers.tsv`、`core/policy/gate-modes.tsv`、 + `core/policy/gate-pass-kinds.tsv`);repo-layout runtime 直接讀 source, + standalone/copy-mode fallback 使用 bounded generated snapshot + freshness ratchet, + 不手寫第二份 policy。三表分別擁有 defaults/topology/initial-reference requirement, + 不建立合併 profile。 +2. CLI canonicalize: + - 新增 `--mode sequential|parallel`;既有 `--parallel`/`--sequential` 為 + compatibility spelling,互相衝突時 fail closed。 + - `--reviewers` 只覆蓋 requested coverage,不再改 tier。 + - `--targeted ` 表示 `pass.kind=targeted`,不再只是 alias;必須搭配 + `--initial-result `。Initial-result 的結構存在性在本票驗,subject freshness + 與 applicability 留給 [[CC-515]]。 + - omitted tier/mode/pass 分別 resolve 為 `auto`→detected tier、 + `default`→sequential、`initial`。 +3. Gate shell 在 dispatch 前決定 coordinates,並在 dispatch/wait 時機械擷取實際 + reviewer/synthesis session evidence;reviewer LLM 不得自行宣稱 tier、mode、 + coverage 或 independence。 +4. 新 producer 寫 `pr_gate_result_v2` Markdown + `gate_assurance_v1` machine-owned + JSON sidecar;Markdown只保留human findings與bounded relative pointer。Envelope + 至少含上述五組coordinates、actual reviewer dispatch outcome與evidence status; + artifact relocation、explicit `--output`、foreground/detached、 + sequential/parallel及copy-mode都須byte-/meaning-parity。 +5. 擴充 shared verifier 只判斷本票擁有的 + **structural + claim consistency**:enum、requested/resolved、selected/skipped + partition、mode/topology/session evidence、pass/initial reference及 Markdown pointer + parity。Stable repo subject、digest/freshness與 consumer policy applicability仍由 + [[CC-515]];risk-based floor仍由 [[CC-513]]。 +6. `pr_gate_result_v1` 維持 legacy structural verification,但明示 + `assurance: unavailable`,不能被新 consumer 當完整 evidence;新 producer 不再 + 產生 v1。不得讓舊 artifact 因缺新欄位被誤報 forged。 +7. CLI help、result contract、`docs/review-model.md`、commands、skills與測試引用相同 + machine source或 bounded generated markers;README只保留 pointer。修正既有 + `full => parallel` rigor-tier敘述,但不把本票號寫入 operational docs。 + +**Delivery slices(同一 ticket;不得提前宣稱完成)**: + +1. **A — coordinate sources + CLI resolution(✅ delivered 2026-07-27)**: + 三份 canonical TSV、repo-layout direct load、copy-mode bounded snapshot + + parity ratchet、canonical `--mode`、targeted initial reference、closed + invalid/conflicting inputs,以及 dispatch brief 中的 requested/resolved/coverage + coordinates。`--tier full --reviewers critic` 保留 full intent + critic-only + selection;新 producer/verifier 尚未交付。 +2. **B — machine-owned envelope + evidence capture(pending)**:sequential combined session、 + parallel per-reviewer/synthesis sessions、targeted initial reference、copy-mode + truthful degradation。 +3. **C — verifier + remaining parity ratchets(pending)**:claim consistency、v1 legacy + classification、result/help/docs parity、affected-test mapping。 + +**Done-when**: + +- `express|standard|full × sequential|parallel × initial|targeted` 的合法矩陣可 + round-trip;targeted fixtures皆帶 initial reference。 +- `full+sequential` 不宣稱 parallel、`express+parallel` 不宣稱 full coverage; + `--tier full --reviewers critic` 可誠實產生「full intent + critic-only actual + coverage」,是否 policy-sufficient 留給後續 verifier。 +- Validator 能抓到 missing/duplicate reviewer partition、targeted 無 initial、 + parallel 無 per-session evidence、combined session 冒充 per-reviewer independent、 + LLM frontmatter與machine envelope不一致、copy-mode冒充 verified,以及v1被誤當新 + assurance。 + +**Non-goals**: 不在本票決定 sensitive-path/risk-based minimum floor([[CC-513]]); +不驗 artifact subject/freshness/publish applicability([[CC-515]]);不建立 scope +manifest或finding schema([[CC-518]]/[[CC-519]]);不新增 gate kind、workflow +engine、FSM或 mandatory parallel policy。 + +**Dependencies**: 無 hard implementation dependency;本票先鎖定 coordinates, +[[CC-513]]再產 policy resolution,[[CC-515]]再把 structural evidence 與 +subject/freshness/applicability接起來。 + +**Cross-link**: [[CC-513]]、[[CC-515]]、[[CC-518]]、[[CC-519]]、 +`docs/review-model.md`。 --- @@ -1671,8 +1738,8 @@ generic 使用者會被不必要強制,maintainer 路徑則可能漏掉必要 **Requirement**: 1. 建立單一可測 resolver,輸入 diff classification、trusted brief metadata、 - generated/untracked/renamed paths、requested tier/mode/reviewers、repo policy 與 - accepted-risk override;輸出: + generated/untracked/renamed paths、requested tier/mode/pass/reviewers、repo policy + 與 accepted-risk override;輸出: `minimum_tier`、`required_reviewers`、`recommended_mode`、`required_mode|null`、 `downgrade_allowed`、matched signals 與 override provenance。 2. **Generic `pmctl gate` risk-based floor**: @@ -1787,7 +1854,8 @@ manifest 與 remediation closure 共同依賴,屬 P1 evidence foundation。 publish authorization 與未來 consumer 呼叫;不得各自 grep `Final:` 或自行重做 repo identity/freshness 邏輯。 4. final artifact 連結 preflight evidence digest/subject、[[CC-512]] resolved tier/ - mode/coverage、[[CC-513]] policy resolution,以及 [[CC-518]] scope manifest。 + mode/pass/coverage/independence、[[CC-513]] policy resolution,以及 [[CC-518]] + scope manifest。 finalize 前重新計算 working subject;HEAD/tree drift 標 stale,不產生可重用 current-subject authorization。 5. 覆蓋 result copy/replay、different physical worktree same git subject、different diff --git a/DECISIONS.md b/DECISIONS.md index 42993831..f08cfdc8 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -7,11 +7,55 @@ H2 標題格式:## YYYY-MM-DD: <短描述> 與 BACKLOG closure 對應的 entry,內文首行寫:Closes: BACKLOG.md#-NNN --> +## 2026-07-27: targeted-review-is-a-pass-kind-not-a-tier + +Relates: CC-512, CC-513, CC-515, CC-517, CC-519 + +**Context**: 2026-07-27 的 runtime planning audit確認:`pr-gate.sh` 在提供 +`--reviewers`、未提供 explicit `--tier` 時直接把 tier 設成 `targeted`; +`--targeted` 只是同一 coverage override 的 alias。同時 final Markdown frontmatter +由 reviewer/synthesis session撰寫,shared verifier只驗 `Final:` parity,無法證明 +實際 tier、mode、reviewer coverage或session independence。這使 +2026-07-23 decision裡「targeted屬於tier」與「tier/coverage/subject必須正交」 +互相矛盾。 + +**Decision**: + +1. Tier closed enum收斂為 `express|standard|full`,表示 rigor intent/preset; + requested與resolved分開,default reviewer/evidence floor不是actual coverage。 +2. Review pass獨立為 `initial|targeted`。Targeted表示remediation delta,必須引用 + initial result;它可和任一tier、sequential/parallel mode組合,但不得呈現為initial + comprehensive coverage。 +3. `--reviewers`只設定requested coverage;`--targeted`不再只是alias,並要求 + `--initial-result`。Mode新增canonical `--mode`,既有`--parallel`/`--sequential` + 保留為compatibility spelling。 +4. Tier、mode、pass、coverage與independence由gate shell產生machine-owned assurance + envelope;LLM Markdown只承載findings與human summary。CC-512 verifier只驗 + structural/claim consistency;CC-513擁有risk/policy resolution,CC-515擁有 + subject validity/freshness/applicability。 + +**Alternatives considered**: (a) 保留`targeted`為第四種tier——否決,scope/delta不是 +rigor depth,且會繼續讓coverage override改寫tier。(b) 只在Markdown增加更多欄位—— +否決,欄位仍由LLM自述,無法作machine assurance。(c) 等CC-515一次解決——否決, +CC-515需要先有穩定coordinates,否則subject verifier會同時承擔CLI語意與policy來源。 + +**Constraints introduced**: 新producer不得再產生把targeted當tier的artifact; +`pr_gate_result_v1`只能被分類為legacy structural evidence,不能被新consumer當成 +完整assurance。不得以tier名稱推論actual reviewer coverage、mode或publish +authorization;不得新增gate kind、workflow engine或FSM。本決策只取代 +2026-07-23 entry中「targeted屬tier」的部分,其餘六維正交與publish authorization +決策維持有效。 + +--- + ## 2026-07-23: gate-delivery-assurance-dimensions-are-orthogonal Relates: CC-511, CC-512, CC-513, CC-514, CC-515, CC-517, CC-518, CC-519, CC-520, CC-521 +> 2026-07-27 clarification:本條目的 `targeted`-as-tier 部分已由 +> `targeted-review-is-a-pass-kind-not-a-tier` 取代;其餘六維正交決策不變。 + **Context**: 現有 runtime 已將 tier detection、reviewer selection 與 `SEQUENTIAL=true|false` 分開處理,但 review 文件與規畫曾把 `full`、五個 reviewers、 parallel independence 與 publish-ready GO 描述成同一件事。這造成兩類漂移: diff --git a/MILESTONES.md b/MILESTONES.md index d94010da..77897732 100644 --- a/MILESTONES.md +++ b/MILESTONES.md @@ -101,11 +101,15 @@ ### Phase 7 — evidence + policy foundations +> 實作順序:CC-512 先鎖定 machine-owned assurance coordinates;CC-513 才能在同一 +> vocabulary 上產 policy resolution;CC-515 最後把 structural evidence 與 immutable +> subject/freshness/consumer applicability 接起來。三者是不同責任,不合併成 profile。 + | 票 | 摘要 | 狀態 | |----|------|------| -| CC-515 | immutable subject;artifact validity、subject freshness、policy applicability 三軸 shared verifier | 🔵 | -| CC-512 | tier/mode/reviewer coverage/independence 正交化;full+sequential、express+parallel 均合法 | 🔵 | +| CC-512 | Slice A coordinate sources/CLI resolution 已交付;machine-owned assurance envelope、evidence capture、verifier 仍待 B/C;targeted 不再是 tier | ⚠️ | | CC-513 | canonical resolver:minimum tier、required reviewers、recommended/required mode、generic vs maintainer policy 與 downgrade audit | 🔵 | +| CC-515 | immutable subject;artifact validity、subject freshness、policy applicability 三軸 shared verifier | 🔵 | ### Phase 8 — existing gate structured evidence diff --git a/commands/pr-gate.md b/commands/pr-gate.md index a13338bd..494b93ec 100644 --- a/commands/pr-gate.md +++ b/commands/pr-gate.md @@ -1,6 +1,6 @@ --- description: Run the tiered pre-PR review pipeline on the current branch. -argument-hint: "[express|standard|full] [--targeted r1,r2] [--scope context] [--parallel]" +argument-hint: "[express|standard|full] [--targeted r1,r2 --initial-result path] [--scope context] [--mode sequential|parallel]" --- Run the PR gate via `pmctl gate run`. The `runtime/bin/pr-gate.sh` script is the @@ -9,15 +9,15 @@ internal implementation; `pmctl gate run` is the preferred invocation surface. **Sequential mode (default):** all reviewers run in one combined session. Low main-thread token cost (~5k dispatch + read result). -**Parallel mode (`--parallel`):** each reviewer runs in its own independent session +**Parallel mode (`--mode parallel`; `--parallel` is compatible):** each reviewer runs in its own independent session followed by a PM synthesis session. Higher token cost — use for auth/payment/migration paths or when reviewer independence matters. | Situation | Args | |---|---| | Routine code / seed / docs changes | _(none)_ | -| Re-gate after fixing specific findings | `--targeted qa-tester,risk-reviewer` | -| Auth / payment / migration / sensitive paths | `--parallel` | +| Re-gate after fixing specific findings | `--targeted qa-tester,risk-reviewer --initial-result ` | +| Auth / payment / migration / sensitive paths | `--mode parallel` | | Force a specific tier | `express` / `standard` / `full` | ## Step 1 - Invoke pmctl directly @@ -78,8 +78,9 @@ implementation model or alternate executor availability cannot be determined. RAW_ARGS="${ARGUMENTS:-}" TIER_OVERRIDE="" TARGETED_REVIEWERS="" +INITIAL_RESULT="" SCOPE_TOKENS=() -PARALLEL=false +GATE_MODE="" GATE_EXECUTOR="" GATE_MODEL="" @@ -105,13 +106,37 @@ while [[ "$idx" -lt "${#TOKENS[@]}" ]]; do --targeted) idx=$((idx + 1)) TARGETED_REVIEWERS="${TOKENS[$idx]:-}" + [[ -n "$TARGETED_REVIEWERS" ]] || { echo "error: --targeted requires a reviewer list" >&2; exit 2; } + ;; + --initial-result) + idx=$((idx + 1)) + INITIAL_RESULT="${TOKENS[$idx]:-}" + [[ -n "$INITIAL_RESULT" ]] || { echo "error: --initial-result requires a path" >&2; exit 2; } ;; --scope) idx=$((idx + 1)) [[ -n "${TOKENS[$idx]:-}" ]] && SCOPE_TOKENS+=("${TOKENS[$idx]}") ;; - --parallel) - PARALLEL=true + --mode) + idx=$((idx + 1)) + requested_mode="${TOKENS[$idx]:-}" + case "$requested_mode" in + sequential|parallel) ;; + *) echo "error: --mode requires sequential or parallel" >&2; exit 2 ;; + esac + if [[ -n "$GATE_MODE" && "$GATE_MODE" != "$requested_mode" ]]; then + echo "error: conflicting gate mode options" >&2 + exit 2 + fi + GATE_MODE="$requested_mode" + ;; + --parallel|--sequential) + requested_mode="${tok#--}" + if [[ -n "$GATE_MODE" && "$GATE_MODE" != "$requested_mode" ]]; then + echo "error: conflicting gate mode options" >&2 + exit 2 + fi + GATE_MODE="$requested_mode" ;; --executor) idx=$((idx + 1)) @@ -129,6 +154,15 @@ while [[ "$idx" -lt "${#TOKENS[@]}" ]]; do idx=$((idx + 1)) done +if [[ -n "$TARGETED_REVIEWERS" && -z "$INITIAL_RESULT" ]]; then + echo "error: --targeted requires --initial-result " >&2 + exit 2 +fi +if [[ -z "$TARGETED_REVIEWERS" && -n "$INITIAL_RESULT" ]]; then + echo "error: --initial-result is only valid with --targeted" >&2 + exit 2 +fi + # Validate after parsing so this also rejects a missing substitution of the # default, not only an invalid explicit --executor value. case "$GATE_EXECUTOR" in @@ -148,9 +182,9 @@ SCOPE="${SCOPE_TOKENS[*]:-}" GATE_ARGS=(--cd "" --executor "$GATE_EXECUTOR") [[ -n "$GATE_MODEL" ]] && GATE_ARGS+=(--model "$GATE_MODEL") [[ -n "$TIER_OVERRIDE" ]] && GATE_ARGS+=(--tier "$TIER_OVERRIDE") -[[ -n "$TARGETED_REVIEWERS" ]] && GATE_ARGS+=(--reviewers "$TARGETED_REVIEWERS") +[[ -n "$TARGETED_REVIEWERS" ]] && GATE_ARGS+=(--targeted "$TARGETED_REVIEWERS" --initial-result "$INITIAL_RESULT") [[ -n "$SCOPE" ]] && GATE_ARGS+=(--scope "$SCOPE") -[[ "$PARALLEL" == "true" ]] && GATE_ARGS+=(--parallel) +[[ -n "$GATE_MODE" ]] && GATE_ARGS+=(--mode "$GATE_MODE") # Launch detached: this call is inline (NOT run_in_background) and returns in # well under a second once the supervisor is forked -- stdout prints exactly diff --git a/commands/ship.md b/commands/ship.md index 25be64fc..129bcac0 100644 --- a/commands/ship.md +++ b/commands/ship.md @@ -173,10 +173,11 @@ once the call returns. assertion or fixture adjustment, or a small guard/error-handling fix. Then re-run `pmctl gate run --executor --cd "" - --lifecycle foreground --reviewers ` (same literal-path - substitution as Step 3's first call — never `"$PWD"`; the `/pr-gate` - `--targeted` flag maps to this same `--reviewers` option) for the reviewers - whose territory the fix touched. Repeat. + --lifecycle foreground --targeted --initial-result + ""` (same literal-path substitution as Step 3's + first call — never `"$PWD"`). The initial-result path is the comprehensive + gate result that opened this remediation loop; keep it explicit on every + targeted pass. Repeat for the reviewers whose territory the fix touched. **Stop the loop only when**: 1. Step 0's check would have caught this but didn't — implementation revealed diff --git a/core/README.md b/core/README.md index 1e4244f7..3426c7d4 100644 --- a/core/README.md +++ b/core/README.md @@ -5,7 +5,8 @@ This directory contains the canonical PM-runtime data contract. **`core/` is def - `schema/` — JSON Schema files (`.schema.json`) for the 8 first-class entities: Task, Run, Event, Review, Decision, Brief, Handover, ContextPack. -- `policy/` — declarative tables (YAML) for enums and state machines. +- `policy/` — declarative YAML/TSV tables for enums, presets, and state + machines. - `state/` — definition of the on-disk state-store layout (`~/.local/share/pm-dispatch/state/`). **Definitions, not the writer.** - `context-pack/` — source-interface contract for ContextPack assembly. @@ -25,7 +26,7 @@ This directory contains the canonical PM-runtime data contract. **`core/` is def ## Dependency graph (acyclic, downward) ``` -core/schema/ → core/policy/ (via inline enum + YAML editing source-of-truth) +core/schema/ → core/policy/ (via inline enum + declarative editing source-of-truth) core/state/ → core/schema/ (layout.yaml names which schema each path conforms to) core/context-pack/ → core/schema/ (source.interface.md references context-pack.schema.json) ``` @@ -33,7 +34,7 @@ core/context-pack/ → core/schema/ (source.interface.md references context-p `core/policy/` is a leaf — depends on nothing. `core/state/` does NOT import `core/policy/`. -The designated writer module in `runtime/lib/state-writer.sh` is the sole manager of writes to `~/.local/share/pm-dispatch/state/` paths. It validates every durable Run, Event, Task, and Decision write against recursive object requirements, constants, primitive types, enums, and `if`/`then` conditionals declared in `core/schema/`, using `jq` only. Runtime enum consumers read the corresponding `core/policy/` YAML, while parity tests keep the inline schema enum mirrors synchronized. Full draft-07 validation remains a development/test concern rather than a runtime dependency. +The designated writer module in `runtime/lib/state-writer.sh` is the sole manager of writes to `~/.local/share/pm-dispatch/state/` paths. It validates every durable Run, Event, Task, and Decision write against recursive object requirements, constants, primitive types, enums, and `if`/`then` conditionals declared in `core/schema/`, using `jq` only. Runtime enum consumers read the corresponding declarative source under `core/policy/`, while parity tests keep inline schema mirrors and generated portability snapshots synchronized. Full draft-07 validation remains a development/test concern rather than a runtime dependency. ## Schema versioning diff --git a/core/policy/gate-modes.tsv b/core/policy/gate-modes.tsv new file mode 100644 index 00000000..96ffc411 --- /dev/null +++ b/core/policy/gate-modes.tsv @@ -0,0 +1,4 @@ +# Gate execution topology. Mode does not imply tier or reviewer coverage. +mode topology synthesis is_default +sequential combined-session inline true +parallel per-reviewer-sessions separate-session false diff --git a/core/policy/gate-pass-kinds.tsv b/core/policy/gate-pass-kinds.tsv new file mode 100644 index 00000000..7296bb61 --- /dev/null +++ b/core/policy/gate-pass-kinds.tsv @@ -0,0 +1,4 @@ +# Gate review-pass semantics. Pass kind does not imply tier or execution mode. +pass_kind scope requires_initial_result is_default +initial comprehensive false true +targeted remediation-delta true false diff --git a/core/policy/gate-tiers.tsv b/core/policy/gate-tiers.tsv new file mode 100644 index 00000000..eb1c8dba --- /dev/null +++ b/core/policy/gate-tiers.tsv @@ -0,0 +1,5 @@ +# Gate rigor presets. Defaults do not assert actual coverage or execution topology. +tier default_reviewers evidence_floor +express critic,qa-tester reviewer-verdicts +standard critic,qa-tester,architecture-reviewer reviewer-verdicts +full critic,qa-tester,architecture-reviewer,security-reviewer,risk-reviewer reviewer-verdicts diff --git a/core/schema/review.schema.json b/core/schema/review.schema.json index 2776aa18..756ceb1a 100644 --- a/core/schema/review.schema.json +++ b/core/schema/review.schema.json @@ -1,6 +1,6 @@ { "title": "Review", - "description": "A PR-gate review verdict, optionally with per-reviewer findings. `tier` mirrors /pr-gate tier names (express/standard/full/targeted). `findings_path` points at .gate-results//.{json,md} — Review row is the INDEX, not the data.", + "description": "A PR-gate review verdict, optionally with per-reviewer findings. Current rigor tiers come from core/policy/gate-tiers.tsv. The schema_version 1 `targeted` tier value is retained only to validate legacy payloads; targeted remediation is now a pass kind. `findings_path` points at .gate-results//.{json,md} — Review row is the INDEX, not the data.", "type": "object", "required": ["id", "schema_version", "task_id", "tier", "verdict", "findings", "created_ts"], "properties": { @@ -20,7 +20,8 @@ "description": "Optional link to the Run that triggered the gate." }, "tier": { - "enum": ["express", "standard", "full", "targeted"] + "enum": ["express", "standard", "full", "targeted"], + "description": "Current editing source: core/policy/gate-tiers.tsv. `targeted` is a schema_version 1 compatibility value and must not be emitted by new producers." }, "verdict": { "enum": ["GO", "NO-GO", "pending"] diff --git a/docs/review-model.md b/docs/review-model.md index cd903a81..8d4f883f 100644 --- a/docs/review-model.md +++ b/docs/review-model.md @@ -143,19 +143,33 @@ When an artifact is missing — no `conceptual_map`, no `self_verify cmd:`, no i --- -## pr-gate rigor tiers +## pr-gate assurance coordinates The `/pr-gate` `--tier` flag selects the **rigor level** required for this change — not just the number of reviewers. Choose based on `architecture_impact` and blast radius: -| Tier | When | Rigor | +| Tier | When | Default reviewer coverage | |---|---|---| -| `express` | hotfix, docs-only, `architecture_impact: none` | machine verification + combined session (critic + qa) | +| `express` | hotfix, docs-only, `architecture_impact: none` | critic + qa | | `standard` | feature, `architecture_impact: minor` | conceptual map required + critic + qa + architecture-reviewer | -| `full` | architectural change, `architecture_impact: major`, sensitive path | parallel cross-context sessions + security + risk hard gates + synthesis | +| `full` | architectural change, `architecture_impact: major`, sensitive path | critic + qa + architecture-reviewer + security + risk | **Tier suggestion**: when a `--brief` is passed to `pr-gate.sh`, it reads `architecture_impact` from the brief and emits an advisory to stderr before dispatch if the auto-detected tier is lower than the impact level implies. The user-selected or auto-detected tier always takes precedence; the advisory is informational only and does not block or alter the tier. -**Tier vs. reviewer count**: `express` is not "fewer reviewers" — it is "this change has bounded impact and does not need architecture-level judgment." `full` is not "more reviewers" — it is "this change has wide blast radius and needs independent parallel review with hard security/risk gates." +**Tier, mode, pass kind, and coverage are independent**: + +- Tier records rigor intent and supplies default reviewer coverage. `--reviewers` + may override the selected coverage without rewriting the tier. +- Mode records execution topology. The default is `sequential`; select + `--mode parallel` when separate reviewer sessions and synthesis are required. + A `full` tier does not select parallel mode by itself. +- Pass kind records whether the review is initial or a remediation-delta + targeted pass. `--targeted ` requires + `--initial-result ` and is not a tier alias. + +The portable policy sources are +[`core/policy/gate-tiers.tsv`](../core/policy/gate-tiers.tsv), +[`core/policy/gate-modes.tsv`](../core/policy/gate-modes.tsv), and +[`core/policy/gate-pass-kinds.tsv`](../core/policy/gate-pass-kinds.tsv). --- diff --git a/runtime/bin/pr-gate.sh b/runtime/bin/pr-gate.sh index 133e5895..4cabee9c 100755 --- a/runtime/bin/pr-gate.sh +++ b/runtime/bin/pr-gate.sh @@ -17,6 +17,168 @@ trap '' PIPE # shellcheck disable=SC2059 # printf passthrough wrapper: the caller owns the format string say() { printf "$@" 2>/dev/null || true; } +# Portable policy reader for the gate assurance coordinates. +# +# Repo-layout runs read the canonical TSV files under core/policy/. A copied +# gate may not carry that tree, so it falls back to the bounded generated +# snapshot below. tests/shell/test-pr-gate.sh compares each heredoc byte-for- +# byte with its canonical source so this fallback cannot drift silently. +_gate_assurance_policy_snapshot() { + case "${1:-}" in + tiers) + # BEGIN GENERATED from core/policy/gate-tiers.tsv + cat <<'GATE_ASSURANCE_TIERS_TSV' +# Gate rigor presets. Defaults do not assert actual coverage or execution topology. +tier default_reviewers evidence_floor +express critic,qa-tester reviewer-verdicts +standard critic,qa-tester,architecture-reviewer reviewer-verdicts +full critic,qa-tester,architecture-reviewer,security-reviewer,risk-reviewer reviewer-verdicts +GATE_ASSURANCE_TIERS_TSV + # END GENERATED from core/policy/gate-tiers.tsv + ;; + modes) + # BEGIN GENERATED from core/policy/gate-modes.tsv + cat <<'GATE_ASSURANCE_MODES_TSV' +# Gate execution topology. Mode does not imply tier or reviewer coverage. +mode topology synthesis is_default +sequential combined-session inline true +parallel per-reviewer-sessions separate-session false +GATE_ASSURANCE_MODES_TSV + # END GENERATED from core/policy/gate-modes.tsv + ;; + pass-kinds) + # BEGIN GENERATED from core/policy/gate-pass-kinds.tsv + cat <<'GATE_ASSURANCE_PASS_KINDS_TSV' +# Gate review-pass semantics. Pass kind does not imply tier or execution mode. +pass_kind scope requires_initial_result is_default +initial comprehensive false true +targeted remediation-delta true false +GATE_ASSURANCE_PASS_KINDS_TSV + # END GENERATED from core/policy/gate-pass-kinds.tsv + ;; + *) + printf 'pr-gate: unknown assurance policy table: %s\n' "${1:-empty}" >&2 + return 2 + ;; + esac +} + +_gate_assurance_policy_filename() { + case "${1:-}" in + tiers) printf 'gate-tiers.tsv\n' ;; + modes) printf 'gate-modes.tsv\n' ;; + pass-kinds) printf 'gate-pass-kinds.tsv\n' ;; + *) return 2 ;; + esac +} + +_gate_assurance_policy_path() { + local filename candidate + filename="$(_gate_assurance_policy_filename "${1:-}")" || return 2 + for candidate in \ + "$SCRIPT_DIR/../../core/policy/$filename" \ + "$SCRIPT_DIR/core/policy/$filename" + do + if [[ -r "$candidate" ]]; then + printf '%s\n' "$candidate" + return 0 + fi + done + return 1 +} + +_gate_assurance_policy_emit() { + local path + if path="$(_gate_assurance_policy_path "${1:-}")"; then + cat "$path" + else + _gate_assurance_policy_snapshot "${1:-}" + fi +} + +# _gate_assurance_policy_lookup +# Requires exactly one matching row and rejects malformed/duplicate tables. +_gate_assurance_policy_lookup() { + local table="${1:-}" key_column="${2:-}" key="${3:-}" value_column="${4:-}" + [[ $# -eq 4 ]] || return 2 + _gate_assurance_policy_emit "$table" | awk -F '\t' \ + -v key_name="$key_column" -v wanted="$key" -v value_name="$value_column" ' + /^[[:space:]]*#/ || /^[[:space:]]*$/ { next } + !header_seen { + header_seen=1 + header_width=NF + for (i=1; i<=NF; i++) { + sub(/\r$/, "", $i) + if ($i == key_name) key_index=i + if ($i == value_name) value_index=i + } + if (!key_index || !value_index) { malformed=1; exit } + next + } + { + sub(/\r$/, "", $NF) + if (NF != header_width) { malformed=1; exit } + if ($(key_index) == wanted) { + matches++ + result=$(value_index) + } + } + END { + if (malformed || !header_seen || matches != 1 || result == "") exit 2 + print result + } + ' +} + +# _gate_assurance_policy_values
+# Emits a unique, non-empty closed enum in source order. +_gate_assurance_policy_values() { + local table="${1:-}" key_column="${2:-}" + [[ $# -eq 2 ]] || return 2 + _gate_assurance_policy_emit "$table" | awk -F '\t' -v key_name="$key_column" ' + /^[[:space:]]*#/ || /^[[:space:]]*$/ { next } + !header_seen { + header_seen=1 + header_width=NF + for (i=1; i<=NF; i++) { + sub(/\r$/, "", $i) + if ($i == key_name) key_index=i + } + if (!key_index) { malformed=1; exit } + next + } + { + sub(/\r$/, "", $NF) + value=$(key_index) + if (NF != header_width || value == "" || seen[value]++) { + malformed=1 + exit + } + values[++count]=value + } + END { + if (malformed || !header_seen || count == 0) exit 2 + for (i=1; i<=count; i++) print values[i] + } + ' +} + +_gate_set_mode_requested() { + local candidate="$1" spelling="$2" + if [[ "$MODE_OPTION_SEEN" == false ]]; then + MODE_REQUESTED="$candidate" + MODE_OPTION_SEEN=true + MODE_OPTION_SPELLING="$spelling" + return 0 + fi + if [[ "$MODE_REQUESTED" != "$candidate" ]]; then + printf 'Error: conflicting gate mode options: %s requested %s, but %s requested %s\n' \ + "$MODE_OPTION_SPELLING" "$MODE_REQUESTED" "$spelling" "$candidate" >&2 + return 2 + fi + return 0 +} + # verify_reviewer_artifact_hashes [...] # Print every reviewer whose artifact differs from its captured baseline. verify_reviewer_artifact_hashes() { @@ -45,6 +207,7 @@ _kill_process_tree() { kill -"$_sig" "$_pid" 2>/dev/null || true } +# pr-gate-help:start # pr-gate.sh -- PR-gate review via a dispatched session # # DEFAULT (single-session / sequential): @@ -52,7 +215,7 @@ _kill_process_tree() { # Lower token cost. All reviewer findings appear in a single output file. # Use this for most routine changes. # -# MULTI-SESSION (--parallel): +# MULTI-SESSION (--mode parallel; --parallel is compatible): # Each reviewer runs in its own INDEPENDENT dispatch session, followed by a # separate PM synthesis session. Reviewers share no context window, which # eliminates anchoring bias across reviewers. @@ -69,9 +232,11 @@ _kill_process_tree() { # Options: # --cd working directory (required) # --tier express|standard|full -- overrides auto-detection +# --mode sequential|parallel execution topology (default: sequential) # --brief dispatch brief for this change; architecture_impact field informs tier suggestion -# --reviewers comma-separated names -- overrides tier default (targeted re-gate) -# --targeted alias for --reviewers (matches /pr-gate skill vocabulary) +# --reviewers comma-separated requested coverage; does not change tier or pass kind +# --targeted remediation-delta pass over these reviewers; requires --initial-result +# --initial-result initial gate result referenced by a --targeted pass; relative to --cd # --reviewer-dir explicit reviewer-definition source; defaults to the repo-owned agents/ directory # --scope context hint passed into the review brief # --base base branch for diff (default: origin/HEAD → main) @@ -94,8 +259,8 @@ _kill_process_tree() { # this to dial reasoning depth up/down without switching models. # --isolation isolation level: none|read-only|workspace-write|workspace-network|sandboxed # --timeout dispatch timeout per session (default: 1200) -# --parallel multi-session: one dispatch per reviewer + synthesis (higher token cost) -# --sequential alias for default single-session mode (kept for backward compatibility) +# --parallel compatibility spelling for --mode parallel +# --sequential compatibility spelling for --mode sequential # --allow-hooks execute repo-local .pm-dispatch hook scripts (trusted branches only) # --allow-dirty review the working tree as-is instead of failing on a dirty tree atop committed changes # --override-file inject accepted-risk overrides into every reviewer brief; auto-discovered @@ -120,18 +285,26 @@ _kill_process_tree() { # --test-timeout timeout for --test-cmd (default: 1800), decoupled from --timeout so a slow # test suite can never cause a reviewer dispatch session to time out. # --skip-preflight-tests force-disable the pre-flight test check even if --test-cmd is passed. +# pr-gate-help:end WORK_DIR="" GATE_RUN_DIR_OVERRIDE="" # out-of-repo artifact root; set via --run-dir from pmctl-gate TIER_OVERRIDE="" +TIER_REQUESTED="auto" REVIEWERS_OVERRIDE="" +REVIEWERS_OPTION_SOURCE="" +MODE_REQUESTED="default" +MODE_OPTION_SEEN=false +MODE_OPTION_SPELLING="" +PASS_KIND_REQUESTED="initial" +INITIAL_RESULT_INPUT="" +INITIAL_RESULT_OPTION_SEEN=false REVIEWER_DIR_OVERRIDE="" SCOPE="" BASE_OVERRIDE="" HEAD_OVERRIDE="" OUTPUT_OVERRIDE="" TIMEOUT="1200" -SEQUENTIAL=true # default: sequential (lower token cost) EXECUTOR_OPTION="auto" ALLOW_HOOKS=false # hooks require explicit --allow-hooks opt-in (security) ALLOW_DIRTY=false # gate refuses a dirty tree atop committed changes unless this opt-in @@ -156,12 +329,42 @@ SKIP_PREFLIGHT_TESTS=false while [[ $# -gt 0 ]]; do case "$1" in - --cd) WORK_DIR="$2"; shift 2;; - --run-dir) GATE_RUN_DIR_OVERRIDE="$2"; shift 2;; - --tier) TIER_OVERRIDE="$2"; shift 2;; + --cd) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --cd requires a directory\n' >&2; exit 2; } + WORK_DIR="$2"; shift 2;; + --run-dir) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --run-dir requires a directory\n' >&2; exit 2; } + GATE_RUN_DIR_OVERRIDE="$2"; shift 2;; + --tier) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --tier requires a value\n' >&2; exit 2; } + TIER_OVERRIDE="$2"; TIER_REQUESTED="$2"; shift 2;; + --mode) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --mode requires sequential or parallel\n' >&2; exit 2; } + _gate_set_mode_requested "$2" "--mode" || exit 2 + shift 2;; --brief) BRIEF_FILE="$2"; shift 2;; - --reviewers) REVIEWERS_OVERRIDE="$2"; shift 2;; - --targeted) REVIEWERS_OVERRIDE="$2"; shift 2;; # alias: /pr-gate skill + script comments say "targeted" + --reviewers) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --reviewers requires a reviewer list\n' >&2; exit 2; } + [[ -z "$REVIEWERS_OPTION_SOURCE" ]] || { + printf 'Error: --reviewers and --targeted may not be combined or repeated\n' >&2 + exit 2 + } + REVIEWERS_OVERRIDE="$2"; REVIEWERS_OPTION_SOURCE="--reviewers"; shift 2;; + --targeted) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --targeted requires a reviewer list\n' >&2; exit 2; } + [[ -z "$REVIEWERS_OPTION_SOURCE" ]] || { + printf 'Error: --reviewers and --targeted may not be combined or repeated\n' >&2 + exit 2 + } + REVIEWERS_OVERRIDE="$2"; REVIEWERS_OPTION_SOURCE="--targeted" + PASS_KIND_REQUESTED="targeted"; shift 2;; + --initial-result) + [[ $# -ge 2 && -n "$2" && "$2" != --* ]] || { printf 'Error: --initial-result requires a result path\n' >&2; exit 2; } + [[ "$INITIAL_RESULT_OPTION_SEEN" == false ]] || { + printf 'Error: --initial-result may only be provided once\n' >&2 + exit 2 + } + INITIAL_RESULT_INPUT="$2"; INITIAL_RESULT_OPTION_SEEN=true; shift 2;; --reviewer-dir) [[ $# -ge 2 ]] || { printf 'Error: --reviewer-dir requires a directory\n' >&2; exit 2; } REVIEWER_DIR_OVERRIDE="$2"; shift 2;; @@ -184,8 +387,12 @@ while [[ $# -gt 0 ]]; do DISPATCH_EFFORT="$2"; shift 2;; --isolation) DISPATCH_ISOLATION="$2"; shift 2;; --timeout) TIMEOUT="$2"; shift 2;; - --parallel) SEQUENTIAL=false; shift;; - --sequential) SEQUENTIAL=true; shift;; # backward compat + --parallel) + _gate_set_mode_requested "parallel" "--parallel" || exit 2 + shift;; + --sequential) + _gate_set_mode_requested "sequential" "--sequential" || exit 2 + shift;; --allow-hooks) ALLOW_HOOKS=true; shift;; --allow-dirty) ALLOW_DIRTY=true; shift;; --override-file) @@ -202,11 +409,19 @@ while [[ $# -gt 0 ]]; do TEST_TIMEOUT="$2"; shift 2;; --skip-preflight-tests) SKIP_PREFLIGHT_TESTS=true; shift;; -h|--help) - sed -n '2,99p' "$0" | sed 's/^# \{0,1\}//' + awk ' + /^# pr-gate-help:start$/ { help=1; next } + /^# pr-gate-help:end$/ { exit } + help { + line=$0 + sub(/^# ?/, "", line) + print line + } + ' "$0" exit 0;; *) printf 'Unknown arg: %s\n' "$1" >&2 - printf 'Accepted: --cd --run-dir --tier --brief --reviewers|--targeted --reviewer-dir --scope --base --head --output --executor --model --effort --isolation --timeout --parallel --sequential --allow-hooks --allow-dirty --override-file --test-cmd --test-timeout --skip-preflight-tests (-h for help)\n' >&2 + printf 'Accepted: --cd --run-dir --tier --mode --brief --reviewers --targeted --initial-result --reviewer-dir --scope --base --head --output --executor --model --effort --isolation --timeout --parallel --sequential --allow-hooks --allow-dirty --override-file --test-cmd --test-timeout --skip-preflight-tests (-h for help)\n' >&2 exit 2;; esac done @@ -284,6 +499,186 @@ else return 0 } fi + +# ── Resolve assurance policy coordinates ───────────────────────────────────── +# These are resolved before any executor routing or dispatch. The LLM receives +# the resolved values as read-only context; it does not choose or infer them. +if ! GATE_TIER_VALUES="$(_gate_assurance_policy_values tiers tier)"; then + printf 'Error: invalid gate tier policy source\n' >&2 + exit 2 +fi +if ! GATE_MODE_VALUES="$(_gate_assurance_policy_values modes mode)"; then + printf 'Error: invalid gate mode policy source\n' >&2 + exit 2 +fi +if ! GATE_PASS_KIND_VALUES="$(_gate_assurance_policy_values pass-kinds pass_kind)"; then + printf 'Error: invalid gate pass-kind policy source\n' >&2 + exit 2 +fi + +if ! GATE_MODE_DEFAULT="$(_gate_assurance_policy_lookup modes is_default true mode)"; then + printf 'Error: gate mode policy must declare exactly one default\n' >&2 + exit 2 +fi +if ! GATE_PASS_KIND_DEFAULT="$(_gate_assurance_policy_lookup pass-kinds is_default true pass_kind)"; then + printf 'Error: gate pass-kind policy must declare exactly one default\n' >&2 + exit 2 +fi +if [[ "$GATE_MODE_DEFAULT" != "sequential" ]]; then + printf 'Error: gate mode policy default must remain sequential\n' >&2 + exit 2 +fi +if [[ "$GATE_PASS_KIND_DEFAULT" != "initial" ]]; then + printf 'Error: gate pass-kind policy default must remain initial\n' >&2 + exit 2 +fi + +if [[ -n "$TIER_OVERRIDE" ]] \ + && ! _gate_assurance_policy_lookup tiers tier "$TIER_OVERRIDE" default_reviewers >/dev/null; then + printf 'Error: --tier must be one of: %s (got: %s)\n' \ + "$(printf '%s\n' "$GATE_TIER_VALUES" | awk 'BEGIN{ORS=" "} {print} END{print "\n"}' | sed 's/[[:space:]]*$//')" \ + "$TIER_OVERRIDE" >&2 + exit 2 +fi + +if [[ "$MODE_OPTION_SEEN" == false ]]; then + MODE_RESOLVED="$GATE_MODE_DEFAULT" +else + MODE_RESOLVED="$MODE_REQUESTED" +fi +if ! MODE_TOPOLOGY="$(_gate_assurance_policy_lookup modes mode "$MODE_RESOLVED" topology)" \ + || ! MODE_SYNTHESIS="$(_gate_assurance_policy_lookup modes mode "$MODE_RESOLVED" synthesis)"; then + printf 'Error: --mode must be one of: %s (got: %s)\n' \ + "$(printf '%s\n' "$GATE_MODE_VALUES" | awk 'BEGIN{ORS=" "} {print} END{print "\n"}' | sed 's/[[:space:]]*$//')" \ + "$MODE_RESOLVED" >&2 + exit 2 +fi +case "$MODE_TOPOLOGY:$MODE_SYNTHESIS" in + combined-session:inline) SEQUENTIAL=true ;; + per-reviewer-sessions:separate-session) SEQUENTIAL=false ;; + *) + printf 'Error: unsupported gate mode topology for %s: %s + %s\n' \ + "$MODE_RESOLVED" "$MODE_TOPOLOGY" "$MODE_SYNTHESIS" >&2 + exit 2 + ;; +esac + +PASS_KIND_RESOLVED="$PASS_KIND_REQUESTED" +if ! PASS_SCOPE="$(_gate_assurance_policy_lookup pass-kinds pass_kind "$PASS_KIND_RESOLVED" scope)" \ + || ! PASS_REQUIRES_INITIAL="$(_gate_assurance_policy_lookup pass-kinds pass_kind "$PASS_KIND_RESOLVED" requires_initial_result)"; then + printf 'Error: gate pass kind must be one of: %s (got: %s)\n' \ + "$(printf '%s\n' "$GATE_PASS_KIND_VALUES" | awk 'BEGIN{ORS=" "} {print} END{print "\n"}' | sed 's/[[:space:]]*$//')" \ + "$PASS_KIND_RESOLVED" >&2 + exit 2 +fi +case "$PASS_REQUIRES_INITIAL" in + true) + if [[ -z "$INITIAL_RESULT_INPUT" ]]; then + printf 'Error: --targeted requires --initial-result \n' >&2 + exit 2 + fi + ;; + false) + if [[ -n "$INITIAL_RESULT_INPUT" ]]; then + printf 'Error: --initial-result is only valid with --targeted\n' >&2 + exit 2 + fi + ;; + *) + printf 'Error: invalid requires_initial_result value for pass kind %s: %s\n' \ + "$PASS_KIND_RESOLVED" "$PASS_REQUIRES_INITIAL" >&2 + exit 2 + ;; +esac + +INITIAL_RESULT_RESOLVED="" +if [[ -n "$INITIAL_RESULT_INPUT" ]]; then + _initial_result_candidate="$INITIAL_RESULT_INPUT" + [[ "$_initial_result_candidate" == /* ]] \ + || _initial_result_candidate="$WORK_DIR/$_initial_result_candidate" + if [[ ! -f "$_initial_result_candidate" || ! -r "$_initial_result_candidate" \ + || ! -s "$_initial_result_candidate" ]]; then + printf 'Error: --initial-result must name a readable, non-empty file: %s\n' \ + "$INITIAL_RESULT_INPUT" >&2 + exit 2 + fi + _initial_result_parent="$(cd "$(dirname "$_initial_result_candidate")" && pwd -P)" || { + printf 'Error: cannot resolve --initial-result parent: %s\n' "$INITIAL_RESULT_INPUT" >&2 + exit 2 + } + INITIAL_RESULT_RESOLVED="$_initial_result_parent/$(basename "$_initial_result_candidate")" + if ! gate_result_verify "$INITIAL_RESULT_RESOLVED" "" "targeted initial result"; then + printf 'Error: --initial-result is not a structurally valid gate result: %s\n' \ + "$INITIAL_RESULT_INPUT" >&2 + exit 2 + fi + unset _initial_result_candidate _initial_result_parent +fi + +# Build the closed reviewer vocabulary from tier defaults in source order. +# This is a vocabulary only: a tier default is not actual selected coverage. +ALL_REVIEWERS="" +while IFS= read -r _policy_tier; do + _policy_reviewers="$(_gate_assurance_policy_lookup tiers tier "$_policy_tier" default_reviewers)" || { + printf 'Error: invalid default_reviewers for gate tier: %s\n' "$_policy_tier" >&2 + exit 2 + } + _gate_assurance_policy_lookup tiers tier "$_policy_tier" evidence_floor >/dev/null || { + printf 'Error: missing evidence_floor for gate tier: %s\n' "$_policy_tier" >&2 + exit 2 + } + for _policy_reviewer in $(printf '%s' "$_policy_reviewers" | tr ',' ' '); do + if [[ ! "$_policy_reviewer" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then + printf 'Error: invalid reviewer name in gate tier policy: %s\n' "$_policy_reviewer" >&2 + exit 2 + fi + if [[ " $ALL_REVIEWERS " != *" $_policy_reviewer "* ]]; then + ALL_REVIEWERS="${ALL_REVIEWERS:+$ALL_REVIEWERS }$_policy_reviewer" + fi + done +done <<< "$GATE_TIER_VALUES" +unset _policy_tier _policy_reviewers _policy_reviewer +if [[ -z "$ALL_REVIEWERS" ]]; then + printf 'Error: gate tier policy did not declare any reviewers\n' >&2 + exit 2 +fi + +_gate_normalize_reviewer_list() { + local raw="$1" source_label="$2" normalized="" reviewer + if [[ -z "${raw//[[:space:]]/}" || "$raw" == ,* || "$raw" == *, || "$raw" == *,,* ]]; then + printf 'Error: %s requires a non-empty comma-separated reviewer list\n' "$source_label" >&2 + return 2 + fi + raw="${raw//,/ }" + for reviewer in $raw; do + if [[ " $ALL_REVIEWERS " != *" $reviewer "* ]]; then + printf 'Error: %s contains unknown reviewer %s (allowed: %s)\n' \ + "$source_label" "$reviewer" "$ALL_REVIEWERS" >&2 + return 2 + fi + if [[ " $normalized " == *" $reviewer "* ]]; then + printf 'Error: %s contains duplicate reviewer: %s\n' "$source_label" "$reviewer" >&2 + return 2 + fi + normalized="${normalized:+$normalized }$reviewer" + done + [[ -n "$normalized" ]] || return 2 + printf '%s\n' "$normalized" +} + +_gate_policy_source_count=0 +for _gate_policy_table in tiers modes pass-kinds; do + if _gate_assurance_policy_path "$_gate_policy_table" >/dev/null; then + _gate_policy_source_count=$((_gate_policy_source_count + 1)) + fi +done +case "$_gate_policy_source_count" in + 3) GATE_ASSURANCE_POLICY_SOURCE="canonical" ;; + 0) GATE_ASSURANCE_POLICY_SOURCE="generated-snapshot" ;; + *) GATE_ASSURANCE_POLICY_SOURCE="mixed" ;; +esac +unset _gate_policy_source_count _gate_policy_table + EXECUTOR_ROUTER_PATH="$SCRIPT_DIR/../lib/executor-router.sh" if [[ -r "$EXECUTOR_ROUTER_PATH" ]]; then # shellcheck source=runtime/lib/executor-router.sh @@ -851,24 +1246,31 @@ fi # ── Detect tier ─────────────────────────────────────────────────────────────── if [[ -n "$TIER_OVERRIDE" ]]; then - TIER="$TIER_OVERRIDE" -elif [[ -n "$REVIEWERS_OVERRIDE" ]]; then - TIER="targeted" + TIER_RESOLVED="$TIER_OVERRIDE" else NON_DOCS=$(printf '%s\n' "$DIFF_FILES" | grep -vE '\.(md|jsonl|txt)$|^\.gitignore$|^audits/|^docs/' || true) SENSITIVE_HIT=$(printf '%s\n' "$DIFF_FILES" | { grep -iE '(^|[/_.-])(auth|oauth|jwt|session|secret|password|token|credential|cors|csrf|webhook|sudo|ssh|payment|billing)([/_.-]|$)|(^|/)migrations?/|^\.github/' || true; } | wc -l) if [[ -z "$NON_DOCS" ]]; then - TIER=express + TIER_RESOLVED=express elif [[ "$SENSITIVE_HIT" -gt 0 || "$LINES" -gt 500 ]]; then - TIER=full + TIER_RESOLVED=full elif [[ "$LINES" -lt 100 && "${BINARY_HIT:-0}" -eq 0 ]]; then # Binary files have no line count but represent real changes -- treat as standard+ - TIER=express + TIER_RESOLVED=express else - TIER=standard + TIER_RESOLVED=standard fi fi +if ! _gate_assurance_policy_lookup tiers tier "$TIER_RESOLVED" default_reviewers >/dev/null; then + printf 'Error: detected gate tier is absent from policy: %s\n' "$TIER_RESOLVED" >&2 + exit 2 +fi +TIER="$TIER_RESOLVED" +TIER_EVIDENCE_FLOOR="$(_gate_assurance_policy_lookup tiers tier "$TIER" evidence_floor)" || { + printf 'Error: gate tier policy has no evidence floor for: %s\n' "$TIER" >&2 + exit 2 +} # ── Brief-based tier suggestion (advisory; never overrides --tier) ──────────── if [[ -n "$BRIEF_FILE" && -f "$BRIEF_FILE" && -z "$TIER_OVERRIDE" ]]; then @@ -888,21 +1290,21 @@ if [[ -n "$BRIEF_FILE" && -f "$BRIEF_FILE" && -z "$TIER_OVERRIDE" ]]; then fi # ── Determine reviewer list ─────────────────────────────────────────────────── -ALL_REVIEWERS="critic qa-tester architecture-reviewer security-reviewer risk-reviewer" - if [[ -n "$REVIEWERS_OVERRIDE" ]]; then - REVIEWERS=$(printf '%s' "$REVIEWERS_OVERRIDE" | tr ',' ' ') + REVIEWERS="$(_gate_normalize_reviewer_list "$REVIEWERS_OVERRIDE" "$REVIEWERS_OPTION_SOURCE")" || exit 2 + COVERAGE_REQUESTED_DISPLAY="$(printf '%s' "$REVIEWERS" | tr ' ' ',')" else - case "$TIER" in - express) REVIEWERS="critic qa-tester";; - standard) REVIEWERS="critic qa-tester architecture-reviewer";; - full) REVIEWERS="$ALL_REVIEWERS";; - *) REVIEWERS="$ALL_REVIEWERS";; - esac + _tier_default_reviewers="$(_gate_assurance_policy_lookup tiers tier "$TIER" default_reviewers)" || { + printf 'Error: gate tier policy has no default reviewer coverage for: %s\n' "$TIER" >&2 + exit 2 + } + REVIEWERS="$(_gate_normalize_reviewer_list "$_tier_default_reviewers" "tier $TIER default_reviewers")" || exit 2 + COVERAGE_REQUESTED_DISPLAY="default" + unset _tier_default_reviewers fi REVIEWER_DISPLAY=$(printf '%s' "$REVIEWERS" | tr ' ' ',') -NUM_REVIEWERS=$(printf '%s\n' $REVIEWERS | wc -l | tr -d ' ') +NUM_REVIEWERS=$(printf '%s\n' "$REVIEWERS" | awk '{print NF}') # Compute skipped dimensions SKIPPED="" @@ -912,6 +1314,8 @@ for r in $ALL_REVIEWERS; do fi done SKIPPED_DISPLAY="${SKIPPED:-none}" +COVERAGE_SELECTED_DISPLAY="$REVIEWER_DISPLAY" +COVERAGE_SKIPPED_DISPLAY="$SKIPPED_DISPLAY" # ── Resolve reviewer definitions ───────────────────────────────────────────── # Definitions outside the reviewed workspace are trusted installation assets. @@ -979,6 +1383,16 @@ OUTPUT_FILE="${OUTPUT_OVERRIDE:-$WORK_DIR/.gate-results/gate-${TIMESTAMP}.md}" # here IS the absolute working dir and a relative OUTPUT_FILE resolves to an absolute path under it. [[ "$OUTPUT_FILE" = /* ]] || OUTPUT_FILE="$PWD/$OUTPUT_FILE" mkdir -p "$(dirname "$OUTPUT_FILE")" +_output_parent="$(cd "$(dirname "$OUTPUT_FILE")" && pwd -P)" +OUTPUT_FILE="$_output_parent/$(basename "$OUTPUT_FILE")" +unset _output_parent +if [[ -n "$INITIAL_RESULT_RESOLVED" \ + && ( "$OUTPUT_FILE" == "$INITIAL_RESULT_RESOLVED" \ + || ( -e "$OUTPUT_FILE" && "$OUTPUT_FILE" -ef "$INITIAL_RESULT_RESOLVED" ) ) ]]; then + printf 'Error: --output must not overwrite the referenced --initial-result: %s\n' \ + "$INITIAL_RESULT_RESOLVED" >&2 + exit 2 +fi touch "$OUTPUT_FILE" # Track all brief files for EXIT cleanup @@ -1131,7 +1545,22 @@ render_gate_overrides_block() { # Pre-format the override block for heredoc injection (empty when no overrides). GATE_OVERRIDES_CONTEXT_BLOCK="$(render_gate_overrides_block "$GATE_OVERRIDES_CONTENT")" -say 'pr-gate: %s tier -- %s\n' "$TIER" "$REVIEWER_DISPLAY" +INITIAL_RESULT_DISPLAY="${INITIAL_RESULT_RESOLVED:-none}" +printf -v GATE_ASSURANCE_CONTEXT_BLOCK \ + ' Assurance coordinates (resolved by the gate shell; do not reinterpret):\n tier.requested: %s\n tier.resolved: %s\n tier.evidence_floor: %s\n mode.requested: %s\n mode.resolved: %s\n mode.topology: %s\n mode.synthesis: %s\n pass.requested: %s\n pass.resolved: %s\n pass.scope: %s\n pass.initial_result: %s\n coverage.requested: %s\n coverage.selected: %s\n coverage.skipped: %s\n policy.source: %s\n' \ + "$TIER_REQUESTED" "$TIER_RESOLVED" "$TIER_EVIDENCE_FLOOR" \ + "$MODE_REQUESTED" "$MODE_RESOLVED" "$MODE_TOPOLOGY" "$MODE_SYNTHESIS" \ + "$PASS_KIND_REQUESTED" "$PASS_KIND_RESOLVED" "$PASS_SCOPE" "$INITIAL_RESULT_DISPLAY" \ + "$COVERAGE_REQUESTED_DISPLAY" \ + "$COVERAGE_SELECTED_DISPLAY" "$COVERAGE_SKIPPED_DISPLAY" \ + "$GATE_ASSURANCE_POLICY_SOURCE" + +say 'pr-gate: tier %s -> %s; mode %s -> %s; pass %s -> %s\n' \ + "$TIER_REQUESTED" "$TIER_RESOLVED" "$MODE_REQUESTED" "$MODE_RESOLVED" \ + "$PASS_KIND_REQUESTED" "$PASS_KIND_RESOLVED" +say 'pr-gate: coverage requested=%s selected=%s skipped=%s; policy=%s\n' \ + "$COVERAGE_REQUESTED_DISPLAY" "$COVERAGE_SELECTED_DISPLAY" \ + "$COVERAGE_SKIPPED_DISPLAY" "$GATE_ASSURANCE_POLICY_SOURCE" [[ "${ADJ_COUNT:-0}" -gt 0 ]] && say ' adjacent test files added: %d\n' "$ADJ_COUNT" say 'result will be written to: %s\n\n' "$OUTPUT_FILE" @@ -1589,7 +2018,7 @@ context: Base: ${BASE}${HEAD_METADATA_LINE} Scope: ${SCOPE:-none} Date: $(date '+%Y-%m-%d') -${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} +${GATE_ASSURANCE_CONTEXT_BLOCK}${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} ${MEMORY_CONTEXT_BLOCK} Verified reference files (exist in working tree -- check before citing): ${REPO_REF_INDEX} @@ -1632,8 +2061,8 @@ output_format: | --- gate_result_version: pr_gate_result_v1 final: GO|NO-GO - tier: express|standard|full|targeted - mode: sequential + tier: ${TIER} + mode: ${MODE_RESOLVED} most_severe: approve|advise|block-soft|block reviewers: critic: approve|advise|block-soft|skipped @@ -1823,7 +2252,7 @@ context: Base: ${BASE}${HEAD_METADATA_LINE} Scope: ${SCOPE:-none} Date: $(date '+%Y-%m-%d') -${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} +${GATE_ASSURANCE_CONTEXT_BLOCK}${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} ${MEMORY_CONTEXT_BLOCK} Verified reference files (exist in working tree -- check before citing): ${REPO_REF_INDEX} @@ -2045,7 +2474,7 @@ context: Base: ${BASE}${HEAD_METADATA_LINE} Scope: ${SCOPE:-none} Date: $(date '+%Y-%m-%d') -${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} +${GATE_ASSURANCE_CONTEXT_BLOCK}${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} Verified reference files (exist in working tree -- check before citing): ${REPO_REF_INDEX} Reviewer findings (embedded -- do NOT attempt to read any external reviewer output file): @@ -2081,7 +2510,7 @@ output_format: | gate_result_version: pr_gate_result_v1 final: GO|NO-GO tier: ${TIER} - mode: parallel + mode: ${MODE_RESOLVED} most_severe: approve|advise|block-soft|block reviewers: critic: approve|advise|block-soft|skipped diff --git a/skills/pr-gate-review/SKILL.md b/skills/pr-gate-review/SKILL.md index feb20d82..6514fa66 100644 --- a/skills/pr-gate-review/SKILL.md +++ b/skills/pr-gate-review/SKILL.md @@ -18,16 +18,16 @@ implement → pr-gate → fix NO-GO → push → PR). - Slash command: `/pr-gate` (see `commands/pr-gate.md`). It dispatches the reviewers and writes a typed result to `.gate-results/`. -- Direct: `bash runtime/bin/pr-gate.sh --cd --executor auto [--parallel]`. +- Direct: `bash runtime/bin/pr-gate.sh --cd --executor auto [--mode sequential|parallel]`. - Reasoning effort defaults to `medium` (`--effort low|medium|high`, independent of `--model`/`--executor`). Only reach for `--effort high` when you need deeper analysis — e.g. a hard-to-diagnose finding, or escalating after repeated NO-GO rounds on the same issue. **Tier / mode** (tiers reflect rigor level, not reviewer count): - `express` — hotfix, docs-only, `architecture_impact: none`; machine verify + critic + qa. - `standard` — feature, `architecture_impact: minor`; adds architecture-reviewer with conceptual map. -- `full` — architectural change, `architecture_impact: major`, sensitive path; parallel cross-context + security + risk hard gates. -- Default = **sequential**, low token cost. `--parallel` gives each reviewer an independent session. +- `full` — architectural change, `architecture_impact: major`, sensitive path; defaults to all reviewer dimensions. +- Default = **sequential**, low token cost. `--mode parallel` gives each reviewer an independent session; `--parallel` remains a compatibility spelling. - Pass `--brief ` to get an advisory tier suggestion based on the brief's `architecture_impact`. -- Force a tier with `--tier express|standard|full`; re-gate a subset with `--targeted r1,r2`. +- Force a tier with `--tier express|standard|full`; re-gate a remediation subset with `--targeted r1,r2 --initial-result `. ## Reading the result diff --git a/tests/bin/run-tests.sh b/tests/bin/run-tests.sh index aa4eca3b..5f4fc7c4 100755 --- a/tests/bin/run-tests.sh +++ b/tests/bin/run-tests.sh @@ -215,6 +215,8 @@ map_path() { add_suite test-pmctl-task; behavioral=1 ;; core/schema/preflight-evidence.schema.json) add_suite test-pr-gate; behavioral=1 ;; + core/policy/gate-tiers.tsv|core/policy/gate-modes.tsv|core/policy/gate-pass-kinds.tsv) + add_suite test-pr-gate; add_suite test-pr-gate-profile; behavioral=1 ;; runtime/lib/pmctl-config.sh) add_suite test-pmctl-dispatch; add_suite test-pmctl-memory; add_suite test-pmctl-context; behavioral=1 ;; runtime/lib/memory.sh|runtime/lib/memory-dir.sh) diff --git a/tests/shell/test-commands.sh b/tests/shell/test-commands.sh index 23a7afe6..b372cfdf 100755 --- a/tests/shell/test-commands.sh +++ b/tests/shell/test-commands.sh @@ -477,6 +477,59 @@ STUB } pr_gate_fence_executor_model_behavior +pr_gate_fence_assurance_behavior() { + local name="pr-gate: assurance argument parser behavior" + should_run "$name" || return 0 + local case_dir="$tmp_root/pr-gate-fence-assurance" + local snippet="$case_dir/snippet.sh" bin_dir="$case_dir/bin" + local args_log="$case_dir/args.log" out="$case_dir/out" err="$case_dir/err" + local valid_status missing_initial_status initial_only_status conflict_status invalid_mode_status + mkdir -p "$bin_dir" + extract_pr_gate_bash_fence "$snippet" + cat > "$bin_dir/pmctl" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$@" > "${PR_GATE_ARGS_LOG:?}" +STUB + chmod +x "$bin_dir/pmctl" + + ARGUMENTS="--executor claude --targeted critic --initial-result /tmp/initial.md --mode parallel" \ + PR_GATE_ARGS_LOG="$args_log" PATH="$bin_dir:$PATH" bash "$snippet" > "$out" 2> "$err" + valid_status=$? + if [[ "$valid_status" -ne 0 ]] || + ! grep -qx -- '--targeted' "$args_log" || + ! grep -qx 'critic' "$args_log" || + ! grep -qx -- '--initial-result' "$args_log" || + ! grep -qx '/tmp/initial.md' "$args_log" || + ! grep -qx -- '--mode' "$args_log" || + ! grep -qx 'parallel' "$args_log" || + grep -qx -- '--reviewers' "$args_log"; then + fail "$name" "valid targeted invocation was not forwarded canonically" + return + fi + + rm -f "$args_log" + ARGUMENTS="--executor claude --targeted critic" \ + PR_GATE_ARGS_LOG="$args_log" PATH="$bin_dir:$PATH" bash "$snippet" > "$out" 2> "$err" + missing_initial_status=$? + ARGUMENTS="--executor claude --initial-result /tmp/initial.md" \ + PR_GATE_ARGS_LOG="$args_log" PATH="$bin_dir:$PATH" bash "$snippet" > "$out" 2> "$err" + initial_only_status=$? + ARGUMENTS="--executor claude --mode parallel --sequential" \ + PR_GATE_ARGS_LOG="$args_log" PATH="$bin_dir:$PATH" bash "$snippet" > "$out" 2> "$err" + conflict_status=$? + ARGUMENTS="--executor claude --mode default" \ + PR_GATE_ARGS_LOG="$args_log" PATH="$bin_dir:$PATH" bash "$snippet" > "$out" 2> "$err" + invalid_mode_status=$? + + if [[ "$missing_initial_status" -eq 2 && "$initial_only_status" -eq 2 && + "$conflict_status" -eq 2 && "$invalid_mode_status" -eq 2 ]]; then + pass "$name" + else + fail "$name" "statuses missing_initial=$missing_initial_status initial_only=$initial_only_status conflict=$conflict_status invalid_mode=$invalid_mode_status" + fi +} +pr_gate_fence_assurance_behavior + assert_frontmatter "ship: frontmatter valid" "$SHIP" should_run "ship: scoped to a single named ticket per invocation" && assert_file_contains "ship: scoped to a single named ticket per invocation" "$SHIP" "one ticket per invocation" && pass "ship: scoped to a single named ticket per invocation" should_run "ship: does not batch-scan BACKLOG for candidates" && assert_file_contains "ship: does not batch-scan BACKLOG for candidates" "$SHIP" "Do not scan" && pass "ship: does not batch-scan BACKLOG for candidates" @@ -536,7 +589,7 @@ fi should_run "ship: explains why detached+wait is unnecessary here" && assert_file_contains "ship: explains why detached+wait is unnecessary here" "$SHIP" "nothing else for the main thread to do while it waits" && pass "ship: explains why detached+wait is unnecessary here" should_run "ship: reads Final GO/NO-GO verdict" && assert_file_contains "ship: reads Final GO/NO-GO verdict" "$SHIP" "Final:" && pass "ship: reads Final GO/NO-GO verdict" should_run "ship: NO-GO fixes every finding not only blocking ones" && assert_file_contains "ship: NO-GO fixes every finding not only blocking ones" "$SHIP" "the blocking ones" && pass "ship: NO-GO fixes every finding not only blocking ones" -should_run "ship: re-runs gate with --reviewers targeting" && assert_file_contains "ship: re-runs gate with --reviewers targeting" "$SHIP" "--reviewers " && pass "ship: re-runs gate with --reviewers targeting" +should_run "ship: re-runs gate with explicit targeted initial reference" && assert_file_contains "ship: re-runs gate with explicit targeted initial reference" "$SHIP" "--targeted " && assert_file_contains "ship: re-runs gate with explicit targeted initial reference" "$SHIP" "--initial-result" && pass "ship: re-runs gate with explicit targeted initial reference" should_run "ship: references project-pm Rules A/B synthesis" && assert_file_contains "ship: references project-pm Rules A/B synthesis" "$SHIP" "Rules A/B" && pass "ship: references project-pm Rules A/B synthesis" should_run "project-pm: classifies refactor/reuse recheck after gate fixes" && assert_file_contains "project-pm: classifies refactor/reuse recheck after gate fixes" "$PROJECT_PM" "refactor_reuse_recheck: required" && assert_file_contains "project-pm: classifies refactor/reuse recheck after gate fixes" "$PROJECT_PM" "refactor_reuse_recheck: skip" && pass "project-pm: classifies refactor/reuse recheck after gate fixes" # exactly two stop conditions, no more diff --git a/tests/shell/test-core-schemas.sh b/tests/shell/test-core-schemas.sh index 01ffd81a..0c6be042 100755 --- a/tests/shell/test-core-schemas.sh +++ b/tests/shell/test-core-schemas.sh @@ -3,13 +3,13 @@ # # These tests validate that: # 1. Every JSON Schema file under core/schema/ is valid JSON -# 2. Every YAML file under core/policy/ and core/state/ is valid YAML +# 2. Every YAML/TSV file under core/policy/ and core/state/ has valid structure # 3. Schemas declaring schema_version do so as a positive integer `const` # 4. Enum values referenced inline in schemas stay in sync with the -# corresponding policy YAML files (the documented editing source). +# corresponding declarative policy files (the documented editing source). # # Per docs/spikes/CC-229-substrate-synthesis.md §E Q1: JSON Schema is the -# ajv-compliant source-of-truth; policy YAML is the human editing surface. +# ajv-compliant source-of-truth; policy YAML/TSV is the human editing surface. # This test enforces the sync that comment-level documentation requests. # # Runs via: tests/shell/test-core-schemas.sh @@ -60,6 +60,29 @@ _schema_enum() { jq -r "$path | .[]" "$file" 2>/dev/null || true } +_tsv_column() { + # Extract a named column from a headered TSV, excluding comments/blank lines. + local file="$1" column="$2" + awk -F '\t' -v wanted="$column" ' + /^[[:space:]]*#/ || /^[[:space:]]*$/ { next } + !header_seen { + header_seen=1 + width=NF + for (i=1; i<=NF; i++) { + sub(/\r$/, "", $i) + if ($i == wanted) column_index=i + } + if (!column_index) exit 2 + next + } + { + sub(/\r$/, "", $NF) + if (NF != width || $(column_index) == "") exit 2 + print $(column_index) + } + ' "$file" +} + # ---------- tests ---------- case_schema_parse() { @@ -103,6 +126,35 @@ case_yaml_parse() { pass "$name" } +case_tsv_parse() { + # Verifies that a policy TSV has a header, at least one row, consistent + # tab-separated width, and no empty cells. + local file="$1" + local name="TSV: $file has valid structure" + should_run "$name" || return 0 + if awk -F '\t' ' + /^[[:space:]]*#/ || /^[[:space:]]*$/ { next } + !header_seen { + header_seen=1 + width=NF + for (i=1; i<=NF; i++) if ($i == "") exit 2 + next + } + { + rows++ + if (NF != width) exit 2 + for (i=1; i<=NF; i++) if ($i == "") exit 2 + } + END { + if (!header_seen || width < 2 || rows < 1) exit 2 + } + ' "$file"; then + pass "$name" + else + fail "$name" "invalid header or row structure" + fi +} + case_schema_version_const() { # Verifies schema_version is a positive integer const OR an enum of positive integers. # context-pack uses enum [1,2] for v1/v2 compat; all others use const. @@ -153,6 +205,28 @@ case_enum_sync() { fi } +case_review_tier_policy_compatibility() { + # Verify current Review tiers match the canonical TSV while schema_version 1 + # retains only `targeted` as its documented legacy compatibility value. + local schema_file="$CORE_DIR/schema/review.schema.json" + local tsv_file="$CORE_DIR/policy/gate-tiers.tsv" + local name="enum-sync: review tiers == gate-tiers.tsv plus legacy targeted" + should_run "$name" || return 0 + + local current_schema_vals tsv_vals legacy_count + current_schema_vals=$(_schema_enum "$schema_file" '.properties.tier.enum' \ + | grep -vx 'targeted' | sort) + tsv_vals=$(_tsv_column "$tsv_file" "tier" | sort) + legacy_count=$(_schema_enum "$schema_file" '.properties.tier.enum' \ + | grep -cx 'targeted' || true) + + if [[ "$current_schema_vals" == "$tsv_vals" && "$legacy_count" -eq 1 ]]; then + pass "$name" + else + fail "$name" "current tiers differ from TSV or legacy targeted is not unique" + fi +} + # ---------- run ---------- # 1. JSON Schema parse @@ -165,6 +239,11 @@ for f in "$CORE_DIR"/policy/*.yaml "$CORE_DIR"/state/layout.yaml; do case_yaml_parse "$f" done +# 2b. TSV parse +for f in "$CORE_DIR"/policy/*.tsv; do + case_tsv_parse "$f" +done + # 3. schema_version: const 1 on every payload schema that declares it for f in "$CORE_DIR"/schema/*.schema.json; do # Skip schemas that intentionally omit schema_version (none in M1) @@ -222,6 +301,10 @@ case_enum_sync "$CORE_DIR/schema/review.schema.json" \ "$CORE_DIR/policy/reviewer-policy.yaml" \ "verdicts" +# Gate rigor tiers: new producers use the TSV; Review schema v1 still accepts +# its historical targeted value so old state remains structurally valid. +case_review_tier_policy_compatibility + case_enum_sync_crlf_input() { # A YAML file checked out with CRLF endings (Windows core.autocrlf re-applying # despite .gitattributes, or a non-git copy) must not produce a false enum-sync diff --git a/tests/shell/test-layer-boundaries.sh b/tests/shell/test-layer-boundaries.sh index a3a7fe35..897e4baf 100755 --- a/tests/shell/test-layer-boundaries.sh +++ b/tests/shell/test-layer-boundaries.sh @@ -15,7 +15,7 @@ # # Boundary the rules encode (calibrated against the real tree, CC-233): # - `core/` is definitions only: no shell/executables, no CLI-product-named -# files/dirs, only .yaml/.json/.md, no CLI product name as a field-name KEY. +# files/dirs, only .yaml/.json/.md/.tsv, no CLI product name as a field-name KEY. # (CLI names MAY appear as enum VALUES / schema descriptions / prose — those # are data, not structure; the rules check structure, ignore prose.) # - `adapters/**/*.sh` must NOT call the shared FLOW (brief-validate, guard, @@ -55,12 +55,13 @@ check_core_no_cli_named_paths() { -o -iname '*antigravity*' -o -iname '*opencode*' \) 2>/dev/null } -# C3: core/ holds only declarative file types (.yaml/.yml/.json/.md). +# C3: core/ holds only declarative file types (.yaml/.yml/.json/.md/.tsv). check_core_only_declarative() { local root="$1" [[ -d "$root/core" ]] || return 0 find "$root/core" -type f \ - ! \( -name '*.yaml' -o -name '*.yml' -o -name '*.json' -o -name '*.md' \) 2>/dev/null + ! \( -name '*.yaml' -o -name '*.yml' -o -name '*.json' -o -name '*.md' \ + -o -name '*.tsv' \) 2>/dev/null } # C4: no CLI product name appears as a field-name KEY (YAML/JSON) in core/. diff --git a/tests/shell/test-pr-gate.sh b/tests/shell/test-pr-gate.sh index 7bf37a9a..811e0cc8 100755 --- a/tests/shell/test-pr-gate.sh +++ b/tests/shell/test-pr-gate.sh @@ -68,6 +68,9 @@ create_runner() { cp -R "$REPO_ROOT/runtime/lib/." "$dir/lib/" mkdir -p "$dir/core/policy" cp "$REPO_ROOT/core/policy/isolation-level.yaml" "$dir/core/policy/isolation-level.yaml" + cp "$REPO_ROOT/core/policy/gate-tiers.tsv" "$dir/core/policy/gate-tiers.tsv" + cp "$REPO_ROOT/core/policy/gate-modes.tsv" "$dir/core/policy/gate-modes.tsv" + cp "$REPO_ROOT/core/policy/gate-pass-kinds.tsv" "$dir/core/policy/gate-pass-kinds.tsv" mkdir -p "$dir/adapters/codex" cat > "$dir/adapters/codex/dispatch.sh" <<'STUB_EOF' #!/usr/bin/env bash @@ -503,6 +506,135 @@ run_gate() { return "$code" } +write_valid_initial_gate_result() { + local path="$1" final="${2:-NO-GO}" + cat > "$path" << INITIAL_GATE_EOF +--- +gate_result_version: pr_gate_result_v1 +final: ${final} +tier: standard +mode: sequential +most_severe: block +reviewers: + critic: block +escalation: + recommended: false + reviewers: [] + reason: [] +--- + +# Initial gate result + +## Gate Conclusion +Final: ${final} +INITIAL_GATE_EOF +} + +# Behavior: the bounded copy-mode policy snapshot is byte-for-byte equivalent +# to all three canonical gate policy TSV sources. +# Steps: extract each generated heredoc from pr-gate.sh, compare it with the +# matching core/policy file, and fail on any drift. +test_gate_assurance_policy_snapshot_matches_sources() { + local name="gate-assurance-policy-snapshot-matches-sources" + should_run "$name" || return 0 + local table delimiter source snapshot + for table in tiers modes pass-kinds; do + case "$table" in + tiers) + delimiter="GATE_ASSURANCE_TIERS_TSV" + source="$REPO_ROOT/core/policy/gate-tiers.tsv" + ;; + modes) + delimiter="GATE_ASSURANCE_MODES_TSV" + source="$REPO_ROOT/core/policy/gate-modes.tsv" + ;; + pass-kinds) + delimiter="GATE_ASSURANCE_PASS_KINDS_TSV" + source="$REPO_ROOT/core/policy/gate-pass-kinds.tsv" + ;; + esac + snapshot="$(awk -v marker="$delimiter" ' + index($0, "cat <<\047" marker "\047") { inside=1; next } + inside && $0 == marker { exit } + inside { print } + ' "$REPO_ROOT/runtime/bin/pr-gate.sh")" + if [[ "$snapshot" != "$(cat "$source")" ]]; then + fail "$name" "generated snapshot drifted from $source" + return + fi + done + pass "$name" +} + +# Behavior: a repo-layout/copy-bundle policy source controls tier-default +# reviewer selection instead of the generated fallback or a hardcoded case. +# Steps: change only the copied express default to critic, run a docs gate, and +# assert the captured brief selects critic while retaining express tier. +test_gate_tier_policy_source_controls_default_coverage() { + local name="gate-tier-policy-source-controls-default-coverage" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" brief="$dir/brief.md" rewritten="$dir/gate-tiers.tsv" + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester architecture-reviewer security-reviewer risk-reviewer + create_repo "$repo" docs + awk -F '\t' -v OFS='\t' ' + $1 == "express" { $2="critic" } + { print } + ' "$runner/core/policy/gate-tiers.tsv" > "$rewritten" + mv "$rewritten" "$runner/core/policy/gate-tiers.tsv" + + set +e + CODEX_GATE_CAPTURE_BRIEF="$brief" run_gate "$home" "$runner" "$repo" "$out" "$err" --base main + local code=$? + set -e + if [[ "$code" -ne 0 ]]; then + fail "$name" "exit $code, expected 0" + return + fi + assert_file_contains "$name" "$brief" "Tier: express" || return + assert_file_contains "$name" "$brief" "coverage.selected: critic" || return + assert_file_contains "$name" "$brief" "Reviewers: critic" || return + assert_not_contains "$name" "$brief" "Process each reviewer IN ORDER: critic,qa-tester" || return + pass "$name" +} + +# Behavior: a copied gate without canonical policy files resolves the same +# defaults from its bounded generated snapshot and reports the degraded source. +# Steps: remove the copied TSV files, run a docs gate, and assert express / +# sequential / initial defaults plus generated-snapshot provenance. +test_gate_assurance_policy_snapshot_is_copy_mode_fallback() { + local name="gate-assurance-policy-snapshot-is-copy-mode-fallback" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" brief="$dir/brief.md" + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester architecture-reviewer security-reviewer risk-reviewer + create_repo "$repo" docs + rm -f \ + "$runner/core/policy/gate-tiers.tsv" \ + "$runner/core/policy/gate-modes.tsv" \ + "$runner/core/policy/gate-pass-kinds.tsv" + + set +e + CODEX_GATE_CAPTURE_BRIEF="$brief" run_gate "$home" "$runner" "$repo" "$out" "$err" --base main + local code=$? + set -e + if [[ "$code" -ne 0 ]]; then + fail "$name" "exit $code, expected 0" + return + fi + assert_file_contains "$name" "$brief" "tier.resolved: express" || return + assert_file_contains "$name" "$brief" "mode.resolved: sequential" || return + assert_file_contains "$name" "$brief" "pass.resolved: initial" || return + assert_file_contains "$name" "$brief" "policy.source: generated-snapshot" || return + pass "$name" +} + # Behavior: express-tier diff with no overrides routes to codex with the # express reviewer set (critic, qa-tester). # Steps: run the gate on a docs-only diff, assert stderr shows dispatch @@ -530,6 +662,15 @@ test_tier_detection() { # Sequential dispatch chatter now lands on stderr, not stdout. assert_file_contains "$name" "$err" "DISPATCH_STUB:success" || return assert_file_contains "$name" "$brief" "Tier: express" || return + assert_file_contains "$name" "$brief" "tier.requested: auto" || return + assert_file_contains "$name" "$brief" "tier.resolved: express" || return + assert_file_contains "$name" "$brief" "tier.evidence_floor: reviewer-verdicts" || return + assert_file_contains "$name" "$brief" "mode.requested: default" || return + assert_file_contains "$name" "$brief" "mode.resolved: sequential" || return + assert_file_contains "$name" "$brief" "mode.synthesis: inline" || return + assert_file_contains "$name" "$brief" "pass.resolved: initial" || return + assert_file_contains "$name" "$brief" "coverage.requested: default" || return + assert_file_contains "$name" "$brief" "coverage.selected: critic,qa-tester" || return assert_file_contains "$name" "$brief" "Executor: codex" || return assert_file_contains "$name" "$brief" "Reviewers: critic,qa-tester" || return pass "$name" @@ -955,14 +1096,13 @@ test_no_changed_files() { pass "$name" } -# Behavior: an explicit --reviewers list overrides tier auto-detection -# (Tier: targeted) and the parallel synthesis brief embeds reviewer findings -# inline rather than pointing at read: paths for reviewers not in the list. +# Behavior: an explicit --reviewers list overrides requested coverage without +# changing auto-detected tier or review pass kind. The parallel synthesis brief +# embeds only selected reviewer findings inline. # Steps: run the gate with --reviewers critic --parallel against a diff that -# would otherwise tier-detect to something else, and assert the captured -# synthesis brief has Tier: targeted, Reviewers: critic, inline -# "--- critic findings ---", and no reviewer-critic-/qa-tester read: paths. -test_reviewers_override_skips_tier_detection() { +# tier-detects to standard, and assert standard/initial coordinates, critic-only +# coverage, inline findings, and no reviewer output read paths. +test_reviewers_override_preserves_tier_detection() { local name="reviewers-override" should_run "$name" || return 0 local dir="$TMP_ROOT/$name" @@ -971,7 +1111,7 @@ test_reviewers_override_skips_tier_detection() { mkdir -p "$dir" create_runner "$runner" create_agents "$home" critic - create_repo "$repo" many + create_repo_with_branch "$repo" standard set +e CODEX_GATE_CAPTURE_BRIEF="$brief" run_gate "$home" "$runner" "$repo" "$out" "$err" --base main --reviewers critic --parallel @@ -982,7 +1122,12 @@ test_reviewers_override_skips_tier_detection() { return fi # Parallel mode: CAPTURE_BRIEF receives the synthesis brief (last dispatch) - assert_file_contains "$name" "$brief" "Tier: targeted" || return + assert_file_contains "$name" "$brief" "Tier: standard" || return + assert_file_contains "$name" "$brief" "tier.requested: auto" || return + assert_file_contains "$name" "$brief" "tier.resolved: standard" || return + assert_file_contains "$name" "$brief" "pass.resolved: initial" || return + assert_file_contains "$name" "$brief" "coverage.requested: critic" || return + assert_file_contains "$name" "$brief" "coverage.selected: critic" || return assert_file_contains "$name" "$brief" "Executor: codex" || return assert_file_contains "$name" "$brief" "Reviewers: critic" || return # Synthesis brief embeds reviewer findings inline — no read: paths to reviewer output files @@ -992,6 +1137,39 @@ test_reviewers_override_skips_tier_detection() { pass "$name" } +# Behavior: explicit full-tier intent and critic-only requested coverage remain +# independent; neither value rewrites the other. +# Steps: run a docs diff with --tier full --reviewers critic, then assert the +# combined brief records full requested/resolved tier and critic-only coverage. +test_full_tier_with_critic_only_coverage_is_truthful() { + local name="full-tier-with-critic-only-coverage-is-truthful" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" brief="$dir/brief.md" + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic + create_repo "$repo" docs + + set +e + CODEX_GATE_CAPTURE_BRIEF="$brief" run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --tier full --reviewers critic --mode sequential + local code=$? + set -e + if [[ "$code" -ne 0 ]]; then + fail "$name" "exit $code, expected 0" + return + fi + assert_file_contains "$name" "$brief" "tier.requested: full" || return + assert_file_contains "$name" "$brief" "tier.resolved: full" || return + assert_file_contains "$name" "$brief" "mode.resolved: sequential" || return + assert_file_contains "$name" "$brief" "coverage.requested: critic" || return + assert_file_contains "$name" "$brief" "coverage.selected: critic" || return + assert_file_contains "$name" "$brief" "Reviewers: critic" || return + pass "$name" +} + # Behavior: the brief handed to the executor exists on disk at the moment of # invocation, and any /tmp brief carries the guard-required `brief-gate-` # prefix. On the pmctl dispatch transport that brief is the /tmp/brief-gate-* @@ -2331,15 +2509,16 @@ test_preflight_runs_even_when_qa_tester_not_targeted() { should_run "$name" || return 0 local dir="$TMP_ROOT/$name" local home="$dir/home" repo="$dir/repo" runner="$dir/runner" - local out="$dir/out" err="$dir/err" result="$dir/result.md" + local out="$dir/out" err="$dir/err" result="$dir/result.md" initial="$dir/initial.md" mkdir -p "$dir" create_runner "$runner" create_agents "$home" critic qa-tester architecture-reviewer security-reviewer risk-reviewer create_repo "$repo" docs + write_valid_initial_gate_result "$initial" set +e run_gate "$home" "$runner" "$repo" "$out" "$err" --base main \ - --targeted critic --test-cmd "exit 0" --output "$result" + --targeted critic --initial-result "$initial" --test-cmd "exit 0" --output "$result" local code=$? set -e if [[ "$code" -ne 0 ]]; then @@ -3446,6 +3625,9 @@ test_untracked_binary_routes_to_standard() { pass "$name" } +run_test test_gate_assurance_policy_snapshot_matches_sources +run_test test_gate_tier_policy_source_controls_default_coverage +run_test test_gate_assurance_policy_snapshot_is_copy_mode_fallback run_test test_tier_detection run_test test_pr_gate_does_not_mutate_gitignore run_test test_artifact_filter_drops_gate_artifacts @@ -3458,7 +3640,8 @@ run_test test_copy_mode_artifact_fallback_body_parity run_test test_missing_reviewer_agent run_test test_invalid_base_ref run_test test_no_changed_files -run_test test_reviewers_override_skips_tier_detection +run_test test_reviewers_override_preserves_tier_detection +run_test test_full_tier_with_critic_only_coverage_is_truthful run_test test_brief_file_snapshot_exists_at_dispatch run_test test_reviewer_definitions_are_workspace_snapshots run_test test_brief_cleanup_on_dispatch_failure @@ -4085,12 +4268,41 @@ test_copy_mode_dispatches_via_adapter() { pass "$name" } +# Behavior: --help prints only the bounded user-facing usage contract and +# includes the canonical assurance flags. +# Steps: invoke a copied gate with --help and assert current flags are present +# while shell implementation and generated policy internals are absent. +test_help_output_is_bounded_and_current() { + local name="help-output-is-bounded-and-current" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local runner="$dir/runner" out="$dir/out" err="$dir/err" + mkdir -p "$dir" + create_runner "$runner" + + set +e + "$runner/pr-gate.sh" --help > "$out" 2> "$err" + local code=$? + set -e + if [[ "$code" -ne 0 ]]; then + fail "$name" "exit $code, expected 0" + return + fi + assert_file_contains "$name" "$out" "Usage:" || return + assert_file_contains "$name" "$out" "--mode " || return + assert_file_contains "$name" "$out" "--targeted " || return + assert_file_contains "$name" "$out" "--initial-result " || return + assert_not_contains "$name" "$out" "_gate_assurance_policy_snapshot" || return + assert_not_contains "$name" "$out" "set -euo pipefail" || return + pass "$name" +} + # Behavior: an unrecognized flag exits 2 and prints an actionable # accepted-flags list (not just a bare "Unknown arg"), so callers # self-correct on first failure. # Steps: run the gate with --bogus-flag, and assert exit 2 and stderr -# contains "Unknown arg: --bogus-flag", "Accepted:", and -# "--reviewers|--targeted". +# contains "Unknown arg: --bogus-flag", "Accepted:", "--targeted", and +# "--initial-result". test_unknown_arg_message() { local name="unknown-arg-message" should_run "$name" || return 0 @@ -4112,30 +4324,31 @@ test_unknown_arg_message() { fi assert_file_contains "$name" "$err" "Unknown arg: --bogus-flag" || return assert_file_contains "$name" "$err" "Accepted:" || return - assert_file_contains "$name" "$err" "--reviewers|--targeted" || return + assert_file_contains "$name" "$err" "--targeted" || return + assert_file_contains "$name" "$err" "--initial-result" || return pass "$name" } -# Behavior: --targeted is accepted as an alias of --reviewers (the -# /pr-gate skill and the script's own comments use "targeted" -# vocabulary). Scoping a parallel gate to critic must launch critic only -- -# same as --reviewers critic. -# Steps: run the gate with --targeted critic --parallel, and assert exit -# 0, no "unknown arg" in stderr, "launched critic" in stdout, and no -# "launched qa-tester" line. -test_targeted_alias() { - local name="targeted-alias" +# Behavior: --targeted selects a remediation-delta pass, preserves detected +# tier, accepts canonical --mode, and carries a structurally valid initial +# result reference into the resolved assurance context. +# Steps: create a valid initial result, run a critic-only targeted gate with +# --mode parallel, and assert the pass/mode/tier/reference/coverage coordinates. +test_targeted_pass_references_initial_result() { + local name="targeted-pass-references-initial-result" should_run "$name" || return 0 local dir="$TMP_ROOT/$name" local home="$dir/home" repo="$dir/repo" runner="$dir/runner" - local out="$dir/out" err="$dir/err" + local out="$dir/out" err="$dir/err" brief="$dir/brief.md" initial="$dir/initial.md" mkdir -p "$dir" create_runner "$runner" create_agents "$home" critic qa-tester architecture-reviewer security-reviewer risk-reviewer create_repo "$repo" docs + write_valid_initial_gate_result "$initial" set +e - run_gate "$home" "$runner" "$repo" "$out" "$err" --base main --targeted critic --parallel + CODEX_GATE_CAPTURE_BRIEF="$brief" run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --targeted critic --initial-result "$initial" --mode parallel local code=$? set -e @@ -4143,10 +4356,15 @@ test_targeted_alias() { fail "$name" "exit $code, expected 0 (stderr: $(head -3 "$err" 2>/dev/null))" return fi - if grep -qi "unknown arg" "$err"; then - fail "$name" "--targeted was rejected as an unknown arg" - return - fi + assert_file_contains "$name" "$brief" "tier.resolved: express" || return + assert_file_contains "$name" "$brief" "mode.requested: parallel" || return + assert_file_contains "$name" "$brief" "mode.resolved: parallel" || return + assert_file_contains "$name" "$brief" "mode.topology: per-reviewer-sessions" || return + assert_file_contains "$name" "$brief" "mode.synthesis: separate-session" || return + assert_file_contains "$name" "$brief" "pass.resolved: targeted" || return + assert_file_contains "$name" "$brief" "pass.scope: remediation-delta" || return + assert_file_contains "$name" "$brief" "pass.initial_result: $initial" || return + assert_file_contains "$name" "$brief" "coverage.selected: critic" || return assert_file_contains "$name" "$out" "launched critic" || return if grep -q "launched qa-tester" "$out"; then fail "$name" "--targeted critic did not scope reviewers — qa-tester was launched" @@ -4155,6 +4373,165 @@ test_targeted_alias() { pass "$name" } +# Behavior: a targeted pass without an initial result fails before dispatch. +# Steps: invoke --targeted critic without --initial-result and assert exit 2, +# the explicit requirement error, and no dispatch marker. +test_targeted_requires_initial_result() { + local name="targeted-requires-initial-result" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" + mkdir -p "$dir" + create_runner "$runner" + create_repo "$repo" docs + + set +e + run_gate "$home" "$runner" "$repo" "$out" "$err" --base main --targeted critic + local code=$? + set -e + if [[ "$code" -ne 2 ]]; then + fail "$name" "exit $code, expected 2" + return + fi + assert_file_contains "$name" "$err" "--targeted requires --initial-result " || return + assert_not_contains "$name" "$out" "DISPATCH_STUB" || return + pass "$name" +} + +# Behavior: a targeted gate cannot reuse its referenced initial result as the +# output destination, including a lexical alias of the same path. +# Steps: pass the initial result back through --output using a ./ alias, assert +# exit 2 before dispatch, and confirm the initial artifact is unchanged. +test_targeted_output_cannot_overwrite_initial_result() { + local name="targeted-output-cannot-overwrite-initial-result" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" initial="$dir/initial.md" before + mkdir -p "$dir" + create_runner "$runner" + create_repo "$repo" docs + write_valid_initial_gate_result "$initial" + before="$(cat "$initial")" + + set +e + run_gate "$home" "$runner" "$repo" "$out" "$err" --base main \ + --targeted critic --initial-result "$initial" --output "$dir/./initial.md" + local code=$? + set -e + if [[ "$code" -ne 2 ]]; then + fail "$name" "exit $code, expected 2" + return + fi + assert_file_contains "$name" "$err" "--output must not overwrite" || return + assert_not_contains "$name" "$out" "DISPATCH_STUB" || return + if [[ "$(cat "$initial")" != "$before" ]]; then + fail "$name" "initial result content changed" + return + fi + pass "$name" +} + +# Behavior: canonical and compatibility mode spellings fail closed when they +# request different topologies. +# Steps: combine --mode parallel with --sequential and assert a controlled +# exit-2 conflict before dispatch. +test_conflicting_mode_options_are_rejected() { + local name="conflicting-mode-options-are-rejected" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" + mkdir -p "$dir" + create_runner "$runner" + create_repo "$repo" docs + + set +e + run_gate "$home" "$runner" "$repo" "$out" "$err" --base main --mode parallel --sequential + local code=$? + set -e + if [[ "$code" -ne 2 ]]; then + fail "$name" "exit $code, expected 2" + return + fi + assert_file_contains "$name" "$err" "conflicting gate mode options" || return + assert_not_contains "$name" "$out" "DISPATCH_STUB" || return + pass "$name" +} + +# Behavior: canonical and compatibility mode spellings may be combined when +# they request the same topology. +# Steps: run --mode parallel with --parallel, capture the synthesis brief, and +# assert one successful parallel resolution. +test_equivalent_mode_spellings_are_accepted() { + local name="equivalent-mode-spellings-are-accepted" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" brief="$dir/brief.md" + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic + create_repo "$repo" docs + + set +e + CODEX_GATE_CAPTURE_BRIEF="$brief" run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic --mode parallel --parallel + local code=$? + set -e + if [[ "$code" -ne 0 ]]; then + fail "$name" "exit $code, expected 0" + return + fi + assert_file_contains "$name" "$brief" "mode.requested: parallel" || return + assert_file_contains "$name" "$brief" "mode.resolved: parallel" || return + assert_file_contains "$name" "$out" "launched critic" || return + pass "$name" +} + +# Behavior: invalid tier, mode, reviewer, and mixed coverage/pass selectors are +# rejected as closed CLI inputs instead of falling back to another profile. +# Steps: run a malformed-input matrix and require exit 2 plus no dispatch for +# every case. +test_invalid_assurance_inputs_are_rejected() { + local name="invalid-assurance-inputs-are-rejected" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" + local home="$dir/home" repo="$dir/repo" runner="$dir/runner" + local out="$dir/out" err="$dir/err" code args + mkdir -p "$dir" + create_runner "$runner" + create_repo "$repo" docs + + for args in \ + "--tier targeted" \ + "--mode default" \ + "--reviewers unknown-reviewer" \ + "--reviewers critic,critic" \ + "--reviewers critic,,qa-tester" \ + "--initial-result missing.md" \ + "--reviewers critic --targeted critic" + do + : > "$out" + : > "$err" + set +e + # shellcheck disable=SC2086 # fixture intentionally expands a small argv matrix + run_gate "$home" "$runner" "$repo" "$out" "$err" --base main $args + code=$? + set -e + if [[ "$code" -ne 2 ]]; then + fail "$name" "args '$args' exited $code, expected 2" + return + fi + if grep -q "DISPATCH_STUB" "$out" "$err"; then + fail "$name" "args '$args' reached dispatch" + return + fi + done + pass "$name" +} + # Behavior: the sequential brief emitted by pr-gate.sh uses ASCII -- # separators and contains no em dash (U+2014) bytes. # Steps: run the gate with --sequential, and assert the captured brief @@ -4763,8 +5140,14 @@ run_test test_isolation_forwarding_through_pr_gate run_test test_effort_forwarding_through_pr_gate run_test test_effort_invalid_value_rejected run_test test_copy_mode_dispatches_via_adapter +run_test test_help_output_is_bounded_and_current run_test test_unknown_arg_message -run_test test_targeted_alias +run_test test_targeted_pass_references_initial_result +run_test test_targeted_requires_initial_result +run_test test_targeted_output_cannot_overwrite_initial_result +run_test test_conflicting_mode_options_are_rejected +run_test test_equivalent_mode_spellings_are_accepted +run_test test_invalid_assurance_inputs_are_rejected run_test test_seq_brief_ascii_separator run_test test_parallel_synthesis_brief_ascii_separator run_test test_parallel_reviewer_brief_ascii_separator diff --git a/tests/shell/test-run-tests.sh b/tests/shell/test-run-tests.sh index 84a7149b..a47cb064 100755 --- a/tests/shell/test-run-tests.sh +++ b/tests/shell/test-run-tests.sh @@ -18,17 +18,20 @@ make_fixture() { local name="$1" repo repo="$TMP_ROOT/$name" mkdir -p "$repo/scripts/lib" "$repo/runtime/lib" "$repo/tests/bin" "$repo/tests/lib" \ - "$repo/core/schema" "$repo/core/state" + "$repo/core/schema" "$repo/core/state" "$repo/core/policy" cp "$REPO_ROOT/tests/bin/run-tests.sh" "$repo/tests/bin/run-tests.sh" cp "$REPO_ROOT/tests/bin/run-all-tests.sh" "$repo/tests/bin/run-all-tests.sh" cp "$SCRIPT_DIR/../lib/test-result.sh" "$repo/tests/lib/test-result.sh" cp "$REPO_ROOT/runtime/lib/artifact-paths.sh" "$repo/runtime/lib/artifact-paths.sh" cp "$REPO_ROOT/core/schema/test-result.schema.json" "$repo/core/schema/test-result.schema.json" + cp "$REPO_ROOT/core/policy/gate-tiers.tsv" "$repo/core/policy/gate-tiers.tsv" + cp "$REPO_ROOT/core/policy/gate-modes.tsv" "$repo/core/policy/gate-modes.tsv" + cp "$REPO_ROOT/core/policy/gate-pass-kinds.tsv" "$repo/core/policy/gate-pass-kinds.tsv" chmod +x "$repo/tests/bin/run-tests.sh" cat > "$repo/tests/lib/test-suite-runner.sh" <<'RUNNER' #!/usr/bin/env bash set -euo pipefail -suites=(lint-agents lint-scripts lint-script-domain-inventory lint-portable-repo-paths test-lint-shellcheck test-script-domain-inventory test-lint-portable-repo-paths test-lint-frontmatter test-commands test-check-docs-freshness test-guards test-migrate test-install test-doctor test-pmctl-dispatch test-pmctl-context test-pmctl-memory test-pr-gate test-pmctl-operation test-host-manifest test-host-write-codex test-codex-dispatch-continuation test-host-write-parity test-core-schemas test-layer-boundaries test-pm-scripts test-run-tests test-state-store test-state-layout-parity) +suites=(lint-agents lint-scripts lint-script-domain-inventory lint-portable-repo-paths test-lint-shellcheck test-script-domain-inventory test-lint-portable-repo-paths test-lint-frontmatter test-commands test-check-docs-freshness test-guards test-migrate test-install test-doctor test-pmctl-dispatch test-pmctl-context test-pmctl-memory test-pr-gate test-pr-gate-profile test-pmctl-operation test-host-manifest test-host-write-codex test-codex-dispatch-continuation test-host-write-parity test-core-schemas test-layer-boundaries test-pm-scripts test-run-tests test-state-store test-state-layout-parity) for arg in "$@"; do if [[ "$arg" == --list ]]; then printf '%s\n' "${suites[@]}"; exit 0; fi done @@ -228,6 +231,23 @@ case_evidence_contract_maps_to_runner_regression() { fi } +case_gate_assurance_policy_maps_gate_consumers() { + local name=gate-assurance-policy-maps-gate-consumers repo out status=0 args + args="$TMP_ROOT/$name.args" + repo="$(make_fixture "$name")" + out=$(RUN_TESTS_ARGS_LOG="$args" "$repo/tests/bin/run-tests.sh" \ + --path core/policy/gate-tiers.tsv --path core/policy/gate-modes.tsv \ + --path core/policy/gate-pass-kinds.tsv --list 2>&1) || status=$? + if [[ "$status" -eq 0 && "$out" == *"test-pr-gate"* && + "$out" == *"test-pr-gate-profile"* && "$out" == *"test-core-schemas"* && + "$out" == *"test-layer-boundaries"* && "$out" != *"coverage gaps"* && + ! -e "$args" ]]; then + pass "$name" + else + fail "$name" "status=$status out=$out" + fi +} + case_high_fanout_escalates_full() { local name=high-fanout-escalates-full repo out status=0 args args="$TMP_ROOT/$name.args" @@ -402,6 +422,7 @@ case_skill_mapping_uses_registered_frontmatter_suite case_guard_family_maps_to_guard_suite case_prompt_context_timeout_contract_maps_all_consumers case_evidence_contract_maps_to_runner_regression +case_gate_assurance_policy_maps_gate_consumers case_high_fanout_escalates_full case_repeated_high_fanout_escalation_succeeds case_unknown_path_fails_without_test_evidence