From d1526248c65bd8463cfc0c9f3208872b9bb05a61 Mon Sep 17 00:00:00 2001 From: SoraSuegami Date: Mon, 6 Jul 2026 10:44:24 +0900 Subject: [PATCH] Remove obsolete code paths --- .codex/config.toml | 1 - .codex/hooks.json | 30 - AGENTS.md | 19 +- BUILDER.md | 118 +- PLANS.md | 108 -- REVIEWER.md | 171 +-- plans/.gitkeep | 1 - schemas/review-decision.schema.json | 15 - schemas/session-start-intent.schema.json | 15 - scripts/lib/codex_workflow/__init__.py | 26 - scripts/lib/codex_workflow/atomic.py | 29 - scripts/lib/codex_workflow/cli.py | 41 - scripts/lib/codex_workflow/hooks.py | 603 --------- scripts/lib/codex_workflow/paths.py | 93 -- scripts/lib/codex_workflow/plan.py | 313 ----- scripts/lib/codex_workflow/runners.py | 244 ---- scripts/lib/codex_workflow/tests/__init__.py | 1 - .../lib/codex_workflow/tests/test_workflow.py | 1180 ----------------- scripts/lib/codex_workflow/transcript.py | 88 -- .../tests/test_repo_validation.py | 0 scripts/run_build_checks.sh | 12 - scripts/run_tests.sh | 2 +- scripts/session_start_hook.sh | 8 - scripts/stop_hook.sh | 8 - 24 files changed, 96 insertions(+), 3030 deletions(-) delete mode 100644 .codex/hooks.json delete mode 100644 PLANS.md delete mode 100644 plans/.gitkeep delete mode 100644 schemas/review-decision.schema.json delete mode 100644 schemas/session-start-intent.schema.json delete mode 100644 scripts/lib/codex_workflow/__init__.py delete mode 100644 scripts/lib/codex_workflow/atomic.py delete mode 100644 scripts/lib/codex_workflow/cli.py delete mode 100644 scripts/lib/codex_workflow/hooks.py delete mode 100644 scripts/lib/codex_workflow/paths.py delete mode 100644 scripts/lib/codex_workflow/plan.py delete mode 100644 scripts/lib/codex_workflow/runners.py delete mode 100644 scripts/lib/codex_workflow/tests/__init__.py delete mode 100644 scripts/lib/codex_workflow/tests/test_workflow.py delete mode 100644 scripts/lib/codex_workflow/transcript.py rename scripts/lib/{codex_workflow => }/tests/test_repo_validation.py (100%) delete mode 100755 scripts/run_build_checks.sh delete mode 100755 scripts/session_start_hook.sh delete mode 100755 scripts/stop_hook.sh diff --git a/.codex/config.toml b/.codex/config.toml index 8492adb8..31eef7f8 100644 --- a/.codex/config.toml +++ b/.codex/config.toml @@ -1,3 +1,2 @@ [features] multi_agent = true -codex_hooks = true diff --git a/.codex/hooks.json b/.codex/hooks.json deleted file mode 100644 index 0ff3209e..00000000 --- a/.codex/hooks.json +++ /dev/null @@ -1,30 +0,0 @@ -{ - "hooks": { - "SessionStart": [ - { - "matcher": ".*", - "hooks": [ - { - "type": "command", - "command": "bash scripts/session_start_hook.sh", - "timeout": 30, - "statusMessage": "Initializing session workflow state" - } - ] - } - ], - "Stop": [ - { - "matcher": ".*", - "hooks": [ - { - "type": "command", - "command": "bash scripts/stop_hook.sh", - "timeout": 10800, - "statusMessage": "Running session completion gates" - } - ] - } - ] - } -} \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index 4373e17c..2385cec6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,25 +1,18 @@ # Repository Guidelines ## Repository Purpose -This repository provides implementations for lattice-cryptography operations (polynomail and matrix operations, preimage samplings, BGG+ encodings, and more), written in Rust and CUDA. +This repository provides implementations for lattice-cryptography operations (polynomial and matrix operations, preimage samplings, BGG+ encodings, and more), written in Rust and CUDA. ## Global Requirements - All documentation in this repository, along with git commit messages and PRs, must be written in English. - When documenting file paths, use only paths relative to the repository top directory. Do not write absolute paths in documentation. - Directories named `references` are read-only reference directories for humans and agents. Agents may read them when relevant and must never edit them. -- Integration tests must not be run unless the user has explicitly asked for them in the current session. Prefer targeted unit tests or other narrow validation until such approval is given. +- Integration tests must not be run unless the user has explicitly asked for them in the current task. Prefer targeted unit tests or other narrow validation until such approval is given. - Rust formatting must use `cargo +nightly fmt --all`. -- Follow a lifecycle defined in `BUILDER.md`. +- Follow the builder/reviewer guidance in `BUILDER.md` and `REVIEWER.md`. ## Codex Workflow -This repository uses a long-running Codex session workflow governed by `BUILDER.md`, `PLANS.md`, and `REVIEWER.md`. - Before starting any task or reading any other files, read and follow `REVIEWER.md` for explicit review tasks and `BUILDER.md` for all other tasks. -- Follow `PLANS.md` to create and update a plan document for each session. -- Discover the current session id from the handoff or hook payload. -- Treat `## Plan approval` and `## Phase` in the session plan as the workflow state. -- The builder behaves differently by plan approval status: -- - When `## Plan approval` is `unapproved`, stay in planning: interview the user, revise the session plan, and ask for explicit approval. -- - When `## Plan approval` is `approved` and `## Phase` is `implementation`, execute the approved subtasks, run the narrowest relevant local validation after each subtask, and only then check the subtask off. The implementation-phase stop hook checks only unfinished tasks plus formatting/build. -- - When `## Plan approval` is `approved` and `## Phase` is `testing`, run or address the selected unit-test gate before entering review. -- - When `## Plan approval` is `approved` and `## Phase` is `review`, address only the concrete follow-up work created by reviewer feedback, keeping completed historical subtasks intact. -- Keep the session plan in sync with the work; detailed phase transitions and hook behavior live in the documents above. +- Builders should clarify unclear requirements, implement scoped changes once requirements are clear, and run the narrowest relevant validation. +- Reviewers should stay read-only, prioritize correctness and regression risk, and ground findings in concrete files, commands, or logs. +- If the work touches CUDA, GPU kernels, GPU wrappers, GPU tests, or GPU-facing performance-sensitive behavior, read `GPU.md` before editing or reviewing. diff --git a/BUILDER.md b/BUILDER.md index 1bc29070..d84702af 100644 --- a/BUILDER.md +++ b/BUILDER.md @@ -2,59 +2,71 @@ Unless the user explicitly asks for review, act as the builder. -At the start of each turn: -1. Obtain the session id from the explicit handoff or hook payload for the current run. -2. Open `plans/active/session-.md`. -3. Inspect `## Plan approval` and `## Phase`. -4. If `## Plan approval` is `unapproved`, stay in planning and keep refining the plan with the user. -5. If `## Plan approval` is `approved`, follow the approved-phase rules for the current `## Phase`. -6. Act according to the rules below. - -## Planning -- Do not implement code yet while `## Plan approval` is `unapproved`. -- Create or update the session-specific plan at `plans/active/session-.md`. -- Discuss the plan with the user and keep interviewing until the scope, constraints, and validation are concrete. -- Keep `## Phase` at `planning` while the plan is still unapproved. -- Update the plan document before ending the turn. -- Ask the user to approve the plan or describe specific revisions. -- The stop hook does nothing special during planning; it simply allows the stop. -- When the user explicitly approves the current plan, update `## Plan approval` from `unapproved` to `approved`, set `## Phase` to `implementation`, record the approval in the plan log, and begin implementation in the same session. - -## Implementation -- Implementation corresponds to `## Plan approval` being `approved` and `## Phase` being `implementation`. -- Read the session plan and work through subtasks in order. -- If the current work touches CUDA, GPU kernels, GPU wrappers, GPU tests, or GPU-facing performance-sensitive behavior, read [GPU.md](GPU.md) and follow its principles. +## Builder Lifecycle +- Read `REVIEWER.md` for explicit review requests; otherwise follow this document. +- Clarify requirements before editing when the requested behavior, scope, or validation target is ambiguous. +- Prefer concrete implementation work once the request is clear. Do not stop at a proposal unless the user asked for one or a blocker remains. +- If the current work touches CUDA, GPU kernels, GPU wrappers, GPU tests, or GPU-facing performance-sensitive behavior, read `GPU.md` and follow its principles before editing. +- Run the narrowest relevant local validation after each meaningful change. Do not run integration tests unless the user explicitly requested them for the current task. - When Rust formatting is needed, use `cargo +nightly fmt --all`. -- After finishing a subtask, run the narrowest relevant local validation immediately. -- Do not run integration tests unless the user has explicitly instructed you to do so in the current session. Until then, keep validation limited to the narrowest relevant tests. -- Only after the related tests pass, mark that subtask checkbox as checked. -- Update the per-subtask validation, progress log, and decision log as work proceeds. -- If build checks, testing, or reviewer feedback create new obligations, do NOT rewrite existing subtasks or erase completed checkmarks. -- Instead, append NEW unchecked follow-up subtasks under `## Follow-up subtasks (append-only)` and continue implementing those follow-up subtasks. -- Expect the outer stop hook to block the current turn when the approved session plan still has unchecked work or when implementation formatting/build checks append new follow-up tasks. If all tracked checkboxes are already checked and formatting/build checks pass or are not selected, the implementation-phase stop hook may accept without running unit tests or the reviewer. -- When implementation is complete and formatting/build checks have passed, update `## Phase` to `testing`, record that transition in the plan log, and stop so the testing-phase stop hook runs the selected final tests. -- If independent subtasks do not share files or mutable context, parallelize them with sub agents. -- If subtasks share files or shared mutable context, do not parallelize them. +- If independent subtasks do not share files or mutable context, parallelize them with subagents. If they share files or shared mutable context, keep the work serialized. +- Do not end the turn before the job is actually complete, including implementation, validation, and any needed documentation updates. + +## Design & Code Style +- Modify existing functions and types instead of adding variants (`new_with_*`, `*_with_shared_inputs`, wrapper structs/traits). Extend `new` with `Option` arguments when needed. +- No backward compatibility: delete legacy formats, fallbacks, and version markers outright. +- Reuse existing environment variables; define all env vars in `env.rs` with explanatory comments. +- Inline private functions that are called once or only a few lines long. Keep files under ~2000 lines excluding tests. +- Delete unused code, arguments, and imports immediately; never silence warnings with `_`. Remove all debug-only scaffolding (extra syncs, flags, timing logs) before finishing. +- Rename anything misleading; names must describe what the value or bound is, not which paper theorem it came from. +- GPU-only code lives in files with "gpu" in the name. CUDA headers (`.cuh`) declare only cross-file and Rust-facing functions; bodies go in `cuda/src/*.cu`. + +## Parallelism & Performance +- Parallelize every loop with rayon unless ownership or peak memory forbids it. Refactors must never reduce parallelism unless the user explicitly requests lower or configurable parallelism, for example to reduce peak memory usage. +- Never fix races or flakiness by adding mutexes or serializing; find the root cause instead. +- Control concurrency with env-var-configured batch sizes over `par_iter`, not `ThreadPoolBuilder`. +- Prefer in-place ops (`add_in_place`, etc.) and ownership transfer over `clone`; precompute loop-invariant constants once. +- After performance-sensitive changes, compare timings against the previous run; investigate regressions with per-stage `tracing` logs, then remove the temporary logs. + +## GPU / CUDA +- Never fall back to CPU to work around a failing GPU path; a GPU failure is an error. +- No device-wide sync (`cudaDeviceSynchronize`); use per-stream events and async APIs, and keep the host non-blocking. Avoid sync-including calls (e.g. `to_compact_bytes`) in hot paths. +- Batch kernel launches and allocations; never launch per element or per limb. +- Multi-GPU: enumerate devices via `detected_gpu_device_ids` (not a fixed `gpu_id` in params) and distribute work evenly; keep all limbs of a matrix on one device; load shared data onto each device once, before loops. +- Matrices stay in evaluation format by default; never compare or concatenate mixed formats without NTT alignment. +- Peak VRAM/RAM must scale with the configured parallelism (env var), never with `num_slots` or total gate count. Matrices of order `d x m_b` or `d x m_g` are acceptable; `m_b^2`, `m_g^2`, and `m_b x m_g` are not; chunk, stream, and store to disk instead. +- Drop large data as early as possible; generate it just before use; pipeline load -> compute -> store. ## Testing -- Testing corresponds to `## Plan approval` being `approved` and `## Phase` being `testing`. -- Treat this as the unit-test gate between implementation and review. -- Keep using the same approved session plan. Do not reset `## Phase` back to `implementation`. -- Expect the outer stop hook to run the selected final tests using the same changed-file selection policy as `scripts/run_tests.sh`. -- If testing-phase final tests fail, append NEW unchecked follow-up subtasks and return to implementation work for those follow-ups. -- When testing-phase final tests pass or are not selected, update `## Phase` to `review`, record that transition in the plan log, and stop so the review-phase stop hook runs the read-only reviewer. - -## Review -- Review corresponds to `## Plan approval` being `approved` and `## Phase` being `review`. -- Treat this as the post-testing acceptance loop: address only the concrete follow-up work created by reviewer feedback, keeping completed historical subtasks intact. -- Keep using the same approved session plan. Do not reset `## Phase` back to `implementation`. -- Expect the outer stop hook to run the hooks-disabled read-only reviewer on every stop while the phase remains `review`. -- If the reviewer returns `revision`, append NEW unchecked follow-up subtasks and continue until the review-phase stop hook can finish with reviewer `accept`. -- If reviewer follow-up work changes code or testable behavior, move the plan back to `testing` before final review so the selected final tests run again. - -## Strong Rule -- Do not end the turn before the job is actually complete. -- In planning, complete the turn only after the plan has been updated and the user has been asked for approval or revisions. -- In implementation, the outer stop hook owns formatting/build checks and does not run unit tests or the reviewer. Builder turns should stop only after the plan's tracked checkboxes are fully satisfied and current build feedback has been incorporated. -- In testing, the outer stop hook owns the final `scripts/run_tests.sh` selection and does not run the reviewer. Builder turns should stop only after the plan's tracked checkboxes are fully satisfied and current test feedback has been incorporated. -- In review, the outer stop hook owns the reviewer acceptance loop. Builder turns should stop only after the plan's tracked checkboxes are fully satisfied and current reviewer feedback has been incorporated; if those gates reveal remaining work, the stop hook blocks the turn so the same session can continue addressing it. +- Use `scripts/run_tests.sh` as a reference for repository validation expectations and helper behavior. +- GPU unit tests must run outside the sandbox because the local GPU is invisible inside it. +- For CUDA/sync-related changes: compile once (`cargo test -r --no-run --features gpu`), then run the built binary directly N consecutive times with the identical command (300 for sync bugs, 3-5 for round-trip smoke checks). Run all N even if one fails and report failure counts. +- Both `cargo test -r --no-run` and the `--features gpu` variant must be warning-free. +- Test names use the established prefixes (e.g. `test_gpu_*`); tests live in the file defining the tested item. +- Test parameters must be overridable via env vars with small defaults. No fixed seeds: sample randomly per run. +- Expected values come from existing trusted primitives or round-trips, never from hand-rolled reference implementations. Never weaken or modify existing tests, or move GPU work to CPU, to make tests pass. +- Each test uses its own directory under `test_data/`; delete stale checkpoints whenever parameters or circuits change. +- Probabilistic norm tests may rarely fail by design; verify statistically over repeated runs instead of "fixing" them. + +## Benchmarks & Measurement +- Benchmark code must call the same functions as production and reproduce its dataflow, including store/load. +- Measurement contract: measure one chunk/wave; latency = one wave; total_time = latency * chunk_count * slots; max_parallelism = the max across stages, never the sum. All times in seconds. +- Log all raw values (per-stage totals, latency, max_parallelism) directly; never leave values to be derived from other logs. +- Estimators use the actual params, never defaults. +- Remote pods (H200 etc.): sync to the latest commit of the local working branch, release build, `RUST_LOG=debug`; record VRAM usage every ~3s to a log; append the command, env vars, and commit to the log; scp logs to local, then stop the pod promptly. +- Append results to the existing CSVs preserving their format, with a date column; verify CSV-vs-log consistency with a script, not by inspection. + +## Lattice-Crypto Domain Rules +- Implement strictly per the PDFs in `references/`; never alter a paper's spec to make a test pass; cite the chapter when explaining bounds. +- Error-norm simulators must stay term-by-term consistent with the evaluator implementations; document the correspondence in English comments; watch for both double-counted and missing terms. If a simulated bound is exceeded in practice, tighten the estimate; do not add `crt_depth` margins. +- Scalars such as `q/q_i` and thresholds are defined against the full modulus, regardless of `q_level`. +- Choose `crt_depth` by simulation (binary search on `eval_ok && decryption_ok`), searching near previously found values in the CSVs. +- Checkpoint ID prefixes embed all parameters; derive randomness deterministically (`hash_sampler` + fixed tags) so checkpoints stay reusable and consistent. + +## Agent Working Style +- When asked to analyze or find a root cause, do not edit or run code until asked; identify the cause at the file/line and mathematical level first. +- Every claim must cite concrete code locations; verify against actual code, diffs, and logs; never answer from assumption. +- Fix root causes, not symptoms. +- If a spec seems ambiguous, contradictory, or mathematically wrong, ask before implementing; report before changing approach. +- Long jobs: run with `nohup` + a log file, monitor to completion, and report progress and ETA; never change parameters or kill runs unprompted. +- Do not touch untracked files that existed before the task; treat the user's manual edits as authoritative and build on them. diff --git a/PLANS.md b/PLANS.md deleted file mode 100644 index bbe42b4d..00000000 --- a/PLANS.md +++ /dev/null @@ -1,108 +0,0 @@ -# PLANS.md - -This repository uses a self-contained plan style for long-running Codex work. Each session plan must be detailed enough that a new contributor, starting only from the current working tree and the single session plan file, can understand the goal, continue the work, and validate the result. - -The session plan is also the workflow state. `## Plan approval` tells the workflow whether the session is still in planning or has entered approved work, and `## Phase` tells the workflow whether approved work is in `implementation`, `testing`, or `review`. - -## General Plan Rules -- Every session plan must be fully self-contained. Do not rely on external articles, prior chat context, or undocumented repository knowledge. -- Every session plan is a living document. Update it as progress is made, discoveries occur, constraints change, or decisions are finalized. -- Write for a novice to this repository. Define non-obvious terms in plain language, restate assumptions explicitly, and name the relevant repository-relative paths, modules, and commands. -- Begin from purpose and observable outcome. The plan must explain why the work matters, what user-visible or maintainer-visible behavior should exist after the change, and how that behavior can be observed. -- Do not outsource important decisions to the reader. If there is ambiguity, resolve it in the plan and record the rationale. -- Do not point the reader to external docs for required context. If knowledge is needed to execute the plan, include that knowledge in the plan itself in your own words. -- Prefer additive, testable, low-risk changes. If a task has meaningful uncertainty, use a prototype or proof-of-concept milestone to validate feasibility before committing to a larger implementation. -- Milestones should be independently verifiable. Each milestone should say what new capability exists after it, what to run, and what success looks like. -- Validation is mandatory. State the exact commands to run, the working directory when relevant, and the concrete behavior or output that demonstrates success. -- Plans should be safe to resume. Record retry paths, rollback-safe habits, or other recovery notes whenever a step can fail partway through. -- Capture meaningful evidence concisely. When tests, logs, or command outputs matter, summarize the proof inside the plan so a future contributor can see what actually validated the work. - -## How To Use Session Plans -- When authoring a session plan, start from the required template in this file and fill it with repository-specific detail rather than placeholders. -- Explicit review-only sessions are outside this session-plan workflow: the session-start hook classifies them from the initial user prompt, does not create `plans/active/session-.md`, and the stop hook later exits without workflow coordination when both `plans/active/session-.md` and `plans/completed/session-.md` are absent. -- While `## Plan approval` is `unapproved`, the builder stays in planning, keeps `## Phase` at `planning`, revises the plan with the user, and does not implement code yet. -- When the user explicitly approves the plan, the builder updates `## Plan approval` to `approved`, sets `## Phase` to `implementation`, and starts implementation from the same plan. -- After implementation formatting/build checks pass, the builder updates `## Phase` from `implementation` to `testing` and stops immediately so the testing-phase stop-hook gate takes over. -- After testing-phase final tests pass, the builder updates `## Phase` from `testing` to `review` and stops immediately so the review-phase stop-hook gate takes over. -- Workflow helpers derive the active session strictly from each hook payload; do not rely on repository-global pointer files. -- When implementing from a session plan, proceed to the next milestone or unchecked subtask without asking the user for "next steps" unless a real blocker remains. -- Keep the plan synchronized with reality at every stop point. If a partially completed task needs to be split into "done" and "remaining" work, update the plan immediately. -- Record key design changes in the plan when they happen, not after the fact. -- If you discover unexpected behavior, performance tradeoffs, test failures, or implementation constraints, capture them in the existing log sections so the next contributor can resume without re-discovering them. -- This repository keeps a machine-checkable session format. Preserve the required headings, approval flag, and checkbox sections exactly so the workflow helpers can parse them reliably. -- This repository's existing session plan template is intentionally narrower than the generic plan skeleton. Keep that template intact, and record extra discoveries, rationale, and retrospective notes inside the existing `Decision log` and `Progress log` unless the task explicitly calls for additional prose sections. - -## Working Rules -- Narrative sections such as Goal, Constraints, Repo facts / assumptions, and Acceptance criteria should be written in plain prose first. Use bullets only when they make the plan clearer. -- When Rust formatting is needed, use `cargo +nightly fmt --all`. -- After each subtask, run the narrowest relevant local validation immediately so bugs are caught with the smallest possible change scope. -- Do not run integration tests unless the user has explicitly requested them in the current session; otherwise keep validation scoped to the narrowest relevant tests and record that limitation in the plan when it matters. -- Each subtask must be small enough that implementation plus debugging fits within one context window. -- If multiple subtasks do not touch the same files and do not require shared context, the builder should actively parallelize them with sub agents. -- Session-specific plans are living documents and must be updated throughout the task. -- Completed subtasks must remain preserved as historical record. If build checks, tests, or review find problems later, add NEW follow-up subtasks instead of rewriting prior completed work. -- Completed checkboxes must never be rewritten back into unchecked boxes. -- While `## Plan approval` is `unapproved`, the stop hook does no workflow coordination beyond allowing the stop. -- Once `## Plan approval` is `approved`, the stop hook first reevaluates the current session plan. If unchecked implementation work remains, it blocks the current turn with an actionable resume message. -- When approved work is in `implementation`, the stop hook runs only formatting and build checks after all tracked checkboxes are checked. Passing checks allow the stop without running unit tests, launching the reviewer, or archiving the session. -- When approved work is in `testing`, the stop hook runs the selected final tests after all tracked checkboxes are checked. Failed final tests append new follow-up tasks and then block the turn so the same session can continue. -- When approved work is in `review`, the stop hook runs the reviewer check after all tracked checkboxes are checked. Non-accepting reviewer results append new follow-up tasks and then block the turn so the same session can continue. -- Acceptance criteria must describe observable behavior, not only internal code structure. -- Per-subtask and final validation entries must name the exact command that was run and enough result detail to distinguish success from failure. -- When work spans multiple files or subsystems, the plan should briefly orient the reader by naming the affected paths and how they fit together. -- If a step is risky or can leave the repo in a partial state, document how to retry safely without losing previous progress. - -## Session Plan Template -Use this exact section structure for `plans/active/session-.md` so the workflow helpers can parse it mechanically. - -```md -# Session Plan: - -## Plan approval -unapproved - -## Phase -planning - -## Goal -Describe the concrete user-visible outcome for this session. - -## Constraints -- Record all important technical constraints here. - -## Repo facts / assumptions -- Record the current repo facts and assumptions here. - -## Acceptance criteria -- Record the concrete acceptance checks here. - -## Ordered subtasks -- [ ] First approved implementation subtask. -- [ ] Second approved implementation subtask. - -## Follow-up subtasks (append-only) -- [x] No follow-up subtasks have been added yet. - -## Per-subtask validation -- Record the most relevant validation command and result immediately after each completed subtask. - -## Final validation -- Record whether the implementation-phase stop hook passed formatting/build checks, whether the testing-phase final test gate passed, and whether the review-phase reviewer accepted here. - -## Decision log -- Append important decisions with timestamps. - -## Progress log -- Append progress updates with timestamps. -``` - -## Subtask Rules -- `## Plan approval` must contain exactly one machine-readable value: `approved` or `unapproved`. -- `## Phase` must contain exactly one machine-readable value: `planning`, `implementation`, `testing`, or `review`. -- `## Phase` must stay `planning` while `## Plan approval` is `unapproved`. -- When `## Plan approval` is `approved`, `## Phase` must be `implementation`, `testing`, or `review`. -- Both `## Ordered subtasks` and `## Follow-up subtasks (append-only)` must use markdown checkboxes. -- If build checks, tests, or review find problems after some tasks were completed, add NEW unchecked items under `## Follow-up subtasks (append-only)`. -- Do not rewrite or delete completed historical subtasks. -- Do not remove prior reviewer or validation obligations from the plan. Add new unchecked follow-up items instead. -- Assume the stop hook may block the turn multiple times in one completion cycle after the plan is approved, may rerun selected final tests while the phase is `testing`, and may rerun the reviewer multiple times once the phase is `review`; keep the plan accurate enough that the same session can resume from the file alone. diff --git a/REVIEWER.md b/REVIEWER.md index b2b4f7cc..09f56fb6 100644 --- a/REVIEWER.md +++ b/REVIEWER.md @@ -1,120 +1,40 @@ # Reviewer Rules -This document applies to every Codex reviewer run in this repository, including the hooks-disabled nested read-only reviewer used by `scripts/stop_hook.sh` and any explicit review request from the user. +This document applies to every explicit Codex review request in this repository. ## Role The reviewer evaluates the current builder result in read-only mode. -The reviewer does not edit tracked files, does not update plan approval or checklist state, and does not manage workflow transitions directly. -The stop hook may invoke the review-phase reviewer multiple times in one outer completion loop; each review must be independent and based only on current observable evidence. +The reviewer does not edit tracked files and does not manage workflow transitions directly. At the start of every review, reset reviewer posture completely. -Review the current work as if it were authored by another party, and do not trust builder claims, summaries, or plan updates without checking the scoped evidence. +Review the current work as if it were authored by another party, and do not trust builder claims or summaries without checking the scoped evidence. ## Default Inputs -Unless the handoff explicitly narrows the scope further, the reviewer should read the minimum set of current-session inputs needed to make a decision: +Unless the user narrows the scope further, read the minimum set of inputs needed to make a decision: -- the explicit session id from the current review handoff or hook payload -- `plans/active/session-.md` when that file exists for the current session -- the latest user message for the current session, or the transcript source needed to recover it when planning review is explicitly requested +- the latest user request that defines the review scope - the relevant changed files for the scoped task -- the validation commands or outputs named in the session plan when they are needed to verify a claim +- the validation commands or outputs needed to verify a claim +- `GPU.md` whenever the reviewed change touches CUDA, GPU kernels, GPU wrappers, GPU tests, or GPU-facing performance-sensitive behavior -Additional repository-specific inputs: - -- read `GPU.md` whenever the reviewed change touches CUDA, GPU kernels, GPU wrappers, GPU tests, or GPU-facing performance-sensitive behavior - -Do not load raw transcripts, temporary hook logs, or unrelated historical artifacts by default. +Do not load raw transcripts, temporary logs, or unrelated historical artifacts by default. Only inspect them when the current evidence is insufficient to verify a concrete claim. ## Core Obligations -1. Review only the current scoped work for the current session. -2. If `plans/active/session-.md` exists, read the current workflow state there by inspecting `## Plan approval` and `## Phase`. Otherwise treat the session as an explicit review request scoped directly by the user prompt. -3. Review against the session plan, not against unstated preferences. -4. Return English feedback. -5. Base the decision on evidence that can be inspected now. -6. In planning, only perform review if planning review was explicitly requested. In implementation or testing, review only when the user explicitly requested review. In review, review the repository state against the current session plan. -7. Prefer the smallest correction that preserves correctness. -8. Treat append-only follow-up-subtask rules and completed-checkbox preservation as hard workflow requirements. - -## Phase-Specific Scope - -### `planning` - -Planning corresponds to `## Plan approval` being `unapproved` and `## Phase` being `planning`. - -In planning, the reviewer is not performing a general quality review of future implementation. -The reviewer is evaluating the current plan and, when explicitly requested, whether the latest user message clearly approves that current plan or instead asks for further revisions. - -Review the latest user message against the current session plan only when that message is part of the review scope. -Do not require implementation yet. - -The reviewer should verify that: - -- the current session plan exists and is concrete enough to be the object being approved, -- the latest user message either clearly approves the current session plan or requests additional revisions, -- the decision is based on the user's actual message rather than on reviewer preference about how the plan could be improved. - -### `implementation` - -Implementation corresponds to `## Plan approval` being `approved` and `## Phase` being `implementation`. - -The stop hook does not launch the nested reviewer in implementation. Review this phase only when the user explicitly requests review before the builder transitions the session to `review`. - -When implementation is explicitly reviewed, review the current repository state against the current session plan. - -The reviewer should verify that: - -- the implementation satisfies the session plan's Goal, Constraints, Repo facts / assumptions, and Acceptance criteria, -- completed subtasks appear genuinely complete for their claimed scope, -- claimed validation is relevant and sufficient for the completed work, -- any later failures are represented as NEW unchecked follow-up subtasks rather than by rewriting prior completed work, -- the code remains within the approved scope instead of drifting into unrelated redesign. - -### `testing` - -Testing corresponds to `## Plan approval` being `approved` and `## Phase` being `testing`. - -The stop hook does not launch the nested reviewer in testing. Review this phase only when the user explicitly requests review before the builder transitions the session to `review`. - -When testing is explicitly reviewed, review the current repository state against the current session plan and the stated final-test evidence. - -The reviewer should verify that: - -- the implementation satisfies the session plan's Goal, Constraints, Repo facts / assumptions, and Acceptance criteria, -- completed subtasks appear genuinely complete for their claimed scope, -- the testing-phase validation is relevant and sufficient for the completed work, -- any failures are represented as NEW unchecked follow-up subtasks rather than by rewriting prior completed work, -- the code remains within the approved scope instead of drifting into unrelated redesign. - -### `review` - -Review corresponds to `## Plan approval` being `approved` and `## Phase` being `review`. - -In review, inspect the current repository state against the approved session plan plus any review-phase follow-up subtasks appended after earlier reviewer passes. - -The reviewer should verify that: - -- the implementation satisfies the session plan's Goal, Constraints, Repo facts / assumptions, and Acceptance criteria, -- completed subtasks and completed historical follow-up items remain preserved, -- open follow-up subtasks capture the remaining concrete obligations created by earlier reviewer feedback, -- claimed validation is relevant and sufficient for the current reviewed state, -- the code remains within the approved scope instead of drifting into unrelated redesign. +1. Review only the current scoped work. +2. Return English feedback. +3. Base the decision on evidence that can be inspected now. +4. Prefer the smallest correction that preserves correctness. +5. Treat user-stated constraints and repository guidance in `AGENTS.md`, `BUILDER.md`, `REVIEWER.md`, and `GPU.md` when relevant as the review contract. ## What To Review -- correctness against the current session plan, -- adherence to the current phase scope, -- in planning, only when explicitly requested, whether the latest user message approves the current session plan or requests further changes, -- in implementation or testing, only explicit user-requested review before the builder transitions to `review`, -- in review, the current repository state plus any review-phase follow-up subtasks, +- correctness against the user's request, - completeness and quality of the stated validation, - whether the claimed completion is observable from code and checks rather than inferred from intent, -- whether the required session plan sections remain coherent after the builder change, -- whether completed subtasks remain preserved as historical record, -- whether new obligations are captured as NEW unchecked items under `## Follow-up subtasks (append-only)` when appropriate, - whether the work introduces unrelated scope expansion, dead fallback logic, or unnecessary redesign, - whether the resulting code follows KISS by minimizing cognitive load and keeping responsibility boundaries rational, - whether GPU-related changes satisfy `GPU.md` when GPU behavior is in scope. @@ -123,24 +43,18 @@ The reviewer should verify that: Before returning a decision, verify all of the following that apply to the current scope: -1. In planning, the decision is grounded in the latest user message and the current session plan, not in reviewer-authored new plan requirements. -2. In implementation, testing, and review, the session plan still matches the current repository state. -3. The required session plan headings, approval flag, and checkbox sections remain intact and machine-checkable when they are relevant to the reviewed scope. -4. Claimed completion checks are appropriate for the scope and are not superficial restatements of intent. -5. The reviewed work did not silently broaden scope beyond the session plan. -6. Completed checklist items were not rewritten back into unchecked items. -7. If review finds remaining work, that remaining work can be described as concrete follow-up subtasks. -8. For GPU changes, the implementation and test strategy respect `GPU.md`, including repeated testing for nondeterministic GPU issues when relevant. +1. The reviewed work satisfies the latest user request and any explicitly stated constraints. +2. Claimed completion checks are appropriate for the scope and are not superficial restatements of intent. +3. The reviewed work did not silently broaden scope beyond the request. +4. For GPU changes, the implementation and test strategy respect `GPU.md`, including repeated testing for nondeterministic GPU issues when relevant. ## What Not To Do - do not perform implementation work, - do not edit repository files, -- do not invent or mutate repository-global session pointers, -- do not rewrite the session plan directly, - do not request unrelated redesigns, - do not block on style preferences that are not tied to correctness, scope, or maintainability, -- do not invent requirements outside the current session plan, `AGENTS.md`, `PLANS.md`, `BUILDER.md`, and `GPU.md` when relevant, +- do not invent requirements outside the user's request, `AGENTS.md`, `BUILDER.md`, `REVIEWER.md`, and `GPU.md` when relevant, - do not rely on builder intent when observable evidence is missing. ## Review Standard @@ -148,7 +62,7 @@ Before returning a decision, verify all of the following that apply to the curre Review against the current contract, not against personal taste. - Prefer observable failures over speculative concerns. -- Call out the exact claim, file, plan section, validation step, or GPU principle that fails the contract. +- Call out the exact claim, file, validation step, or GPU principle that fails the contract. - If the work is acceptable, do not ask for extra polish outside the approved scope. - If multiple outcomes are possible, choose the narrowest result that preserves correctness. - Evaluate simplicity by readability, local reasoning cost, and scope separation, not by maximizing abstraction. @@ -161,55 +75,18 @@ When a structured reviewer result is required, use exactly this result space: - `accept` - `revision` -### `planning` - -- `accept` - - the latest user message approves the current session plan and does not request further changes. -- `revision` - - the latest user message requests additional changes, or does not clearly approve the current session plan. - -### `implementation` - -- `accept` - - the explicitly requested implementation review satisfies the current session plan and the validation is sufficient for the claimed completion. -- `revision` - - the explicitly requested implementation review still has concrete deficiencies that must be addressed before the builder should transition into `review`. - -### `testing` - -- `accept` - - the explicitly requested testing review satisfies the current session plan and the validation is sufficient for the claimed completion. -- `revision` - - the explicitly requested testing review still has concrete deficiencies that must be addressed before the builder should transition into `review`. - -### `review` - -- `accept` - - the review-phase implementation satisfies the current session plan and the validation is sufficient for final acceptance. -- `revision` - - the review-phase implementation, validation, or workflow bookkeeping still has concrete deficiencies that must be addressed before final acceptance. +Use `accept` when the reviewed work satisfies the request and the validation is sufficient for the claimed completion. +Use `revision` when concrete deficiencies remain. ## Feedback Quality Feedback should be: - specific, -- phase-scoped, - actionable, -- tied to the session plan, the latest user message, or other named evidence, -- narrow enough that the builder can convert it into concrete follow-up subtasks without guessing, +- tied to the user request or other named evidence, +- narrow enough that the builder can convert it into concrete follow-up work without guessing, - focused on what must change for acceptance, not on optional improvements. If the work is acceptable, say so plainly. If it is not, identify the smallest correction that would make the review pass. - -## Workflow Implications - -Reviewer outcomes do not update workflow state directly. -The stop hook and builder handle the next step. - -Important invariants: - -- reviewer feedback should map cleanly to NEW unchecked follow-up subtasks when more work is required, -- reviewer rejection must not imply rewriting or erasing already completed subtasks, -- reviewer approval should mean the current session can finish without additional hidden conditions. diff --git a/plans/.gitkeep b/plans/.gitkeep deleted file mode 100644 index 8b137891..00000000 --- a/plans/.gitkeep +++ /dev/null @@ -1 +0,0 @@ - diff --git a/schemas/review-decision.schema.json b/schemas/review-decision.schema.json deleted file mode 100644 index 1b6f6a63..00000000 --- a/schemas/review-decision.schema.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", - "properties": { - "result": { - "type": "string", - "enum": ["accept", "revision"] - }, - "msg": { - "type": "string" - } - }, - "required": ["result", "msg"], - "additionalProperties": false -} diff --git a/schemas/session-start-intent.schema.json b/schemas/session-start-intent.schema.json deleted file mode 100644 index 4dbad3d7..00000000 --- a/schemas/session-start-intent.schema.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", - "properties": { - "result": { - "type": "string", - "enum": ["review", "build"] - }, - "msg": { - "type": "string" - } - }, - "required": ["result", "msg"], - "additionalProperties": false -} diff --git a/scripts/lib/codex_workflow/__init__.py b/scripts/lib/codex_workflow/__init__.py deleted file mode 100644 index 446e1a03..00000000 --- a/scripts/lib/codex_workflow/__init__.py +++ /dev/null @@ -1,26 +0,0 @@ -from .atomic import atomic_write_json, atomic_write_text -from .hooks import HookOutcome, handle_session_start, handle_stop -from .paths import RepoPaths -from .plan import ( - FOLLOW_UP_SUBTASKS_HEADING, - ORDERED_SUBTASKS_HEADING, - PLAN_APPROVAL_HEADING, - analyze_plan, - append_follow_up_subtasks, - render_session_plan, -) - -__all__ = [ - "FOLLOW_UP_SUBTASKS_HEADING", - "ORDERED_SUBTASKS_HEADING", - "PLAN_APPROVAL_HEADING", - "HookOutcome", - "RepoPaths", - "analyze_plan", - "append_follow_up_subtasks", - "atomic_write_json", - "atomic_write_text", - "handle_session_start", - "handle_stop", - "render_session_plan", -] diff --git a/scripts/lib/codex_workflow/atomic.py b/scripts/lib/codex_workflow/atomic.py deleted file mode 100644 index 7fec8042..00000000 --- a/scripts/lib/codex_workflow/atomic.py +++ /dev/null @@ -1,29 +0,0 @@ -from __future__ import annotations - -import json -import os -import tempfile -from pathlib import Path -from typing import Any - - -def atomic_write_text(path: Path, content: str, mode: int = 0o644) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - fd, tmp_path = tempfile.mkstemp(prefix=f".{path.name}.tmp-", dir=str(path.parent)) - try: - with os.fdopen(fd, "w", encoding="utf-8", newline="") as handle: - handle.write(content) - handle.flush() - os.fsync(handle.fileno()) - os.replace(tmp_path, path) - try: - os.chmod(path, mode) - except PermissionError: - pass - finally: - if os.path.exists(tmp_path): - os.unlink(tmp_path) - - -def atomic_write_json(path: Path, payload: Any, mode: int = 0o644) -> None: - atomic_write_text(path, json.dumps(payload, indent=2, sort_keys=False) + "\n", mode=mode) diff --git a/scripts/lib/codex_workflow/cli.py b/scripts/lib/codex_workflow/cli.py deleted file mode 100644 index 3caafee6..00000000 --- a/scripts/lib/codex_workflow/cli.py +++ /dev/null @@ -1,41 +0,0 @@ -from __future__ import annotations - -import argparse -import json -import sys -from pathlib import Path -from typing import Any - -from .hooks import HookOutcome, handle_session_start, handle_stop - - -def emit_outcome(outcome: HookOutcome, stdout_stream: Any, stderr_stream: Any) -> int: - if outcome.stdout_payload is not None: - stdout_stream.write(json.dumps(outcome.stdout_payload, separators=(",", ":"))) - if outcome.stderr_message: - stderr_stream.write(outcome.stderr_message) - if not outcome.stderr_message.endswith("\n"): - stderr_stream.write("\n") - return outcome.exit_code - - -def run_command(command: str, payload: dict[str, Any], repo_root: Path) -> HookOutcome: - if command == "session-start": - return handle_session_start(payload, repo_root) - if command == "stop": - return handle_stop(payload, repo_root) - raise ValueError(f"Unsupported workflow command: {command}") - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser(description="Repository-local Codex workflow harness") - parser.add_argument("command", choices=("session-start", "stop")) - args = parser.parse_args(argv) - payload = json.load(sys.stdin) - repo_root = Path.cwd() - outcome = run_command(args.command, payload, repo_root) - return emit_outcome(outcome, sys.stdout, sys.stderr) - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/scripts/lib/codex_workflow/hooks.py b/scripts/lib/codex_workflow/hooks.py deleted file mode 100644 index 5b97c32c..00000000 --- a/scripts/lib/codex_workflow/hooks.py +++ /dev/null @@ -1,603 +0,0 @@ -from __future__ import annotations - -from dataclasses import dataclass -from pathlib import Path, PurePosixPath -import sys -import time -from typing import Any, Callable - -from repo_validation import edited_paths_from_git - -from .atomic import atomic_write_text -from .paths import RepoPaths -from .plan import ( - FOLLOW_UP_SUBTASKS_HEADING, - ORDERED_SUBTASKS_HEADING, - PLAN_APPROVAL_HEADING, - PLAN_PHASE_HEADING, - analyze_plan, - append_follow_up_subtasks_file, - render_session_plan, -) -from .runners import ( - BuildCheckRunner, - CodexExecRunner, - FinalTestRunner, - ShellBuildCheckRunner, - ShellFinalTestRunner, - StructuredExecResult, - StructuredExecRunner, -) -from .transcript import initial_user_message_from_transcript - -RETRY_DELAY_SECONDS = 1.0 -MAX_INFRA_RETRY_ATTEMPTS = 100 - - -@dataclass(frozen=True) -class HookOutcome: - exit_code: int - stdout_payload: dict[str, object] | None = None - stderr_message: str | None = None - - -class RetryLimitExceeded(RuntimeError): - """Raised when repeated nested Codex infra failures should stop the hook.""" - - -def stop_outcome(reason: str) -> HookOutcome: - return HookOutcome(exit_code=0, stdout_payload={"continue": False, "stopReason": reason}) - - -def block_outcome(message: str) -> HookOutcome: - return HookOutcome(exit_code=2, stderr_message=message) - - -def _find_string_field(payload: object, candidate_keys: tuple[str, ...]) -> str | None: - if isinstance(payload, dict): - for key in candidate_keys: - value = payload.get(key) - if isinstance(value, str) and value.strip(): - return value - for value in payload.values(): - found = _find_string_field(value, candidate_keys) - if found: - return found - elif isinstance(payload, list): - for item in payload: - found = _find_string_field(item, candidate_keys) - if found: - return found - return None - - -def extract_session_id(payload: dict[str, Any]) -> str | None: - return _find_string_field(payload, ("session_id", "sessionId")) - - -def extract_initial_user_message(payload: dict[str, Any]) -> str | None: - direct_message = _find_string_field( - payload, - ("initial_user_message", "initialUserMessage", "first_user_message", "firstUserMessage", "user_message", "userMessage"), - ) - if direct_message: - return direct_message - transcript_path = _find_string_field(payload, ("transcript_path", "transcriptPath")) - if not transcript_path: - return None - return initial_user_message_from_transcript(Path(transcript_path)) - - -def _ensure_plan_exists(paths: RepoPaths, session_id: str) -> Path: - plan_path = paths.active_plan_path(session_id) - if plan_path.exists(): - return plan_path - reactivated_path = paths.move_completed_plan_to_active(session_id) - if reactivated_path is not None: - return reactivated_path - atomic_write_text(plan_path, render_session_plan(session_id)) - return plan_path - - -def _find_stop_plan(paths: RepoPaths, session_id: str) -> Path | None: - active_path = paths.active_plan_path(session_id) - if active_path.exists(): - return active_path - return paths.move_completed_plan_to_active(session_id) - - -def _finalize_completed_session(paths: RepoPaths, session_id: str) -> None: - paths.move_session_revision_logs_to_completed(session_id) - paths.move_active_plan_to_completed(session_id) - - -def _compact_message(text: str, max_len: int = 220) -> str: - cleaned = " ".join(text.split()) - if len(cleaned) <= max_len: - return cleaned - return cleaned[: max_len - 3].rstrip() + "..." - - -def _make_test_follow_ups(summary: str) -> list[str]: - detail = _compact_message(summary) - return [f"Fix the failing final validation from `scripts/run_tests.sh`: {detail}"] - - -def _make_build_follow_ups(summary: str) -> list[str]: - detail = _compact_message(summary) - return [f"Fix the failing implementation formatting/build validation from `scripts/run_build_checks.sh`: {detail}"] - - -def _make_review_follow_ups(summary: str) -> list[str]: - detail = _compact_message(summary) - return [f"Address reviewer feedback from the final read-only Codex review: {detail}"] - - -def _missing_plan_structure_message( - missing_sections: list[str], - invalid_sections: list[str], - empty_sections: list[str] | None = None, -) -> str: - missing_text = ", ".join(f"`{section}`" for section in missing_sections) - invalid_text = ", ".join(f"`{section}`" for section in invalid_sections) - empty_text = ", ".join(f"`{section}`" for section in (empty_sections or [])) - details: list[str] = [] - if missing_text: - details.append(f"missing: {missing_text}") - if invalid_text: - details.append(f"invalid: {invalid_text}") - if empty_text: - details.append(f"empty: {empty_text}") - detail_text = "; ".join(details) - return ( - "The session plan is missing required machine-checkable structure. Restore " - f"`{PLAN_APPROVAL_HEADING}`, `{PLAN_PHASE_HEADING}`, `{ORDERED_SUBTASKS_HEADING}`, and `{FOLLOW_UP_SUBTASKS_HEADING}`. " - f"Current issues: {detail_text}" - ) - - -def _log_progress(message: str) -> None: - sys.stderr.write(message) - if not message.endswith("\n"): - sys.stderr.write("\n") - sys.stderr.flush() - - -def _raise_retry_limit_exceeded(kind: str, detail: str) -> None: - raise RetryLimitExceeded( - "Stop hook aborted after " - f"{MAX_INFRA_RETRY_ATTEMPTS} failed nested {kind} attempts. Last summary: {detail}" - ) - - -@dataclass(frozen=True) -class ImplementationCheckResult: - status: str - message: str - should_archive: bool = False - - -@dataclass(frozen=True) -class FinalTestSelection: - run_python: bool - run_rust: bool - - @property - def requires_any(self) -> bool: - return self.run_python or self.run_rust - - -def _default_edited_paths_provider(repo_root: Path) -> list[str] | None: - try: - return edited_paths_from_git(repo_root) - except RuntimeError: - return None - - -def _select_final_tests(edited_paths: list[str]) -> FinalTestSelection: - run_python = False - run_rust = False - for path in edited_paths: - normalized = PurePosixPath(path) - if normalized.suffix == ".py": - run_python = True - if normalized.name == "Cargo.toml": - run_rust = True - if normalized.suffix == ".rs": - run_rust = True - if normalized.parts and normalized.parts[0] == "cuda": - run_rust = True - return FinalTestSelection(run_python=run_python, run_rust=run_rust) - - -def _rust_build_check_required(path: str) -> bool: - normalized = PurePosixPath(path) - return ( - normalized.suffix == ".rs" - or normalized.name == "Cargo.toml" - or bool(normalized.parts and normalized.parts[0] == "cuda") - ) - - -def _build_session_start_prompt(initial_user_message: str) -> str: - return ( - "You are classifying the user's initial prompt for a Codex workflow session.\n" - "Return `result=review` only when the user explicitly asks for a review, code review, audit, or inspection of " - "existing work.\n" - "Return `result=build` for everything else, including implementation requests, bug fixes, explanations, " - "brainstorming, or ambiguous prompts.\n" - "Set `msg` to a short reason.\n" - "\n" - "User prompt:\n" - f"{initial_user_message}\n" - ) - - -def _build_review_prompt(session_id: str, plan_display_path: str) -> str: - return ( - "You are a read-only reviewer.\n" - f"The builder session id is {session_id}.\n" - f"Read the corresponding session plan at {plan_display_path}.\n" - "Verify whether the implementation satisfies the goal, constraints, and acceptance criteria in that plan.\n" - "If acceptable, output result=accept.\n" - "Otherwise output result=revision and msg=.\n" - ) - - -def _run_review_with_retry( - paths: RepoPaths, - exec_runner: StructuredExecRunner, - prompt: str, - sleep_fn: Callable[[float], None], -) -> StructuredExecResult: - schema_path = paths.schemas_dir / "review-decision.schema.json" - for attempt in range(1, MAX_INFRA_RETRY_ATTEMPTS + 1): - review_result = exec_runner.run(prompt=prompt, schema_path=schema_path, label="final-review") - if review_result.ok and review_result.result in {"accept", "revision"}: - return review_result - detail = review_result.error or "Reviewer did not return a valid structured decision." - if attempt == MAX_INFRA_RETRY_ATTEMPTS: - _raise_retry_limit_exceeded("reviewer", detail) - _log_progress( - "[stop hook] Nested reviewer failed. " - f"Attempt {attempt}/{MAX_INFRA_RETRY_ATTEMPTS}. " - f"Retrying in {RETRY_DELAY_SECONDS:.1f}s. Summary: {detail}" - ) - sleep_fn(RETRY_DELAY_SECONDS) - _raise_retry_limit_exceeded("reviewer", "Retry loop exited unexpectedly.") - - -def _classify_session_start_intent( - paths: RepoPaths, - session_id: str, - initial_user_message: str, - exec_runner: StructuredExecRunner, -) -> StructuredExecResult: - schema_path = paths.schemas_dir / "session-start-intent.schema.json" - return exec_runner.run( - prompt=_build_session_start_prompt(initial_user_message), - schema_path=schema_path, - label="session-start-intent", - ) - - -def _run_selected_final_tests( - paths: RepoPaths, - plan_path: Path, - test_runner: FinalTestRunner, - edited_paths_provider: Callable[[Path], list[str] | None], -) -> tuple[str, str | None]: - edited_paths = edited_paths_provider(paths.repo_root) - if edited_paths == []: - return "skipped-no-changes", None - - test_selection = ( - FinalTestSelection(run_python=True, run_rust=True) - if edited_paths is None - else _select_final_tests(edited_paths) - ) - if not test_selection.requires_any: - return "skipped-selection", None - - test_result = test_runner.run( - label="final-tests", - run_python=test_selection.run_python, - run_rust=test_selection.run_rust, - ) - if not test_result.ok: - follow_up_items = _make_test_follow_ups(test_result.summary) - append_follow_up_subtasks_file(plan_path, follow_up_items) - return "failed", test_result.summary - return "passed", None - - -def _run_selected_build_checks( - paths: RepoPaths, - plan_path: Path, - build_runner: BuildCheckRunner, - edited_paths_provider: Callable[[Path], list[str] | None], -) -> tuple[str, str | None]: - edited_paths = edited_paths_provider(paths.repo_root) - if edited_paths == []: - return "skipped-no-changes", None - if edited_paths is not None and not any(_rust_build_check_required(path) for path in edited_paths): - return "skipped-selection", None - - build_result = build_runner.run(label="implementation-build") - if not build_result.ok: - append_follow_up_subtasks_file(plan_path, _make_build_follow_ups(build_result.summary)) - return "failed", build_result.summary - return "passed", None - - -def _evaluate_implementation_once( - paths: RepoPaths, - session_id: str, - plan_path: Path, - plan_display_path: str, - exec_runner: StructuredExecRunner, - test_runner: FinalTestRunner, - build_runner: BuildCheckRunner, - sleep_fn: Callable[[float], None], - edited_paths_provider: Callable[[Path], list[str] | None], -) -> ImplementationCheckResult: - analysis = analyze_plan(plan_path.read_text(encoding="utf-8")) - if analysis.missing_sections or analysis.invalid_sections or analysis.empty_sections: - return ImplementationCheckResult( - status="blocked", - message=_missing_plan_structure_message( - analysis.missing_sections, - analysis.invalid_sections, - analysis.empty_sections, - ), - ) - - if not analysis.is_approved: - return ImplementationCheckResult( - status="blocked", - message=( - "The session plan is still unapproved. Continue the planning interview and do not start implementation " - "until the plan approval flag is `approved`." - ), - ) - - if not analysis.all_checked: - next_item = analysis.first_unchecked - detail = f" Next unchecked subtask: {next_item.text}" if next_item else "" - return ImplementationCheckResult( - status="blocked", - message=( - "Implementation is not complete. Continue with the next unchecked subtask and keep the plan document updated." - + detail - ), - ) - - if analysis.phase == "implementation": - build_status, build_summary = _run_selected_build_checks( - paths=paths, - plan_path=plan_path, - build_runner=build_runner, - edited_paths_provider=edited_paths_provider, - ) - if build_status == "failed": - return ImplementationCheckResult( - status="blocked", - message=( - "Implementation formatting/build checks failed. New follow-up subtasks were appended to the session plan. " - f"Address them first. Failure summary: {build_summary}" - ), - ) - if build_status == "passed": - return ImplementationCheckResult( - status="accepted", - message=( - "All subtasks are complete and implementation formatting/build checks passed. " - "Unit tests and reviewer execution were skipped because the session phase is `implementation`." - ), - ) - if build_status == "skipped-no-changes": - return ImplementationCheckResult( - status="accepted", - message="All subtasks are complete and no files changed, so implementation formatting/build checks were skipped.", - ) - return ImplementationCheckResult( - status="accepted", - message=( - "All subtasks are complete, implementation formatting/build checks were skipped because no Rust, Cargo.toml, " - "or cuda/ files changed, and unit tests plus reviewer execution were skipped because the session phase is `implementation`." - ), - ) - - if analysis.phase == "testing": - test_status, test_summary = _run_selected_final_tests( - paths=paths, - plan_path=plan_path, - test_runner=test_runner, - edited_paths_provider=edited_paths_provider, - ) - if test_status == "failed": - return ImplementationCheckResult( - status="blocked", - message=( - "Testing-phase final tests failed. New follow-up subtasks were appended to the session plan. " - f"Address them first. Failure summary: {test_summary}" - ), - ) - if test_status == "passed": - return ImplementationCheckResult( - status="accepted", - message=( - "All subtasks are complete and testing-phase final tests passed. " - "Reviewer execution was skipped because the session phase is `testing`." - ), - ) - if test_status == "skipped-no-changes": - return ImplementationCheckResult( - status="accepted", - message="All subtasks are complete and no files changed, so testing-phase final tests were skipped.", - ) - return ImplementationCheckResult( - status="accepted", - message=( - "All subtasks are complete, testing-phase final tests were skipped because no Python, Rust, Cargo.toml, " - "or cuda/ files changed, and reviewer execution was skipped because the session phase is `testing`." - ), - ) - - if analysis.phase != "review": - return ImplementationCheckResult( - status="blocked", - message=f"Unsupported approved workflow phase `{analysis.phase}`.", - ) - - review_result = _run_review_with_retry( - paths=paths, - exec_runner=exec_runner, - prompt=_build_review_prompt(session_id, plan_display_path), - sleep_fn=sleep_fn, - ) - if review_result.result == "revision": - append_follow_up_subtasks_file( - plan_path, - _make_review_follow_ups(review_result.msg or "Reviewer requested changes."), - ) - return ImplementationCheckResult( - status="blocked", - message=( - "Reviewer requested changes. New follow-up subtasks were appended to the session plan. " - f"Address them first. Reviewer feedback: {review_result.msg or 'No review message was provided.'}" - ), - ) - - return ImplementationCheckResult( - status="accepted", - message="All subtasks are complete and reviewer approved.", - should_archive=True, - ) - - -def _run_implementation_loop( - paths: RepoPaths, - session_id: str, - plan_path: Path, - plan_display_path: str, - exec_runner: StructuredExecRunner, - test_runner: FinalTestRunner, - build_runner: BuildCheckRunner, - sleep_fn: Callable[[float], None], - edited_paths_provider: Callable[[Path], list[str] | None], -) -> HookOutcome: - try: - check_result = _evaluate_implementation_once( - paths=paths, - session_id=session_id, - plan_path=plan_path, - plan_display_path=plan_display_path, - exec_runner=exec_runner, - test_runner=test_runner, - build_runner=build_runner, - sleep_fn=sleep_fn, - edited_paths_provider=edited_paths_provider, - ) - except RetryLimitExceeded as exc: - return block_outcome(str(exc)) - if check_result.status == "accepted": - if check_result.should_archive: - try: - _finalize_completed_session(paths, session_id) - except OSError as exc: - return block_outcome(f"Failed to archive completed workflow artifacts: {exc}") - return stop_outcome(check_result.message) - _log_progress(f"[stop hook] Additional implementation required. {check_result.message}") - return block_outcome(check_result.message) - - -def handle_session_start( - payload: dict[str, Any], - repo_root: Path, - exec_runner: StructuredExecRunner | None = None, -) -> HookOutcome: - paths = RepoPaths(repo_root) - paths.ensure_directories() - session_id = extract_session_id(payload) - if not session_id: - return HookOutcome(exit_code=1, stderr_message="SessionStart hook requires `session_id` in the hook payload.") - initial_user_message = extract_initial_user_message(payload) - if not initial_user_message: - return HookOutcome( - exit_code=1, - stderr_message="SessionStart hook requires the initial user prompt in the hook payload or transcript.", - ) - - exec_runner = exec_runner or CodexExecRunner(paths, session_id) - intent_result = _classify_session_start_intent(paths, session_id, initial_user_message, exec_runner) - if not intent_result.ok: - detail = intent_result.error or "Intent classifier did not return a valid structured decision." - return HookOutcome(exit_code=1, stderr_message=f"SessionStart hook could not classify the initial prompt. {detail}") - if intent_result.result == "review": - return HookOutcome(exit_code=0) - if intent_result.result != "build": - return HookOutcome( - exit_code=1, - stderr_message="SessionStart hook classifier returned an unexpected intent result.", - ) - - _ensure_plan_exists(paths, session_id) - return HookOutcome(exit_code=0) - - -def handle_stop( - payload: dict[str, Any], - repo_root: Path, - exec_runner: StructuredExecRunner | None = None, - test_runner: FinalTestRunner | None = None, - build_runner: BuildCheckRunner | None = None, - sleep_fn: Callable[[float], None] | None = None, - edited_paths_provider: Callable[[Path], list[str] | None] | None = None, -) -> HookOutcome: - paths = RepoPaths(repo_root) - paths.ensure_directories() - session_id = extract_session_id(payload) - if not session_id: - return block_outcome("Stop hook requires `session_id` in the hook payload.") - - plan_path = _find_stop_plan(paths, session_id) - if plan_path is None: - return HookOutcome(exit_code=0) - plan_display_path = str(plan_path.relative_to(paths.repo_root)) - analysis = analyze_plan(plan_path.read_text(encoding="utf-8")) - if analysis.missing_sections or analysis.invalid_sections: - return block_outcome( - _missing_plan_structure_message( - analysis.missing_sections, - analysis.invalid_sections, - analysis.empty_sections, - ) - ) - if analysis.approval_status == "unapproved": - return HookOutcome(exit_code=0) - if analysis.empty_sections: - return block_outcome( - _missing_plan_structure_message( - analysis.missing_sections, - analysis.invalid_sections, - analysis.empty_sections, - ) - ) - - exec_runner = exec_runner or CodexExecRunner(paths, session_id) - test_runner = test_runner or ShellFinalTestRunner(paths, session_id) - build_runner = build_runner or ShellBuildCheckRunner(paths, session_id) - sleep_fn = sleep_fn or time.sleep - edited_paths_provider = edited_paths_provider or _default_edited_paths_provider - return _run_implementation_loop( - paths=paths, - session_id=session_id, - plan_path=plan_path, - plan_display_path=plan_display_path, - exec_runner=exec_runner, - test_runner=test_runner, - build_runner=build_runner, - sleep_fn=sleep_fn, - edited_paths_provider=edited_paths_provider, - ) diff --git a/scripts/lib/codex_workflow/paths.py b/scripts/lib/codex_workflow/paths.py deleted file mode 100644 index 2c299641..00000000 --- a/scripts/lib/codex_workflow/paths.py +++ /dev/null @@ -1,93 +0,0 @@ -from __future__ import annotations - -from dataclasses import dataclass -from pathlib import Path - - -@dataclass(frozen=True) -class RepoPaths: - repo_root: Path - - @property - def agents_dir(self) -> Path: - return self.repo_root / ".agents" - - @property - def plans_dir(self) -> Path: - return self.repo_root / "plans" - - @property - def active_plans_dir(self) -> Path: - return self.plans_dir / "active" - - @property - def completed_plans_dir(self) -> Path: - return self.plans_dir / "completed" - - @property - def revision_logs_dir(self) -> Path: - return self.repo_root / "revision_logs" - - @property - def active_revision_logs_dir(self) -> Path: - return self.revision_logs_dir / "active" - - @property - def completed_revision_logs_dir(self) -> Path: - return self.revision_logs_dir / "completed" - - @property - def scripts_dir(self) -> Path: - return self.repo_root / "scripts" - - @property - def schemas_dir(self) -> Path: - return self.repo_root / "schemas" - - def active_plan_path(self, session_id: str) -> Path: - return self.active_plans_dir / f"session-{session_id}.md" - - def completed_plan_path(self, session_id: str) -> Path: - return self.completed_plans_dir / f"session-{session_id}.md" - - def default_plan_path(self, session_id: str) -> Path: - return self.active_plan_path(session_id) - - def active_revision_log_paths(self, session_id: str) -> list[Path]: - pattern = f"{session_id}-*" - return sorted(self.active_revision_logs_dir.glob(pattern)) - - def move_session_revision_logs_to_completed(self, session_id: str) -> list[Path]: - moved_paths: list[Path] = [] - for source in self.active_revision_log_paths(session_id): - destination = self.completed_revision_logs_dir / source.name - source.replace(destination) - moved_paths.append(destination) - return moved_paths - - def move_completed_plan_to_active(self, session_id: str) -> Path | None: - active_path = self.active_plan_path(session_id) - if active_path.exists(): - return active_path - completed_path = self.completed_plan_path(session_id) - if not completed_path.exists(): - return None - completed_path.replace(active_path) - return active_path - - def move_active_plan_to_completed(self, session_id: str) -> Path | None: - active_path = self.active_plan_path(session_id) - if not active_path.exists(): - return None - destination = self.completed_plans_dir / f"session-{session_id}.md" - active_path.replace(destination) - return destination - - def ensure_directories(self) -> None: - self.agents_dir.mkdir(parents=True, exist_ok=True) - self.plans_dir.mkdir(parents=True, exist_ok=True) - self.active_plans_dir.mkdir(parents=True, exist_ok=True) - self.completed_plans_dir.mkdir(parents=True, exist_ok=True) - self.revision_logs_dir.mkdir(parents=True, exist_ok=True) - self.active_revision_logs_dir.mkdir(parents=True, exist_ok=True) - self.completed_revision_logs_dir.mkdir(parents=True, exist_ok=True) diff --git a/scripts/lib/codex_workflow/plan.py b/scripts/lib/codex_workflow/plan.py deleted file mode 100644 index 8a93e403..00000000 --- a/scripts/lib/codex_workflow/plan.py +++ /dev/null @@ -1,313 +0,0 @@ -from __future__ import annotations - -import re -from dataclasses import dataclass -from datetime import datetime, timezone -from pathlib import Path - -from .atomic import atomic_write_text - -PLAN_APPROVAL_HEADING = "## Plan approval" -PLAN_APPROVAL_VALUES = {"approved", "unapproved"} -PLAN_PHASE_HEADING = "## Phase" -PLAN_PHASE_VALUES = {"planning", "implementation", "testing", "review"} -ORDERED_SUBTASKS_HEADING = "## Ordered subtasks" -FOLLOW_UP_SUBTASKS_HEADING = "## Follow-up subtasks (append-only)" -CHECKBOX_RE = re.compile(r"^\s*-\s\[(?P[ xX])\]\s+(?P.+?)\s*$") - - -@dataclass(frozen=True) -class CheckboxItem: - section: str - text: str - checked: bool - line_no: int - - -@dataclass(frozen=True) -class PlanAnalysis: - approval_status: str | None - phase_status: str | None - ordered_items: list[CheckboxItem] - follow_up_items: list[CheckboxItem] - missing_sections: list[str] - empty_sections: list[str] - invalid_sections: list[str] - - @property - def tracked_items(self) -> list[CheckboxItem]: - return [*self.ordered_items, *self.follow_up_items] - - @property - def is_approved(self) -> bool: - return self.approval_status == "approved" - - @property - def phase(self) -> str: - if self.phase_status is None: - raise ValueError("PlanAnalysis.phase requires a valid `## Phase` value.") - return self.phase_status - - @property - def all_checked(self) -> bool: - return not self.missing_sections and not self.empty_sections and not self.invalid_sections and all( - item.checked for item in self.tracked_items - ) - - @property - def first_unchecked(self) -> CheckboxItem | None: - for item in self.tracked_items: - if not item.checked: - return item - return None - - -def utc_now_rfc3339() -> str: - return datetime.now(timezone.utc).isoformat(timespec="seconds").replace("+00:00", "Z") - - -def render_session_plan( - session_id: str, - created_at: str | None = None, - approval_status: str = "unapproved", - phase: str | None = None, -) -> str: - if approval_status not in PLAN_APPROVAL_VALUES: - raise ValueError(f"approval_status must be one of {sorted(PLAN_APPROVAL_VALUES)}") - if phase is None: - phase = "planning" if approval_status == "unapproved" else "implementation" - if phase not in PLAN_PHASE_VALUES: - raise ValueError(f"phase must be one of {sorted(PLAN_PHASE_VALUES)}") - if approval_status == "unapproved" and phase != "planning": - raise ValueError("phase must be `planning` while approval_status is `unapproved`") - if approval_status == "approved" and phase == "planning": - raise ValueError("phase must be `implementation`, `testing`, or `review` while approval_status is `approved`") - stamp = created_at or utc_now_rfc3339() - return f"""# Session Plan: {session_id} - -{PLAN_APPROVAL_HEADING} -{approval_status} - -{PLAN_PHASE_HEADING} -{phase} - -## Goal -Describe the concrete user-visible outcome for this session. - -## Constraints -- Preserve completed subtasks as historical record. -- Keep each implementation subtask small enough to complete, debug, and validate within one context window. -- Run the most relevant local validation immediately after each subtask before checking it off. - -## Repo facts / assumptions -- Each workflow hook invocation must provide `session_id` in its JSON payload. -- The stop hook derives planning vs approved work from `## Plan approval` and derives the approved-work stage from `## Phase`. -- The active plan path for this session is `plans/active/session-{session_id}.md`; accepted sessions are archived under `plans/completed/`. - -## Acceptance criteria -- The workflow harness uses repository-local Codex hooks. -- Planning transitions to implementation only after plan approval. -- Implementation stops run only formatting and build checks. -- Testing stops run the selected final tests. -- Final completion requires the review-phase stop hook to block with actionable resume messages until all tracked checkboxes are checked and the reviewer approves. - -{ORDERED_SUBTASKS_HEADING} -- [ ] Replace this placeholder with the first approved implementation subtask. - -{FOLLOW_UP_SUBTASKS_HEADING} -- [x] No follow-up subtasks have been added yet. - -## Per-subtask validation -- Record the validation command and result immediately after each completed subtask. - -## Final validation -- Implementation-phase stop hook ran only formatting and build checks -- Testing-phase stop hook ran the selected final `scripts/run_tests.sh` gate -- Review-phase stop hook blocked with actionable resume messages until all tracked plan checkboxes were checked and the reviewer accepted -- `scripts/run_tests.sh` -- Hooks-disabled read-only reviewer via `codex exec` until acceptance - -## Decision log -- {stamp}: Session plan initialized. - -## Progress log -- {stamp}: Session plan initialized. -""" - - -def _find_section_bounds(lines: list[str], heading: str) -> tuple[int, int, int] | None: - heading_idx = None - for index, line in enumerate(lines): - if line.strip() == heading: - heading_idx = index - break - if heading_idx is None: - return None - start = heading_idx + 1 - end = len(lines) - for index in range(start, len(lines)): - if lines[index].startswith("## "): - end = index - break - return heading_idx, start, end - - -def _parse_checkboxes(lines: list[str], start: int, end: int, section: str) -> list[CheckboxItem]: - items: list[CheckboxItem] = [] - for line_no in range(start, end): - match = CHECKBOX_RE.match(lines[line_no]) - if not match: - continue - items.append( - CheckboxItem( - section=section, - text=match.group("text").strip(), - checked=match.group("mark").lower() == "x", - line_no=line_no + 1, - ) - ) - return items - - -def _parse_plan_approval(lines: list[str], heading: str) -> tuple[str | None, list[str], list[str]]: - bounds = _find_section_bounds(lines, heading) - missing_sections: list[str] = [] - invalid_sections: list[str] = [] - if bounds is None: - missing_sections.append(heading) - return None, missing_sections, invalid_sections - _, start, end = bounds - value: str | None = None - for line_no in range(start, end): - candidate = lines[line_no].strip() - if not candidate: - continue - value = candidate - break - if value not in PLAN_APPROVAL_VALUES: - invalid_sections.append(heading) - return None, missing_sections, invalid_sections - return value, missing_sections, invalid_sections - - -def _parse_plan_phase(lines: list[str], heading: str) -> tuple[str | None, list[str], list[str]]: - bounds = _find_section_bounds(lines, heading) - missing_sections: list[str] = [] - invalid_sections: list[str] = [] - if bounds is None: - missing_sections.append(heading) - return None, missing_sections, invalid_sections - _, start, end = bounds - value: str | None = None - for line_no in range(start, end): - candidate = lines[line_no].strip() - if not candidate: - continue - value = candidate - break - if value not in PLAN_PHASE_VALUES: - invalid_sections.append(heading) - return None, missing_sections, invalid_sections - return value, missing_sections, invalid_sections - - -def analyze_plan(plan_text: str) -> PlanAnalysis: - lines = plan_text.splitlines() - approval_status, missing_sections, invalid_sections = _parse_plan_approval(lines, PLAN_APPROVAL_HEADING) - phase_status, phase_missing_sections, phase_invalid_sections = _parse_plan_phase(lines, PLAN_PHASE_HEADING) - missing_sections.extend(phase_missing_sections) - invalid_sections.extend(phase_invalid_sections) - if approval_status == "unapproved" and phase_status != "planning": - invalid_sections.append(PLAN_PHASE_HEADING) - if approval_status == "approved" and phase_status == "planning": - invalid_sections.append(PLAN_PHASE_HEADING) - ordered_bounds = _find_section_bounds(lines, ORDERED_SUBTASKS_HEADING) - follow_up_bounds = _find_section_bounds(lines, FOLLOW_UP_SUBTASKS_HEADING) - empty_sections: list[str] = [] - ordered_items: list[CheckboxItem] = [] - follow_up_items: list[CheckboxItem] = [] - - if ordered_bounds is None: - missing_sections.append(ORDERED_SUBTASKS_HEADING) - else: - ordered_items = _parse_checkboxes(lines, ordered_bounds[1], ordered_bounds[2], ORDERED_SUBTASKS_HEADING) - if not ordered_items: - empty_sections.append(ORDERED_SUBTASKS_HEADING) - - if follow_up_bounds is None: - missing_sections.append(FOLLOW_UP_SUBTASKS_HEADING) - else: - follow_up_items = _parse_checkboxes( - lines, follow_up_bounds[1], follow_up_bounds[2], FOLLOW_UP_SUBTASKS_HEADING - ) - if not follow_up_items: - empty_sections.append(FOLLOW_UP_SUBTASKS_HEADING) - - return PlanAnalysis( - approval_status=approval_status, - phase_status=phase_status, - ordered_items=ordered_items, - follow_up_items=follow_up_items, - missing_sections=missing_sections, - empty_sections=empty_sections, - invalid_sections=invalid_sections, - ) - - -def _normalize_item_text(text: str) -> str: - cleaned = re.sub(r"^\s*-\s+\[[ xX]\]\s*", "", text.strip()) - cleaned = re.sub(r"^\s*-\s*", "", cleaned) - cleaned = re.sub(r"\s+", " ", cleaned) - return cleaned.strip() - - -def _dedupe_key(text: str) -> str: - return _normalize_item_text(text).rstrip(".").lower() - - -def append_follow_up_subtasks(plan_text: str, raw_items: list[str]) -> str: - lines = plan_text.splitlines() - bounds = _find_section_bounds(lines, FOLLOW_UP_SUBTASKS_HEADING) - analysis = analyze_plan(plan_text) - existing_open = {_dedupe_key(item.text) for item in analysis.follow_up_items if not item.checked} - new_items: list[str] = [] - seen_new: set[str] = set() - for raw in raw_items: - cleaned = _normalize_item_text(raw) - if not cleaned: - continue - key = _dedupe_key(cleaned) - if key in existing_open or key in seen_new: - continue - seen_new.add(key) - new_items.append(cleaned) - - if not new_items: - return plan_text - - if bounds is None: - insert_at = len(lines) - for index, line in enumerate(lines): - if line.strip() == "## Per-subtask validation": - insert_at = index - break - section_lines = [FOLLOW_UP_SUBTASKS_HEADING, *[f"- [ ] {item}" for item in new_items], ""] - if insert_at < len(lines) and insert_at > 0 and lines[insert_at - 1].strip(): - section_lines.insert(0, "") - lines[insert_at:insert_at] = section_lines - else: - _, start, end = bounds - insert_at = end - while insert_at > start and not lines[insert_at - 1].strip(): - insert_at -= 1 - lines[insert_at:insert_at] = [f"- [ ] {item}" for item in new_items] - - return "\n".join(lines) + "\n" - - -def append_follow_up_subtasks_file(plan_path: Path, raw_items: list[str]) -> str: - original = plan_path.read_text(encoding="utf-8") - updated = append_follow_up_subtasks(original, raw_items) - if updated != original: - atomic_write_text(plan_path, updated) - return updated diff --git a/scripts/lib/codex_workflow/runners.py b/scripts/lib/codex_workflow/runners.py deleted file mode 100644 index a7b10dd7..00000000 --- a/scripts/lib/codex_workflow/runners.py +++ /dev/null @@ -1,244 +0,0 @@ -from __future__ import annotations - -import json -import subprocess -import uuid -from dataclasses import dataclass -from pathlib import Path -from typing import Any, Protocol - -from .paths import RepoPaths - - -def _sanitize_label(label: str) -> str: - cleaned = "".join(ch if ch.isalnum() or ch in {"-", "_"} else "-" for ch in label.lower()) - return cleaned.strip("-") or "run" - - -def _summarize_text(text: str, max_lines: int = 6) -> str: - lines = [line.strip() for line in text.splitlines() if line.strip()] - if not lines: - return "" - return " | ".join(lines[-max_lines:]) - - -def _has_failure_signal(text: str) -> bool: - return any( - needle in text - for needle in ( - "test result: FAILED", - "error: test failed", - "[gpu-repeat] FAIL", - "FAILED.", - "panicked at ", - ) - ) - - -def summarize_logs(stdout_path: Path | None, stderr_path: Path | None) -> str: - stderr_text = stderr_path.read_text(encoding="utf-8") if stderr_path and stderr_path.exists() else "" - stdout_text = stdout_path.read_text(encoding="utf-8") if stdout_path and stdout_path.exists() else "" - if _has_failure_signal(stdout_text) and not _has_failure_signal(stderr_text): - summary = _summarize_text(stdout_text) - if summary: - return summary - summary = _summarize_text(stderr_text) - if summary: - return summary - summary = _summarize_text(stdout_text) - if summary: - return summary - return "No diagnostic output was captured." - - -@dataclass(frozen=True) -class StructuredExecResult: - ok: bool - payload: dict[str, Any] | None = None - result: str | None = None - msg: str | None = None - error: str | None = None - stdout_path: Path | None = None - stderr_path: Path | None = None - output_path: Path | None = None - - -@dataclass(frozen=True) -class FinalTestResult: - ok: bool - summary: str - returncode: int - stdout_path: Path | None = None - stderr_path: Path | None = None - - -class StructuredExecRunner(Protocol): - def run(self, prompt: str, schema_path: Path, label: str) -> StructuredExecResult: - ... - - -class FinalTestRunner(Protocol): - def run(self, label: str, *, run_python: bool = True, run_rust: bool = True) -> FinalTestResult: - ... - - -class BuildCheckRunner(Protocol): - def run(self, label: str) -> FinalTestResult: - ... - - -class CodexExecRunner: - def __init__(self, paths: RepoPaths, session_id: str) -> None: - self.paths = paths - self.session_id = session_id - - def run(self, prompt: str, schema_path: Path, label: str) -> StructuredExecResult: - run_id = uuid.uuid4().hex - safe_label = _sanitize_label(label) - stdout_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.stdout.log" - stderr_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.stderr.log" - output_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.json" - command = [ - "codex", - "exec", - "--skip-git-repo-check", - "--disable", - "codex_hooks", - # The default Linux bubblewrap backend can fail inside the desktop hook - # environment before commands run (`bwrap: loopback: Failed RTM_NEWADDR`). - # Landlock preserves read-only command sandboxing without requiring that - # loopback setup. - "--enable", - "use_legacy_landlock", - "--sandbox", - "read-only", - "--output-schema", - str(schema_path), - "-o", - str(output_path), - "--cd", - str(self.paths.repo_root), - prompt, - ] - with stdout_path.open("w", encoding="utf-8") as stdout_handle, stderr_path.open( - "w", encoding="utf-8" - ) as stderr_handle: - completed = subprocess.run( - command, - cwd=self.paths.repo_root, - stdout=stdout_handle, - stderr=stderr_handle, - text=True, - check=False, - ) - if completed.returncode != 0: - return StructuredExecResult( - ok=False, - error=summarize_logs(stdout_path, stderr_path), - stdout_path=stdout_path, - stderr_path=stderr_path, - output_path=output_path, - ) - try: - payload = json.loads(output_path.read_text(encoding="utf-8")) - except (FileNotFoundError, OSError, json.JSONDecodeError) as exc: - return StructuredExecResult( - ok=False, - error=f"Failed to parse structured Codex output: {exc}", - stdout_path=stdout_path, - stderr_path=stderr_path, - output_path=output_path, - ) - if not isinstance(payload, dict): - return StructuredExecResult( - ok=False, - error="Structured Codex output was not a JSON object.", - stdout_path=stdout_path, - stderr_path=stderr_path, - output_path=output_path, - ) - result = payload.get("result") - msg = payload.get("msg") - if result is not None and not isinstance(result, str): - result = None - if msg is not None and not isinstance(msg, str): - msg = None - return StructuredExecResult( - ok=True, - payload=payload, - result=result, - msg=msg, - stdout_path=stdout_path, - stderr_path=stderr_path, - output_path=output_path, - ) - - -class ShellFinalTestRunner: - def __init__(self, paths: RepoPaths, session_id: str) -> None: - self.paths = paths - self.session_id = session_id - - def run(self, label: str, *, run_python: bool = True, run_rust: bool = True) -> FinalTestResult: - run_id = uuid.uuid4().hex - safe_label = _sanitize_label(label) - stdout_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.stdout.log" - stderr_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.stderr.log" - command = [str(self.paths.scripts_dir / "run_tests.sh")] - if run_python and not run_rust: - command.append("--python") - elif run_rust and not run_python: - command.append("--rust") - elif not run_python and not run_rust: - raise ValueError("ShellFinalTestRunner.run requires at least one test suite.") - with stdout_path.open("w", encoding="utf-8") as stdout_handle, stderr_path.open( - "w", encoding="utf-8" - ) as stderr_handle: - completed = subprocess.run( - command, - cwd=self.paths.repo_root, - stdout=stdout_handle, - stderr=stderr_handle, - text=True, - check=False, - ) - summary = summarize_logs(stdout_path, stderr_path) - return FinalTestResult( - ok=completed.returncode == 0, - summary=summary, - returncode=completed.returncode, - stdout_path=stdout_path, - stderr_path=stderr_path, - ) - - -class ShellBuildCheckRunner: - def __init__(self, paths: RepoPaths, session_id: str) -> None: - self.paths = paths - self.session_id = session_id - - def run(self, label: str) -> FinalTestResult: - run_id = uuid.uuid4().hex - safe_label = _sanitize_label(label) - stdout_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.stdout.log" - stderr_path = self.paths.active_revision_logs_dir / f"{self.session_id}-{safe_label}-{run_id}.stderr.log" - command = [str(self.paths.scripts_dir / "run_build_checks.sh")] - with stdout_path.open("w", encoding="utf-8") as stdout_handle, stderr_path.open( - "w", encoding="utf-8" - ) as stderr_handle: - completed = subprocess.run( - command, - cwd=self.paths.repo_root, - stdout=stdout_handle, - stderr=stderr_handle, - text=True, - check=False, - ) - summary = summarize_logs(stdout_path, stderr_path) - return FinalTestResult( - ok=completed.returncode == 0, - summary=summary, - returncode=completed.returncode, - stdout_path=stdout_path, - stderr_path=stderr_path, - ) diff --git a/scripts/lib/codex_workflow/tests/__init__.py b/scripts/lib/codex_workflow/tests/__init__.py deleted file mode 100644 index 8b137891..00000000 --- a/scripts/lib/codex_workflow/tests/__init__.py +++ /dev/null @@ -1 +0,0 @@ - diff --git a/scripts/lib/codex_workflow/tests/test_workflow.py b/scripts/lib/codex_workflow/tests/test_workflow.py deleted file mode 100644 index 654f7584..00000000 --- a/scripts/lib/codex_workflow/tests/test_workflow.py +++ /dev/null @@ -1,1180 +0,0 @@ -from __future__ import annotations - -import io -import json -import tempfile -import unittest -from pathlib import Path -from unittest import mock - -from codex_workflow.atomic import atomic_write_text -from codex_workflow.cli import emit_outcome -from codex_workflow.hooks import MAX_INFRA_RETRY_ATTEMPTS, handle_session_start, handle_stop -from codex_workflow.paths import RepoPaths -from codex_workflow.plan import ( - FOLLOW_UP_SUBTASKS_HEADING, - ORDERED_SUBTASKS_HEADING, - PLAN_APPROVAL_HEADING, - PLAN_PHASE_HEADING, - analyze_plan, - append_follow_up_subtasks, - render_session_plan, -) -from codex_workflow.runners import ( - CodexExecRunner, - FinalTestResult, - ShellBuildCheckRunner, - ShellFinalTestRunner, - StructuredExecResult, - summarize_logs, -) -from codex_workflow.transcript import ( - initial_user_message_from_transcript, - latest_assistant_message_from_transcript, - latest_user_message_from_transcript, -) - - -def build_plan_text( - session_id: str, - ordered_items: list[tuple[bool, str]], - follow_up_items: list[tuple[bool, str]] | None = None, - approval_status: str = "approved", - phase: str | None = None, -) -> str: - if follow_up_items is None: - follow_up_items = [(True, "No follow-up subtasks have been added yet.")] - if phase is None: - phase = "planning" if approval_status == "unapproved" else "implementation" - ordered_block = "\n".join(f"- [{'x' if checked else ' '}] {text}" for checked, text in ordered_items) - follow_up_block = "\n".join(f"- [{'x' if checked else ' '}] {text}" for checked, text in follow_up_items) - return f"""# Session Plan: {session_id} - -{PLAN_APPROVAL_HEADING} -{approval_status} - -{PLAN_PHASE_HEADING} -{phase} - -## Goal -Goal - -## Constraints -- Constraint - -## Repo facts / assumptions -- Fact - -## Acceptance criteria -- Criterion - -{ORDERED_SUBTASKS_HEADING} -{ordered_block} - -{FOLLOW_UP_SUBTASKS_HEADING} -{follow_up_block} - -## Per-subtask validation -- Validation - -## Final validation -- Final validation - -## Decision log -- Initialized - -## Progress log -- Initialized -""" - - -class FakeExecRunner: - def __init__(self, *results: StructuredExecResult) -> None: - self.results = list(results) - self.calls: list[tuple[str, Path, str]] = [] - - def run(self, prompt: str, schema_path: Path, label: str) -> StructuredExecResult: - self.calls.append((prompt, schema_path, label)) - return self.results.pop(0) - - -class FakeTestRunner: - def __init__(self, *results: FinalTestResult) -> None: - self.results = list(results) - self.calls: list[tuple[str, bool, bool]] = [] - - def run(self, label: str, *, run_python: bool = True, run_rust: bool = True) -> FinalTestResult: - self.calls.append((label, run_python, run_rust)) - return self.results.pop(0) - - -class FakeBuildRunner: - def __init__(self, *results: FinalTestResult) -> None: - self.results = list(results) - self.calls: list[str] = [] - - def run(self, label: str) -> FinalTestResult: - self.calls.append(label) - return self.results.pop(0) - - -class WorkflowHarnessTests(unittest.TestCase): - def setUp(self) -> None: - self.tmpdir = tempfile.TemporaryDirectory() - self.repo_root = Path(self.tmpdir.name) - self.paths = RepoPaths(self.repo_root) - self.paths.ensure_directories() - (self.repo_root / "schemas").mkdir(parents=True, exist_ok=True) - atomic_write_text(self.paths.schemas_dir / "review-decision.schema.json", "{}\n") - atomic_write_text(self.paths.schemas_dir / "session-start-intent.schema.json", "{}\n") - - def tearDown(self) -> None: - self.tmpdir.cleanup() - - def write_plan( - self, - session_id: str = "session-1", - ordered_items: list[tuple[bool, str]] | None = None, - follow_up_items: list[tuple[bool, str]] | None = None, - approval_status: str = "approved", - phase: str | None = None, - ) -> Path: - if ordered_items is None: - ordered_items = [(False, "Replace this placeholder with a real subtask.")] - plan_path = self.paths.default_plan_path(session_id) - atomic_write_text( - plan_path, - build_plan_text( - session_id, - ordered_items=ordered_items, - follow_up_items=follow_up_items, - approval_status=approval_status, - phase=phase, - ), - ) - return plan_path - - def write_completed_plan( - self, - session_id: str = "session-1", - ordered_items: list[tuple[bool, str]] | None = None, - follow_up_items: list[tuple[bool, str]] | None = None, - approval_status: str = "approved", - phase: str | None = None, - ) -> Path: - if ordered_items is None: - ordered_items = [(False, "Replace this placeholder with a real subtask.")] - plan_path = self.paths.completed_plan_path(session_id) - atomic_write_text( - plan_path, - build_plan_text( - session_id, - ordered_items=ordered_items, - follow_up_items=follow_up_items, - approval_status=approval_status, - phase=phase, - ), - ) - return plan_path - - def test_session_start_initializes_plan_for_non_review_requests(self) -> None: - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="build", msg="implementation request")) - outcome = handle_session_start( - {"session_id": "abc123", "initial_user_message": "Implement the stop hook changes."}, - self.repo_root, - exec_runner=exec_runner, - ) - - self.assertEqual(outcome.exit_code, 0) - plan_path = self.paths.default_plan_path("abc123") - self.assertTrue(plan_path.exists()) - analysis = analyze_plan(plan_path.read_text(encoding="utf-8")) - self.assertEqual(analysis.approval_status, "unapproved") - self.assertEqual(analysis.phase, "planning") - self.assertFalse((self.paths.agents_dir / "current-session-id").exists()) - self.assertEqual(list(self.paths.agents_dir.glob("session-abc123.json")), []) - self.assertEqual(len(exec_runner.calls), 1) - prompt, schema_path, label = exec_runner.calls[0] - self.assertIn("Implement the stop hook changes.", prompt) - self.assertTrue(str(schema_path).endswith("session-start-intent.schema.json")) - self.assertEqual(label, "session-start-intent") - - def test_repo_paths_creates_revision_logs_directory(self) -> None: - self.assertTrue(self.paths.revision_logs_dir.exists()) - self.assertEqual(self.paths.revision_logs_dir, self.repo_root / "revision_logs") - self.assertTrue(self.paths.active_revision_logs_dir.exists()) - self.assertTrue(self.paths.completed_revision_logs_dir.exists()) - self.assertEqual(self.paths.active_revision_logs_dir, self.repo_root / "revision_logs" / "active") - self.assertEqual(self.paths.completed_revision_logs_dir, self.repo_root / "revision_logs" / "completed") - self.assertTrue(self.paths.active_plans_dir.exists()) - self.assertTrue(self.paths.completed_plans_dir.exists()) - self.assertEqual(self.paths.active_plans_dir, self.repo_root / "plans" / "active") - self.assertEqual(self.paths.completed_plans_dir, self.repo_root / "plans" / "completed") - - def test_session_start_skips_plan_creation_for_explicit_review_requests(self) -> None: - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="review", msg="explicit review request")) - outcome = handle_session_start( - {"session_id": "review-123", "initial_user_message": "Please review this patch."}, - self.repo_root, - exec_runner=exec_runner, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertFalse(self.paths.default_plan_path("review-123").exists()) - self.assertEqual(len(exec_runner.calls), 1) - - def test_session_start_is_idempotent_for_existing_plan(self) -> None: - plan_path = self.write_plan( - session_id="abc123", - ordered_items=[(True, "Done")], - approval_status="approved", - ) - original = plan_path.read_text(encoding="utf-8") - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="build", msg="implementation request")) - - outcome = handle_session_start( - {"session_id": "abc123", "initial_user_message": "Continue implementing the approved plan."}, - self.repo_root, - exec_runner=exec_runner, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(plan_path.read_text(encoding="utf-8"), original) - - def test_session_start_requires_session_id_in_payload(self) -> None: - outcome = handle_session_start({}, self.repo_root) - - self.assertEqual(outcome.exit_code, 1) - self.assertIn("requires `session_id`", outcome.stderr_message) - - def test_session_start_requires_initial_user_prompt(self) -> None: - outcome = handle_session_start({"session_id": "abc123"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 1) - self.assertIn("initial user prompt", outcome.stderr_message) - - def test_session_start_rejects_latest_user_message_as_initial_prompt(self) -> None: - outcome = handle_session_start({"session_id": "abc123", "latest_user_message": "Please review this patch."}, self.repo_root) - - self.assertEqual(outcome.exit_code, 1) - self.assertIn("initial user prompt", outcome.stderr_message) - - def test_session_start_can_extract_initial_prompt_from_transcript(self) -> None: - transcript_path = self.repo_root / "session-start-transcript.jsonl" - atomic_write_text( - transcript_path, - "\n".join( - [ - "{\"type\":\"event_msg\",\"payload\":{\"type\":\"user_message\",\"message\":\"Implement this workflow change.\"}}", - "{\"type\":\"event_msg\",\"payload\":{\"type\":\"user_message\",\"message\":\"Please review the implementation.\"}}", - ] - ) - + "\n", - ) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="build", msg="initial prompt is a build request")) - - outcome = handle_session_start( - {"session_id": "review-transcript", "transcript_path": str(transcript_path)}, - self.repo_root, - exec_runner=exec_runner, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertTrue(self.paths.default_plan_path("review-transcript").exists()) - self.assertEqual(len(exec_runner.calls), 1) - prompt, _, _ = exec_runner.calls[0] - self.assertIn("Implement this workflow change.", prompt) - self.assertNotIn("Please review the implementation.", prompt) - - def test_session_start_blocks_when_intent_classifier_fails(self) -> None: - exec_runner = FakeExecRunner(StructuredExecResult(ok=False, error="classifier unavailable")) - - outcome = handle_session_start( - {"session_id": "abc123", "initial_user_message": "Implement the change."}, - self.repo_root, - exec_runner=exec_runner, - ) - - self.assertEqual(outcome.exit_code, 1) - self.assertIn("could not classify", outcome.stderr_message) - - def test_review_decision_schema_requires_all_defined_properties(self) -> None: - schema_path = Path(__file__).resolve().parents[4] / "schemas" / "review-decision.schema.json" - schema = json.loads(schema_path.read_text(encoding="utf-8")) - - self.assertEqual(set(schema["properties"]), {"result", "msg"}) - self.assertEqual(set(schema["required"]), {"result", "msg"}) - - def test_session_start_intent_schema_requires_all_defined_properties(self) -> None: - schema_path = Path(__file__).resolve().parents[4] / "schemas" / "session-start-intent.schema.json" - schema = json.loads(schema_path.read_text(encoding="utf-8")) - - self.assertEqual(set(schema["properties"]), {"result", "msg"}) - self.assertEqual(set(schema["required"]), {"result", "msg"}) - - def test_planning_stop_allows_stop_without_running_nested_work(self) -> None: - self.write_plan(session_id="plan-first", approval_status="unapproved") - exec_runner = FakeExecRunner() - test_runner = FakeTestRunner() - - outcome = handle_stop( - {"session_id": "plan-first"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertIsNone(outcome.stdout_payload) - self.assertEqual(exec_runner.calls, []) - self.assertEqual(test_runner.calls, []) - - def test_planning_stop_allows_stop_even_when_required_checkbox_section_is_empty(self) -> None: - plan_path = self.write_plan(session_id="planning-empty", approval_status="unapproved") - broken = plan_path.read_text(encoding="utf-8").replace("- [ ] Replace this placeholder with a real subtask.\n", "", 1) - atomic_write_text(plan_path, broken) - exec_runner = FakeExecRunner() - test_runner = FakeTestRunner() - - outcome = handle_stop( - {"session_id": "planning-empty"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertIsNone(outcome.stdout_payload) - self.assertEqual(exec_runner.calls, []) - self.assertEqual(test_runner.calls, []) - - def test_planning_stop_blocks_when_phase_section_is_missing(self) -> None: - plan_path = self.write_plan(session_id="planning-missing-phase", approval_status="unapproved") - original = plan_path.read_text(encoding="utf-8") - broken = original.replace(f"{PLAN_PHASE_HEADING}\nplanning\n\n", "", 1) - atomic_write_text(plan_path, broken) - - outcome = handle_stop({"session_id": "planning-missing-phase"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn(PLAN_PHASE_HEADING, outcome.stderr_message) - - def test_stop_requires_session_id_in_payload(self) -> None: - self.write_plan( - session_id="pointer-session", - ordered_items=[(True, "Approved work already completed.")], - approval_status="approved", - ) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - - outcome = handle_stop( - {}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("requires `session_id`", outcome.stderr_message) - self.assertEqual(test_runner.calls, []) - - def test_stop_allows_explicit_review_sessions_without_plan_file(self) -> None: - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - - outcome = handle_stop( - {"session_id": "review-only"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertIsNone(outcome.stdout_payload) - self.assertEqual(exec_runner.calls, []) - self.assertEqual(test_runner.calls, []) - - def test_stop_reactivates_completed_plan_before_evaluating(self) -> None: - self.write_completed_plan( - session_id="reactivate-plan", - ordered_items=[(False, "Continue the interrupted task.")], - approval_status="approved", - ) - - outcome = handle_stop({"session_id": "reactivate-plan"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 2) - self.assertTrue(self.paths.active_plan_path("reactivate-plan").exists()) - self.assertFalse(self.paths.completed_plan_path("reactivate-plan").exists()) - self.assertIn("Continue the interrupted task.", outcome.stderr_message) - - def test_stop_blocks_when_plan_approval_section_is_missing(self) -> None: - plan_path = self.write_plan(session_id="missing-approval", approval_status="approved") - original = plan_path.read_text(encoding="utf-8") - broken = original.replace(f"{PLAN_APPROVAL_HEADING}\napproved\n\n", "", 1) - atomic_write_text(plan_path, broken) - - outcome = handle_stop({"session_id": "missing-approval"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn(PLAN_APPROVAL_HEADING, outcome.stderr_message) - - def test_stop_blocks_when_phase_section_is_missing(self) -> None: - plan_path = self.write_plan(session_id="missing-phase", approval_status="approved") - original = plan_path.read_text(encoding="utf-8") - broken = original.replace(f"{PLAN_PHASE_HEADING}\nimplementation\n\n", "", 1) - atomic_write_text(plan_path, broken) - - outcome = handle_stop({"session_id": "missing-phase"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn(PLAN_PHASE_HEADING, outcome.stderr_message) - - def test_stop_blocks_when_required_checkbox_section_is_empty(self) -> None: - plan_path = self.write_plan(session_id="empty-ordered", approval_status="approved") - original = plan_path.read_text(encoding="utf-8") - broken = original.replace("- [ ] Replace this placeholder with a real subtask.\n", "", 1) - atomic_write_text(plan_path, broken) - - outcome = handle_stop({"session_id": "empty-ordered"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("empty", outcome.stderr_message) - self.assertIn(ORDERED_SUBTASKS_HEADING, outcome.stderr_message) - - def test_stop_blocks_when_phase_is_incompatible_with_approval(self) -> None: - self.write_plan( - session_id="invalid-phase", - ordered_items=[(True, "Approved work already completed.")], - approval_status="approved", - phase="planning", - ) - - outcome = handle_stop({"session_id": "invalid-phase"}, self.repo_root) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn(PLAN_PHASE_HEADING, outcome.stderr_message) - - def test_implementation_phase_runs_build_checks_without_unit_tests_reviewer_or_archiving(self) -> None: - self.write_plan( - session_id="approved-loop", - ordered_items=[(True, "Approved work already completed.")], - approval_status="approved", - ) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - build_runner = FakeBuildRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - - outcome = handle_stop( - {"session_id": "approved-loop"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - build_runner=build_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual( - outcome.stdout_payload, - { - "continue": False, - "stopReason": "All subtasks are complete and implementation formatting/build checks passed. Unit tests and reviewer execution were skipped because the session phase is `implementation`.", - }, - ) - self.assertEqual(build_runner.calls, ["implementation-build"]) - self.assertEqual(test_runner.calls, []) - self.assertEqual(exec_runner.calls, []) - self.assertTrue(self.paths.active_plan_path("approved-loop").exists()) - self.assertFalse(self.paths.completed_plan_path("approved-loop").exists()) - - def test_stop_blocks_when_unchecked_ordered_subtask_remains(self) -> None: - self.write_plan( - session_id="impl-unchecked", - ordered_items=[(False, "Implement the session start hook.")], - approval_status="approved", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "impl-unchecked"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("Implementation is not complete.", outcome.stderr_message) - self.assertIn("Implement the session start hook.", outcome.stderr_message) - self.assertEqual(test_runner.calls, []) - self.assertEqual(exec_runner.calls, []) - - def test_stop_blocks_when_unchecked_follow_up_subtask_remains(self) -> None: - self.write_plan( - session_id="impl-follow-up-open", - ordered_items=[(True, "Done ordered work.")], - follow_up_items=[(False, "Fix the reviewer-reported edge case.")], - approval_status="approved", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "impl-follow-up-open"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("Implementation is not complete.", outcome.stderr_message) - self.assertIn("Fix the reviewer-reported edge case.", outcome.stderr_message) - self.assertEqual(test_runner.calls, []) - self.assertEqual(exec_runner.calls, []) - - def test_testing_phase_test_failure_appends_follow_up_and_blocks(self) -> None: - plan_path = self.write_plan( - session_id="tests-fail", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="testing", - ) - test_runner = FakeTestRunner( - FinalTestResult(ok=False, summary="workflow unit tests failed", returncode=1) - ) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "tests-fail"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("Testing-phase final tests failed.", outcome.stderr_message) - self.assertEqual(exec_runner.calls, []) - plan_text = plan_path.read_text(encoding="utf-8") - self.assertIn("Fix the failing final validation from `scripts/run_tests.sh`", plan_text) - self.assertIn("- [x] Implement everything.", plan_text) - self.assertEqual(test_runner.calls, [("final-tests", True, True)]) - - def test_review_phase_reviewer_revision_appends_follow_up_and_blocks(self) -> None: - plan_path = self.write_plan( - session_id="review-revision", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="review", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner( - StructuredExecResult(ok=True, result="revision", msg="Handle the malformed state path."), - ) - - outcome = handle_stop( - {"session_id": "review-revision"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("Reviewer requested changes.", outcome.stderr_message) - plan_text = plan_path.read_text(encoding="utf-8") - self.assertIn("Address reviewer feedback from the final read-only Codex review", plan_text) - - def test_review_phase_runs_reviewer_before_archiving(self) -> None: - self.write_plan( - session_id="precheck-complete", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="review", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "precheck-complete"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(test_runner.calls, []) - self.assertEqual(len(exec_runner.calls), 1) - self.assertFalse(self.paths.active_plan_path("precheck-complete").exists()) - self.assertTrue(self.paths.completed_plan_path("precheck-complete").exists()) - - def test_accepted_stop_moves_active_revision_logs_to_completed(self) -> None: - self.write_plan( - session_id="archive-logs", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="review", - ) - active_stdout = self.paths.active_revision_logs_dir / "archive-logs-final-review-1.stdout.log" - active_stderr = self.paths.active_revision_logs_dir / "archive-logs-final-review-1.stderr.log" - atomic_write_text(active_stdout, "stdout\n") - atomic_write_text(active_stderr, "stderr\n") - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "archive-logs"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertFalse(active_stdout.exists()) - self.assertFalse(active_stderr.exists()) - self.assertTrue((self.paths.completed_revision_logs_dir / active_stdout.name).exists()) - self.assertTrue((self.paths.completed_revision_logs_dir / active_stderr.name).exists()) - self.assertFalse(self.paths.active_plan_path("archive-logs").exists()) - self.assertTrue(self.paths.completed_plan_path("archive-logs").exists()) - - def test_blocked_stop_keeps_plan_and_revision_logs_active(self) -> None: - plan_path = self.write_plan( - session_id="keep-active", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="testing", - ) - active_stdout = self.paths.active_revision_logs_dir / "keep-active-final-tests-1.stdout.log" - atomic_write_text(active_stdout, "stdout\n") - test_runner = FakeTestRunner(FinalTestResult(ok=False, summary="workflow unit tests failed", returncode=1)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "keep-active"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertTrue(plan_path.exists()) - self.assertFalse(self.paths.completed_plan_path("keep-active").exists()) - self.assertTrue(active_stdout.exists()) - self.assertFalse((self.paths.completed_revision_logs_dir / active_stdout.name).exists()) - - def test_implementation_phase_skips_build_checks_when_no_files_changed(self) -> None: - self.write_plan( - session_id="no-file-changes", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - build_runner = FakeBuildRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - - outcome = handle_stop( - {"session_id": "no-file-changes"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - build_runner=build_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: [], - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual( - outcome.stdout_payload, - { - "continue": False, - "stopReason": "All subtasks are complete and no files changed, so implementation formatting/build checks were skipped.", - }, - ) - self.assertEqual(build_runner.calls, []) - self.assertEqual(test_runner.calls, []) - self.assertEqual(exec_runner.calls, []) - - def test_implementation_phase_build_failure_appends_follow_up_and_blocks(self) -> None: - plan_path = self.write_plan( - session_id="build-fail", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - build_runner = FakeBuildRunner(FinalTestResult(ok=False, summary="build failed", returncode=1)) - - outcome = handle_stop( - {"session_id": "build-fail"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - build_runner=build_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: ["src/lib.rs"], - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("Implementation formatting/build checks failed.", outcome.stderr_message) - self.assertEqual(build_runner.calls, ["implementation-build"]) - self.assertEqual(test_runner.calls, []) - self.assertEqual(exec_runner.calls, []) - plan_text = plan_path.read_text(encoding="utf-8") - self.assertIn("Fix the failing implementation formatting/build validation from `scripts/run_build_checks.sh`", plan_text) - - def test_review_phase_skips_tests_but_still_runs_reviewer_for_non_selected_changes(self) -> None: - self.write_plan( - session_id="docs-only-changes", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="review", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "docs-only-changes"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: ["README.md", "docs/notes.txt"], - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual( - outcome.stdout_payload, - { - "continue": False, - "stopReason": "All subtasks are complete and reviewer approved.", - }, - ) - self.assertEqual(test_runner.calls, []) - self.assertEqual(len(exec_runner.calls), 1) - - def test_complete_plan_runs_final_tests_when_cargo_toml_changed(self) -> None: - self.write_plan( - session_id="cargo-toml-change", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="testing", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "cargo-toml-change"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: ["Cargo.toml", "README.md"], - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(test_runner.calls, [("final-tests", False, True)]) - self.assertEqual(exec_runner.calls, []) - - def test_complete_plan_runs_final_tests_when_rust_file_changed(self) -> None: - self.write_plan( - session_id="rust-change", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="testing", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "rust-change"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: ["src/lib.rs"], - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(test_runner.calls, [("final-tests", False, True)]) - self.assertEqual(exec_runner.calls, []) - - def test_complete_plan_runs_only_python_tests_when_python_file_changed(self) -> None: - self.write_plan( - session_id="python-change", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="testing", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "python-change"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: ["scripts/lib/codex_workflow/hooks.py"], - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(test_runner.calls, [("final-tests", True, False)]) - self.assertEqual(exec_runner.calls, []) - - def test_complete_plan_runs_python_and_rust_tests_for_mixed_changes(self) -> None: - self.write_plan( - session_id="mixed-change", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="testing", - ) - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner(StructuredExecResult(ok=True, result="accept", msg=None)) - - outcome = handle_stop( - {"session_id": "mixed-change"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=lambda _: None, - edited_paths_provider=lambda _: ["scripts/lib/codex_workflow/hooks.py", "src/lib.rs"], - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(test_runner.calls, [("final-tests", True, True)]) - self.assertEqual(exec_runner.calls, []) - - def test_shell_final_test_runner_uses_python_only_flag(self) -> None: - runner = ShellFinalTestRunner(self.paths, "python-only") - with mock.patch("codex_workflow.runners.subprocess.run") as run_mock: - run_mock.return_value.returncode = 0 - - runner.run(label="final-tests", run_python=True, run_rust=False) - - command = run_mock.call_args.args[0] - self.assertEqual(command, [str(self.paths.scripts_dir / "run_tests.sh"), "--python"]) - - def test_shell_final_test_runner_uses_rust_only_flag(self) -> None: - runner = ShellFinalTestRunner(self.paths, "rust-only") - with mock.patch("codex_workflow.runners.subprocess.run") as run_mock: - run_mock.return_value.returncode = 0 - - runner.run(label="final-tests", run_python=False, run_rust=True) - - command = run_mock.call_args.args[0] - self.assertEqual(command, [str(self.paths.scripts_dir / "run_tests.sh"), "--rust"]) - - def test_shell_final_test_runner_uses_default_command_for_mixed_selection(self) -> None: - runner = ShellFinalTestRunner(self.paths, "mixed") - with mock.patch("codex_workflow.runners.subprocess.run") as run_mock: - run_mock.return_value.returncode = 0 - - runner.run(label="final-tests", run_python=True, run_rust=True) - - command = run_mock.call_args.args[0] - self.assertEqual(command, [str(self.paths.scripts_dir / "run_tests.sh")]) - - def test_shell_build_check_runner_uses_build_check_script(self) -> None: - runner = ShellBuildCheckRunner(self.paths, "build-check") - with mock.patch("codex_workflow.runners.subprocess.run") as run_mock: - run_mock.return_value.returncode = 0 - - runner.run(label="implementation-build") - - command = run_mock.call_args.args[0] - self.assertEqual(command, [str(self.paths.scripts_dir / "run_build_checks.sh")]) - - def test_codex_exec_runner_uses_landlock_read_only_sandbox(self) -> None: - runner = CodexExecRunner(self.paths, "review-sandbox") - - def fake_run(command: list[str], **_: object) -> object: - output_path = Path(command[command.index("-o") + 1]) - output_path.write_text('{"result":"accept"}\n', encoding="utf-8") - completed = mock.Mock() - completed.returncode = 0 - return completed - - with mock.patch("codex_workflow.runners.subprocess.run", side_effect=fake_run) as run_mock: - result = runner.run( - prompt="review", - schema_path=self.paths.schemas_dir / "review-decision.schema.json", - label="final-review", - ) - - self.assertTrue(result.ok) - command = run_mock.call_args.args[0] - self.assertIn("--sandbox", command) - self.assertEqual(command[command.index("--sandbox") + 1], "read-only") - self.assertIn("--enable", command) - self.assertEqual(command[command.index("--enable") + 1], "use_legacy_landlock") - - def test_summarize_logs_prefers_stdout_when_it_contains_test_failure_and_stderr_only_has_compile_noise( - self, - ) -> None: - stdout_path = self.repo_root / "stdout.log" - stderr_path = self.repo_root / "stderr.log" - stdout_path.write_text( - "\n".join( - [ - "test something ... ok", - "failures:", - "---- bench_estimator::tests::test_estimate_gate_bench_panics_on_sub_circuit_output_placeholder stdout ----", - "note: panic did not contain expected string", - 'expected substring: "unexpected SubCircuitOutput"', - "test result: FAILED. 284 passed; 1 failed; 4 ignored; 0 measured; 0 filtered out; finished in 430.53s", - "error: test failed, to rerun pass `--lib`", - ] - ) - + "\n", - encoding="utf-8", - ) - stderr_path.write_text( - "\n".join( - [ - "warning: `mxx` (lib) generated 3 warnings", - "warning: `mxx` (lib test) generated 3 warnings (3 duplicates)", - "Finished `release` profile [optimized] target(s) in 1m 05s", - "Executable tests/test_gpu_ggh15_negacyclic_conv_mul.rs (target/release/deps/test_gpu_ggh15_negacyclic_conv_mul-b997dc1c6882de89)", - ] - ) - + "\n", - encoding="utf-8", - ) - - summary = summarize_logs(stdout_path, stderr_path) - - self.assertIn("test result: FAILED", summary) - self.assertIn("error: test failed", summary) - self.assertNotIn("Executable tests/test_gpu_ggh15_negacyclic_conv_mul.rs", summary) - - def test_reviewer_infra_failure_retries_with_backoff(self) -> None: - self.write_plan( - session_id="review-retry", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="review", - ) - sleep_calls: list[float] = [] - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner( - StructuredExecResult(ok=False, error="review infra failure"), - StructuredExecResult(ok=True, result="accept", msg=None), - ) - - outcome = handle_stop( - {"session_id": "review-retry"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=sleep_calls.append, - ) - - self.assertEqual(outcome.exit_code, 0) - self.assertEqual(sleep_calls, [1.0]) - self.assertEqual(len(exec_runner.calls), 2) - - def test_reviewer_infra_failure_stops_after_retry_limit(self) -> None: - self.write_plan( - session_id="review-retry-limit", - ordered_items=[(True, "Implement everything.")], - approval_status="approved", - phase="review", - ) - sleep_calls: list[float] = [] - test_runner = FakeTestRunner(FinalTestResult(ok=True, summary="ok", returncode=0)) - exec_runner = FakeExecRunner( - *[ - StructuredExecResult(ok=False, error="permanent review failure") - for _ in range(MAX_INFRA_RETRY_ATTEMPTS) - ] - ) - - outcome = handle_stop( - {"session_id": "review-retry-limit"}, - self.repo_root, - exec_runner=exec_runner, - test_runner=test_runner, - sleep_fn=sleep_calls.append, - ) - - self.assertEqual(outcome.exit_code, 2) - self.assertIn("100 failed nested reviewer attempts", outcome.stderr_message) - self.assertIn("permanent review failure", outcome.stderr_message) - self.assertEqual(len(exec_runner.calls), MAX_INFRA_RETRY_ATTEMPTS) - self.assertEqual(len(sleep_calls), MAX_INFRA_RETRY_ATTEMPTS - 1) - - def test_transcript_parser_returns_latest_user_message(self) -> None: - transcript_path = self.repo_root / "transcript.jsonl" - atomic_write_text( - transcript_path, - "\n".join( - [ - "{\"type\":\"response_item\",\"payload\":{\"type\":\"message\",\"role\":\"user\",\"content\":[{\"type\":\"input_text\",\"text\":\"First\"}]}}", - "not-json", - "{\"type\":\"event_msg\",\"payload\":{\"type\":\"user_message\",\"message\":\"Final accept\"}}", - ] - ) - + "\n", - ) - - self.assertEqual(latest_user_message_from_transcript(transcript_path), "Final accept") - - def test_transcript_parser_returns_initial_user_message(self) -> None: - transcript_path = self.repo_root / "initial-transcript.jsonl" - atomic_write_text( - transcript_path, - "\n".join( - [ - "{\"type\":\"event_msg\",\"payload\":{\"type\":\"user_message\",\"message\":\"First prompt\"}}", - "{\"type\":\"event_msg\",\"payload\":{\"type\":\"user_message\",\"message\":\"Later follow-up\"}}", - ] - ) - + "\n", - ) - - self.assertEqual(initial_user_message_from_transcript(transcript_path), "First prompt") - - def test_transcript_parser_returns_latest_assistant_message(self) -> None: - transcript_path = self.repo_root / "assistant-transcript.jsonl" - atomic_write_text( - transcript_path, - "\n".join( - [ - "{\"type\":\"response_item\",\"payload\":{\"type\":\"message\",\"role\":\"assistant\",\"content\":[{\"type\":\"output_text\",\"text\":\"First draft\"}]}}", - "{\"type\":\"response_item\",\"payload\":{\"type\":\"message\",\"role\":\"assistant\",\"content\":[{\"type\":\"output_text\",\"text\":\"Final plan prompt\"}]}}", - ] - ) - + "\n", - ) - - self.assertEqual(latest_assistant_message_from_transcript(transcript_path), "Final plan prompt") - - def test_atomic_write_text_overwrites_without_leaving_temp_files(self) -> None: - target = self.repo_root / "sample.txt" - atomic_write_text(target, "one\n") - atomic_write_text(target, "two\n") - - self.assertEqual(target.read_text(encoding="utf-8"), "two\n") - leftovers = list(target.parent.glob(".sample.txt.tmp-*")) - self.assertEqual(leftovers, []) - - def test_emit_outcome_outputs_only_strict_json(self) -> None: - stdout = io.StringIO() - stderr = io.StringIO() - - code = emit_outcome( - outcome=type( - "Outcome", - (), - {"exit_code": 0, "stdout_payload": {"continue": False, "stopReason": "ok"}, "stderr_message": None}, - )(), - stdout_stream=stdout, - stderr_stream=stderr, - ) - - self.assertEqual(code, 0) - self.assertEqual(stdout.getvalue(), "{\"continue\":false,\"stopReason\":\"ok\"}") - self.assertEqual(stderr.getvalue(), "") - - def test_plan_helper_preserves_completed_history_and_appends_new_follow_up_tasks(self) -> None: - original = build_plan_text( - "plan-helper", - ordered_items=[(True, "Completed historical task.")], - follow_up_items=[ - (True, "Already fixed earlier."), - (False, "Existing open obligation."), - ], - ) - - updated = append_follow_up_subtasks( - original, - [ - "Existing open obligation.", - "Investigate the new regression from reviewer feedback.", - ], - ) - - self.assertIn("- [x] Completed historical task.", updated) - self.assertEqual(updated.count("Existing open obligation."), 1) - self.assertIn("- [ ] Investigate the new regression from reviewer feedback.", updated) - - def test_plan_analysis_requires_required_sections_and_checkboxes(self) -> None: - analysis = analyze_plan(render_session_plan("analysis-test")) - - self.assertFalse(analysis.all_checked) - self.assertEqual(analysis.approval_status, "unapproved") - self.assertEqual(analysis.phase_status, "planning") - self.assertFalse(analysis.is_approved) - self.assertEqual(analysis.phase, "planning") - self.assertEqual(analysis.missing_sections, []) - self.assertEqual(analysis.empty_sections, []) - self.assertEqual(analysis.invalid_sections, []) - self.assertEqual(len(analysis.tracked_items), 2) - - def test_plan_analysis_detects_approved_flag(self) -> None: - analysis = analyze_plan( - build_plan_text( - "approved-analysis", - ordered_items=[(True, "Done ordered work.")], - approval_status="approved", - ) - ) - - self.assertEqual(analysis.approval_status, "approved") - self.assertEqual(analysis.phase_status, "implementation") - self.assertTrue(analysis.is_approved) - self.assertEqual(analysis.phase, "implementation") - - def test_plan_analysis_preserves_explicit_review_phase(self) -> None: - analysis = analyze_plan( - build_plan_text( - "review-analysis", - ordered_items=[(True, "Done ordered work.")], - approval_status="approved", - phase="review", - ) - ) - - self.assertEqual(analysis.approval_status, "approved") - self.assertEqual(analysis.phase_status, "review") - self.assertEqual(analysis.phase, "review") - - def test_plan_analysis_preserves_explicit_testing_phase(self) -> None: - analysis = analyze_plan( - build_plan_text( - "testing-analysis", - ordered_items=[(True, "Done ordered work.")], - approval_status="approved", - phase="testing", - ) - ) - - self.assertEqual(analysis.approval_status, "approved") - self.assertEqual(analysis.phase_status, "testing") - self.assertEqual(analysis.phase, "testing") - - def test_plan_analysis_requires_phase_section(self) -> None: - missing_phase_plan = build_plan_text( - "missing-phase-analysis", - ordered_items=[(True, "Done ordered work.")], - approval_status="approved", - ).replace(f"\n{PLAN_PHASE_HEADING}\nimplementation\n", "", 1) - - analysis = analyze_plan(missing_phase_plan) - - self.assertEqual(analysis.approval_status, "approved") - self.assertIsNone(analysis.phase_status) - self.assertIn(PLAN_PHASE_HEADING, analysis.missing_sections) - - -if __name__ == "__main__": - unittest.main() diff --git a/scripts/lib/codex_workflow/transcript.py b/scripts/lib/codex_workflow/transcript.py deleted file mode 100644 index 43899e69..00000000 --- a/scripts/lib/codex_workflow/transcript.py +++ /dev/null @@ -1,88 +0,0 @@ -from __future__ import annotations - -import json -from pathlib import Path - - -def _extract_role_message_from_record(record: dict[str, object], role: str) -> str | None: - record_type = record.get("type") - payload = record.get("payload") - if not isinstance(payload, dict): - return None - if record_type == "event_msg" and role == "user" and payload.get("type") == "user_message": - message = payload.get("message") - if isinstance(message, str) and message.strip(): - return message - if record_type == "response_item" and payload.get("type") == "message" and payload.get("role") == role: - content = payload.get("content") - if not isinstance(content, list): - return None - parts: list[str] = [] - for item in content: - if not isinstance(item, dict): - continue - text = item.get("text") - if isinstance(text, str) and text.strip(): - parts.append(text) - if parts: - return "\n".join(parts) - return None - - -def initial_user_message_from_transcript(transcript_path: Path) -> str | None: - try: - lines = transcript_path.read_text(encoding="utf-8").splitlines() - except (FileNotFoundError, OSError): - return None - - for line in lines: - try: - record = json.loads(line) - except json.JSONDecodeError: - continue - if not isinstance(record, dict): - continue - message = _extract_role_message_from_record(record, "user") - if message: - return message - return None - - -def latest_user_message_from_transcript(transcript_path: Path) -> str | None: - try: - lines = transcript_path.read_text(encoding="utf-8").splitlines() - except (FileNotFoundError, OSError): - return None - - latest: str | None = None - for line in lines: - try: - record = json.loads(line) - except json.JSONDecodeError: - continue - if not isinstance(record, dict): - continue - message = _extract_role_message_from_record(record, "user") - if message: - latest = message - return latest - - -def latest_assistant_message_from_transcript(transcript_path: Path) -> str | None: - try: - lines = transcript_path.read_text(encoding="utf-8").splitlines() - except (FileNotFoundError, OSError): - return None - - latest: str | None = None - for line in lines: - try: - record = json.loads(line) - except json.JSONDecodeError: - continue - if not isinstance(record, dict): - continue - message = _extract_role_message_from_record(record, "assistant") - if message: - latest = message - return latest diff --git a/scripts/lib/codex_workflow/tests/test_repo_validation.py b/scripts/lib/tests/test_repo_validation.py similarity index 100% rename from scripts/lib/codex_workflow/tests/test_repo_validation.py rename to scripts/lib/tests/test_repo_validation.py diff --git a/scripts/run_build_checks.sh b/scripts/run_build_checks.sh deleted file mode 100755 index 2762d521..00000000 --- a/scripts/run_build_checks.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -cd "$REPO_ROOT" - -if ! command -v cargo >/dev/null 2>&1 && [[ -x "${HOME:-}/.cargo/bin/cargo" ]]; then - export PATH="${HOME}/.cargo/bin:${PATH:-}" -fi - -cargo +nightly fmt --all -cargo test -r --features gpu --no-run diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 162ea147..0fb30f1d 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -34,7 +34,7 @@ else fi if [[ $run_python -eq 1 ]]; then - python3 -m unittest discover -s scripts/lib/codex_workflow/tests -p 'test_*.py' + python3 -m unittest discover -s scripts/lib/tests -p 'test_*.py' fi if [[ $run_rust -eq 1 ]]; then diff --git a/scripts/session_start_hook.sh b/scripts/session_start_hook.sh deleted file mode 100755 index 77200d03..00000000 --- a/scripts/session_start_hook.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -cd "$REPO_ROOT" - -PYTHONPATH="$REPO_ROOT/scripts/lib${PYTHONPATH:+:$PYTHONPATH}" \ - exec python3 -m codex_workflow.cli session-start diff --git a/scripts/stop_hook.sh b/scripts/stop_hook.sh deleted file mode 100755 index 93bfd743..00000000 --- a/scripts/stop_hook.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -cd "$REPO_ROOT" - -PYTHONPATH="$REPO_ROOT/scripts/lib${PYTHONPATH:+:$PYTHONPATH}" \ - exec python3 -m codex_workflow.cli stop