review: record named-token pass 9 #38

Closed

vnprc-agent wants to merge 0 commits from vnprc-agent/pi-named-tokens-pass-9 into master AGit

vnprc-agent commented

2026-08-22 16:24:44 +01:00

Contributor

Scoped diff verification of pass-8 BLOCKER fix (b6d95ec) plus fresh sweep.
Verifying model claude-opus-4-8, distinct from pass-8's gpt-5.6-sol.

New findings by tag: 1 GAP, 0 BLOCKER, 0 SIMPLIFY.
Classification: 1 original-plan (GAP), 0 review-introduced.

GAP (original-plan, present since pass-3 31ae5c1): the plan pruned the
Nexus secrets token directory and the legacy runtime directory with
empty-only rmdir but never pruned the new runtime nested
// subdirectories, stranding empty
directories on credential/last-token removal and undermining the
final-empty-state and partial-removal contracts. Fixed in f917884.

Scoped diff verification of b6d95ec HELD: the live reconciler's v1
credentialLinks strictly records every managed legacy link with exact
path/target, so the bounded transitional legacyCredentialLinks map is the
complete and only ownership set; retirement reads only that durable map,
never a directory scan, so an unrecorded exact-target operator symlink
survives. Crash boundaries and Pi 0.84.2 registerProvider/session_start/
agent_settled surfaces re-confirmed.

SIMPLIFY sweep considered: the transitional legacyCredentialLinks map
(kept), directory-wide scan (rejected by pass 8), durable journal /
pre-unlink sync / restore-symlink recovery (superseded by distinct root),
same-leaf conversion, desired-set-keyed retirement, rotationStrategy,
legacy fallback, startup flags, selector/provider split, preference-state
helpers, deployment scope, and quota/failover machinery; nothing further
removable.

Stop rule not met: review-introduced (0) do not outnumber original-plan
(1), and this pass found an original-plan GAP.

Model: claude-opus-4-8

Scoped diff verification of pass-8 BLOCKER fix (b6d95ec) plus fresh sweep. Verifying model claude-opus-4-8, distinct from pass-8's gpt-5.6-sol. New findings by tag: 1 GAP, 0 BLOCKER, 0 SIMPLIFY. Classification: 1 original-plan (GAP), 0 review-introduced. - GAP (original-plan, present since pass-3 31ae5c1): the plan pruned the Nexus secrets token directory and the legacy runtime directory with empty-only rmdir but never pruned the new runtime nested <token-link-root>/<credential>/ subdirectories, stranding empty directories on credential/last-token removal and undermining the final-empty-state and partial-removal contracts. Fixed in f917884. Scoped diff verification of b6d95ec HELD: the live reconciler's v1 credentialLinks strictly records every managed legacy link with exact path/target, so the bounded transitional legacyCredentialLinks map is the complete and only ownership set; retirement reads only that durable map, never a directory scan, so an unrecorded exact-target operator symlink survives. Crash boundaries and Pi 0.84.2 registerProvider/session_start/ agent_settled surfaces re-confirmed. SIMPLIFY sweep considered: the transitional legacyCredentialLinks map (kept), directory-wide scan (rejected by pass 8), durable journal / pre-unlink sync / restore-symlink recovery (superseded by distinct root), same-leaf conversion, desired-set-keyed retirement, rotationStrategy, legacy fallback, startup flags, selector/provider split, preference-state helpers, deployment scope, and quota/failover machinery; nothing further removable. Stop rule not met: review-introduced (0) do not outnumber original-plan (1), and this pass found an original-plan GAP. Model: claude-opus-4-8

vnprc-agent added 19 commits

2026-08-22 16:24:44 +01:00

docs: plan managed Pi named tokens a6db27f360

plan: fix manifest migration, token-directory lifecycle, drop rotationStrategy e04ccc03d9

Review pass 1 findings on managed Pi named tokens:

[BLOCKER] Reconciler strictly requires ownership manifest .version==1 with
per-credential credentialLinks; per-token links change that shape, so the
first post-upgrade activation on a machine that already owns a provider would
refuse a valid prior-version manifest as corrupt and never converge. Plan now
requires the reconciler to recognize and upgrade a prior-version manifest in
place.

[GAP] Token-directory lifecycle was implicit: assert_recoverable_plan's
realpath -e parent check rejects a not-yet-existent <credential>/ dir,
safe_journal_path allows only single-segment *.age paths, and rollback/
recover/retire never remove the created directory. Plan now makes the token
directory a journaled transaction object that is created safely and pruned so
no empty/untracked directory survives.

[SIMPLIFY] rotationStrategy is stored/validated/tested but no code branches on
it, and provider-scoped rotate is being removed. Plan now drops it from the
record and migration; witnesses assert its absence.

Acceptance-test coverage extended for all three.

review: record pass 1 metadata and refocus for pass 2 0b141d6cd2

Pass 1 new findings: 1 BLOCKER, 1 GAP, 1 SIMPLIFY (3 total).
Classification: all 3 original-plan; 0 review-introduced.
Fixing hash: e04ccc0 (all three).

SIMPLIFY sweep considered: the migrate command (kept \u2014 one-shot forward-only
reader is safer than auto-migration), token list/default-none subcommands
(kept \u2014 required by the contract), and the rotationStrategy credential field
(removed \u2014 stored/validated/tested but no behavioral consumer once
provider-scoped rotate is dropped). No generic quota/failover machinery was
introduced.

Pass 1 confirmed via Pi 0.84.2 docs that session_start registerProvider,
agent_settled, and ctx.hasPendingMessages()/isIdle() exist, so the startup and
queued-work pinning claims (Focus Areas 1-2) are implementable \u2014 no findings
there. Because a BLOCKER was fixed, pass 2 is a scoped diff verification of
e04ccc0 by a different model.

Model: claude-opus-4-8

plan: tighten token migration ownership safety 9897fcd680

Clarify that legacy manifest records are strictly validated and used only to retire exact old links before desired per-token ownership is re-derived. Require rmdir-only token-directory pruning so concurrent or operator contents survive.

Findings: 0 BLOCKER, 2 GAP, 0 SIMPLIFY; both GAPs were introduced by review commit e04ccc0.

review: record named-token pass 2 4cbd5b0485

Pass 2 found 0 BLOCKER, 2 GAP, and 0 SIMPLIFY: 0 original-plan findings and 2 review-introduced findings. Both review-introduced GAPs are fixed in 9897fcd; metadata and pass-3 focus are updated here.

The SIMPLIFY sweep considered rotationStrategy, legacy fallback, startup flags, provider/token command separation, state helpers, deployment scope, and quota/failover machinery; no additional simplification was justified. Stop rule is not yet met.

Model: gpt-5.6

plan: pin per-token runtime link naming and legacy-path conversion order 31ae5c1791

GAP: the plan never specified per-token runtime credential link naming.
Nested (mirroring the ciphertext layout) is required because both
credential and token names may contain hyphens, so a flat
<credential>-<token> link name cannot be split back into its parts for
per-credential token listing. Nesting under the credential name also
means a legacy single-file per-credential symlink and the new
per-credential token directory occupy the identical leaf path
(archetypes/modules/pi-provider-reconcile.sh: 'path=$credential_dir/$credential'),
so the manifest-version-transition fix from 9897fcd needs an explicit
retire-before-create ordering and a same-path collision refusal, or an
implementation could mkdir over the still-live legacy symlink and fail
the very upgrade path pass 1/2 fixed. Added the ordering guarantee and
a witness item for the same-path conversion.

Findings: 1 GAP, 0 BLOCKER, 0 SIMPLIFY. Original-plan (present since a6db27f,
missed by passes 1-2).

review: record named-token pass 3 852725322a

Findings: 1 GAP, 0 BLOCKER, 0 SIMPLIFY (original-plan, present since a6db27f).
0 review-introduced findings. Fixing hash: 31ae5c1.

SIMPLIFY sweep re-grounded rotationStrategy against the live Nexus script
(three read sites: stage_registries add-time check, credential-registry
schema exact()/enum check, print_impact's rotate in-place warning), all
reachable only through the provider-scoped add/rotate commands the plan
already replaces; pass 1's drop confirmed lossless, no further scope found
among legacy fallback, startup flags, command separation, state helpers,
deployment scope, or quota/failover machinery.

Stop rule not met: pass 3 produced an original-plan GAP, so neither
disjunct (review-introduced outnumbering originals, or neither
BLOCKER-nor-original-GAP for two consecutive passes) holds. Next pass is a
scoped diff verification of 31ae5c1.

Model: claude-sonnet-5

plan: recover interrupted legacy link conversion c7a5caa427

BLOCKER: the pass-3 same-leaf conversion required unlinking a legacy\nsymlink before creating its token directory, but the plan had only in-process\nrollback. A kill or reboot in that interval would leave neither object; the old\nstrict ownership validation would then refuse forever. Require a synced,\nbearer-free conversion journal that recovery consumes before manifest\nvalidation, restoring exact old ownership or preserving a foreign occupant.\n\nGAP: pin pending /token selections as process-local until activation succeeds,\nso /token cancel cannot accidentally leave a new remembered preference.\n\nFindings: 1 BLOCKER (review-introduced by 31ae5c1), 1 GAP (original-plan).

review: record named-token pass 4 37233547f3

Findings: 1 BLOCKER (review-introduced by 31ae5c1), 1 GAP (original-plan).
Fixing hash: c7a5caa.

Pass 4 verified the same-leaf collision/order requirement but found its
SIGKILL/reboot gap. Pass 5 is a scoped blocker-fix review by a different model.

SIMPLIFY considered conversion-journal alternatives, preference helpers, legacy
fallback, startup flags, selector/provider separation, deployment scope, and
quota/failover machinery; no removable scope remains.

Model: gpt-5.6-terra

plan: install per-token links under a distinct root, drop conversion journal 362d3d00f9

SIMPLIFY: the pass-3 same-leaf nesting put each credential's new token
directory at the exact legacy per-credential symlink leaf, forcing a
symlink-to-directory type change with an unavoidable absent-name interval,
which pass 4 then had to cover with a durable synced conversion journal,
recovery-before-manifest-validation, restore-exact-symlink, retire-before-create
ordering, and same-path collision refusal.

The credential-link path is an internal reconciler contract: the generated
auth.json !command is emitted against the same root in the same operation
(pi-provider-reconcile.sh line 352), so nothing external pins the legacy
location. Installing the new nested per-token links under a distinct root
that never contains a legacy leaf removes the type change entirely: create the
additive new links, commit auth/models/manifest atomically, then retire each
legacy symlink by exact recorded-target match. Every crash point is already
crash-safe with ordinary atomic renames and idempotent retry, so the whole
pass-3/pass-4 migration journal machinery is deleted while all contracts hold.

Findings: 1 SIMPLIFY (removes review-introduced machinery from 31ae5c1/c7a5caa).

review: record named-token pass 5 50b2515a7c

Pass 5 scoped diff verification of c7a5caa held: the conversion journal is
internally sound for the same-leaf design (synced before unlink, consumed
before manifest validation, restores exact legacy symlink or preserves a
foreign occupant, empty-only rmdir, never recursive). The directed SIMPLIFY
sweep then dissolved that design.

New findings by tag: 1 SIMPLIFY, 0 BLOCKER, 0 GAP, 0 QUESTION.
Origin: the SIMPLIFY is review-introduced (removes machinery added by review
passes 3 (31ae5c1) and 4 (c7a5caa)); 0 original-plan findings.
Fixing hash: 362d3d0 (plan), this commit (prompt metadata).

SIMPLIFY sweep considered: same-leaf conversion + crash journal vs installing
the new nested per-token links under a distinct managed root (adopted \u2014 the
link path is an internal reconciler contract; auth.json !command is generated
against the same root in one operation, pi-provider-reconcile.sh line 352, so
nothing external pins the legacy leaf; distinct root removes the type change,
absent-name interval, journal, pre-unlink sync, restore-exact-symlink recovery,
and same-path collision handling while keeping unambiguous per-credential token
listing, forward-only migration, and operator-collision preservation); plus
rotationStrategy, legacy fallback, startup flags, selector/provider split,
preference-state helpers, deployment scope, and quota/failover machinery
(no further removable scope).

Startup (Focus 1) and queued-work pinning (Focus 2) re-confirmed against Pi
0.84.2 docs/extensions.md, docs/rpc.md, and docs/models.md. Pass-2 empty-only
rmdir and pass-4 preference-after-activation fixes remain intact.

Stop rule not met: neither disjunct has two consecutive qualifying passes
(pass 4 had a BLOCKER and a 1-vs-1 count). Next: scoped diff verification of
362d3d0 by a different model.

Model: claude-opus-4-8

plan: make distinct-root publication ordering explicit 293362d14e

review: record named-token pass 6 5fc3d3a369

New findings: 1 GAP, 0 BLOCKER, 0 SIMPLIFY; 1 original-plan, 0 review-introduced.
Fixed by 293362d. SIMPLIFY sweep found no removable scope.
Model: gpt-5.6-luna

plan: make legacy-link retirement a directory-wide sweep, not a desired-set lookup 5ed45c65f6

Scoped diff review of 293362d. The migration ordering retires legacy
per-credential symlinks after the new-version manifest commit, but the
only prior source for which credentials had a legacy link was the old
manifest's credentialLinks list, and that list is gone once the new
manifest is committed. A credential retired in the same operation that
performs the version bump would never appear in any future desired-
credential set, so retirement keyed off "currently desired credentials"
could never find and clean up its legacy link, and a crash between the
manifest commit and retirement had no persisted list to resume from.

Fix: legacy retirement is a standing, credential-set-independent sweep
of the legacy link root's direct entries, retiring any entry that is a
symlink to its own name's computed runtime target regardless of current
desired status, run unconditionally on every activation rather than
only right after a version transition. This needs no persisted list of
which credentials used to have a legacy link, so it self-heals across a
crash between manifest commit and retirement and across simultaneous
credential removal. Added the combined migration+removal case to the
archetypes acceptance-test witness.

review: record named-token pass 7 055a8dfa05

Pass 7 (claude-sonnet-5, reasoning medium): scoped diff verification
of 293362d. 1 original-plan GAP, 0 review-introduced. Legacy-link
retirement after the new-version manifest commit had no source for a
credential's legacy target once the old manifest was overwritten, so a
credential retired in the same operation as the version bump could
never be found by retirement keyed on the current desired-credential
set, and a crash between manifest commit and retirement had no
persisted list to resume from. Fixed in 5ed45c6 by making retirement
an unconditional, credential-set-independent sweep of the legacy link
root, self-healing from the directory alone.

Fresh sweep found no further BLOCKER/GAP; SIMPLIFY reconsidered the
sweep fix against a persisted pending-retirement list (rejected,
reintroduces journal-like bookkeeping) and re-confirmed prior SIMPLIFY
conclusions (conversion journals, pre-unlink sync, restore-symlink
recovery, same-leaf collision handling, legacy fallback, startup
flags, provider/token separation, preference helpers, deployment
scope, quota/failover machinery); no further removable scope found.

Stop rule not met: review-introduced findings (0) do not outnumber
original-plan findings (1), and this pass found an original-plan GAP.
Continue with a scoped diff verification of 5ed45c6 by a different
eligible model than claude-sonnet-5.

Model: claude-sonnet-5

plan: bound legacy-link retirement to recorded ownership b6d95ecba4

review: record named-token pass 8 e2070cd454

Findings: 1 BLOCKER, 0 GAP, 0 SIMPLIFY; 0 original-plan and 1 review-introduced.

The review-introduced BLOCKER in 5ed45c6 allowed an unconditional legacy-root sweep to delete unrecorded exact-target symlinks. Fixed in b6d95ec with bounded transitional ownership in the new manifest.

SIMPLIFY considered the unconditional scan, desired-set lookup, manifest-carried transitional ownership, a separate journal, conversion recovery, legacy fallback, startup flags, selector/provider separation, preference helpers, deployment scope, and quota/failover machinery; no further scope can be removed.

Model: gpt-5.6-sol

plan: prune emptied runtime nested credential directories f9178840a5

Empty-only rmdir pruning applied to the Nexus secrets token directory and
the legacy runtime directory was never extended to the new nested
<token-link-root>/<credential>/ subdirectories the pass-3 design created.
Credential/last-token removal stranded empty directories, undermining
final-empty-state and partial-removal contracts. Require empty-only rmdir
pruning of emptied nested subdirectories and the token-link root, preserving
operator contents, and cover it in the archetypes witness.

review: record named-token pass 9 5622359e77

Scoped diff verification of pass-8 BLOCKER fix (b6d95ec) plus fresh sweep.
Verifying model claude-opus-4-8, distinct from pass-8's gpt-5.6-sol.

New findings by tag: 1 GAP, 0 BLOCKER, 0 SIMPLIFY.
Classification: 1 original-plan (GAP), 0 review-introduced.

- GAP (original-plan, present since pass-3 31ae5c1): the plan pruned the
  Nexus secrets token directory and the legacy runtime directory with
  empty-only rmdir but never pruned the new runtime nested
  <token-link-root>/<credential>/ subdirectories, stranding empty
  directories on credential/last-token removal and undermining the
  final-empty-state and partial-removal contracts. Fixed in f917884.

Scoped diff verification of b6d95ec HELD: the live reconciler's v1
credentialLinks strictly records every managed legacy link with exact
path/target, so the bounded transitional legacyCredentialLinks map is the
complete and only ownership set; retirement reads only that durable map,
never a directory scan, so an unrecorded exact-target operator symlink
survives. Crash boundaries and Pi 0.84.2 registerProvider/session_start/
agent_settled surfaces re-confirmed.

SIMPLIFY sweep considered: the transitional legacyCredentialLinks map
(kept), directory-wide scan (rejected by pass 8), durable journal /
pre-unlink sync / restore-symlink recovery (superseded by distinct root),
same-leaf conversion, desired-set-keyed retirement, rotationStrategy,
legacy fallback, startup flags, selector/provider split, preference-state
helpers, deployment scope, and quota/failover machinery; nothing further
removable.

Stop rule not met: review-introduced (0) do not outnumber original-plan
(1), and this pass found an original-plan GAP.

Model: claude-opus-4-8