Skip to content

fix(verification): restore manual checks and make plan resume idempotent - #387

Merged
datacas merged 2 commits into
mainfrom
task-385
Nov 2, 2026
Merged

datacas merged 2 commits into
mainfrom
task-385

Conversation

@datacas

@datacas datacas commented Nov 2, 2026

Copy link
Copy Markdown
Owner

Closes #385

Work item: #385

Diagnosis

  • Symptom: a change request whose frozen verification plan holds a manual:* check cannot be resumed. restore_plan raises HistoryError, and the fallback then stops BLOCKED with cannot rewrite frozen check identity, on every later resume.
  • Reproduction: a persisted plan with configured-test, an agent check, manual-orca-live (manual:orca-live) and a previously added configured-test-<digest> entry, resumed with --from rereview. Before the change the run stops with cannot rewrite frozen check identity (observed by temporarily removing the digest match from prepare_plan and running the new test).
  • Cause: matches the issue's hypothesis. (1) authorized_bindings() only binds journal proposals that _is_verification_command accepts, and a manual check is not a runnable command. (2) In the fallback, stored commands are sha256: fingerprints, so prepare_plan never finds the configured check by its plain command and re-derives configured-test-<digest>, which VerificationPlan.extend rejects against the one added by the previous resume.
  • Invariant: frozen check identities are never rewritten, and a resume is idempotent in the set of checks it leaves.
  • Related work: basic search (the issue's own links and Use the shared execution plan in supervised Orca cycles #330, Accept host-attested evidence for required verification checks #382, PR feat(orca): share execution plans in supervised cycles (#330) #384); no duplicate or superseding item. Accept host-attested evidence for required verification checks #382 is independent and builds on this.
  • Fix shape: bind the manual identity where restore authorizes commands, and recognize the configured check by plain command or digest where the plan is extended. extend's frozen-identity rule, .code-cycle.yml and the allowlist rules are untouched.

Work units

Unit Behaviour Files Tests Verification Rollback
WU-1 Bind journal-proposed manual:<name> commands on restore (fixed grammar, still non-executable and never a verify prefix); recognize the configured check by plain command or digest so resume is idempotent scripts/cycle_plan.py, scripts/verification_plan.py, docs/verification.md tests/test_run_cycle.py (NativeExecutionPlanTests: three resumes with and without the proposal, restored manual check stays unverified and caps, unauthorized non-manual still fails restore, binding grammar) python3 -m unittest discover -s tests: 2134 tests OK (1 skipped); python3 scripts/validate-package.py: VALID independent

Documentation

docs/verification.md now states that manual checks survive resume and that resume is idempotent. README.md is unchanged because no skill or entry point was added or removed.

Verification (agent-reported)

  • python3 -m unittest discover -s tests — exit 0, 2134 tests, 1 skipped.
  • python3 scripts/validate-package.py — exit 0, VALID: 16 skills.
  • python3 -m unittest — exit 0, 0 tests (no discovery from the repository root; discover -s tests above is the real run).
  • Not run: the issue's pre-resume check against a copy of the persisted PR feat(orca): share execution plans in supervised cycles (#330) #384 state, which needs the host telemetry database and is outside this stage's sandbox. The new fixture reproduces its revision-2 shape instead.

Local run shares with CI: no use of the global Git identity or host credentials by the new tests.

…ent (#385)

A persisted plan with a manual:* check could not be restored, because the journal proposal never passed the verification-command policy. Every resume then fell back to the fingerprinted plan, where the configured test command was not recognized, so each resume derived a configured-test-<digest> check that collided with the one added before and stopped with 'cannot rewrite frozen check identity'.

authorized_bindings now binds journal-proposed manual:<name> commands (fixed grammar) while _is_verification_command still rejects them; prepare_plan recognizes the configured check by its plain command or its command digest; verify_pending never offers a manual check as a verify prefix. docs/verification.md states that manual checks survive resume and that resume is idempotent.
…385)

Security review follow-up: only the runtime's configured-test* checks may stand in for the configured test command by digest, so a proposed check carrying the same fingerprint cannot suppress that obligation on resume.
@datacas

datacas commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Implementation of #385 is published at 9d95271db70f7709d2d8a6335790f787ae13c8cb (plan plan-cycle-8a3c3d9902d84b1d91bb864ce946ffe2, revision 5, digest 2fb3724cf8b3442e498d8f1ef530fa0f6287f23ff73e9aabff7442f61e05ad64).

Security review (security_audit_ran=true). Scope: scripts/cycle_plan.py, scripts/verification_plan.py.

  • Confirmed vulnerabilities: none. Bound manual:<name> commands (fixed grammar manual:[A-Za-z0-9][A-Za-z0-9._-]{0,63}) are only used by restore_plan to match stored fingerprints; _is_verification_command is unchanged and still rejects them, and verify_pending filters them out, so they never become a verify prefix or an executable command.
  • Potential risk, low, fixed in 9d95271: matching the configured test by digest on any check would let a proposed check carrying that fingerprint suppress the configured obligation. The digest match is now limited to configured-test* identities, with a regression test.
  • Not reviewed: the host telemetry database and journal integrity (an agent-proposed manual:* name is recorded as before and only binds an identical stored fingerprint). Residual: agent-reported evidence for a manual check remains unattested until Accept host-attested evidence for required verification checks #382.

Verification (agent-reported): python3 -m unittest discover -s tests exit 0, 2135 tests (1 skipped); python3 scripts/validate-package.py exit 0; python3 -m unittest exit 0, 0 tests. The code host's checks are left to the cycle's CI gate.

@datacas

datacas commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Initial review — #387 (work item #385)

[CCR-20261102-001] · senior_reviewer · anthropic/claude-opus-5-5→? · high · schema:2

Verdict: APPROVED for head 9d95271db70f7709d2d8a6335790f787ae13c8cb. No open finding blocks approval; both required checks passed on this head.

Plan: plan-cycle-09e00f805ff44de5b05b1b61779a041d, revision 4, digest fedae33c0053a2c9c64a7b37a49054ab9574d6e6d409413c25fa9d32c219fc51.

Summary and scope

Reviewed the accumulated diff origin/main...9d95271 (merge base 7c0f28b, 269 lines, 4 files: docs/verification.md, scripts/cycle_plan.py, scripts/verification_plan.py, tests/test_run_cycle.py), plus the surrounding code it relies on: restore_plan/command_digest in scripts/cycle_history.py, the fallback branch and verify_pending in scripts/cycle_plan.py, VerificationPlan.extend/cap in scripts/verification_plan.py and _is_verification_command in scripts/stage_permissions.py.

The fix does two things:

  • authorized_bindings() also binds journal-proposed commands that fully match manual:[A-Za-z0-9][A-Za-z0-9._-]{0,63}, so a plan holding a manual check restores. The bound command is never executable: _is_verification_command still rejects it, and verify_pending drops manual checks before it builds verify_prefixes.
  • When the plan is extended, the configured check is recognized by its plain command, or by its sha256: digest when the check id starts with configured-test. Resume therefore no longer adds a second configured-test-<digest> that extend would reject. Limiting the digest match to the runtime's own ids stops a proposed check from replacing the configured obligation. The test test_a_proposed_check_cannot_stand_in_for_the_configured_test_by_digest covers this.

extend's frozen-identity rule, .code-cycle.yml and scripts/stage_permissions.py are unchanged.

Findings

[REV-001] · low · open · - · blocks:no — Fallback resume does not assert that result history survives every resume

tests/test_run_cycle.py:211-217. In test_manual_check_survives_three_resumes_with_stable_identities, the proposal=False subtest (unrestored plan, sha256: fingerprints) compares check ids and required/criteria across resumes. It checks the historical focused pass and the manual-orca-live result only on the first resume (seen[0][1]). AC-4 requires historical results to be preserved across resumes. On the fallback path, each resume appends unverified records, so the history is preserved by construction (VerificationPlan.record appends). No test asserts this, though: a later regression that rebuilt results on the second or third resume would still pass. Suggested check: assert that seen[n][1] is a prefix of seen[n+1][1], or that ("focused", "a"*40, "pass") is in every seen[n][1]. This does not block approval: the behaviour is correct today, and the restored path (proposal=True) asserts exact equality.

[REV-002] · low · open · - · blocks:no — Documented manual-name grammar omits the leading-character rule

docs/verification.md:309-310. The page says a manual name uses "letters, digits, ., _ and -, at most 64 characters". MANUAL_COMMAND (scripts/verification_plan.py:22) also requires the first character to be a letter or digit. A check named manual:_smoke or manual:.live matches the documented grammar but is not bound. Its plan then fails restore, which is the very failure this change documents as fixed. Suggested fix: state that the name starts with a letter or digit.

Security triage and audit

  • Deterministic rule: no changed path matches the mandatory minimum (CI/automation, secrets/keys, agent/toolkit configuration), and the PR has no labels.
  • Reviewer judgement and runtime plan: the plan sets security_required=true, and the change touches command authorization: which journal-proposed commands restore_plan may bind. cc-security-review ran on this head (security_audit_ran=true).
  • Result: no confirmed vulnerability and no potential risk introduced by this change.
    • Trust boundary: journal proposals are agent-originated text. A bound manual: identity only lets restore_plan rebuild the stored check. It grants no execution: _is_verification_command("manual:…") is false under every policy the tests exercise, and verify_pending filters manual checks out of verify_prefixes (scripts/cycle_plan.py:433,456), the only place check commands become command authority.
    • Grammar: re.fullmatch with a fixed character class rejects whitespace, newlines, ;, $(, the wrong case and over-long names. The test cases cover each of these.
    • Digest stand-in: only required checks with a configured-test* id and the digest of the current allowed_test_command qualify, so an agent cannot suppress the configured obligation with a lookalike check.
    • Non-manual unauthorized commands still raise HistoryError (test_an_unauthorized_non_manual_command_still_fails_restore).
    • Not reviewed: how a later stage reports manual evidence through normalize_tests (an agent-reported pass for a manual check). That path predates this change, and this diff does not change it.

Executed verification (local, on 9d95271)

Command Exit Result
python3 -m unittest tests.test_run_cycle.NativeExecutionPlanTests -v 0 22 tests OK, including the 5 new tests
python3 -m unittest discover -s tests 0 Ran 2135 tests in 231.802s, 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 discovery from the repository root; the discover -s tests run above is the real suite)

Acceptance criteria

AC Evidence State
AC-1 manual plan restores when its proposal exists test_restored_manual_check_stays_unverified_and_caps_the_conclusion; test_manual_check_survives_three_resumes_with_stable_identities (proposal=True) — executed, pass verified
AC-2 restored manual check stays unverified and non-executable test_restored_manual_check_stays_unverified_and_caps_the_conclusion, test_manual_proposals_bind_without_gaining_execution_authority, test_manual_command_grammar_is_fixed — executed, pass verified
AC-3 three resumes, same identities, no duplicate configured-test-<digest> test_manual_check_survives_three_resumes_with_stable_identities (both subtests) — executed, pass verified
AC-4 results, identities and obligations preserved Restored path asserts equal results; the fallback path asserts identities and obligations but checks result history only on the first resume (REV-001) verified_with_reservations
AC-5 genuinely unauthorized command still fails restore test_an_unauthorized_non_manual_command_still_fails_restore — executed, pass verified
AC-6 .code-cycle.yml and allowlist rules unchanged git diff --stat 7c0f28b 9d95271 lists 4 files and none of them; static evidence only verified (static)
AC-7 docs/verification.md documents manual-check survival and idempotent resume docs/verification.md:311-324 (diff) — static evidence; see REV-002 for a grammar detail verified (static)

Unverified items

  • The issue's pre-resume check against a copy of the persisted PR feat(orca): share execution plans in supervised cycles (#330) #384 state was not run: it needs the host telemetry database, which is outside this read-only clone. The new fixture reproduces that plan's shape (configured, agent, manual:orca-live and configured-test-<digest> checks).

CI

CI policy: parallel (read once, not waited on). All 6 checks on 9d95271 passed:

  • Skills, manifests, and Bash installer — pass (required)
  • PowerShell installer — pass (required)
  • Windows test shard (windows-1) — pass
  • Windows test shard (windows-2) — pass
  • Windows test shard (windows-3) — pass
  • Windows test shard (windows-4) — pass

No pending, failed, cancelled or skipped check.

Residual risk

  • A manual check whose name falls outside the grammar (for example one with a leading _ or a space) still forces the unrestored fallback, which is the documented behaviour for any unbound command (see REV-002 for the doc gap).
  • On the fallback path, the digest match depends on the configured test command being unchanged. If allowed_test_command changes between resumes, the runtime adds a new configured-test-<digest-of-new-command> identity. This is the expected obligation for a new command and is outside this issue's acceptance criteria.

@datacas

datacas commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Host-side evidence: recovery of the persisted PR #384 plan (host-verified, not runtime-verified)

This comment records a manual check run on the host. It is not a result of the cycle runtime: it does not satisfy the required pre-resume check, does not clear the verification_unverified stop, and is not a review or approval. The final decision stays with manual review.

Tested SHA: 9d95271db70f7709d2d8a6335790f787ae13c8cb, equal to the PR head when this comment was written. Worktree task-385 was clean (git status --short empty), and the test imported scripts/ from that worktree.

Procedure

  1. Copied the host telemetry database to an isolated file (cp, read-only use afterwards); copy SHA-256 7acbffcb543a073cb0c9d835bceca6900ae85510ee1cac39cf983fd2998764a9. The live database was not opened for writing.
  2. Read the latest persisted plan for work item 330 / change request 384 with CycleHistoryStore.latest_plan: plan-cycle-6de74646b15646a5a3a9987a338ed7ce, revision 2, stop reason readiness_unconfirmed, digest 19e035f4e175fcac1d81ebd3b6fb6b14f67ab13db062e28b372a75b6240d8bf0.
  3. Built the command bindings as the PR's authorized_bindings does (configured test, verify prefixes, and journal-proposed commands that pass _is_verification_command or the new is_manual_command) and called restore_plan. Exit code of the script: 0.

Result

Check Command Stored fingerprint (command_digest) Restored Executable
configured-test python3 -m unittest discover -s tests sha256:3b904b6440a1431e3bc43d30c6941d115d622ee1423e9241fa937e71de5b0564 yes yes
orca-parity python3 -m unittest discover -s tests -p test_orca_plan.py sha256:dc6c1481e1809f98548a7d1b510d764726612a06a5dd2e240621fe735c660017 yes yes
package-validation python3 scripts/validate-package.py sha256:374a2f47eeb8d55cd905b1295d89d7d1dcb2df582f38f9232ccd1d1fde53bbda yes yes
orca-live manual:orca-live sha256:f6373f8b7909ca08697f5038aa8a4af8c68b08dae9b6a588e2fa07476cff9623 yes no (_is_verification_command still rejects it)
configured-test-3b904b6440a1431e python3 -m unittest discover -s tests same as configured-test yes yes
  • Without the PR's change, the same restore raises HistoryError: stored check has no currently authorized command binding (reproduced earlier on 7c0f28b), and the next resume stops with cannot rewrite frozen check identity.
  • With the change: 5 checks and 14 results restored; identities and required flags unchanged; the leftover configured-test-3b904b6440a1431e entry is recognized as the configured obligation, so prepare_plan adds no extension.

Final states on c7f0390 (the head #384 was last reviewed on)

All 5 checks read unverified as their latest result. The two earlier failed resumes appended unverified results after the original passes, and the latest result wins. The history is preserved: configured-test, orca-parity and package-validation each still hold their earlier pass on c7f0390 (and configured-test its fail/defect on 7c0f28b); orca-live holds unverified/environment. #384 therefore needs a fresh verification run to show those checks as current again.

Limits of this evidence

  • One restore of a copy of the state, not three consecutive resumes through prepare_plan; the consecutive-resume behaviour is covered by the PR's own tests (test_manual_check_survives_three_resumes_with_stable_identities), which ran in the review.
  • The test script is a throwaway and is not in the repository.
  • Not run on the live database, and not run by the runtime.

REV-001 (test-coverage gap) and REV-002 (documentation omits the leading-character rule) stay open as low follow-ups. REV-001 changes no behaviour. REV-002 means a name such as manual:_smoke would be documented as valid yet not bind, so its plan would use the (now idempotent) fallback; neither affects security.

@datacas

datacas commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Human exception to verification_unverified

This merge is a human decision based on external evidence. It is not an automatic verification success of the cycle.

Authorized by the maintainer; applied by the coordinator on that instruction, with the head pinned to 9d95271.

@datacas
datacas merged commit df52fc9 into main Nov 2, 2026
6 checks passed
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.

Restore manual checks and make plan resume idempotent

1 participant