Repository navigation
Conversation
…me conflicts (#393) A check's execution identity is now its command, executor, profile, context and level. Provenance and reason are descriptive: the history projection normalizes them to `runtime`, so a report or reconciliation that describes a frozen check differently keeps the frozen description instead of failing with "cannot rewrite frozen check identity". An executor, profile or context kept by the store as a `ref-<sha256>` reference matches the value it came from. When a resume cannot bind every stored command, `restore_plan(partial=True)` still binds the commands today's policy authorizes and leaves the rest as inert fingerprints, keeping every check, obligation and result. The configured check therefore keeps one identity, and agent reports for it are no longer rejected because the frozen command was left as a fingerprint. A rejected identity names the check and each differing field, showing commands only as fingerprint prefixes (marked `unbound` while unbound) and non-token values as references. Before dispatch, that text is the BLOCKED stop reason. A rejected stage report keeps `not_verified` and adds a report note. Demotion errors name the check and the lost obligation. Regression tests use a sanitized legacy plan with two configured IDs. They cover three resumes (resolve, rereview, resolve), partial restore, normalized metadata, genuine identity changes, an unbound fingerprint and the pre-dispatch stop. Closes #393
Implement stage summaryStatus:
Run: implement · profile |
Code Cycle initial review — PR #404[CCR-20261103-001] · senior_reviewer · anthropic/claude-opus-5-5→? · high · schema:2Verdict: Scope
SummaryThe fix restores the invariant that a frozen check's identity is what it executes (
Findings[REV-001] · low · open · - · blocks:no — A persisted
|
| Command | Exit | Result |
|---|---|---|
python3 -m unittest tests.test_run_cycle.NativeExecutionPlanTests |
0 | 29 tests OK, including the 7 new #393 regressions |
python3 -m unittest discover -s tests |
0 | Ran 2185 tests in 558.400s, OK (skipped=1) |
python3 scripts/validate-package.py |
0 | VALID: 16 skills (7 cycle, 9 supporting), shared sections in sync, cross-agent manifests aligned |
python3 -m unittest |
0 | Ran 0 tests (no top-level package; the configured suite is the discover run) |
basedpyright (5 changed scripts/ modules) |
1 | 99 errors, 2724 warnings. None of the errors is on a changed line range except cycle_history.py:51:6, which is reportImplicitRelativeImport for the new from verification_plan import persisted_token. That is the same class as the existing import on line 52 and the rest of the module. The other diagnostics are pre-existing and reported separately, as AC-8 asks, not as a finding. |
Acceptance criteria
| AC | State | Evidence |
|---|---|---|
| AC-1 | met | test_observed_pre_dispatch_collision_names_the_check_and_its_fields passes. The installed-runtime/source comparison is recorded in the PR description. |
| AC-2 | met | test_three_resumes_keep_identities_obligations_and_history (resolve, rereview, resolve) passes. REV-001 is a non-configured edge case outside the tested shape. |
| AC-3 | met | The legacy fixture with both configured IDs passes, and test_partial_restore_binds_only_authorized_commands passes. |
| AC-4 | met | test_persisted_metadata_is_not_an_identity_conflict passes. |
| AC-5 | met | test_genuine_identity_changes_are_rejected_and_borrow_no_evidence passes. |
| AC-6 | met | The three-resume test checks per-HEAD unverified and the not_verified cap. CI does not feed cap(). |
| AC-7 | met | test_pre_dispatch_identity_stop_names_check_and_fields and test_rejected_report_note_is_safe_for_a_terminal pass. |
| AC-8 | met | docs/verification.md and docs/execution-plan.md are updated. The suites pass, and the type-check diagnostics are reported above. |
Unverified
- REV-001 is supported by code inspection only. Running an ad-hoc helper (
python3 $TMPDIR/report.md) was denied by this stage's command table ('python3' is not in the review stage's command table). This review did not bypass it. basedpyrightwas run on the changed modules, not on the whole repository.
CI (read once under the parallel policy; not waited on)
| Check | State |
|---|---|
| Skills, manifests, and Bash installer | pass |
| Windows test shard (windows-1) | pass |
| Windows test shard (windows-2) | pass |
| Windows test shard (windows-4) | pass |
| Windows test shard (windows-3) | pending |
Residual risk
- The live runtime that produced the original Resume blocks before dispatch on frozen check identity: recover persisted plans and name conflicting fields #393 stop cannot be identified (
toolkit.identityhas an emptycommit). The regression is a sanitized reconstruction, not a replay of the stored snapshot. - An agent report that describes a configured check under a different
executororprofilethan the runtime-owned identity is still rejected asnot_verified. This is intended identity behaviour, but it can still cost a verification round.
Closes #393
Work item: GitHub issue #393 (#393).
Diagnosis
Symptom. Resuming with
--from resolveor--from rereviewstopped before any dispatch with the genericcannot rewrite frozen check identity. The stop named no check and no field.Hypothesis in the item. It listed several candidates and kept the cause open: stale runtime, a restore failure, a legacy polluted plan, a metadata mismatch, or something else.
Reproduction.
tests.test_run_cycle.NativeExecutionPlanTests.test_observed_pre_dispatch_collision_names_the_check_and_its_fieldsuses a sanitized legacy revision with the documented shape:configured-testandconfigured-test-<digest>) with one command, runtime executor and profile, project context, and the projection'sruntimeprovenance and reason;The strict restore fails (
HistoryError), so the resume keeps the fingerprinted plan. The pre-#387 reconciliation derivedconfigured-test-<digest>again. Extending the plan with that spec raises the observed error. The actual check isconfigured-test-<16 hex of the command digest>. The fields that differ are:command: persisted fingerprint against the plain command;provenanceandreason: normalized toruntimeby the projection.executor,profile,contextandlevelare equal. All the new regressions were run against the pre-fixscripts/(tests from this branch, scripts frommain). They failed with 10 failures and 5 errors, including the three-resume test. They pass with the fix.Installed runtime against source. The plan modules in
~/.code-cycle/runtimeare byte-identical tomainat0032676today, with nogit diff --no-indexdifferences forcycle_plan.py,verification_plan.py,cycle_history.py,cycle_dispatch.py,execution_plan.pyandrun_cycle.py. They were reinstalled on 2026-11-03 at 16:37 +0100, after the reported failures.toolkit.identityrecordsversion=0.6.1with an emptycommit, so the runtime that actually ran the failing resumes can't be recovered from here. With the current reconciliation, the pre-dispatch collision for this exact shape no longer happens, because #387 recognizes the configured digest. The same root cause still broke the current code after dispatch. The restored configured check carriedprovenance/reason: runtime, and after a partial restore its command was a fingerprint. So a correct report forconfigured-test, for example withprovenance: projectas the skill instructs, or with the plain command, was rejected._normalize_test_payloadswallowed that rejection, so the stage's evidence was silently discarded asnot_verified. This stage could not read the persisted plan snapshot from the history database: the implement stage's command table deniespython3 -cand helper scripts, and the TMPDIR policy allows only fixed file names. The sanitized fixture uses the facts confirmed during triage instead.Cause and invariant. The broken invariant: a frozen check's identity is what it executes (command, executor, profile, context, level).
extendcompared every field, including the descriptive ones that persistence normalizes. On top of that, the partial-restore path left every command, authorized or not, as a fingerprint. The cause matches the item's "metadata mismatch + restoration" candidates. The "stale runtime" candidate explains only the pre-dispatch form.Related work (widened search: bug label, recurrence after #385). #385 / PR #387 (closed, same symptom, configured-digest recognition), #388, #382, #330 (independent, per the item). No other open item is addressed.
Fix shape.
VerificationPlan.extendcompares onlyIDENTITY_FIELDS. It keeps the frozen provenance and reason, and treats a storedref-<sha256>executor, profile or context as equal to the value it came from.unboundwhile unbound, and other non-token values as references. Demotion names the check and the lost obligation.not_verifiedand adds a sanitized report note. Before dispatch, the diagnostic is theBLOCKEDstop reason (readiness_unconfirmed).extend, so an agent can't bind a hidden command.Work units
WU-1scripts/verification_plan.py,scripts/cycle_history.py,scripts/cycle_plan.py,scripts/cycle_dispatch.py,scripts/run_cycle.py,tests/test_run_cycle.py,docs/verification.md,docs/execution-plan.mdNativeExecutionPlanTestsindependent(no schema or data migration: the stored projection format is unchanged)Acceptance criteria coverage
test_observed_pre_dispatch_collision_names_the_check_and_its_fields(installed-runtime/source comparison in Diagnosis)test_three_resumes_keep_identities_obligations_and_history(resolve, rereview, resolve)test_three_resumes_…andtest_partial_restore_binds_only_authorized_commands(legacy plan with both configured IDs)test_persisted_metadata_is_not_an_identity_conflict; configuration and checkout paths are not part of any runtime-owned identitytest_genuine_identity_changes_are_rejected_and_borrow_no_evidence,test_an_unauthorized_non_manual_command_still_fails_restoretest_three_resumes_…(new HEAD per resume, history retained, unbound/manual/unexecuted checks stayunverified, capnot_verified)test_pre_dispatch_identity_stop_names_check_and_fields,test_rejected_report_note_is_safe_for_a_terminalVerification (agent-reported)
python3 -m unittest tests.test_run_cycle.NativeExecutionPlanTestspython3 -m unittest discover -s tests(configured-test)python3 scripts/validate-package.pyVALID: 16 skills (7 cycle, 9 supporting), shared sections in sync, cross-agent manifests alignedpython3 -m unittestbasedpyrightAny, import cycles). The newfrom verification_plan import persisted_tokenfollows the module's existing import style.Environment: the tests use temporary SQLite stores and Git fixtures with their own identity, and
isolate_host_environmentkeeps out the global Git config, credentials and the user's history database. Local runs and CI share Python stdlib-only dependencies. They differ in the host: WSL2, Python 3.10.12.Security
cc-security-reviewran on the diff. SEC-001 (low): the new report note could carry an agent-chosen check ID with terminal control bytes. Fixed: the note goes throughreadable(), and a test covers it. No other finding: partial binding grants nothing beyond today's authorization, a fingerprint can't be bound from a report, and the diagnostics don't reveal command text.Documentation
docs/verification.md(identity fields, partial restore, diagnostics format) anddocs/execution-plan.md(reconciliation) are updated. The idempotent-resume behaviour from #387 was already documented. This change extends that section.