Skip to content

refactor(driver): split cycle control and transact workspace setup - #369

Merged
datacas merged 1 commit into
mainfrom
refactor/189-split-run-cycle
Oct 30, 2026
Merged

datacas merged 1 commit into
mainfrom
refactor/189-split-run-cycle

Conversation

@datacas

@datacas datacas commented Oct 30, 2026

Copy link
Copy Markdown
Owner

Split the native driver into CLI, workspace selection, stage dispatch, publication history, review gates, shared state, and an explicit control-transition table. Keep public imports from run_cycle, flags, reports, retries, resume behavior and CI policies compatible.

Workspace setup now records owned resources and compensates them in reverse order before the first adapter invocation. Successful detach commits the setup; failed handoff rolls it back. Reused worktrees/branches are preserved, changed or used refs are retained, and cleanup errors are collected.

Closes #189.

Diagnosis

Decision: implement (feature_request). The source confirms the issue's mechanism: run_cycle contains about 1,880 lines of nested closures and workspace creation has no ownership transaction. The invariant is unchanged stage behavior plus rollback limited to resources created by this setup. Characterization tests passed before extraction; all 31 moved methods and their setup/cleanup compare identically by AST after normalizing facade/state wiring.

The widened related-work search covered open and closed issues mentioning run_cycle.py, the issue's links, and #188/#329. #188 remains integration coordination, and plan-driven execution/modes in #329 stay outside this change. No duplicate was found. This PR adds no telemetry schema or dependency changes.

Work units

Unit Behavior Files Tests Verification Rollback
WU-1 Separate driver responsibilities while preserving control behavior; transact owned workspace setup scripts/run_cycle.py, nine focused runtime modules, scripts/cycle.py, runtime/shard manifests, docs/workflows.md CLI/report/stage characterization; Git/Herdr/reuse/PR-ref/detach/dispatch-boundary rollback; existing driver and installed CLI suites 2,063 executable tests pass; package validation passes; full suite remains unverified for nested Codex dependency setup independent: revert commit 99b062db9fa97cd075e6b84d02f378faf7b5f0be

Verification

Agent-reported evidence. Commands ran in the stage sandbox through rtk proxy, in the foreground, with output captured under the dispatch TMPDIR.

Command Exit Executed Result
python3 scripts/validate-package.py 0 — pass
python3 -m unittest discover -s tests 1 2,091 not_verified: 15 failures and 7 errors, all in dependency setup; the runtime reports the known nested Codex socket limitation (#268)
python3 -m unittest discover -s /tmp/code-cycle-tmp-x0wqnwd_ -p verification_suite.py 0 2,063 pass against final executable source; excludes all 29 dependency-setup tests and does not replace or pass the original check
python3 -m unittest discover -s tests -p test_installed_cycle.py 0 18 pass through installed CLI/process boundaries
python3 scripts/run_cycle.py --repo datacas/code-cycle-toolkit --task 189 --dry-run --json 0 — real CLI returns its JSON plan
python3 -m unittest 0 0 default discovery finds no tests; no coverage claim
basedpyright scripts/run_cycle.py scripts/cycle.py scripts/cycle_cli.py scripts/cycle_workspace.py scripts/cycle_session.py scripts/cycle_state.py scripts/cycle_dispatch.py scripts/cycle_publication.py scripts/cycle_review_gate.py scripts/cycle_loop.py scripts/workspace_setup.py --outputjson 1 — 105 errors; pre-change baseline has 112. This is not a clean type-check: existing diagnostics remain and the compatibility facade introduces five type-only import-cycle diagnostics.

Focused characterization (4), workspace setup (15), existing driver (243), manual rereview (20), and workflow inventory (10) checks also passed. No nested Codex launch, host install, or isolation bypass was used to recover the unavailable dependency checks.

The self-check found no additional functional issue after correcting extraction errors. Independent review and exact-head CI remain pending. Under the parallel CI policy, this stage reads checks once after publication and does not wait.

Documentation

The existing workspace policy already lives in docs/workflows.md; this PR extends it with ownership, rollback, dispatch/detach commit boundaries, and the runtime module split. README entry points are unchanged.

Routing: profile deep_coder; requested model openai/gpt-6.1-sol; resolved model ?; effort medium.

@datacas

datacas commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

Implemented #189 at 99b062db9fa97cd075e6b84d02f378faf7b5f0be.

  • Split CLI, workspace selection, dispatch, publication history, review gates and shared state from the compatibility entry point; added an explicit control-transition table.
  • Added owned-resource setup rollback until first adapter invocation or successful detach.
  • Agent-reported: 2,063 executable tests pass; package validation, 18 installed CLI tests, and the real dry-run entry point pass.
  • Full-suite verification remains not_verified: the original run reports 15 failures and 7 errors in dependency setup under the known nested Codex socket limitation. All 29 dependency-setup tests were excluded from the separately reported executable suite; that suite does not pass or replace the original command.
  • BasedPyright remains nonzero (105 errors, versus a 112-error pre-change baseline), including retained diagnostics and five type-only compatibility-facade cycles. No clean type-check is claimed.
  • Documentation: updated the existing workspace-policy section in docs/workflows.md.

Checks read once for 99b062db9fa97cd075e6b84d02f378faf7b5f0be: 0 passed, 0 failed, 5 pending. No CI wait was performed; independent review and the cycle's CI gate remain pending.

Routing: profile deep_coder, requested model openai/gpt-6.1-sol, resolved model ?, effort medium.

@datacas

datacas commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

[CCR-20261030-001] · reviewer · anthropic/claude-sonnet-5-5→? · high · schema:2

Initial review — 99b062db9fa97cd075e6b84d02f378faf7b5f0be

Status: REVIEW_PASSED_AWAITING_CI. The review of this head is complete and no open finding blocks approval. It is not an approval: Windows test shard (windows-4) was still pending when read, and the cycle's CI gate decides.

Scope

Accumulated diff origin/main…head (merge base 4a58a2e, 5,194 lines, 16 files): the run_cycle.py split into cycle_cli, cycle_workspace, cycle_session, cycle_state, cycle_dispatch, cycle_publication, cycle_review_gate and cycle_loop; the new workspace_setup.py ownership ledger; the cycle.py dispatch hook; docs/workflows.md; the runtime manifest and Windows shards; two new test modules. I read the new logic in full (ledger, workspace selection and rollback, CLI wrapper, session wiring, control table, detach) and the moved stage-dispatch, recovery and CI-gate code in sections. Moved bodies were not diffed line by line; I relied on the author's AST comparison and on the full suite.

Findings

[REV-001] · low · open · - · blocks:no — CycleControl.move raises KeyError for an interrupt that arrives during stop()

scripts/cycle_loop.py:967, scripts/cycle_session.py:2183,2230. stop() calls control.move("stop") first, and TRANSITIONS[STOPPED] is empty. If the signal handler raises CycleInterrupted while stop() is still writing its journal and status records, execute() calls interrupted(), whose control.move("interrupt") raises KeyError from STOPPED. The interruption record is lost and the run ends in a traceback; the original closure code closed the cycle. Found by reading the code; not reproduced, because this stage may not run helper scripts. Suggested fix: make move tolerant of events from terminal states, or move the stop transition to the end of stop().

[REV-002] · low · open · - · blocks:no — A stop before the first dispatch deletes the cycle's own final status snapshot

scripts/cycle_session.py:2093, scripts/run_cycle.py:3244-3246,3273-3290. own_file(status_writer.path, …) registers the status file for rollback, so a routing or preflight stop (no dispatch) removes the snapshot that finish() just wrote. Foreground runs still print the report. A detached run's child stops the same way, and cycle_status.py --line --cursor-file (the progress command detach prints) then finds no terminal line and prints "No cycle status files". The report remains in the detached log. test_routing_stop_keeps_rollback_active_and_removes_owned_status pins this, and the docs describe the rollback of the worktree but not this loss of the stop line. Either keep the snapshot when the cycle reached its own finish(), or state it in docs/workflows.md.

Observations (not findings)

  • Open questions Q-001/Q-002 from the issue review went unanswered. The PR chose a passive control table (CycleControl records transitions; drive() still decides them) and a commit at the first adapter invocation or at a successful detach. This matches the roadmap's bounded extraction. Whether a table that does not drive control satisfies "explicit state machine" is a scope call for the maintainer.
  • The modules reach the facade through self.api.<name> over sys.modules["run_cycle"]. This keeps every test patch point working, but the split is structural rather than a dependency boundary. The author reports five new type-only import-cycle diagnostics from basedpyright.
  • A fetched PR ref is retained, with a collected error, whenever any local branch contains its commit (for example an already merged PR). This is safe, and the docs describe it as "unused".

Security triage

The deterministic rule did not fire: no changed path or label matches the mandatory minimum (CI, secrets and keys, agent/toolkit configuration) or a project rule. By reviewer judgement the change is sensitive, because it moves command-policy construction (stage_policy), trusted-author receipt checks, the publication audit and new ref and worktree deletion code. I audited those inline rather than through a separate cc-security-review pass. Rollback deletes only resources recorded by this run (rmdir of directories it created, git worktree remove without force, update-ref -d guarded by the recorded SHA, unlink of its own files). It uses argument lists, builds the ref name from a validated numeric PR ID, and does no recursive delete. I found no vulnerability.

Verification (executed in this clone, foreground)

Command Result
python3 scripts/validate-package.py exit 0: VALID: 16 skills (7 cycle, 9 supporting)…
python3 -m unittest discover -s tests exit 0: 2,092 tests OK in 221.7 s, including the new test_driver_characterization and test_workspace_setup. The author's run could not finish this check; here it passed in full.
python3 -m unittest exit 0: 0 tests (default discovery finds none; no coverage claim)

Unverified: REV-001 was not reproduced (code reading only). Type-checking (basedpyright) was not rerun, because it is not among the configured commands.

Documentation

docs/workflows.md covers ownership, rollback, the dispatch/detach commit boundary and the module split, as the repository instructions require. README.md is unchanged, which is correct because no entry point changed. REV-002 is the one behaviour the page does not state.

CI (read once, not awaited — policy parallel)

Check State
Skills, manifests, and Bash installer pass
Windows test shard (windows-1) pass
Windows test shard (windows-2) pass
Windows test shard (windows-3) pass
Windows test shard (windows-4) pending

Residual risk

The extraction rests on the author's AST equivalence and on the test suite; the unread portions of the moved publication and gate code are covered by tests only. The two low findings concern interruption and status edge cases in failure paths.

Verdict

No open finding blocks approval. REVIEW_PASSED_AWAITING_CI for 99b062db9fa97cd075e6b84d02f378faf7b5f0be; approval depends on windows-4 passing.

@datacas
datacas merged commit 2904cac into main Oct 30, 2026
6 checks passed
@datacas
datacas deleted the refactor/189-split-run-cycle branch October 30, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(F2): split run_cycle.py into CLI, workspace selection and an explicit stage loop

1 participant