From d7bcbbed6ca9b4c5a1ac39383051604cbbffc406 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Tue, 1 Sep 2026 15:49:20 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix(review):=20=EB=A8=B8=EC=A7=80=20?= =?UTF-8?q?=EA=B0=90=EC=A7=80=20=EC=8B=9C=20=EB=A6=AC=EB=B7=B0=EB=A5=BC=20?= =?UTF-8?q?=EC=A6=89=EC=8B=9C=20=EC=A4=91=EB=8B=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 리뷰 도중 PR lifecycle이 MERGED로 바뀌면 추가 검사, 수정, push, GitHub 게시를 중단합니다. 두 runtime projection과 brief/output 계약에 동일한 sentinel을 연결하고 회귀 검증을 추가했습니다. --- .codex/skills/ddalggak/SKILL.md | 6 +++--- .../ddalggak/references/cross-review-loop.md | 13 +++++++++++-- .../ddalggak/references/review-output-contract.md | 2 +- .codex/skills/ddalggak/templates/review-brief.md | 6 +++++- core/commands/review.yaml | 2 +- ddalggak/SKILL.md | 6 ++++-- ddalggak/references/cross-review-loop.md | 13 +++++++++++-- ddalggak/references/review-output-contract.md | 2 +- ddalggak/templates/review-brief.md | 6 +++++- scripts/test-conditional-command-assets.mjs | 15 +++++++++++++++ 10 files changed, 57 insertions(+), 14 deletions(-) diff --git a/.codex/skills/ddalggak/SKILL.md b/.codex/skills/ddalggak/SKILL.md index f276d44..f874fc2 100644 --- a/.codex/skills/ddalggak/SKILL.md +++ b/.codex/skills/ddalggak/SKILL.md @@ -69,7 +69,7 @@ If a non-writing subcommand would need a source edit to continue, report the nee | Subcommand | Mode | Show-doc heading | Purpose | Side effects | Stop condition | Required assets | | --- | --- | --- | --- | --- | --- | --- | | `start` | source-edit | Start Workflow | Issue implementation from live issue body/comments; one issue PR by default | Repo source edits in issue scope; start publishes the issue PR via the ship procedure (ship.md); cross-review comments come through the review gate. | Stop on stale base, missing issue body/comments, duplicate PR, or required files outside the issue-owned scope. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/agent-runtime-contract.md`, `references/start-workflow.md`; templates: -; conditional: see conditional map | -| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Stop before approve if intake is not ready, current-head CI is not terminal green/skipped, semantic coverage has gaps, blockers remain, or wiki/evidence preflight has blocking gaps. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | +| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Re-check lifecycle at intake, after delegated work, and before writes. On state=MERGED or mergedAt, emit REVIEW_STOPPED_PR_MERGED and stop review/writes. Uncertain lifecycle is BLOCKED. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | | `status` | read-only | Status | Read-only live git/GitHub/session state snapshot | No source, GitHub, or local cleanup mutation; report live git/GitHub/session state only. | Stop after a live state snapshot and next-action recommendation. | refs: `references/wiki-context-preflight.md`, `references/status.md`, `references/pr-check-evidence-bundle.md`; templates: - | | `plan` | plan-only | Issue-Ready Plan | Issue-ready implementation plan from issue/wiki/code evidence | No source edits; no GitHub writes unless the user separately requests issue creation. | Stop after an issue-ready plan with evidence/unknowns and PR topology. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/wiki-bridge.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/issue-ready-plan.md`; templates: -; conditional: see conditional map | | `issue` | github-write | Plan to Issues | Create GitHub issues from an approved plan | Create/edit GitHub issues and comments only; no repository source edits. | Stop after live issue URLs/labels/assignees/body UTF-8 verification or on metadata permission failure. | refs: `references/wiki-context-preflight.md`, `references/plan-to-issues.md`; templates: `templates/issue-body.md`, `templates/epic-body.md` | @@ -173,11 +173,11 @@ Execution contract index: target repo/base freshness, issue body+comments, base ## `review` - Cross-Review Loop -Command contract: mode `review-fix`; source edits are allowed only for accepted Critical/High blockers; top-level review comments are allowed; stop before APPROVE when current-head CI/checks are not terminal, blockers remain, or evidence/wiki preflight has blocking gaps. +Command contract: mode `review-fix`; only accepted Critical/High fixes and review comments may write. Re-check lifecycle at intake, after delegated work, and before writes. On `state=MERGED` or `mergedAt`, emit `REVIEW_STOPPED_PR_MERGED` and stop; uncertainty is `BLOCKED`. Full procedure: `references/cross-review-loop.md`; public renderer: `references/review-output-contract.md` + `references/review-comment-style.md`; wiki authority: `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`; delegated review only loads `templates/review-brief.md`. -Execution contract index: live PR/diff/files/checks/issue/head SHA, Wiki Context Preflight, base Router/Evidence, activation-bound optional gates, Admission schema v3, candidate disposition, lifecycle aggregate, publication authority, canonical-candidate-bound two-sentence findings, deterministic fixed summary, zero-finding substantive validation, and a top-level conclusion comment when formal approval is inappropriate. +Execution contract index: live PR/diff/files/checks/issue/head SHA; lifecycle rechecks at intake, after delegated work, and before writes; merged hard stop with authoritative readback and default `MERGED / NO_FOLLOW_UP`; lookup ambiguity as `BLOCKED`; Wiki Context Preflight; base Router/Evidence; activation-bound optional gates; Admission schema v3; aggregate/publication authority; canonical-candidate-bound two-sentence findings; deterministic fixed summary; substantive zero-finding validation. ## `status` - Current State Snapshot diff --git a/.codex/skills/ddalggak/references/cross-review-loop.md b/.codex/skills/ddalggak/references/cross-review-loop.md index bb5cdde..45ed0fa 100644 --- a/.codex/skills/ddalggak/references/cross-review-loop.md +++ b/.codex/skills/ddalggak/references/cross-review-loop.md @@ -18,13 +18,22 @@ Load human-feedback, CI-triage, security-posture, and regression references only ## Lifecycle gate -Resolve the live lifecycle before evaluating candidates: +Resolve the live lifecycle before evaluating candidates. Re-check it at intake, after every delegated or long-running review phase, and immediately before every fix, push, formal/inline review, or top-level GitHub comment write. - OPEN: `APPROVE | CHANGES_REQUESTED | BLOCKED` - MERGED: `NO_FOLLOW_UP | FOLLOW_UP_REQUIRED | BLOCKED` - CLOSED_UNMERGED: `NO_ACTION | FOLLOW_UP_REQUIRED | BLOCKED` -Re-read current head, base, files, checks, linked requirements, issue/body comments, prior review decisions, and Wiki Context Preflight. `checksStatus: PASS | FAIL | PENDING | NOT_APPLICABLE`; `NOT_APPLICABLE` requires a non-empty `checksJustification`. FAIL or PENDING makes the aggregate BLOCKED and completion-ineligible. +`state=MERGED` or a non-null `mergedAt` is a hard stop sentinel. Immediately: + +1. Record authoritative readback for `state`, `mergedAt`, `mergeCommit`, and `headRefOid`. +2. Emit `REVIEW_STOPPED_PR_MERGED` and attempt to stop any in-flight delegated review work. +3. Stop all further probes, candidate evaluation, delegation, fixes, source edits, pushes, formal/inline reviews, and top-level GitHub comments. +4. Return `MERGED / NO_FOLLOW_UP` by default. A follow-up requires fresh live evidence of a material residual defect and separate authorization; it is not a continuation of the stopped review. + +If lifecycle lookup fails or is ambiguous, return `BLOCKED` and perform no mutation or publication. Never infer OPEN from a stale brief, worker report, or earlier readback. + +For an OPEN PR, re-read current head, base, files, checks, linked requirements, issue/body comments, prior review decisions, and Wiki Context Preflight. `checksStatus: PASS | FAIL | PENDING | NOT_APPLICABLE`; `NOT_APPLICABLE` requires a non-empty `checksJustification`. FAIL or PENDING makes the aggregate BLOCKED and completion-ineligible. ## Accepted finding authority diff --git a/.codex/skills/ddalggak/references/review-output-contract.md b/.codex/skills/ddalggak/references/review-output-contract.md index b59287d..68e76b9 100644 --- a/.codex/skills/ddalggak/references/review-output-contract.md +++ b/.codex/skills/ddalggak/references/review-output-contract.md @@ -34,7 +34,7 @@ Public output excludes internal evidence inventories. Wiki Context Manifest, gat - MERGED: `no follow-up`, `follow-up required`, `blocked` - CLOSED_UNMERGED: `no action`, `follow-up required`, `blocked` -Do not reuse `change request` for MERGED or CLOSED_UNMERGED. Merged/closed reviews default to no GitHub mutation; an authorized follow-up comment requires a reproducible or deterministic residual defect with material impact. +Do not reuse `change request` for MERGED or CLOSED_UNMERGED. Immediately before rendering or publishing, re-read lifecycle. If `state=MERGED` or `mergedAt` is present, emit `REVIEW_STOPPED_PR_MERGED`, stop rendering/publication, and perform no GitHub mutation; report `MERGED / NO_FOLLOW_UP` by default. An authorized follow-up comment requires a reproducible or deterministic residual defect with material impact and is a separate action, not continuation of the stopped review. Lifecycle lookup failure or ambiguity is `BLOCKED`. ## Substantive gate requirement diff --git a/.codex/skills/ddalggak/templates/review-brief.md b/.codex/skills/ddalggak/templates/review-brief.md index 756829e..3c48d88 100644 --- a/.codex/skills/ddalggak/templates/review-brief.md +++ b/.codex/skills/ddalggak/templates/review-brief.md @@ -7,6 +7,8 @@ - checks Head SHA: - Linked issue contract / acceptance criteria: - Lifecycle: +- Lifecycle checked at / evidence: intake; after delegated or long-running work; immediately before each fix, push, or GitHub write +- Merge stop sentinel: `state=MERGED` or non-null `mergedAt` → `REVIEW_STOPPED_PR_MERGED`; record `state`, `mergedAt`, `mergeCommit`, `headRefOid`; stop delegation/probes/edits/pushes/GitHub writes - Authority boundary: - Purpose / non-goals: @@ -66,6 +68,8 @@ Worker output has no publication authority. - Internal Review Quality sensor verdict: `approve | change request | comment | blocked` - Canonical lifecycle outcome and public rendering: `references/cross-review-loop.md` plus `references/review-output-contract.md` -`REVIEW_DONE PR#: critical=N high=N medium=N low=N head=` +Open lifecycle completion: `REVIEW_DONE PR#: critical=N high=N medium=N low=N head=` + +Merged lifecycle hard stop: `REVIEW_STOPPED_PR_MERGED PR#: state=MERGED mergedAt= mergeCommit= head= outcome=NO_FOLLOW_UP` A zero-finding review is valid only with semantic coverage evidence. High/Critical zero-finding reviews additionally require the documented counterexample/challenger rule in `references/review-quality-contract.md`. diff --git a/core/commands/review.yaml b/core/commands/review.yaml index e645653..ca264a2 100644 --- a/core/commands/review.yaml +++ b/core/commands/review.yaml @@ -7,7 +7,7 @@ github_write_allowed: true purpose: "Risk-adaptive current-head semantic review and accepted fix loop." mode: "review-fix" write_side_effects: "Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch." -stop_condition: "Stop before approve if intake is not ready, current-head CI is not terminal green/skipped, semantic coverage has gaps, blockers remain, or wiki/evidence preflight has blocking gaps." +stop_condition: "Re-check lifecycle at intake, after delegated work, and before writes. On state=MERGED or mergedAt, emit REVIEW_STOPPED_PR_MERGED and stop review/writes. Uncertain lifecycle is BLOCKED." required_references: - wiki-context-preflight.md - 2026-06-04-brain-v0-wiki-authority-in-ddalggak.md diff --git a/ddalggak/SKILL.md b/ddalggak/SKILL.md index c0496d0..84c1b87 100644 --- a/ddalggak/SKILL.md +++ b/ddalggak/SKILL.md @@ -81,7 +81,7 @@ user-invocable: true | subcommand | mode | show-doc heading | 목적 | side effects | stop condition | 상세 reference rule | |---|---|---|---|---|---|---| | `start` | source-edit | Start Workflow | Issue implementation from live issue body/comments; one issue PR by default | Repo source edits in issue scope; start publishes the issue PR via the ship procedure (ship.md); cross-review comments come through the review gate. | Stop on stale base, missing issue body/comments, duplicate PR, or required files outside the issue-owned scope. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/agent-runtime-contract.md`, `references/start-workflow.md`; templates: -; conditional: see conditional map | -| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Stop before approve if intake is not ready, current-head CI is not terminal green/skipped, semantic coverage has gaps, blockers remain, or wiki/evidence preflight has blocking gaps. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | +| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Re-check lifecycle at intake, after delegated work, and before writes. On state=MERGED or mergedAt, emit REVIEW_STOPPED_PR_MERGED and stop review/writes. Uncertain lifecycle is BLOCKED. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | | `status` | read-only | Status | Read-only live git/GitHub/session state snapshot | No source, GitHub, or local cleanup mutation; report live git/GitHub/session state only. | Stop after a live state snapshot and next-action recommendation. | refs: `references/wiki-context-preflight.md`, `references/status.md`, `references/pr-check-evidence-bundle.md`; templates: - | | `plan` | plan-only | Issue-Ready Plan | Issue-ready implementation plan from issue/wiki/code evidence | No source edits; no GitHub writes unless the user separately requests issue creation. | Stop after an issue-ready plan with evidence/unknowns and PR topology. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/wiki-bridge.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/issue-ready-plan.md`; templates: -; conditional: see conditional map | | `issue` | github-write | Plan to Issues | Create GitHub issues from an approved plan | Create/edit GitHub issues and comments only; no repository source edits. | Stop after live issue URLs/labels/assignees/body UTF-8 verification or on metadata permission failure. | refs: `references/wiki-context-preflight.md`, `references/plan-to-issues.md`; templates: `templates/issue-body.md`, `templates/epic-body.md` | @@ -161,12 +161,14 @@ Execution contract index: ## Cross-Review Loop -Command contract: mode `review-fix`; source edits are allowed only for accepted Critical/High blockers; top-level review comments are allowed; stop before APPROVE when current-head CI/checks are not terminal, blockers remain, or evidence/wiki preflight has blocking gaps. +Command contract: mode `review-fix`; only accepted Critical/High fixes and review comments may write. Re-check lifecycle at intake, after delegated work, and before writes. On `state=MERGED` or `mergedAt`, emit `REVIEW_STOPPED_PR_MERGED` and stop; uncertainty is `BLOCKED`. Full procedure: `references/cross-review-loop.md`; public renderer: `references/review-output-contract.md` + `references/review-comment-style.md`; wiki authority: `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`; delegated-review만 `templates/review-brief.md`를 로드한다. Execution contract index: - Re-read live PR state, diff/files/checks, linked issue, current head SHA, and wiki-context preflight. +- Treat `state=MERGED` or non-null `mergedAt` as a hard stop sentinel: attempt to stop in-flight delegated review work, perform no further probes or candidate processing, and prohibit fixes, pushes, formal/inline reviews, and top-level comments. Record authoritative lifecycle readback (`state`, `mergedAt`, `mergeCommit`, `headRefOid`) and return `MERGED / NO_FOLLOW_UP` unless live evidence requires a separately authorized follow-up. +- Lifecycle lookup failure or ambiguity is `BLOCKED`; it never permits continued mutation or publication. - Gates: Router/Evidence는 base; 나머지는 activation evidence applies일 때만 로드한다. - Findings must separate live evidence, wiki-strengthened rationale, non-wiki inference, and retrieval gaps. - Run admission schema v3 and the finding signal gate first (`references/cross-review-loop.md`): only conductor-promoted admitted findings become inline comments in one `COMMENT`-event batch; each finding is rendered from its aggregate-member canonical candidate, filtered Low/nit notes stay internal, and the top-level body is the deterministic fixed summary. A zero-finding review with substantive validation evidence is valid. diff --git a/ddalggak/references/cross-review-loop.md b/ddalggak/references/cross-review-loop.md index bb5cdde..45ed0fa 100644 --- a/ddalggak/references/cross-review-loop.md +++ b/ddalggak/references/cross-review-loop.md @@ -18,13 +18,22 @@ Load human-feedback, CI-triage, security-posture, and regression references only ## Lifecycle gate -Resolve the live lifecycle before evaluating candidates: +Resolve the live lifecycle before evaluating candidates. Re-check it at intake, after every delegated or long-running review phase, and immediately before every fix, push, formal/inline review, or top-level GitHub comment write. - OPEN: `APPROVE | CHANGES_REQUESTED | BLOCKED` - MERGED: `NO_FOLLOW_UP | FOLLOW_UP_REQUIRED | BLOCKED` - CLOSED_UNMERGED: `NO_ACTION | FOLLOW_UP_REQUIRED | BLOCKED` -Re-read current head, base, files, checks, linked requirements, issue/body comments, prior review decisions, and Wiki Context Preflight. `checksStatus: PASS | FAIL | PENDING | NOT_APPLICABLE`; `NOT_APPLICABLE` requires a non-empty `checksJustification`. FAIL or PENDING makes the aggregate BLOCKED and completion-ineligible. +`state=MERGED` or a non-null `mergedAt` is a hard stop sentinel. Immediately: + +1. Record authoritative readback for `state`, `mergedAt`, `mergeCommit`, and `headRefOid`. +2. Emit `REVIEW_STOPPED_PR_MERGED` and attempt to stop any in-flight delegated review work. +3. Stop all further probes, candidate evaluation, delegation, fixes, source edits, pushes, formal/inline reviews, and top-level GitHub comments. +4. Return `MERGED / NO_FOLLOW_UP` by default. A follow-up requires fresh live evidence of a material residual defect and separate authorization; it is not a continuation of the stopped review. + +If lifecycle lookup fails or is ambiguous, return `BLOCKED` and perform no mutation or publication. Never infer OPEN from a stale brief, worker report, or earlier readback. + +For an OPEN PR, re-read current head, base, files, checks, linked requirements, issue/body comments, prior review decisions, and Wiki Context Preflight. `checksStatus: PASS | FAIL | PENDING | NOT_APPLICABLE`; `NOT_APPLICABLE` requires a non-empty `checksJustification`. FAIL or PENDING makes the aggregate BLOCKED and completion-ineligible. ## Accepted finding authority diff --git a/ddalggak/references/review-output-contract.md b/ddalggak/references/review-output-contract.md index b59287d..68e76b9 100644 --- a/ddalggak/references/review-output-contract.md +++ b/ddalggak/references/review-output-contract.md @@ -34,7 +34,7 @@ Public output excludes internal evidence inventories. Wiki Context Manifest, gat - MERGED: `no follow-up`, `follow-up required`, `blocked` - CLOSED_UNMERGED: `no action`, `follow-up required`, `blocked` -Do not reuse `change request` for MERGED or CLOSED_UNMERGED. Merged/closed reviews default to no GitHub mutation; an authorized follow-up comment requires a reproducible or deterministic residual defect with material impact. +Do not reuse `change request` for MERGED or CLOSED_UNMERGED. Immediately before rendering or publishing, re-read lifecycle. If `state=MERGED` or `mergedAt` is present, emit `REVIEW_STOPPED_PR_MERGED`, stop rendering/publication, and perform no GitHub mutation; report `MERGED / NO_FOLLOW_UP` by default. An authorized follow-up comment requires a reproducible or deterministic residual defect with material impact and is a separate action, not continuation of the stopped review. Lifecycle lookup failure or ambiguity is `BLOCKED`. ## Substantive gate requirement diff --git a/ddalggak/templates/review-brief.md b/ddalggak/templates/review-brief.md index 756829e..3c48d88 100644 --- a/ddalggak/templates/review-brief.md +++ b/ddalggak/templates/review-brief.md @@ -7,6 +7,8 @@ - checks Head SHA: - Linked issue contract / acceptance criteria: - Lifecycle: +- Lifecycle checked at / evidence: intake; after delegated or long-running work; immediately before each fix, push, or GitHub write +- Merge stop sentinel: `state=MERGED` or non-null `mergedAt` → `REVIEW_STOPPED_PR_MERGED`; record `state`, `mergedAt`, `mergeCommit`, `headRefOid`; stop delegation/probes/edits/pushes/GitHub writes - Authority boundary: - Purpose / non-goals: @@ -66,6 +68,8 @@ Worker output has no publication authority. - Internal Review Quality sensor verdict: `approve | change request | comment | blocked` - Canonical lifecycle outcome and public rendering: `references/cross-review-loop.md` plus `references/review-output-contract.md` -`REVIEW_DONE PR#: critical=N high=N medium=N low=N head=` +Open lifecycle completion: `REVIEW_DONE PR#: critical=N high=N medium=N low=N head=` + +Merged lifecycle hard stop: `REVIEW_STOPPED_PR_MERGED PR#: state=MERGED mergedAt= mergeCommit= head= outcome=NO_FOLLOW_UP` A zero-finding review is valid only with semantic coverage evidence. High/Critical zero-finding reviews additionally require the documented counterexample/challenger rule in `references/review-quality-contract.md`. diff --git a/scripts/test-conditional-command-assets.mjs b/scripts/test-conditional-command-assets.mjs index 447e654..5fb55b0 100644 --- a/scripts/test-conditional-command-assets.mjs +++ b/scripts/test-conditional-command-assets.mjs @@ -30,6 +30,8 @@ assert(byName.get("plan").conditional_references.includes("ambiguous-intent=deep assert.equal(byName.get("start").required_templates.length, 0); assert(byName.get("start").conditional_templates.includes("delegated-work=worker-brief.md")); assert(byName.get("review").conditional_references.includes("package-workflow-release-or-security-posture=security-posture-gate.md")); +assert(byName.get("review").stop_condition.includes("REVIEW_STOPPED_PR_MERGED")); +assert(byName.get("review").stop_condition.includes("state=MERGED")); console.log("[PASS] expensive gates/templates are absent from the base hot path and activation-bound"); assert.deepEqual(parseConditionalAssetSpec("package-workflow-release-or-security-posture=security-posture-gate.md"), { @@ -99,6 +101,19 @@ for (const skillPath of ["ddalggak/SKILL.md", ".codex/skills/ddalggak/SKILL.md"] } console.log("[PASS] rendered skills keep conditional routing and prose aligned"); +for (const root of ["ddalggak", ".codex/skills/ddalggak"]) { + for (const asset of [ + "SKILL.md", + "references/cross-review-loop.md", + "references/review-output-contract.md", + "templates/review-brief.md", + ]) { + const text = readFileSync(`${root}/${asset}`, "utf8"); + assert(text.includes("REVIEW_STOPPED_PR_MERGED"), `${root}/${asset}: merged-review stop sentinel must remain projected`); + } +} +console.log("[PASS] merged-during-review hard stop remains projected across both runtime roots"); + const router = readFileSync("ddalggak/references/quality-lens-router.md", "utf8"); assert(router.includes("`security-posture` | Package manifests/admission")); assert(router.includes("CI/workflows, release/publish")); From 793a0ef2b9e8649d84ad715200205434d2743b86 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Tue, 1 Sep 2026 15:54:53 +0900 Subject: [PATCH 2/2] =?UTF-8?q?fix(review):=20=EB=A6=AC=EB=B7=B0=20?= =?UTF-8?q?=EA=B3=84=EC=95=BD=20=ED=86=A0=ED=81=B0=20=EC=98=88=EC=82=B0=20?= =?UTF-8?q?=EC=A4=80=EC=88=98?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit merge stop 규칙의 중복 설명을 reference로 모으고 hot path를 압축해 모든 subcommand token budget admission을 통과시킵니다. --- .codex/skills/ddalggak/SKILL.md | 6 +++--- core/commands/review.yaml | 2 +- ddalggak/SKILL.md | 9 ++++----- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.codex/skills/ddalggak/SKILL.md b/.codex/skills/ddalggak/SKILL.md index f874fc2..b4f4153 100644 --- a/.codex/skills/ddalggak/SKILL.md +++ b/.codex/skills/ddalggak/SKILL.md @@ -69,7 +69,7 @@ If a non-writing subcommand would need a source edit to continue, report the nee | Subcommand | Mode | Show-doc heading | Purpose | Side effects | Stop condition | Required assets | | --- | --- | --- | --- | --- | --- | --- | | `start` | source-edit | Start Workflow | Issue implementation from live issue body/comments; one issue PR by default | Repo source edits in issue scope; start publishes the issue PR via the ship procedure (ship.md); cross-review comments come through the review gate. | Stop on stale base, missing issue body/comments, duplicate PR, or required files outside the issue-owned scope. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/agent-runtime-contract.md`, `references/start-workflow.md`; templates: -; conditional: see conditional map | -| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Re-check lifecycle at intake, after delegated work, and before writes. On state=MERGED or mergedAt, emit REVIEW_STOPPED_PR_MERGED and stop review/writes. Uncertain lifecycle is BLOCKED. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | +| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Before writes, re-check lifecycle. state=MERGED/mergedAt emits REVIEW_STOPPED_PR_MERGED and stops; uncertainty is BLOCKED. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | | `status` | read-only | Status | Read-only live git/GitHub/session state snapshot | No source, GitHub, or local cleanup mutation; report live git/GitHub/session state only. | Stop after a live state snapshot and next-action recommendation. | refs: `references/wiki-context-preflight.md`, `references/status.md`, `references/pr-check-evidence-bundle.md`; templates: - | | `plan` | plan-only | Issue-Ready Plan | Issue-ready implementation plan from issue/wiki/code evidence | No source edits; no GitHub writes unless the user separately requests issue creation. | Stop after an issue-ready plan with evidence/unknowns and PR topology. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/wiki-bridge.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/issue-ready-plan.md`; templates: -; conditional: see conditional map | | `issue` | github-write | Plan to Issues | Create GitHub issues from an approved plan | Create/edit GitHub issues and comments only; no repository source edits. | Stop after live issue URLs/labels/assignees/body UTF-8 verification or on metadata permission failure. | refs: `references/wiki-context-preflight.md`, `references/plan-to-issues.md`; templates: `templates/issue-body.md`, `templates/epic-body.md` | @@ -173,11 +173,11 @@ Execution contract index: target repo/base freshness, issue body+comments, base ## `review` - Cross-Review Loop -Command contract: mode `review-fix`; only accepted Critical/High fixes and review comments may write. Re-check lifecycle at intake, after delegated work, and before writes. On `state=MERGED` or `mergedAt`, emit `REVIEW_STOPPED_PR_MERGED` and stop; uncertainty is `BLOCKED`. +Command contract: `review-fix`; accepted Critical/High fixes/comments only. Follow `cross-review-loop.md` lifecycle checkpoints; merged emits `REVIEW_STOPPED_PR_MERGED`, uncertainty is `BLOCKED`. Full procedure: `references/cross-review-loop.md`; public renderer: `references/review-output-contract.md` + `references/review-comment-style.md`; wiki authority: `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`; delegated review only loads `templates/review-brief.md`. -Execution contract index: live PR/diff/files/checks/issue/head SHA; lifecycle rechecks at intake, after delegated work, and before writes; merged hard stop with authoritative readback and default `MERGED / NO_FOLLOW_UP`; lookup ambiguity as `BLOCKED`; Wiki Context Preflight; base Router/Evidence; activation-bound optional gates; Admission schema v3; aggregate/publication authority; canonical-candidate-bound two-sentence findings; deterministic fixed summary; substantive zero-finding validation. +Execution contract index: live PR/diff/files/checks/issue/head SHA; lifecycle stop/readback; Wiki Context Preflight; base Router/Evidence; activation-bound optional gates; Admission schema v3; canonical-candidate-bound two-sentence findings; deterministic fixed summary. ## `status` - Current State Snapshot diff --git a/core/commands/review.yaml b/core/commands/review.yaml index ca264a2..8cdd1c4 100644 --- a/core/commands/review.yaml +++ b/core/commands/review.yaml @@ -7,7 +7,7 @@ github_write_allowed: true purpose: "Risk-adaptive current-head semantic review and accepted fix loop." mode: "review-fix" write_side_effects: "Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch." -stop_condition: "Re-check lifecycle at intake, after delegated work, and before writes. On state=MERGED or mergedAt, emit REVIEW_STOPPED_PR_MERGED and stop review/writes. Uncertain lifecycle is BLOCKED." +stop_condition: "Before writes, re-check lifecycle. state=MERGED/mergedAt emits REVIEW_STOPPED_PR_MERGED and stops; uncertainty is BLOCKED." required_references: - wiki-context-preflight.md - 2026-06-04-brain-v0-wiki-authority-in-ddalggak.md diff --git a/ddalggak/SKILL.md b/ddalggak/SKILL.md index 84c1b87..c971104 100644 --- a/ddalggak/SKILL.md +++ b/ddalggak/SKILL.md @@ -81,7 +81,7 @@ user-invocable: true | subcommand | mode | show-doc heading | 목적 | side effects | stop condition | 상세 reference rule | |---|---|---|---|---|---|---| | `start` | source-edit | Start Workflow | Issue implementation from live issue body/comments; one issue PR by default | Repo source edits in issue scope; start publishes the issue PR via the ship procedure (ship.md); cross-review comments come through the review gate. | Stop on stale base, missing issue body/comments, duplicate PR, or required files outside the issue-owned scope. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/agent-runtime-contract.md`, `references/start-workflow.md`; templates: -; conditional: see conditional map | -| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Re-check lifecycle at intake, after delegated work, and before writes. On state=MERGED or mergedAt, emit REVIEW_STOPPED_PR_MERGED and stop review/writes. Uncertain lifecycle is BLOCKED. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | +| `review` | review-fix | Cross-Review Loop | Risk-adaptive current-head semantic review and accepted fix loop | Top-level review comment plus inline line-anchored review comments for every triage-passing finding in one COMMENT-event batch; accepted Critical/High fixes may edit source and push to the reviewed PR branch. | Before writes, re-check lifecycle. state=MERGED/mergedAt emits REVIEW_STOPPED_PR_MERGED and stops; uncertainty is BLOCKED. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/cross-review-loop.md`, `references/review-quality-contract.md`; templates: -; conditional: see conditional map | | `status` | read-only | Status | Read-only live git/GitHub/session state snapshot | No source, GitHub, or local cleanup mutation; report live git/GitHub/session state only. | Stop after a live state snapshot and next-action recommendation. | refs: `references/wiki-context-preflight.md`, `references/status.md`, `references/pr-check-evidence-bundle.md`; templates: - | | `plan` | plan-only | Issue-Ready Plan | Issue-ready implementation plan from issue/wiki/code evidence | No source edits; no GitHub writes unless the user separately requests issue creation. | Stop after an issue-ready plan with evidence/unknowns and PR topology. | refs: `references/wiki-context-preflight.md`, `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`, `references/wiki-bridge.md`, `references/quality-lens-router.md`, `references/evidence-contract.md`, `references/issue-ready-plan.md`; templates: -; conditional: see conditional map | | `issue` | github-write | Plan to Issues | Create GitHub issues from an approved plan | Create/edit GitHub issues and comments only; no repository source edits. | Stop after live issue URLs/labels/assignees/body UTF-8 verification or on metadata permission failure. | refs: `references/wiki-context-preflight.md`, `references/plan-to-issues.md`; templates: `templates/issue-body.md`, `templates/epic-body.md` | @@ -161,17 +161,16 @@ Execution contract index: ## Cross-Review Loop -Command contract: mode `review-fix`; only accepted Critical/High fixes and review comments may write. Re-check lifecycle at intake, after delegated work, and before writes. On `state=MERGED` or `mergedAt`, emit `REVIEW_STOPPED_PR_MERGED` and stop; uncertainty is `BLOCKED`. +Command contract: `review-fix`; accepted Critical/High fixes/comments only. Follow `cross-review-loop.md` lifecycle checkpoints; merged emits `REVIEW_STOPPED_PR_MERGED`, uncertainty is `BLOCKED`. Full procedure: `references/cross-review-loop.md`; public renderer: `references/review-output-contract.md` + `references/review-comment-style.md`; wiki authority: `references/2026-06-04-brain-v0-wiki-authority-in-ddalggak.md`; delegated-review만 `templates/review-brief.md`를 로드한다. Execution contract index: - Re-read live PR state, diff/files/checks, linked issue, current head SHA, and wiki-context preflight. -- Treat `state=MERGED` or non-null `mergedAt` as a hard stop sentinel: attempt to stop in-flight delegated review work, perform no further probes or candidate processing, and prohibit fixes, pushes, formal/inline reviews, and top-level comments. Record authoritative lifecycle readback (`state`, `mergedAt`, `mergeCommit`, `headRefOid`) and return `MERGED / NO_FOLLOW_UP` unless live evidence requires a separately authorized follow-up. -- Lifecycle lookup failure or ambiguity is `BLOCKED`; it never permits continued mutation or publication. +- Lifecycle: merged stops probes/delegation/edits/pushes/GitHub writes after authoritative readback; default `MERGED / NO_FOLLOW_UP`. Lookup ambiguity is `BLOCKED`. - Gates: Router/Evidence는 base; 나머지는 activation evidence applies일 때만 로드한다. - Findings must separate live evidence, wiki-strengthened rationale, non-wiki inference, and retrieval gaps. -- Run admission schema v3 and the finding signal gate first (`references/cross-review-loop.md`): only conductor-promoted admitted findings become inline comments in one `COMMENT`-event batch; each finding is rendered from its aggregate-member canonical candidate, filtered Low/nit notes stay internal, and the top-level body is the deterministic fixed summary. A zero-finding review with substantive validation evidence is valid. +- Apply admission schema v3 first: only aggregate-member canonical candidate findings publish, filtered notes stay internal, deterministic fixed summary renders, and zero-finding needs substantive evidence. ---