Skip to content

refactor(delivery,identity,vrs): delete a label branch, give DELTA-006 a producer, fold two duplications - #500

Merged
schickling-assistant merged 2 commits into
mainfrom
schickling-assistant/2026-09-07-clean-2
Sep 7, 2026
Merged

refactor(delivery,identity,vrs): delete a label branch, give DELTA-006 a producer, fold two duplications#500
schickling-assistant merged 2 commits into
mainfrom
schickling-assistant/2026-09-07-clean-2

Conversation

@schickling-assistant

Copy link
Copy Markdown
Contributor

What

Four items from reports/deepclean-added-today.md, each small and independent.

C2 — HoldReason::UnattestedClaim was a label, not a decision

retention()'s branch order is quarantine → negative → profile.releases>= Persisted → attestation, so the attestation arm was reachable only for a phase below Persisted that the profile does not release — i.e. Attempted — which already holds as AmbiguousAttempt. retry() collapses both through one arm, and no caller reads the reason: codex_app_server.rs:1087 and opencode_session.rs:1510 both test != RetryDecision::Retry, and HoldReason reaches no output path. So the arm changed no Release/Hold/Retry outcome anywhere.

Deleted, along with the three doc claims that credited the attestation with work the phase does. The safety property is delivered by Phase + Profile::proves — no harness profile proves Attempted, and seed re-checks an asserted phase against the profile and fails closed.

C3 — DELTA-006's deletion trigger named a query nobody ran

Both commands were runnable, but nothing produced them: no st2 doctor row, no axe check, no timer. "Prints nothing on every admitted host for seven consecutive days" then resolves on someone remembering — a date in disguise, which /sk-patterns-legacy-boundary's own Smells section forbids. Clause 1 additionally required "rollback to a pre-ledger release has stopped being supported", which no artifact records, so it could never be satisfied.

  • migrations::delivery_state::resolution_signal(&[state_dirs]) counts both clauses. Read-only by construction: it opens no Ledger, so measuring cannot consume what it measures (opening translates and persists).
  • delivery_ledger::asserted_entries reads the ledger bytes directly; a missing ledger is zero, unparseable bytes are an error.
  • st2 doctor prints one advisory per seat and nothing when both clauses are clear, so the trigger keeps the shape of the query it replaces.
  • Clause 1 now names an artifact: a ratified decision under docs/vrs/.decisions/ recording that the pre-ledger release line is no longer a rollback target. Its existence is what closes the clause.

DELTA-006 also loses its false claim that the attestation concept is permanent: it is trigger instrumentation for this arm, and it is deleted with the arm. Deletion cost is restated accordingly.

C4 — identity API wider than its callers

resolve (0 external callers) and resolve_address (0) become private; delivery_ledger::{LEDGER_SCHEMA, LEDGER_FILE} become pub(crate). Three sites re-derived the identical {id, bus_identity, host, address} mapping from the same spec address_book already performs; that derivation becomes AddressBookEntry::of, which address_book itself uses. message.rs:1373 keeps its own literal — it maps an AddressableAgent, not an AgentSpec.

C5 — tests/vrs_ledger.rs allow-list

The expected literal spelled 4 numbers × 8 full filename stems (35 of 122 LOC). Comparing number -> count keeps every property that matters — a new number colliding, a third file joining an existing collision, a repair — and drops only "a stem was retitled", which is not a ledger property and forced a test edit on every rename. Renaming the 8 files is explicitly not done: 26 inbound references cite them and decision filenames are historical IDs.

Checks

  • Gate nix build .#st2 --no-link -L: EXIT=0 (pty st2.clean-2.gate, log /tmp/st2-clean2-gate.log). --lib 742 passed (741 before; +1 is the new signal test), agent_publish 15 passed / 9 ignored, validate 64, message 6, agent_address 12, vrs_ledger 3, all other targets unchanged.
  • New proofs: migrations::delivery_state::tests::the_resolution_signal_counts_each_clause_without_consuming_it (each clause counts nonzero exactly while its subject is present, and two consecutive measurements agree) and the reworded an_asserted_phase_suppresses_a_duplicate_and_authorizes_no_transport, which now shows the hold coming from the phase.
  • cargo check --all-targets: EXIT=0, no new warnings.
  • Real-binary smoke on the Nix-built st2 with a planted state dir:
state doctor line
nothing planted (silent)
delivery-state.json present ⚠ h.root pre-ledger delivery state (DELTA-006) — preLedgerRecords=1 assertedEntries=0
plus a ledger with 1 asserted + 1 observed entry … preLedgerRecords=1 assertedEntries=1
ledger bytes corrupt ✗ h.root delivery ledger readable — parsing delivery ledger …

LOC delta

+254/−126 across 9 files. Production: C2 −11, C4 −18; C3 +~75 (producer, doctor row, and its test); docs +49/−22; tests/vrs_ledger.rs −34.

Posted on behalf of @schickling
field value
agent_identity dev3.direct.omp.43sz6ujq
session dev3.43sz6ujq
agent_persona generalist
agent_supervisor unavailable
agent_tool OMP
agent_tool_version 18.1.7
agent_runtime OMP 18.1.7
tooling_profile dotfiles@39a19af

…allow-list

C4: four sites re-derived the same `{id, bus_identity, host, address}`
from the same spec that `identity::address_book` already maps. The
derivation becomes `AddressBookEntry::of`, which `address_book` uses too;
`resolve` and `resolve_address` had zero callers outside the module and
become private.

C5: `tests/vrs_ledger.rs` spelled four decision numbers and eight full
filename stems to assert a set equality. Comparing `number -> count`
keeps every ledger property that matters — a new number colliding, or a
third file joining an existing collision — and drops only "a stem was
retitled", which is not a ledger property and forced a test edit on
every rename.

agent-identity: dev3.direct.omp.43sz6ujq
agent-persona: generalist
agent-supervisor: unavailable
agent-tool: OMP
agent-tool-version: 18.1.7
agent-runtime: OMP 18.1.7
tooling-profile: dotfiles@39a19af
…6 a producer

C2: `HoldReason::UnattestedClaim` was a label, not a decision.
`retention()` reaches the attestation branch only for a phase below
`Persisted` that the profile does not release — i.e. `Attempted` — which
already holds as `AmbiguousAttempt`, and `retry()` collapses both through
one arm. No caller reads the reason: both transports test
`!= RetryDecision::Retry`. So the arm changed no Release/Hold/Retry
outcome anywhere; the safety property is delivered by `Phase` +
`Profile::proves`, which `seed` re-checks. Deleted, with the three docs
that claimed the attestation did that work.

C3: DELTA-006's Resolution Signal named a query nobody ran, so the
trigger resolved on someone remembering — a date in disguise. Adds the
producer: `migrations::delivery_state::resolution_signal` counts both
clauses read-only (it opens no ledger, so measuring cannot consume what
it measures) and `st2 doctor` prints
`<seat> pre-ledger delivery state (DELTA-006) — preLedgerRecords=N
assertedEntries=M`, silent when both are zero. Clause 1's unevidenceable
"rollback has stopped being supported" is replaced by a named artifact: a
ratified decision recording that the pre-ledger release line is no longer
a rollback target.

Consequence recorded in DELTA-006: the `attestation` field is trigger
instrumentation for this arm, not a permanent concept, so it is deleted
*with* the arm.

agent-identity: dev3.direct.omp.43sz6ujq
agent-persona: generalist
agent-supervisor: unavailable
agent-tool: OMP
agent-tool-version: 18.1.7
agent-runtime: OMP 18.1.7
tooling-profile: dotfiles@39a19af
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.

1 participant