Fold the fault-seam evidence into #177

[George-RD]

Part of #182

Question

Comment onto #177: 13 fault-injection mechanisms across 8 files with three naming schemes and two arm semantics; four flags compiled into release builds (the seam must be test-gated wholesale, not per-field); ~9 test files hand-writing raw SQL through pub(crate) conn for crash-state setup; the duplicated shell-E2E harness; test-support fragmented with its de-facto home in api/dispatch_tests.rs.

Correctness/test shape — needs no story. Evidence: review, candidate 5.

Resolved when the comment is posted and linked from this ticket.

[George-RD]

Resolution. Folded. Fault-seam evidence posted: #177 (comment). Headline for the fix: four fault flags are compiled into release builds, so the seam must be test-gated wholesale.

[coderabbitai]

Coding Plan

Summary

Treat this ticket as an evidence-recording task, not a code change. The deliverable is a comment on #177.

Verify every candidate-5 claim against the kernel source. Correct the two claims that the code contradicts: the SQL-setup file count and the shell-E2E harness location.

Carry the evidence inline in the comment because the cited review document is not resolvable.

Post the comment to #177 and link it back from this ticket to meet the resolution criterion.

Design Choices

Design Choice 1: Handling of the dead review-document link

Options Considered:

1. Cite the review link only, as written in the ticket.
2. Carry the verified evidence inline in the comment, and mention the review link as the original source.

Chosen Option: 2

Rationale: The ticket cites .raw/openspine-architecture-review-2026-08-20.md as evidence, but that file does not exist in the repository and returns 404 on GitHub. The link is not resolvable, so the comment must be self-contained with concrete file and field references so #177 has usable evidence.

Design Choice 2: Treatment of claims that the code contradicts

Options Considered:

1. Repeat the raw claims verbatim.
2. State the corrected, code-verified facts and note where they differ from the raw claim.

Chosen Option: 2

Rationale: Verification found the "~9 SQL test files" count and the "duplicated shell-E2E harness" location differ from the raw claim. #177 needs accurate evidence to plan the proof work, so the comment must reflect what the code actually shows.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Consolidate and Correct the Fault-Seam Evidence

This phase turns the raw candidate-5 claims into a verified evidence set. The goal is one accurate list of findings with concrete source citations. No source code changes occur in this phase.

Task 1: Confirm the fault-injection inventory and the release-build gap

Record the fault-injection mechanisms and the exact test-gating gap.

• Cite the seven Store flags in crates/openspine-kernel/src/store/mod.rs, and name the four that lack #[cfg(test)]: fail_next_owner_reconfirmation, fail_next_standing_rule_remaining, fail_next_effective_allow_audit, fail_next_reservation_cancel.
• Note that these four fields and their check sites compile into release builds, while their setters are #[cfg(test)]-gated, which makes gating depend on discipline rather than the type system.
• Cite the check sites: store/activation.rs, store/learned_reconfirmation.rs, store/audit_append.rs, and store/standing_rules_budget.rs.
• Cite the correctly-gated injectors in secret_store.rs, artifact_store.rs, and counterparty_keys.rs as the contrast set.
• Cite store/test_hooks.rs as a grab-bag of test-only helpers, not a central home, to support the "test-gate the seam wholesale, not per-field" point.
• Record the three naming schemes (fault_*, fail_next_* / fail_*_for_test, and arm_* / break_* / force_* / install_*) and the two arm semantics (one-shot self-consuming AtomicBool::swap, and persistent toggle via set_*_for_test).

Task 2: Correct the SQL-setup and shell-E2E claims to match the code

Reconcile the two claims that verification contradicts.

• Confirm the pub(crate) conn: Arc<Mutex<Connection>> field in store/mod.rs and the raw-SQL setup pattern (store.conn.lock()...execute(...)).
• State the corrected scope: the raw-SQL-through-conn pattern appears in roughly 26 test files, wider than the "~9" claim; the crash/recovery-specific example is store/worker_supervision_tests.rs.
• State the corrected shell-E2E harness location: the duplication is between pipeline/tests/terminal_e2e.rs and pipeline/worker_e2e_tests.rs inside the kernel crate, not against openspine-shell/src/client_tests.rs.
• List the duplicated helpers: the shell-binary locator (shell_binary / worker_shell_binary) and the audit_action_count helper, plus the shared process-spawning setup skeleton.

Task 3: Confirm the test-support fragmentation claim

Record the split between the intended and de-facto shared test-support modules.

• Cite test_support.rs as the intended shared fixture module (AppState builders, owner-surface helpers).
• Cite api/dispatch_tests.rs as the de-facto second home that exports OWNER_CHAT_ID and mint_grant_with_selection_token, is imported by about 22 modules, and is already flagged allow-large-module.

🤖 Prompt for AI agents

Implement Phase 1: build a single, verified evidence set for the fault-injection
seam by checking each candidate-5 claim against the kernel source. This phase
does not change any source code; it only records findings for use in the comment
drafted in Phase 2.

For the fault-injection inventory:
- Cite the seven `Store` flags in `crates/openspine-kernel/src/store/mod.rs`.
- Name the four flags that lack `#[cfg(test)]`:
`fail_next_owner_reconfirmation`, `fail_next_standing_rule_remaining`,
`fail_next_effective_allow_audit`, `fail_next_reservation_cancel`.
- Note these four fields and their check sites compile into release builds,
while their setters are `#[cfg(test)]`-gated, so gating depends on discipline,
not the type system.
- Cite the check sites: `store/activation.rs`,
`store/learned_reconfirmation.rs`, `store/audit_append.rs`,
`store/standing_rules_budget.rs`.
- Cite the correctly-gated injectors in `secret_store.rs`, `artifact_store.rs`,
`counterparty_keys.rs` as the contrast set.
- Cite `store/test_hooks.rs` as a grab-bag of test-only helpers, not a central
home, to support gating the seam wholesale rather than per-field.
- Record the three naming schemes (`fault_*`; `fail_next_*` / `fail_*_for_test`;
`arm_*` / `break_*` / `force_*` / `install_*`) and the two arm semantics
(one-shot self-consuming `AtomicBool::swap`, and persistent toggle via
`set_*_for_test`).

For the SQL-setup and shell-E2E claims:
- Confirm the `pub(crate) conn: Arc<Mutex<Connection>>` field in `store/mod.rs`
and the raw-SQL setup pattern (`store.conn.lock()...execute(...)`).
- Correct the scope: the raw-SQL-through-`conn` pattern appears in roughly 26
test files, not the claimed "~9"; cite `store/worker_supervision_tests.rs` as
the crash/recovery-specific example.
- Correct the shell-E2E harness location: the duplication is between
`pipeline/tests/terminal_e2e.rs` and `pipeline/worker_e2e_tests.rs` inside the
kernel crate, not against `openspine-shell/src/client_tests.rs`.
- List the duplicated helpers: the shell-binary locator (`shell_binary` /
`worker_shell_binary`), the `audit_action_count` helper, and the shared
process-spawning setup skeleton.

For the test-support fragmentation claim:
- Cite `test_support.rs` as the intended shared fixture module (`AppState`
builders, owner-surface helpers).
- Cite `api/dispatch_tests.rs` as the de-facto second home that exports
`OWNER_CHAT_ID` and `mint_grant_with_selection_token`, is imported by about 22
modules, and is already flagged `allow-large-module`.

Phase 2: Draft the #177 Comment

This phase produces the comment body. The comment must be self-contained, accurate, and framed for the security-seam proof work that #177 tracks.

Write the comment body

Compose the comment that folds the fault-seam evidence into #177.

• Open with the intent: consolidate and test-gate the fault-injection seam so the two #177 security seams can be proven with deterministic, mutation-killing tests.
• Present the evidence from Phase 1 as a short structured list: the 13 mechanisms across 8 files, the three naming schemes, the two arm semantics, and the four release-build flags.
• State the corrected facts for the SQL-setup file scope and the shell-E2E harness location.
• Include the test-support fragmentation point with the api/dispatch_tests.rs de-facto-home finding.
• State the correctness/test-shape framing: this is test-shape debt and needs no product story.
• Reference the original source as "review, candidate 5" and note the linked document path, while keeping the evidence inline because the link is not resolvable.

🤖 Prompt for AI agents

Implement Phase 2: write the full text of the comment to post on issue `#177`,
using the verified evidence from Phase 1. The comment must stand on its own, be
accurate, and frame the findings for the security-seam proof work that `#177`
tracks.

Structure the comment as follows:
- Open by stating the intent: consolidate and test-gate the fault-injection seam
so the two `#177` security seams can be proven with deterministic,
mutation-killing tests.
- Present the Phase 1 evidence as a short structured list, covering: the 13
mechanisms across 8 files, the three naming schemes, the two arm semantics, and
the four release-build flags.
- State the corrected facts for the SQL-setup file scope (roughly 26 files, not
~9) and the corrected shell-E2E harness location (between
`pipeline/tests/terminal_e2e.rs` and `pipeline/worker_e2e_tests.rs` inside the
kernel crate).
- Include the test-support fragmentation point, citing the
`api/dispatch_tests.rs` de-facto-home finding.
- State the correctness/test-shape framing: describe this as test-shape debt
that needs no product story.
- Reference the original source as "review, candidate 5" and note the linked
document path, while keeping the evidence inline in the comment because the link
is not resolvable.

Phase 3: Post the Comment and Link It Back

This phase satisfies the resolution criterion.

Task 1: Post the comment to `#177`

Publish the drafted comment on issue #177.

• Post the Phase 2 comment onto GitHub issue #177 in the George-RD/openspine repository.

Task 2: Link the comment from this ticket

Close the loop as the ticket requires.

• Add the permalink of the posted #177 comment to this ticket.
• Confirm both actions are complete, because the ticket resolves only when the comment is posted and linked.

🤖 Prompt for AI agents

Implement Phase 3: post the comment drafted in Phase 2 and close the loop back
to this ticket.

- Post the Phase 2 comment onto GitHub issue `#177` in the `George-RD/openspine`
repository.
- Add the permalink of the posted `#177` comment to this ticket.
- Confirm both actions are complete. The ticket resolves only when the comment
is posted and linked.

Research

The work centers on crates/openspine-kernel. Store in crates/openspine-kernel/src/store/mod.rs holds several fault-injection flags. Four of these flags lack #[cfg(test)] and compile into release builds. Additional fault injectors live in secret_store.rs, artifact_store.rs, counterparty_keys.rs, and store/test_hooks.rs, with inconsistent naming and arm semantics. Test setup across many store/*_tests.rs and api/*_tests.rs files reaches through pub(crate) conn to run raw SQL. Shared test helpers are split between test_support.rs and api/dispatch_tests.rs. Two kernel E2E test files, pipeline/tests/terminal_e2e.rs and pipeline/worker_e2e_tests.rs, duplicate a shell-binary locator and audit-count helper. This ticket does not change any of this code. It records the evidence on #177.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement Phase 1: build a single, verified evidence set for the fault-injection
seam by checking each candidate-5 claim against the kernel source. This phase
does not change any source code; it only records findings for use in the comment
drafted in Phase 2.

For the fault-injection inventory:
- Cite the seven `Store` flags in `crates/openspine-kernel/src/store/mod.rs`.
- Name the four flags that lack `#[cfg(test)]`:
`fail_next_owner_reconfirmation`, `fail_next_standing_rule_remaining`,
`fail_next_effective_allow_audit`, `fail_next_reservation_cancel`.
- Note these four fields and their check sites compile into release builds,
while their setters are `#[cfg(test)]`-gated, so gating depends on discipline,
not the type system.
- Cite the check sites: `store/activation.rs`,
`store/learned_reconfirmation.rs`, `store/audit_append.rs`,
`store/standing_rules_budget.rs`.
- Cite the correctly-gated injectors in `secret_store.rs`, `artifact_store.rs`,
`counterparty_keys.rs` as the contrast set.
- Cite `store/test_hooks.rs` as a grab-bag of test-only helpers, not a central
home, to support gating the seam wholesale rather than per-field.
- Record the three naming schemes (`fault_*`; `fail_next_*` / `fail_*_for_test`;
`arm_*` / `break_*` / `force_*` / `install_*`) and the two arm semantics
(one-shot self-consuming `AtomicBool::swap`, and persistent toggle via
`set_*_for_test`).

For the SQL-setup and shell-E2E claims:
- Confirm the `pub(crate) conn: Arc<Mutex<Connection>>` field in `store/mod.rs`
and the raw-SQL setup pattern (`store.conn.lock()...execute(...)`).
- Correct the scope: the raw-SQL-through-`conn` pattern appears in roughly 26
test files, not the claimed "~9"; cite `store/worker_supervision_tests.rs` as
the crash/recovery-specific example.
- Correct the shell-E2E harness location: the duplication is between
`pipeline/tests/terminal_e2e.rs` and `pipeline/worker_e2e_tests.rs` inside the
kernel crate, not against `openspine-shell/src/client_tests.rs`.
- List the duplicated helpers: the shell-binary locator (`shell_binary` /
`worker_shell_binary`), the `audit_action_count` helper, and the shared
process-spawning setup skeleton.

For the test-support fragmentation claim:
- Cite `test_support.rs` as the intended shared fixture module (`AppState`
builders, owner-surface helpers).
- Cite `api/dispatch_tests.rs` as the de-facto second home that exports
`OWNER_CHAT_ID` and `mint_grant_with_selection_token`, is imported by about 22
modules, and is already flagged `allow-large-module`.
===============================================================================

Task: 2

Implement Phase 2: write the full text of the comment to post on issue `#177`,
using the verified evidence from Phase 1. The comment must stand on its own, be
accurate, and frame the findings for the security-seam proof work that `#177`
tracks.

Structure the comment as follows:
- Open by stating the intent: consolidate and test-gate the fault-injection seam
so the two `#177` security seams can be proven with deterministic,
mutation-killing tests.
- Present the Phase 1 evidence as a short structured list, covering: the 13
mechanisms across 8 files, the three naming schemes, the two arm semantics, and
the four release-build flags.
- State the corrected facts for the SQL-setup file scope (roughly 26 files, not
~9) and the corrected shell-E2E harness location (between
`pipeline/tests/terminal_e2e.rs` and `pipeline/worker_e2e_tests.rs` inside the
kernel crate).
- Include the test-support fragmentation point, citing the
`api/dispatch_tests.rs` de-facto-home finding.
- State the correctness/test-shape framing: describe this as test-shape debt
that needs no product story.
- Reference the original source as "review, candidate 5" and note the linked
document path, while keeping the evidence inline in the comment because the link
is not resolvable.
===============================================================================

Task: 3

Implement Phase 3: post the comment drafted in Phase 2 and close the loop back
to this ticket.

- Post the Phase 2 comment onto GitHub issue `#177` in the `George-RD/openspine`
repository.
- Add the permalink of the posted `#177` comment to this ticket.
- Confirm both actions are complete. The ticket resolves only when the comment
is posted and linked.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!