test(threads): complete retired corpus recovery evidence - #1046
Conversation
Add bounded authenticated two-worker clock ticks, real corpus contention and unsupported-declaration journeys, and exact successful audit exports. Document the existing pending-history classifier without claiming a global historical repair. Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Complete finite corpus engineering packet, for parent review#1046 is at
All paths above are under Original corpus-specific red is now reproducedAt historical test-only commit cargo +1.95.0 test --locked -p coven-cli --features threads-test-clock \
--test threads_e2e \
retired_corpus_scheduled_intake_survives_restart_and_applies_once -- --nocaptureActual red: one failed / zero passed / eight filtered. Public New-source native and current-core proof
The first native attempt remains a real preserved failure: existing same-home Recommendation: the finite seven-scenario and original-red engineering |
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate findings and one documentation nit remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds feature-gated Threads E2E evidence for retired-corpus recovery, concurrent scheduler workers, and unsupported identity rejection without changing default production scheduling.
Changes:
- Adds bounded two-worker test-clock scheduling and contention checkpoints.
- Expands migration, recovery, terminal, audit, and artifact assertions.
- Documents test-clock behavior and recovery classification boundaries.
File summaries
| File | Summary | Final comments |
|---|---|---|
docs/reference/threads-e2e.md |
Documents corpus evidence outputs. | Nit (2 votes): Document worker completion order as order-agnostic; it may be [0, 1] or [1, 0]. |
docs/design/threads-test-clock.md |
Documents multi-worker fixture behavior. | None. |
docs/design/threads-terminal-recovery.md |
Defines recovery classification boundaries. | None. |
crates/coven-cli/tests/threads_e2e.rs |
Adds migration and evidence capture helpers. | None. |
crates/coven-cli/tests/support/threads_corpus_closure_cases.rs |
Adds contention and unsupported-corpus journeys. | Moderate (1 vote): Improve the 5-second guard and include elapsed time in failures. Moderate (1 vote): Count all terminal proposal event types. Moderate (1 vote): Verify apply_audit rows prove exactly-once writes. |
crates/coven-cli/src/api.rs |
Adds bounded concurrent scheduler workers. | None. |
Review details
Suppressed comments (3)
crates/coven-cli/tests/support/threads_corpus_closure_cases.rs:43
- This new 5-second wall-clock guard is the only bound for proving the second worker entered, so a busy shared runner can turn scheduler jitter into a false failure; the timeout message also omits the observed wait. Use a more generous hang guard and include the elapsed duration so failures remain diagnosable.
let deadline = Instant::now() + Duration::from_secs(5);
while scheduler_entries(fixture)? < before + 2 {
anyhow::ensure!(
Instant::now() < deadline,
"second daemon recovery worker did not reach the held pass lock"
crates/coven-cli/tests/support/threads_corpus_closure_cases.rs:166
- This invariant check omits
proposal_rejectedandproposal_vetoed. If live intake ever emitted a terminal refusal while leaving no pending proposal, the count would still be zero and this journey could falsely certify that the unsupported declaration produced no audit authority. Include all terminal proposal event types (or count every proposal audit row) in this check.
let authority: i64 = fixture.store()?.query_row(
"SELECT COUNT(*) FROM ward_audit WHERE event_type IN
('proposal_submitted','proposal_window_opened','proposal_approved','apply_audit')",
crates/coven-cli/tests/support/threads_corpus_closure_cases.rs:83
- The final bytes and a single apply intent do not prove exactly-once writing:
append_apply_audit_records_atrecords oneapply_auditrow per written surface, so a duplicate same-content apply could leave the files unchanged while adding extra write evidence. Count theapply_auditrows for this proposal and require one per corpus surface before claiming duplicate apply was prevented.
let intents: i64 = fixture.store()?.query_row(
"SELECT COUNT(*) FROM ward_audit WHERE proposal_id=?1 AND decision='proposal-apply-intent'",
[id],
|row| row.get(0),
)?;
anyhow::ensure!(intents == 1, "recovery created {intents} apply intents");
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Integrate accepted scheduler leadership fencing without altering the retired-corpus contribution. PR landing awaits parent review disposition. Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Normal landing preparation: forward-merged accepted main d00cc86 into head805ade0c9e478f1b28d6cbebf7147144fe0268b8. No conflict and no contribution change: the binary diff versus d00 is byte-identical to originalfcab0eaa versus5d662749 (SHA256af96faf01cc3e9ac860770dadde5985188167a23d1116e713fd8dcfd6c43ed04). Local20 automation-conformance,103 E2E and30 terminal tests, feature-complete all-target Clippy and formatting pass. Fresh CI34777893160 is running/queued, not yet acceptance. Original receipts remain intact. PR landing awaits the parent independent-review disposition and explicit merge authority; no root or human gate changed. |
Address review4000158050: the raw result preserves worker order, while acceptance requires one zero and one one regardless of which worker wins the pass lock. Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Context
Engineering evidence for #886 and #888; does not close either root.
Base
5d662749b734430f84b15c73a03637e106c06140; headfcab0eaa62955bb0d2b1240a13e323fa4104d8f7.Acceptance: exercise two actual daemon scheduler workers on restarted,
publicly submitted migrated corpus state; refuse a canonical unsupported
identity declaration through real migration and live intake; retain exact
corpus/submission/terminal evidence. Non-goals: governance, principal-write
credentials, production scheduling changes, global historical audit repair,
Threads pin/workflow changes, and Automations leadership work in #1042.
Consulted Coven AGENTS/CONTRIBUTING,
api.rs,ward_migrate.rs, existingscheduler/terminal/corpus tests; Threads AGENTS, manifest, Phase-5 decision,
typed approval/audit contracts, synthetic corpus, E2E contract and compatibility
manifest.
Implementation
workers: 2test-clock tick starts two concurrentcalls to the production Threads scheduler. The outer clock-control lock,
scheduler pass lock, decision claims and final-commit checks remain in place.
Other counts fail closed; default single-worker responses are unchanged.
the existing final-commit pause is released. Two ordinary HTTP ticks would
serialize at the clock-control lock and would not prove this contention.
normally, restarted pending, then contended: one apply intent, one typed
approval, per-worker completions
[0, 1], exact target bytes and stable audithistory after another restart.
unknown-identity-factdeclaration is refused by actualward migrate --apply, leaves its original file and absence of backup intact,and cannot stage through live intake.
and source hashes, plus the original daemon audit rows. Existing corpus veto,
downtime divergence, unavailable identity and supersession cases are mapped,
not replaced with generic cases.
classifier from a missing global historical-opening census. Quarantine is
not a typed close or historical repair.
Only six files changed:
crates/coven-cli/src/api.rs,crates/coven-cli/tests/threads_e2e.rs,crates/coven-cli/tests/support/threads_corpus_closure_cases.rs,docs/design/threads-test-clock.md,docs/design/threads-terminal-recovery.md,docs/reference/threads-e2e.md.Verification
eight-case corpus replay loop); four real-daemon legacy terminal cases passed.
controls, scheduler, typed terminal and recovery cases. The broad terminal
selector also includes unrelated pre-existing terminal tests; this is not a
count of 159 distinct Threads journeys.
cargo +1.95.0 clippy --locked --workspace --all-targets --features threads-test-clock -- -D warnings, formatting, diff, secret and stagedprivacy guards passed.
current-clean-Threads overlay observation are being recorded separately;
this draft does not claim their results prematurely.
The exact original corpus red was also reproduced at the preserved test-only
commit
d15d493914ed85f304ef8d89bcfef528b2df781a, whose parent-to-commit diffchanges only
threads_e2e.rs. Its real migration/intake returned 200 appliedinstead of canonical pending scheduling, immediately writing
TOOLS.md(26bytes) and
HEARTBEAT.md(22 bytes). Original Git corec3bd46bcand rawmanifest/audit/response evidence are retained. Production repair lineage
d3abad07and the later merged #1022/#972/#1029 integration leads to the currentgreen corpus journey; this is not an artificially broken current daemon.
Initial test-authoring failures are retained: an expected 409 was actually the
existing typed 500
ward_config_invalid, and two separate tick requestsserialized at the fixture lock. Neither is misrepresented as a production red.
Risk and Rollback
R4 boundary evidence; no public Rust exhaustive type, credential, audit schema,
second audit store, or migration is introduced. New control behavior is
feature-gated and requires the existing private synthetic fixture and owner-local
transport. Default production scheduling is unchanged.
Rollback is this scoped commit; do not revert the landed identity/protected/
terminal authority repairs. All fixtures are repository-owned and synthetic.
Agent Handoff
The supported seven-case corpus and nine-case terminal evidence remains
distinct from unprovable interrupted apply or arbitrary historical corruption.
The latter requires an explicit inventory/operator resolution procedure, not
fabricated approval/rejection rows. Root #886 therefore retains that concrete
historical obligation.
Native CI for this new head, integrated current-core results, required stable
Threads compatibility activation, and parent review remain separate. No human
gate or root issue is closed by this PR. Same-daemon contention is not evidence
for a second production daemon or an OS-level write counter.
Signed-off-by: Val Alexander 68980965+BunsDev@users.noreply.github.com
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com