feat: classify verification failures before rollback - #107
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ada2920d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| to: "CLASSIFICATION_REQUIRED", | ||
| action: "capture-evidence-before-mutation", | ||
| description: "a failed verification enters classification before the next mutable action" |
There was a problem hiding this comment.
Gate escalation transitions on rollback classification
For every FAIL_PLAN or FAIL_IMPLEMENTATION, this rule requires classification before mutation, but WORKFLOW_ESCALATION_CONTRACT.transitionRules still immediately selects INITIAL_FAIL, SOL_REPLAN_FAIL, or a Terra failure transition and schedules the next corrective action. A host consuming that contract can therefore modify the plan or workspace before evidence capture and before discovering that the policy is selective or mandatory rollback; the existing failure transitions need to be gated on rollback classification and completion/waiver.
Useful? React with 👍 / 👎.
| ${workflowRecoveryContract} | ||
| ${workflowEscalationContract} | ||
| ${workflowLunaInteractionContract} | ||
| ${workflowRollbackContract} |
There was a problem hiding this comment.
Retain the previous managed templates for migration
Adding this contract changes the exact managed skill template, while the shared role-action-gate change also changes the other four templates, but none of the five templates from the parent commit is present in its corresponding LEGACY_TEMPLATES array. On an existing managed install, router-core.js only updates tracked content when it exactly matches a recognized legacy template, so rerunning install preserves every old file as “not a recognized package template” and existing users never receive this rollback feature.
Useful? React with 👍 / 👎.
| "rollback_class", | ||
| "rollback_policy", | ||
| "rollback_status", | ||
| "rollback_target_hash", |
There was a problem hiding this comment.
Persist rollback target identities alongside their hashes
The selective-target rule requires an exact identity and pre-rollback hash for each target, and the sequencing contract says that target identity is persisted atomically, but the task-scoped state contains only this singular hash and no target reference or identity. After a primary switch/resume—or whenever rollback covers multiple files—the host cannot determine which path each saved hash protects, so it cannot safely perform the promised stale-target check; persist target references paired with their hashes.
Useful? React with 👍 / 👎.
| Object.freeze({ failureClass: "CORRECTABLE", policy: "NONE", safeToContinue: true, description: "preserve valid work and evidence, then continue with an incremental correction" }), | ||
| Object.freeze({ failureClass: "SCOPE_VIOLATION", policy: "SELECTIVE", safeToContinue: true, description: "revert only exact unauthorized or unrelated targets after evidence capture" }), | ||
| Object.freeze({ failureClass: "WORKSPACE_POLLUTION", policy: "SELECTIVE", safeToContinue: true, description: "remove only known generated artifacts; unknown untracked files are never deleted automatically" }), | ||
| Object.freeze({ failureClass: "WORKSPACE_CORRUPTION", policy: "FULL_WORKSPACE", safeToContinue: false, description: "full workspace rollback requires an exact trusted checkpoint and proof that unrelated work is preserved; otherwise BLOCK_AND_ESCALATE" }), |
There was a problem hiding this comment.
Unify the workspace-corruption default policy
When no trusted checkpoint is available, defaultPolicies.WORKSPACE_CORRUPTION says BLOCK_AND_ESCALATE, but this machine-readable classification rule says FULL_WORKSPACE, and the generated skill prompt is built from this latter value. Consequently different contract consumers classify the same corruption differently—one blocks while another enters mandatory full-workspace rollback—so the conditional checkpoint decision needs one unambiguous representation.
Useful? React with 👍 / 👎.
Summary
Validation
pm test — 109 passed
pm run check — passed
pm run test:package — passed
Closes #105