From 0da11670124a1cd90f3326d1552981fdfa7f1bc5 Mon Sep 17 00:00:00 2001 From: hyoseok Date: Thu, 16 Apr 2026 13:09:47 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20Harness=20hook=20=EC=B6=94=EB=A1=A0?= =?UTF-8?q?=EA=B3=BC=20runtime=20surface=20=EA=B2=80=EC=A6=9D=20=EA=B0=95?= =?UTF-8?q?=ED=99=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - completed task scope 기준 pre-push 검증과 fail-closed task 추론 보강 - init과 doctor에 githook 및 hooks.json 동기화와 drift 검출 추가 - dead hook surface 제거와 문서 및 회귀 테스트 정리 --- .codex/skills/README.md | 9 +- .codex/skills/repair-reopen/SKILL.md | 3 +- .codex/skills/verification-runner/SKILL.md | 9 +- .githooks/pre-push | 6 +- AGENTS.md | 9 +- docs/hooks.md | 16 +- docs/runbook.md | 9 +- docs/runtime.md | 9 +- scripts/hooks/circuit_breaker.py | 27 -- scripts/hooks/dangerous_cmd_guard.py | 27 -- scripts/hooks/tdd_guard.py | 40 -- scripts/workflow_runtime/cli.py | 21 +- scripts/workflow_runtime/constants.py | 5 +- scripts/workflow_runtime/doctor.py | 43 +- scripts/workflow_runtime/engine.py | 158 +++++-- tests/test_workflow_cli.py | 505 +++++++++++++++++++++ workflows/system/hooks.json | 4 +- 17 files changed, 724 insertions(+), 176 deletions(-) delete mode 100755 scripts/hooks/circuit_breaker.py delete mode 100755 scripts/hooks/dangerous_cmd_guard.py delete mode 100755 scripts/hooks/tdd_guard.py diff --git a/.codex/skills/README.md b/.codex/skills/README.md index 47e6351..726fba6 100644 --- a/.codex/skills/README.md +++ b/.codex/skills/README.md @@ -10,6 +10,7 @@ - 전역 guard 정책은 `workflows/system/hooks.json`이 가진다. - 내부 런타임 구현은 `scripts/workflow_runtime/`이 가진다. - skill은 위 규칙을 복제하지 않고, 한 단계의 handoff와 CLI 진입점만 안내한다. +- `python3 scripts/workflow.py init`는 runtime surface를 source 기준으로 다시 동기화하고, `doctor`는 drift를 실패로 보고한다. ## Active Skill Set @@ -27,10 +28,8 @@ 1. `request-intake` 2. `socratic-spec-authoring` 3. `phase-planner` -4. `boundary-check` -5. `phase-executor` -6. `verification-runner` -7. `review-closeout` -8. `repair-reopen` +4. `boundary-check -> phase-executor -> verification-runner`를 active phase마다 반복 +5. 모든 phase가 끝나면 `review-closeout` +6. 실패, block, follow-up이면 `repair-reopen` 새 workflow에서는 markdown spec과 JSON state를 분리한다. `spec.md`는 사람용 요구사항 초안이고, `approve`가 이를 `task.json.intake`로 잠근다. `task.json`/`phases.json`/`runs/*.json`은 자동화용 canonical source다. diff --git a/.codex/skills/repair-reopen/SKILL.md b/.codex/skills/repair-reopen/SKILL.md index 9e3f39d..1c416e6 100644 --- a/.codex/skills/repair-reopen/SKILL.md +++ b/.codex/skills/repair-reopen/SKILL.md @@ -21,7 +21,7 @@ description: failed, blocked, review follow-up, 추가 요구사항이 들어온 1. 왜 reopen이 필요한지 note를 한 줄로 잠근다. 2. 필요하면 target phase를 정한다. -3. `python3 scripts/workflow.py reopen --note "..." [--phase-id ...]`로 task를 `approved` 상태로 되돌린다. +3. `python3 scripts/workflow.py reopen --note "..." [--phase-id ...]`로 task를 `approved` 상태로 되돌리고 target phase를 `pending`으로 복구한다. 4. 추가 요구사항이면 먼저 `spec.md`를 다시 잠근다. 5. spec이 바뀌었으면 `python3 scripts/workflow.py approve --note "..."`로 `task.json.intake`를 다시 잠근다. 6. 필요하면 `plan`으로 phase를 다시 적재한다. @@ -30,5 +30,6 @@ description: failed, blocked, review follow-up, 추가 요구사항이 들어온 ## 결과 - `state=approved`인 `task.json` +- rerunnable target phase (`status=pending`) - cleared `blocked_reason` - new reopen run evidence diff --git a/.codex/skills/verification-runner/SKILL.md b/.codex/skills/verification-runner/SKILL.md index 79a48b6..67f3ce0 100644 --- a/.codex/skills/verification-runner/SKILL.md +++ b/.codex/skills/verification-runner/SKILL.md @@ -7,7 +7,7 @@ description: active phase의 acceptance command를 실행하고 `runs/*.json`에 ## 언제 사용하나 -- phase 구현이 끝났고 검증을 실행해야 한다. +- phase 구현이 끝났고 `run --complete`까지 닫은 뒤 검증을 실행해야 한다. ## 먼저 확인할 것 @@ -19,12 +19,13 @@ description: active phase의 acceptance command를 실행하고 `runs/*.json`에 ## 작업 방식 1. `acceptance.commands`를 확인한다. -2. `python3 scripts/workflow.py verify `를 실행한다. -3. `task.json.last_verified_run_id`가 현재 phase의 passed verification으로 갱신됐는지 확인한다. +2. 현재 active phase가 `completed` 상태인지 확인한다. +3. `python3 scripts/workflow.py verify `를 실행한다. +4. `task.json.last_verified_run_id`가 현재 phase의 passed verification으로 갱신됐는지 확인한다. ## 결과 - `runs/.json` - `task.json.latest_run_id` - `task.json.last_verified_run_id` -- 다음 stage: `review-closeout` +- 다음 stage: 다음 pending phase가 있으면 `phase-executor`, 없으면 `review-closeout` diff --git a/.githooks/pre-push b/.githooks/pre-push index 4b907c2..7999f03 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -5,14 +5,12 @@ python3 scripts/workflow.py doctor python3 scripts/workflow.py hook pre_command --command-text "git push $*" task_id="${WORKFLOW_TASK_ID:-}" -if [ -z "$task_id" ]; then - task_id="$(python3 scripts/workflow.py status --infer-active-task 2>/dev/null || true)" -fi - if [ -n "$task_id" ]; then phase_id="${WORKFLOW_PHASE_ID:-}" if [ -z "$phase_id" ]; then phase_id="$(python3 scripts/workflow.py status "$task_id" --field active_phase_id 2>/dev/null || true)" fi python3 scripts/workflow.py hook pre_push --task-id "$task_id" --phase-id "$phase_id" --command-text "git push $*" +else + python3 scripts/workflow.py hook pre_push --command-text "git push $*" fi diff --git a/AGENTS.md b/AGENTS.md index d94ce58..90b1f9c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,10 +43,10 @@ NEVER: control plane 문서를 앱 source-of-truth보다 우선시하지 않는 5. `plan --from ` 또는 `plan --stdin`이 phase 초안을 읽어 `phases.json`으로 적재한다. `spec.md`와 `task.json.intake`가 어긋나 있으면 plan을 시작하지 않는다. 6. `run --start`가 active phase를 시작한다. 7. 구현 후 `run --complete --changed-path ...`가 write scope와 TDD guard를 통과해야 한다. -8. `verify`가 acceptance command를 실행하고 `runs/*.json`에 evidence를 남긴다. +8. `verify`가 completed phase의 acceptance command를 실행하고 `runs/*.json`에 evidence를 남긴다. 9. 모든 phase가 완료되면 `review`가 review readiness를 기록한다. 10. `review --close --user-validation-note ...`만 최종 완료를 닫을 수 있다. -11. 실패, block, 추가 요구사항이 생기면 `reopen`으로 repair loop를 재개한다. +11. 실패, block, 추가 요구사항이 생기면 `reopen`으로 target phase를 `pending`으로 되돌리고 repair loop를 재개한다. ## TDD Contract @@ -74,9 +74,14 @@ python3 scripts/workflow.py review --note "..." python3 scripts/workflow.py review --close --user-validation-note "..." python3 scripts/workflow.py reopen --note "..." python3 scripts/workflow.py status +python3 scripts/workflow.py hook pre_command --command-text "..." +python3 scripts/workflow.py hook pre_commit --staged +python3 scripts/workflow.py hook pre_push --command-text "..." ``` - `scripts/workflow.py`는 영구적인 CLI entrypoint다. +- hook enforcement의 공식 surface도 `python3 scripts/workflow.py hook ...` 하나다. +- `init`는 `.githooks/*`와 `workflows/system/hooks.json`을 source 기준으로 다시 동기화하고, `doctor`는 drift를 실패로 취급한다. - 실제 실행 엔진은 `scripts/workflow_runtime/cli.py`, `engine.py`, `models.py`, `guards.py`, `doctor.py`, `git_ops.py`가 분담한다. - `workflow.py`를 유지하는 이유는 사람과 AI가 하나의 안정적인 명령 surface만 기억하면 되기 때문이다. diff --git a/docs/hooks.md b/docs/hooks.md index 8391bed..071343a 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -3,14 +3,13 @@ ## Canonical Hook Config - 설정 파일: [workflows/system/hooks.json](../workflows/system/hooks.json) -- 구현 파일: [scripts/hooks](../scripts/hooks/) +- 구현 파일: [scripts/workflow_runtime/guards.py](../scripts/workflow_runtime/guards.py), [scripts/workflow_runtime/cli.py](../scripts/workflow_runtime/cli.py) - trigger surface: workflow CLI, `.githooks/`, CI ## Hook Events - `pre_command` - `pre_commit` -- `pre_phase_start` - `post_change` - `pre_phase_complete` - `pre_review` @@ -38,7 +37,14 @@ ### Latest Verification Guard - `pre_review`에서는 active phase에 대한 latest passed verification을 요구한다. -- `pre_push`에서는 unpushed diff가 active phase scope와 겹칠 때 latest passed verification을 요구한다. +- `pre_push`에서는 unpushed diff를 기준으로 task를 먼저 해석하고, 그 scope와 겹칠 때 latest passed verification을 요구한다. +- active task가 없더라도 unpushed diff가 정확히 하나의 completed task scope에 매핑되면 그 task를 push gate context로 재사용한다. +- unpushed diff가 어느 task에도 단일하게 매핑되지 않으면 `pre_push`는 fail-closed 한다. + +### User Validation Guard + +- `pre_complete`에서는 non-empty user validation note를 요구한다. +- `review --close --user-validation-note ...`만 최종 완료를 닫을 수 있다. ### Circuit Breaker @@ -55,3 +61,7 @@ git hook만으로는 충분하지 않다. 로컬 CLI와 phase runner가 같은 hook 설정을 읽어야 guard가 유지된다. `python3 scripts/workflow.py init`는 로컬 git 저장소의 `core.hooksPath`를 `.githooks`로 맞추고, `doctor`는 이 설정이 빠지면 실패한다. +`init`는 `.githooks/pre-commit`, `.githooks/pre-push`, `workflows/system/hooks.json`을 source 기준으로 다시 동기화한다. +`doctor`는 위 runtime surface가 source와 drift하면 실패하고 `python3 scripts/workflow.py init`를 다시 요구한다. +`pre_commit`은 active task가 하나로 추론되지 않으면 fail-closed 한다. 이때는 `WORKFLOW_TASK_ID` 또는 `--task-id`로 task binding을 명시해야 한다. +`pre_push`는 먼저 unpushed diff를 task scope에 매핑한다. scope가 단일 task로 정해지지 않으면 역시 fail-closed 한다. diff --git a/docs/runbook.md b/docs/runbook.md index 500631e..bf907d8 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -8,7 +8,7 @@ python3 scripts/workflow.py doctor python3 -m unittest discover -s tests -v ``` -`init`가 로컬 git 저장소의 `core.hooksPath`를 `.githooks`로 맞춘다. `doctor`는 이 값이 어긋나면 실패한다. +`init`가 로컬 git 저장소의 `core.hooksPath`를 `.githooks`로 맞추고 `.githooks/*`, `workflows/system/hooks.json`을 source 기준으로 다시 동기화한다. `doctor`는 이 값이나 runtime surface가 어긋나면 실패한다. ## 새 작업 시작 @@ -41,6 +41,8 @@ cat /tmp/task-001-phases.json | python3 scripts/workflow.py plan task-001 --stdi ## 실행과 검증 +`verify`는 `run --complete`로 닫힌 phase에 대해서만 실행한다. + ```bash python3 scripts/workflow.py run task-001 --start python3 scripts/workflow.py run task-001 --complete --changed-path scripts/workflow.py --changed-path tests/test_workflow_cli.py @@ -51,7 +53,7 @@ python3 scripts/workflow.py review task-001 --close --user-validation-note "vali ## Repair / Reopen -실패, block, review follow-up, 추가 요구사항이 들어오면 `reopen`으로 다시 `approved` 상태로 되돌린다. 추가 요구사항이면 `spec.md`를 다시 잠근 뒤 `approve`를 다시 실행하고, 그 다음 `plan`으로 phase를 갱신한다. +실패, block, review follow-up, 추가 요구사항이 들어오면 `reopen`으로 다시 `approved` 상태로 되돌린다. `reopen`은 target phase를 `pending`으로 복구해서 바로 `run --start`로 재개할 수 있게 만든다. 추가 요구사항이면 `spec.md`를 다시 잠근 뒤 `approve`를 다시 실행하고, 그 다음 `plan`으로 phase를 갱신한다. ```bash python3 scripts/workflow.py reopen task-001 --note "추가 요구사항 반영" @@ -68,3 +70,6 @@ python3 scripts/workflow.py hook pre_command --command-text "git push origin fea python3 scripts/workflow.py hook pre_commit --staged python3 scripts/workflow.py hook pre_push --command-text "git push origin feature" ``` + +`pre_commit`은 active task가 여러 개라면 자동 추론을 중단하고 실패한다. 이 경우 `WORKFLOW_TASK_ID` 또는 `--task-id`로 명시적으로 task binding을 넣는다. +`pre_push`는 먼저 unpushed diff를 task scope에 매핑한다. active task가 없어도 completed task scope 하나로 매핑되면 그 task를 재사용하고, 단일 task로 정해지지 않으면 실패한다. diff --git a/docs/runtime.md b/docs/runtime.md index 47d527a..3be89e5 100644 --- a/docs/runtime.md +++ b/docs/runtime.md @@ -9,7 +9,7 @@ 5. `plan`으로 `phases.json` 적재 6. active phase 실행 7. `run --complete`로 phase closure -8. `verify`로 acceptance command 실행 +8. `verify`로 completed phase의 acceptance command 실행 9. 모든 phase 완료 후 `review` 10. `review --close`로 user validation과 완료 고정 11. 필요하면 `reopen`으로 repair loop 재개 @@ -41,7 +41,7 @@ - `plan`은 `task.json.intake`와 현재 `spec.md`가 일치할 때만 허용한다. - `status`는 `spec.md`를 읽어 `ready_for_approval`, `clarification_count`, `unresolved_clarifications`를 JSON으로 노출한다. - `run --complete`는 `allowed_write_paths`와 `workflows/tasks//` 밖 변경을 차단한다. -- `verify`는 active phase 또는 지정 phase의 `acceptance.commands`를 실행해 run evidence를 남긴다. +- `verify`는 completed active phase 또는 지정된 completed phase의 `acceptance.commands`를 실행해 run evidence를 남긴다. - phase completion 뒤 verification이 통과하기 전까지 `last_verified_run_id`는 비어 있어야 한다. - `review`는 `last_verified_run_id`가 active phase와 일치하는 passed verification일 때만 허용한다. - task-level review는 모든 phase가 `completed`일 때만 연다. @@ -58,7 +58,8 @@ - `scripts/workflow_runtime/git_ops.py`: staged/unpushed path 조회 `workflow.py`를 유지하는 이유는 사람과 AI가 단일 고정 명령 surface를 기억하는 편이 가장 효율적이기 때문이다. -`init`는 로컬 git 저장소의 `core.hooksPath`를 `.githooks`로 맞추고, `doctor`는 이 설정이 어긋나면 실패한다. +`init`는 로컬 git 저장소의 `core.hooksPath`를 `.githooks`로 맞추고, `.githooks/*`와 `workflows/system/hooks.json`을 source 기준으로 다시 동기화한다. +`doctor`는 이 설정이 어긋나거나 runtime surface가 source와 drift하면 실패한다. ## Retry / Blocked Semantics @@ -66,5 +67,5 @@ - verification 실패도 retry에 포함한다. - 동일한 `error_fingerprint`가 짧은 시간 안에 반복되면 Circuit Breaker가 `blocked`로 전이한다. - `blocked`는 외부 입력, spec 재정의, phase 재계획이 필요한 상태다. -- `reopen`은 `failed`, `blocked`, `review_ready`, `completed` task를 다시 `approved`로 되돌리고 repair loop를 재시작한다. +- `reopen`은 `failed`, `blocked`, `review_ready`, `completed` task를 다시 `approved`로 되돌리고 target phase를 `pending`으로 복구한 뒤 repair loop를 재시작한다. - 추가 요구사항이면 `reopen` 뒤 `spec.md`를 다시 잠그고 `approve`를 재실행한 뒤 `plan`으로 phase를 다시 적재한다. diff --git a/scripts/hooks/circuit_breaker.py b/scripts/hooks/circuit_breaker.py deleted file mode 100755 index 772f323..0000000 --- a/scripts/hooks/circuit_breaker.py +++ /dev/null @@ -1,27 +0,0 @@ -#!/usr/bin/env python3 - -from __future__ import annotations - -import argparse -import json -import sys -from pathlib import Path - -sys.path.insert(0, str(Path(__file__).resolve().parents[1])) - -from workflow_runtime.engine import WorkflowService # noqa: E402 - - -def main() -> int: - parser = argparse.ArgumentParser(description="Record a failure fingerprint") - parser.add_argument("--error-fingerprint", required=True) - args = parser.parse_args() - - service = WorkflowService() - result = service.record_circuit_breaker(args.error_fingerprint) - print(json.dumps(result.as_payload(), indent=2, ensure_ascii=False)) - return 0 if result.status != "blocked" else 2 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/scripts/hooks/dangerous_cmd_guard.py b/scripts/hooks/dangerous_cmd_guard.py deleted file mode 100755 index 1b26f41..0000000 --- a/scripts/hooks/dangerous_cmd_guard.py +++ /dev/null @@ -1,27 +0,0 @@ -#!/usr/bin/env python3 - -from __future__ import annotations - -import argparse -import json -import sys -from pathlib import Path - -sys.path.insert(0, str(Path(__file__).resolve().parents[1])) - -from workflow_runtime.engine import WorkflowService # noqa: E402 - - -def main() -> int: - parser = argparse.ArgumentParser(description="Run Dangerous Command Guard") - parser.add_argument("--command", required=True) - args = parser.parse_args() - - service = WorkflowService() - result = service.dangerous_cmd_guard(args.command) - print(json.dumps(result.as_payload(), indent=2, ensure_ascii=False)) - return 0 if result.status == "passed" else 1 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/scripts/hooks/tdd_guard.py b/scripts/hooks/tdd_guard.py deleted file mode 100755 index 74d9c8b..0000000 --- a/scripts/hooks/tdd_guard.py +++ /dev/null @@ -1,40 +0,0 @@ -#!/usr/bin/env python3 - -from __future__ import annotations - -import argparse -import json -import sys -from pathlib import Path - -sys.path.insert(0, str(Path(__file__).resolve().parents[1])) - -from workflow_runtime.engine import WorkflowService # noqa: E402 - - -def main() -> int: - parser = argparse.ArgumentParser(description="Run TDD Guard") - parser.add_argument("--task-id") - parser.add_argument("--phase-id") - parser.add_argument("--changed-path", action="append", default=[]) - parser.add_argument("--staged", action="store_true") - args = parser.parse_args() - - service = WorkflowService() - task = service.load_task(args.task_id) if args.task_id else None - phase = None - if args.task_id and args.phase_id: - _, _, phase = service.current_phase(args.task_id, args.phase_id) - elif task and task["active_phase_id"]: - _, _, phase = service.current_phase(args.task_id, task["active_phase_id"]) - - changed_paths = list(args.changed_path) - if args.staged: - changed_paths.extend(service.staged_or_worktree_paths(staged=True)) - result = service.tdd_guard(sorted(set(changed_paths)), phase) - print(json.dumps(result.as_payload(), indent=2, ensure_ascii=False)) - return 0 if result.status == "passed" else 1 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/scripts/workflow_runtime/cli.py b/scripts/workflow_runtime/cli.py index 81060ec..939f075 100644 --- a/scripts/workflow_runtime/cli.py +++ b/scripts/workflow_runtime/cli.py @@ -213,13 +213,6 @@ def main(argv: list[str] | None = None) -> int: return 0 if args.command == "hook": - task = service.load_task(args.task_id) if args.task_id else None - phase = None - if args.task_id and args.phase_id: - _, _, phase = service.current_phase(args.task_id, args.phase_id) - elif args.task_id and task and task["active_phase_id"]: - _, _, phase = service.current_phase(args.task_id, task["active_phase_id"]) - changed_paths = list(args.changed_path) if args.staged: changed_paths.extend(service.staged_or_worktree_paths(staged=True)) @@ -227,12 +220,24 @@ def main(argv: list[str] | None = None) -> int: changed_paths.extend(service.staged_or_worktree_paths(staged=False)) if args.event == "pre_push" and not changed_paths: changed_paths.extend(service.unpushed_paths()) + changed_paths = service.normalize_changed_paths(changed_paths) + + task_id = args.task_id + if not task_id and args.event in {"pre_commit", "pre_push"}: + task_id = service.infer_hook_task(args.event, changed_paths) + + task = service.load_task(task_id) if task_id else None + phase = None + if task_id and args.phase_id: + _, _, phase = service.current_phase(task_id, args.phase_id) + elif task_id and task and task["active_phase_id"]: + _, _, phase = service.current_phase(task_id, task["active_phase_id"]) result = service.run_hooks( args.event, task=task, phase=phase, - changed_paths=service.normalize_changed_paths(changed_paths), + changed_paths=changed_paths, command=args.command_text, user_validation_note=args.user_validation_note, ) diff --git a/scripts/workflow_runtime/constants.py b/scripts/workflow_runtime/constants.py index 6c560a0..330511b 100644 --- a/scripts/workflow_runtime/constants.py +++ b/scripts/workflow_runtime/constants.py @@ -20,7 +20,7 @@ "in_progress": {"completed", "failed", "blocked"}, "failed": {"pending", "in_progress", "blocked"}, "blocked": {"pending", "in_progress"}, - "completed": {"failed", "blocked"}, + "completed": {"pending", "failed", "blocked"}, } SPEC_REQUIRED_SECTIONS = [ @@ -64,7 +64,7 @@ "docs/README.md": ["AGENTS.md", "docs/runtime.md", "docs/artifact-model.md"], "docs/artifact-model.md": ["task.json", "intake", "clarifications"], "docs/runtime.md": ["소크라테스 질문", "approve", "plan", "review --close"], - "docs/hooks.md": [".githooks/", "TDD Guard", "Dangerous Command Guard", "Circuit Breaker"], + "docs/hooks.md": [".githooks/", "TDD Guard", "Dangerous Command Guard", "Circuit Breaker", "workflow.py hook"], "docs/runbook.md": ["사용자가 현재 spec 초안에 명시적으로 동의하면 approve 한다.", "approve", "plan"], } @@ -77,6 +77,7 @@ "workflows/config/", "workflows/runtime/", "workflows/schemas/", + "scripts/hooks/", "workflow_lib.py", ] diff --git a/scripts/workflow_runtime/doctor.py b/scripts/workflow_runtime/doctor.py index 2e397f6..e2042ad 100644 --- a/scripts/workflow_runtime/doctor.py +++ b/scripts/workflow_runtime/doctor.py @@ -13,6 +13,13 @@ STALE_REFERENCE_SCAN_EXCLUDES, ) +SOURCE_ROOT = Path(__file__).resolve().parents[2] +RUNTIME_SURFACE_FILES = ( + Path(".githooks/pre-commit"), + Path(".githooks/pre-push"), + Path("workflows/system/hooks.json"), +) + def validate_agents_constitution(agents_path: Path) -> list[str]: text = agents_path.read_text(encoding="utf-8", errors="ignore") @@ -79,6 +86,21 @@ def stale_reference_errors(root: Path) -> list[str]: return errors +def runtime_surface_errors(root: Path) -> list[str]: + errors: list[str] = [] + for relative in RUNTIME_SURFACE_FILES: + target = root / relative + source = SOURCE_ROOT / relative + if not target.exists(): + errors.append(f"missing canonical artifact: {relative}") + continue + if not source.exists(): + continue + if target.read_bytes() != source.read_bytes(): + errors.append(f"{relative} is out of sync with source; run `python3 scripts/workflow.py init`") + return errors + + def build_doctor_report( root: Path, *, @@ -105,6 +127,12 @@ def build_doctor_report( else: checks.append("task artifacts are internally consistent") + runtime_errors = runtime_surface_errors(root) + if runtime_errors: + errors.extend(runtime_errors) + else: + checks.append("runtime surface matches source") + repo_surface = any((root / name).exists() for name in ("AGENTS.md", "docs", ".github", ".codex")) if repo_surface: agents_path = root / "AGENTS.md" @@ -125,11 +153,16 @@ def build_doctor_report( markers = DOC_REQUIRED_MARKERS.get(relative) if markers: errors.extend(validate_doc_markers(doc_path, relative, markers)) - for relative in (".githooks/pre-commit", ".githooks/pre-push", "workflows/system/hooks.json"): - if not (root / relative).exists(): - errors.append(f"missing canonical artifact: {relative}") - for legacy_dir in ("workflows/config", "workflows/runtime", "workflows/schemas"): - if (root / legacy_dir).exists(): + for legacy_dir in ("workflows/config", "workflows/runtime", "workflows/schemas", "scripts/hooks"): + legacy_path = root / legacy_dir + if not legacy_path.exists(): + continue + legacy_files = [ + candidate + for candidate in legacy_path.rglob("*") + if candidate.is_file() and "__pycache__" not in candidate.parts and candidate.suffix != ".pyc" + ] + if legacy_files: errors.append(f"legacy directory must not exist: {legacy_dir}") stale_errors = stale_reference_errors(root) diff --git a/scripts/workflow_runtime/engine.py b/scripts/workflow_runtime/engine.py index ac840f7..93a36fe 100644 --- a/scripts/workflow_runtime/engine.py +++ b/scripts/workflow_runtime/engine.py @@ -17,6 +17,7 @@ dangerous_cmd_guard, latest_verified_run_guard, out_of_scope_paths, + phase_scope, tdd_guard, user_validation_guard, write_scope_guard, @@ -30,6 +31,7 @@ normalize_repo_path, now_iso, read_json, + scope_matches, validate_phases, validate_relative_repo_path, validate_run, @@ -42,6 +44,8 @@ SOURCE_ROOT = Path(__file__).resolve().parents[2] DEFAULT_ROOT = Path(os.environ.get("WORKFLOW_ROOT", SOURCE_ROOT)) +ACTIVE_TASK_STATES = {"approved", "in_progress", "failed", "blocked", "review_ready"} +PRE_PUSH_TASK_STATES = ACTIVE_TASK_STATES | {"completed"} class WorkflowService: @@ -52,23 +56,31 @@ def __init__(self, root: Path | None = None): self.tasks_dir = self.workflows_dir / "tasks" self.hooks_config_path = self.system_dir / "hooks.json" - def ensure_layout(self) -> None: + def sync_runtime_surface(self, *, overwrite: bool) -> None: + surfaces = ( + (SOURCE_ROOT / "workflows" / "system" / "hooks.json", self.hooks_config_path), + (SOURCE_ROOT / ".githooks" / "pre-commit", self.root / ".githooks" / "pre-commit"), + (SOURCE_ROOT / ".githooks" / "pre-push", self.root / ".githooks" / "pre-push"), + ) + for source, target in surfaces: + if not source.exists(): + continue + if source == target: + continue + if target.exists(): + try: + if source.samefile(target): + continue + except FileNotFoundError: + pass + target.parent.mkdir(parents=True, exist_ok=True) + if overwrite or not target.exists(): + shutil.copy2(source, target) + + def ensure_layout(self, *, sync: bool = False) -> None: self.system_dir.mkdir(parents=True, exist_ok=True) self.tasks_dir.mkdir(parents=True, exist_ok=True) - - source_hook = SOURCE_ROOT / "workflows" / "system" / "hooks.json" - if not self.hooks_config_path.exists() and source_hook.exists(): - shutil.copy2(source_hook, self.hooks_config_path) - - source_githooks_dir = SOURCE_ROOT / ".githooks" - target_githooks_dir = self.root / ".githooks" - if source_githooks_dir.exists(): - target_githooks_dir.mkdir(parents=True, exist_ok=True) - for hook_name in ("pre-commit", "pre-push"): - source_hook_script = source_githooks_dir / hook_name - target_hook_script = target_githooks_dir / hook_name - if not target_hook_script.exists() and source_hook_script.exists(): - shutil.copy2(source_hook_script, target_hook_script) + self.sync_runtime_surface(overwrite=sync) def task_dir(self, task_id: str) -> Path: return self.tasks_dir / validate_task_id(task_id) @@ -148,7 +160,7 @@ def configure_git_hooks(self) -> str | None: return ".githooks" def init_workspace(self) -> dict[str, Any]: - self.ensure_layout() + self.ensure_layout(sync=True) hooks_path = self.configure_git_hooks() return { "status": "ready", @@ -288,9 +300,10 @@ def next_pending_phase_id(self, phases_payload: dict[str, Any]) -> str | None: ) return pending[0]["id"] if pending else None - def infer_active_task(self) -> str | None: + def task_ids_by_state(self, states: set[str]) -> list[str]: if not self.tasks_dir.exists(): - return None + return [] + active = [] for candidate in sorted(self.tasks_dir.iterdir()): if not candidate.is_dir(): @@ -299,8 +312,61 @@ def infer_active_task(self) -> str | None: if not task_file.exists(): continue task = read_json(task_file) - if task.get("state") in {"approved", "in_progress", "failed", "blocked", "review_ready"}: - active.append(task["id"]) + if task.get("state") in states: + active.append(task.get("id") or candidate.name) + return active + + def active_task_ids(self) -> list[str]: + return self.task_ids_by_state(ACTIVE_TASK_STATES) + + def infer_active_task(self) -> str | None: + active = self.active_task_ids() + if len(active) == 1: + return active[0] + return None + + def task_matches_changed_paths(self, task: dict[str, Any], phase: dict[str, Any], changed_paths: list[str]) -> bool: + scopes = phase_scope(task["id"], phase) + return any(any(scope_matches(path, scope) for scope in scopes) for path in changed_paths) + + def scoped_task_ids(self, changed_paths: list[str], *, states: set[str]) -> list[str]: + if not changed_paths: + return [] + + matches: list[str] = [] + for task_id in self.task_ids_by_state(states): + task = self.load_task(task_id) + active_phase_id = task.get("active_phase_id") + if not active_phase_id: + continue + try: + _, _, phase = self.current_phase(task_id, active_phase_id) + except WorkflowError: + continue + if self.task_matches_changed_paths(task, phase, changed_paths): + matches.append(task_id) + return matches + + def infer_hook_task(self, event: str, changed_paths: list[str]) -> str | None: + if event == "pre_push": + if not changed_paths: + return None + matches = self.scoped_task_ids(changed_paths, states=PRE_PUSH_TASK_STATES) + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + raise WorkflowError( + f"{event} requires explicit --task-id or WORKFLOW_TASK_ID when changed paths match multiple tasks: {', '.join(matches)}" + ) + raise WorkflowError( + f"{event} requires explicit --task-id or WORKFLOW_TASK_ID when unpushed changes do not map to a single task" + ) + + active = self.active_task_ids() + if len(active) > 1: + raise WorkflowError( + f"{event} requires explicit --task-id or WORKFLOW_TASK_ID when multiple active tasks exist: {', '.join(active)}" + ) if len(active) == 1: return active[0] return None @@ -395,15 +461,18 @@ def run_hooks( elif guard_name == "write_scope_guard": result = write_scope_guard(task["id"] if task else None, phase, changed_paths or []) elif guard_name == "latest_verified_run_guard": - if task is None: + if task is None and event == "pre_push" and not (changed_paths or []): + result = HookResult("latest_verified_run_guard", "passed", ["no changed paths detected"]) + elif task is None: raise WorkflowError(f"{event} requires task context") - result = latest_verified_run_guard( - task, - phase, - changed_paths or [], - load_run=self.load_run, - scope_sensitive=(event == "pre_push"), - ) + else: + result = latest_verified_run_guard( + task, + phase, + changed_paths or [], + load_run=self.load_run, + scope_sensitive=(event == "pre_push"), + ) elif guard_name == "user_validation_guard": if task is None: raise WorkflowError("pre_complete requires task context") @@ -443,10 +512,6 @@ def start_phase(self, task_id: str, phase_id: str | None) -> None: if phase["status"] not in {"pending", "failed", "blocked"}: raise WorkflowError(f"phase state does not allow start: {phase['status']}") - hook_result = self.run_hooks("pre_phase_start", task=task, phase=phase) - if hook_result.status != "passed": - raise WorkflowError("; ".join(hook_result.messages)) - self.update_phase_state(phase, "in_progress") task["state"] = "in_progress" task["active_phase_id"] = phase["id"] @@ -572,8 +637,8 @@ def verification_fingerprint(self, phase: dict[str, Any], command: str) -> str: def verify_task(self, task_id: str, phase_id: str | None, commands: list[str] | None, evidence: list[str]) -> int: task, phases_payload, phase = self.current_phase(task_id, phase_id) - if phase["status"] not in {"completed", "in_progress"}: - raise WorkflowError("verification requires phase status completed or in_progress") + if phase["status"] != "completed": + raise WorkflowError("verification requires phase status completed") command_list = commands or phase["acceptance"]["commands"] if not command_list: @@ -735,7 +800,7 @@ def reopen_task(self, task_id: str, note: str, phase_id: str | None) -> None: target_phase = candidate break - if target_phase and target_phase["status"] in {"failed", "blocked"}: + if target_phase and target_phase["status"] in {"failed", "blocked", "completed"}: self.update_phase_state(target_phase, "pending") task["state"] = "approved" @@ -775,10 +840,25 @@ def check_all(self) -> list[str]: task = self.load_task(task_id) phases = self.load_phases(task_id) spec = inspect_spec(self.spec_path(task_id)) - if task["active_phase_id"] is not None and not any( - phase["id"] == task["active_phase_id"] for phase in phases["phases"] - ): - raise WorkflowError("active phase does not exist") + active_phase = None + if task["active_phase_id"] is not None: + active_phase = next((phase for phase in phases["phases"] if phase["id"] == task["active_phase_id"]), None) + if active_phase is None: + raise WorkflowError("active phase does not exist") + elif task["state"] != "draft" and phases["phases"]: + raise WorkflowError("planned task requires active phase") + + if active_phase is not None: + if task["state"] == "approved" and active_phase["status"] != "pending": + raise WorkflowError("approved task requires active phase status pending") + if task["state"] == "in_progress" and active_phase["status"] not in {"in_progress", "completed"}: + raise WorkflowError("in_progress task requires active phase status in_progress or completed") + if task["state"] == "failed" and active_phase["status"] != "failed": + raise WorkflowError("failed task requires active phase status failed") + if task["state"] == "blocked" and active_phase["status"] != "blocked": + raise WorkflowError("blocked task requires active phase status blocked") + if task["state"] in {"review_ready", "completed"} and active_phase["status"] != "completed": + raise WorkflowError(f"{task['state']} task requires active phase status completed") if task["state"] != "draft": if not spec["ready_for_approval"]: raise WorkflowError("approved-or-later task requires approval-ready spec.md") diff --git a/tests/test_workflow_cli.py b/tests/test_workflow_cli.py index a2e496d..d266368 100644 --- a/tests/test_workflow_cli.py +++ b/tests/test_workflow_cli.py @@ -2,6 +2,7 @@ import json import os +import shutil import subprocess import tempfile import unittest @@ -34,11 +35,54 @@ def run_cli(self, root: Path, *args: str, expected: int = 0, stdin: str | None = def read_json(self, path: Path) -> dict: return json.loads(path.read_text(encoding="utf-8")) + def run_hook_script( + self, + root: Path, + hook_name: str, + *args: str, + expected: int = 0, + env_extra: dict[str, str] | None = None, + ) -> subprocess.CompletedProcess[str]: + env = os.environ.copy() + env["WORKFLOW_ROOT"] = str(root) + if env_extra: + env.update(env_extra) + result = subprocess.run( + [str(root / ".githooks" / hook_name), *args], + cwd=root, + env=env, + capture_output=True, + text=True, + ) + self.assertEqual( + result.returncode, + expected, + msg=f"stdout={result.stdout}\nstderr={result.stderr}", + ) + return result + def init_git_repo(self, root: Path) -> None: subprocess.run(["git", "init"], cwd=root, capture_output=True, text=True, check=True) subprocess.run(["git", "config", "user.email", "test@example.com"], cwd=root, capture_output=True, text=True, check=True) subprocess.run(["git", "config", "user.name", "Workflow Test"], cwd=root, capture_output=True, text=True, check=True) + def install_runtime_surface(self, root: Path) -> None: + shutil.copytree( + REPO_ROOT / "scripts", + root / "scripts", + dirs_exist_ok=True, + ignore=shutil.ignore_patterns("__pycache__", "*.pyc"), + ) + + def git_add(self, root: Path, *paths: str) -> None: + subprocess.run(["git", "add", *paths], cwd=root, capture_output=True, text=True, check=True) + + def git_commit(self, root: Path, message: str) -> None: + subprocess.run(["git", "commit", "-m", message], cwd=root, capture_output=True, text=True, check=True) + + def git_commit_no_verify(self, root: Path, message: str) -> None: + subprocess.run(["git", "commit", "--no-verify", "-m", message], cwd=root, capture_output=True, text=True, check=True) + def git_config(self, root: Path, key: str) -> str: result = subprocess.run( ["git", "config", "--local", "--get", key], @@ -180,6 +224,7 @@ def write_repo_docs(self, root: Path) -> None: "- TDD Guard\n" "- Dangerous Command Guard\n" "- Circuit Breaker\n" + "- workflow.py hook\n" ), encoding="utf-8", ) @@ -366,6 +411,160 @@ def test_review_requires_last_verified_run_and_closeout(self) -> None: self.assertIsNotNone(task["last_verified_run_id"]) self.assertNotEqual(task["latest_run_id"], task["last_verified_run_id"]) + def test_verify_requires_completed_phase(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-005a", + test_policy_mode="evidence_only", + test_policy_evidence=["verification should only run after phase completion"], + ) + self.run_cli(root, "run", "task-005a", "--start") + + result = self.run_cli(root, "verify", "task-005a", expected=1) + self.assertIn("verification requires phase status completed", result.stderr) + + task = self.read_json(root / "workflows" / "tasks" / "task-005a" / "task.json") + phases = self.read_json(root / "workflows" / "tasks" / "task-005a" / "phases.json") + self.assertIsNone(task["last_verified_run_id"]) + self.assertEqual(task["state"], "in_progress") + self.assertEqual(phases["phases"][0]["status"], "in_progress") + + def test_hook_post_change_blocks_missing_tests_without_evidence(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task(root, "task-005b") + self.run_cli(root, "run", "task-005b", "--start") + + result = self.run_cli( + root, + "hook", + "post_change", + "--task-id", + "task-005b", + "--phase-id", + "phase-1", + "--changed-path", + "scripts/workflow.py", + expected=1, + ) + payload = json.loads(result.stdout) + self.assertEqual(payload["status"], "failed") + self.assertTrue(any("implementation changes require tests" in message for message in payload["messages"])) + + def test_hook_pre_phase_complete_blocks_out_of_scope_paths(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-005c", + test_policy_mode="evidence_only", + test_policy_evidence=["scope guard should still reject out-of-scope files"], + ) + self.run_cli(root, "run", "task-005c", "--start") + + result = self.run_cli( + root, + "hook", + "pre_phase_complete", + "--task-id", + "task-005c", + "--phase-id", + "phase-1", + "--changed-path", + "docs/README.md", + expected=1, + ) + payload = json.loads(result.stdout) + self.assertEqual(payload["status"], "failed") + self.assertTrue(any("outside allowed_write_paths" in message for message in payload["messages"])) + + def test_hook_pre_review_requires_and_then_accepts_verification(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task(root, "task-005d") + self.run_cli(root, "run", "task-005d", "--start") + self.run_cli( + root, + "run", + "task-005d", + "--complete", + "--changed-path", + "scripts/workflow.py", + "--changed-path", + "tests/test_workflow_cli.py", + ) + + fail_result = self.run_cli( + root, + "hook", + "pre_review", + "--task-id", + "task-005d", + "--phase-id", + "phase-1", + expected=1, + ) + fail_payload = json.loads(fail_result.stdout) + self.assertEqual(fail_payload["status"], "failed") + self.assertTrue(any("no passed verification" in message for message in fail_payload["messages"])) + + self.run_cli(root, "verify", "task-005d") + pass_result = self.run_cli( + root, + "hook", + "pre_review", + "--task-id", + "task-005d", + "--phase-id", + "phase-1", + ) + pass_payload = json.loads(pass_result.stdout) + self.assertEqual(pass_payload["status"], "passed") + self.assertTrue(any("latest passed verification" in message for message in pass_payload["messages"])) + + def test_hook_pre_complete_requires_user_validation_note(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-005e", + test_policy_mode="evidence_only", + test_policy_evidence=["pre_complete should require explicit validation note"], + ) + + fail_result = self.run_cli(root, "hook", "pre_complete", "--task-id", "task-005e", expected=1) + fail_payload = json.loads(fail_result.stdout) + self.assertEqual(fail_payload["status"], "failed") + self.assertTrue(any("user validation note is required" in message for message in fail_payload["messages"])) + + pass_result = self.run_cli( + root, + "hook", + "pre_complete", + "--task-id", + "task-005e", + "--user-validation-note", + "validated", + ) + pass_payload = json.loads(pass_result.stdout) + self.assertEqual(pass_payload["status"], "passed") + self.assertTrue(any("user validation provided" in message for message in pass_payload["messages"])) + + def test_hook_unknown_pre_phase_start_fails_after_surface_cleanup(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-005f", + test_policy_mode="evidence_only", + test_policy_evidence=["pre_phase_start event has been removed from the canonical hook surface"], + ) + + result = self.run_cli(root, "hook", "pre_phase_start", "--task-id", "task-005f", "--phase-id", "phase-1", expected=1) + self.assertIn("unknown hook event", result.stderr) + def test_pre_push_requires_verification_for_active_phase_scope(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -414,6 +613,255 @@ def test_pre_push_requires_verification_for_active_phase_scope(self) -> None: "git push origin feature", ) + def test_hook_requires_explicit_task_when_active_task_is_ambiguous(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-006a", + test_policy_mode="evidence_only", + test_policy_evidence=["ambiguous active-task detection should fail closed"], + ) + self.bootstrap_task( + root, + "task-006b", + test_policy_mode="evidence_only", + test_policy_evidence=["second active task makes hook inference ambiguous"], + ) + + commit_result = self.run_cli( + root, + "hook", + "pre_commit", + "--changed-path", + "scripts/workflow.py", + expected=1, + ) + self.assertIn("WORKFLOW_TASK_ID", commit_result.stderr) + + push_result = self.run_cli( + root, + "hook", + "pre_push", + "--changed-path", + "scripts/workflow.py", + "--command-text", + "git push origin feature", + expected=1, + ) + self.assertIn("WORKFLOW_TASK_ID", push_result.stderr) + + def test_githook_pre_commit_blocks_implementation_change_without_tests(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task(root, "task-006c") + + scripts_dir = root / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + self.git_add(root, "scripts/sample.py") + + result = self.run_hook_script(root, "pre-commit", expected=1) + self.assertIn("implementation changes require tests", result.stdout) + + def test_githook_pre_commit_passes_with_matching_test_change(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task(root, "task-006d") + + scripts_dir = root / "scripts" + tests_dir = root / "tests" + scripts_dir.mkdir(parents=True, exist_ok=True) + tests_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + (tests_dir / "test_sample.py").write_text("def test_sample():\n assert True\n", encoding="utf-8") + self.git_add(root, "scripts/sample.py", "tests/test_sample.py") + + result = self.run_hook_script(root, "pre-commit") + self.assertIn("test changes detected", result.stdout) + + def test_githook_pre_push_blocks_force_push(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + + result = self.run_hook_script(root, "pre-push", "origin", "feature", "--force", expected=1) + self.assertIn("blocked command", result.stdout) + + def test_githook_pre_push_requires_verification_for_unpushed_scope_changes(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task( + root, + "task-006e", + test_policy_mode="evidence_only", + test_policy_evidence=["pre-push script should enforce latest verification on unpushed scope changes"], + ) + + scripts_dir = root / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + self.git_add(root, "scripts/sample.py") + self.git_commit(root, "hook pre-push smoke") + + result = self.run_hook_script(root, "pre-push", "origin", "feature", expected=1) + self.assertIn("no passed verification recorded for active phase", result.stdout) + + def test_githook_pre_push_passes_after_phase_verification(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task( + root, + "task-006f", + test_policy_mode="evidence_only", + test_policy_evidence=["verified unpushed scope changes should pass pre-push"], + ) + + scripts_dir = root / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + self.run_cli(root, "run", "task-006f", "--start") + self.run_cli( + root, + "run", + "task-006f", + "--complete", + "--changed-path", + "scripts/sample.py", + ) + self.run_cli(root, "verify", "task-006f") + + self.git_add(root, "scripts/sample.py") + self.git_commit(root, "verified pre-push smoke") + + result = self.run_hook_script(root, "pre-push", "origin", "feature") + self.assertIn("latest passed verification", result.stdout) + + def test_githook_pre_push_passes_for_completed_task_scope(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task( + root, + "task-006g", + test_policy_mode="evidence_only", + test_policy_evidence=["completed task scope should still bind pre-push verification"], + ) + + scripts_dir = root / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + self.run_cli(root, "run", "task-006g", "--start") + self.run_cli( + root, + "run", + "task-006g", + "--complete", + "--changed-path", + "scripts/sample.py", + ) + self.run_cli(root, "verify", "task-006g") + self.run_cli(root, "review", "task-006g", "--note", "ready") + self.run_cli(root, "review", "task-006g", "--close", "--user-validation-note", "validated") + + self.git_add(root, "scripts/sample.py", "workflows/tasks/task-006g") + self.git_commit_no_verify(root, "completed task pre-push smoke") + + result = self.run_hook_script(root, "pre-push", "origin", "feature") + self.assertIn("latest passed verification", result.stdout) + + def test_githook_pre_push_prefers_scoped_completed_task_over_unrelated_active_task(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task( + root, + "task-006h", + test_policy_mode="evidence_only", + test_policy_evidence=["completed task scope should remain usable even when another task is active"], + ) + scripts_dir = root / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + self.run_cli(root, "run", "task-006h", "--start") + self.run_cli( + root, + "run", + "task-006h", + "--complete", + "--changed-path", + "scripts/sample.py", + ) + self.run_cli(root, "verify", "task-006h") + self.run_cli(root, "review", "task-006h", "--note", "ready") + self.run_cli(root, "review", "task-006h", "--close", "--user-validation-note", "validated") + + self.git_add(root, "scripts/sample.py", "workflows/tasks/task-006h") + self.git_commit_no_verify(root, "completed task with unrelated active task") + + self.bootstrap_task( + root, + "task-006i", + allowed_write_paths=["docs/"], + test_policy_mode="evidence_only", + test_policy_evidence=["unrelated active task should not steal pre-push context"], + ) + + result = self.run_hook_script(root, "pre-push", "origin", "feature") + self.assertIn("latest passed verification", result.stdout) + + def test_githook_pre_push_passes_without_task_when_no_unpushed_changes(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + + result = self.run_hook_script(root, "pre-push", "origin", "feature") + self.assertIn("no changed paths detected", result.stdout) + + def test_doctor_detects_runtime_surface_drift_and_init_resyncs(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + + hooks_path = root / "workflows" / "system" / "hooks.json" + pre_push_path = root / ".githooks" / "pre-push" + hooks_payload = self.read_json(hooks_path) + hooks_payload["guards"]["dangerous_cmd_guard"]["blocked_patterns"] = [] + hooks_path.write_text(json.dumps(hooks_payload, indent=2, ensure_ascii=False) + "\n", encoding="utf-8") + pre_push_path.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + + drift_result = self.run_cli(root, "doctor", expected=1) + drift_report = json.loads(drift_result.stdout) + self.assertTrue(any("hooks.json is out of sync" in error for error in drift_report["errors"])) + self.assertTrue(any(".githooks/pre-push is out of sync" in error for error in drift_report["errors"])) + + self.run_cli(root, "init") + self.assertEqual(hooks_path.read_text(encoding="utf-8"), (REPO_ROOT / "workflows" / "system" / "hooks.json").read_text(encoding="utf-8")) + self.assertEqual(pre_push_path.read_text(encoding="utf-8"), (REPO_ROOT / ".githooks" / "pre-push").read_text(encoding="utf-8")) + self.run_cli(root, "doctor") + def test_reopen_resets_failed_task_for_repair_loop(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -445,6 +893,39 @@ def test_reopen_resets_failed_task_for_repair_loop(self) -> None: self.assertEqual(task["active_phase_id"], "phase-1") self.assertEqual(phases["phases"][0]["status"], "pending") + def test_reopen_resets_completed_task_for_follow_up(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-007a", + test_policy_mode="evidence_only", + test_policy_evidence=["completed task should reopen into a rerunnable phase"], + ) + self.run_cli(root, "run", "task-007a", "--start") + self.run_cli( + root, + "run", + "task-007a", + "--complete", + "--changed-path", + "scripts/workflow.py", + ) + self.run_cli(root, "verify", "task-007a") + self.run_cli(root, "review", "task-007a", "--note", "ready") + self.run_cli(root, "review", "task-007a", "--close", "--user-validation-note", "validated") + self.run_cli(root, "reopen", "task-007a", "--note", "follow-up change") + + task = self.read_json(root / "workflows" / "tasks" / "task-007a" / "task.json") + phases = self.read_json(root / "workflows" / "tasks" / "task-007a" / "phases.json") + self.assertEqual(task["state"], "approved") + self.assertEqual(task["active_phase_id"], "phase-1") + self.assertIsNone(task["last_verified_run_id"]) + self.assertFalse(task["user_validated"]) + self.assertEqual(phases["phases"][0]["status"], "pending") + + self.run_cli(root, "run", "task-007a", "--start") + def test_circuit_breaker_triggers_from_verification_failures(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -545,6 +1026,30 @@ def test_init_and_doctor_enforce_git_hooks_path(self) -> None: report = json.loads(result.stdout) self.assertTrue(any("core.hooksPath" in error for error in report["errors"])) + def test_check_fails_when_approved_task_points_to_completed_phase(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.bootstrap_task( + root, + "task-010", + test_policy_mode="evidence_only", + test_policy_evidence=["artifact consistency check should reject non-startable approved phase"], + ) + + task_path = root / "workflows" / "tasks" / "task-010" / "task.json" + task = self.read_json(task_path) + task["state"] = "approved" + task_path.write_text(json.dumps(task, indent=2, ensure_ascii=False) + "\n", encoding="utf-8") + + phases_path = root / "workflows" / "tasks" / "task-010" / "phases.json" + phases = self.read_json(phases_path) + phases["phases"][0]["status"] = "completed" + phases_path.write_text(json.dumps(phases, indent=2, ensure_ascii=False) + "\n", encoding="utf-8") + + result = self.run_cli(root, "status", "--check", "--all", expected=1) + payload = json.loads(result.stdout) + self.assertTrue(any("approved task requires active phase status pending" in error for error in payload["errors"])) + def test_doctor_fails_when_runbook_loses_socratic_contract(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) diff --git a/workflows/system/hooks.json b/workflows/system/hooks.json index 0613bbf..13e5fab 100644 --- a/workflows/system/hooks.json +++ b/workflows/system/hooks.json @@ -8,7 +8,6 @@ "write_scope_guard", "tdd_guard" ], - "pre_phase_start": [], "post_change": [ "tdd_guard" ], @@ -70,8 +69,7 @@ "git\\s+push(?:\\s+\\S+)*\\s+-[A-Za-z]*f[A-Za-z]*", "DROP\\s+TABLE", "TRUNCATE\\s+TABLE" - ], - "allowlist": [] + ] }, "latest_verified_run_guard": { "require_phase_match": true, From 5b17ee5ff71a0c5bb10ebb0088b2e7e46ee04731 Mon Sep 17 00:00:00 2001 From: hyoseok Date: Thu, 16 Apr 2026 13:24:14 +0900 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20pre-push=20task=20=EC=B6=94=EB=A1=A0?= =?UTF-8?q?=EC=97=90=20=EC=A0=84=EC=B2=B4=20diff=20=EB=A7=A4=ED=95=91?= =?UTF-8?q?=EC=9D=84=20=EA=B0=95=EC=A0=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/workflow_runtime/engine.py | 6 ++--- tests/test_workflow_cli.py | 38 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/scripts/workflow_runtime/engine.py b/scripts/workflow_runtime/engine.py index 93a36fe..d71716c 100644 --- a/scripts/workflow_runtime/engine.py +++ b/scripts/workflow_runtime/engine.py @@ -325,9 +325,9 @@ def infer_active_task(self) -> str | None: return active[0] return None - def task_matches_changed_paths(self, task: dict[str, Any], phase: dict[str, Any], changed_paths: list[str]) -> bool: + def task_covers_changed_paths(self, task: dict[str, Any], phase: dict[str, Any], changed_paths: list[str]) -> bool: scopes = phase_scope(task["id"], phase) - return any(any(scope_matches(path, scope) for scope in scopes) for path in changed_paths) + return all(any(scope_matches(path, scope) for scope in scopes) for path in changed_paths) def scoped_task_ids(self, changed_paths: list[str], *, states: set[str]) -> list[str]: if not changed_paths: @@ -343,7 +343,7 @@ def scoped_task_ids(self, changed_paths: list[str], *, states: set[str]) -> list _, _, phase = self.current_phase(task_id, active_phase_id) except WorkflowError: continue - if self.task_matches_changed_paths(task, phase, changed_paths): + if self.task_covers_changed_paths(task, phase, changed_paths): matches.append(task_id) return matches diff --git a/tests/test_workflow_cli.py b/tests/test_workflow_cli.py index d266368..e141107 100644 --- a/tests/test_workflow_cli.py +++ b/tests/test_workflow_cli.py @@ -828,6 +828,44 @@ def test_githook_pre_push_prefers_scoped_completed_task_over_unrelated_active_ta result = self.run_hook_script(root, "pre-push", "origin", "feature") self.assertIn("latest passed verification", result.stdout) + def test_githook_pre_push_fails_when_unpushed_diff_only_partially_maps_to_task(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + self.init_git_repo(root) + self.install_runtime_surface(root) + self.run_cli(root, "init") + self.bootstrap_task( + root, + "task-006j", + test_policy_mode="evidence_only", + test_policy_evidence=["pre-push inference must require the full unpushed diff to map to one task"], + ) + + scripts_dir = root / "scripts" + notes_dir = root / "notes" + scripts_dir.mkdir(parents=True, exist_ok=True) + notes_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "sample.py").write_text("print('sample')\n", encoding="utf-8") + (notes_dir / "mixed.txt").write_text("not covered by any task scope\n", encoding="utf-8") + self.run_cli(root, "run", "task-006j", "--start") + self.run_cli( + root, + "run", + "task-006j", + "--complete", + "--changed-path", + "scripts/sample.py", + ) + self.run_cli(root, "verify", "task-006j") + self.run_cli(root, "review", "task-006j", "--note", "ready") + self.run_cli(root, "review", "task-006j", "--close", "--user-validation-note", "validated") + + self.git_add(root, "scripts/sample.py", "notes/mixed.txt", "workflows/tasks/task-006j") + self.git_commit_no_verify(root, "mixed scope pre-push smoke") + + result = self.run_hook_script(root, "pre-push", "origin", "feature", expected=1) + self.assertIn("do not map to a single task", result.stderr) + def test_githook_pre_push_passes_without_task_when_no_unpushed_changes(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp)