Repository navigation
feat(s15): stage adoption without erasing state or widening policy - #364
Conversation
[High impact] The PR reaches the state and worker-release path for an explicitly selected S14 installation. Fix discard recovery, external continuity checks, and historical request reservations before merging. Findings
Fix with agent prompt### Issue 1
src/s15/adoption/service.ts:290
If the process crashes after `discard` saves `rolling_back`, retrying the exact request fails because this guard accepts only `preparing` or `validated`. The saved pending request also blocks replacement commands, leaving both runtimes fenced. Allow the recorded discard to resume from `rolling_back`, and add a crash test for this path.
### Issue 2
src/s15/adoption/source.ts:94-96
If browser continuity lives outside `stateDirectory`, even in an explicit `preservedPaths` root, `protectedState` is false. Its unversioned SQLite database is then merely checksummed, so adoption can proceed despite an unresolved browser action and no supported mapper. Apply the unknown-store check to explicit protected roots too, keeping the named native/profile exceptions separate.
### Issue 3
src/s15/adoption/source.ts:192-195
This query reserves only IDs from `command_receipts`, but S14 consumes `takeover.input` IDs separately in `computer_inputs`. Settled `succeeded` or `unknown` inputs pass adoption checks. After release, POLICY can accept those old IDs as fresh work requests because its guard checks only `source_requests`. Reserve the consumed input IDs too, without copying private input.
### Issue 4
src/s15/adoption/service.ts:284-287
If the owner revokes the validated policy before `release`, `currentPolicy()` still returns the same stored policy. Revocation changes the separate `policyRevoked` flag, so the hash check passes and adoption reports workers released. New work then fails with `authority_revoked`. Check that the bound policy is still usable before reporting release complete.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds explicit staged S14-to-S15 adoption while retaining original files and keeping POLICY empty until owner installation.
|
| stage.state = 'complete'; stage.revision++; finish(stage, actor); checkpoint('workers_released'); | ||
| } else if (command.kind === 'rollback' || command.kind === 'discard') { | ||
| const discard = command.kind === 'discard'; | ||
| if (discard ? !['preparing', 'validated'].includes(stage.state) : !['activating', 'active_fenced', 'complete', 'rolling_back', 'rolled_back'].includes(stage.state)) adoptionFail('transition_invalid_state'); |
There was a problem hiding this comment.
Interrupted discard cannot resume
If the process crashes after discard saves rolling_back, retrying the exact request fails because this guard accepts only preparing or validated. The saved pending request also blocks replacement commands, leaving both runtimes fenced. Allow the recorded discard to resume from rolling_back, and add a crash test for this path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/s15/adoption/service.ts
Line: 290
Comment:
**Interrupted discard cannot resume**
If the process crashes after `discard` saves `rolling_back`, retrying the exact request fails because this guard accepts only `preparing` or `validated`. The saved pending request also blocks replacement commands, leaving both runtimes fenced. Allow the recorded discard to resume from `rolling_back`, and add a crash test for this path.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const protectedState = containsPath(config.stateDirectory, path) && !containsPath(join(config.stateDirectory, 'native'), path) && !containsPath(`${o.source.database}.ops`, path) && path !== `${o.source.database}.lifecycle.sqlite`; | ||
| if (protectedState && /-(wal|shm|journal)$/.test(path)) adoptionFail('transition_sqlite_settlement_required'); | ||
| const entry = fileEntry(path, protectedState); total += entry.bytes; |
There was a problem hiding this comment.
External browser continuity is overlooked
If browser continuity lives outside stateDirectory, even in an explicit preservedPaths root, protectedState is false. Its unversioned SQLite database is then merely checksummed, so adoption can proceed despite an unresolved browser action and no supported mapper. Apply the unknown-store check to explicit protected roots too, keeping the named native/profile exceptions separate.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/s15/adoption/source.ts
Line: 94-96
Comment:
**External browser continuity is overlooked**
If browser continuity lives outside `stateDirectory`, even in an explicit `preservedPaths` root, `protectedState` is false. Its unversioned SQLite database is then merely checksummed, so adoption can proceed despite an unresolved browser action and no supported mapper. Apply the unknown-store check to explicit protected roots too, keeping the named native/profile exceptions separate.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const requests = (db.query('SELECT request_id,payload_hash,result_json FROM command_receipts ORDER BY request_id').all() as { request_id: string; payload_hash: string; result_json: string }[]).map(r => { | ||
| id(r.request_id); hash(r.payload_hash); | ||
| return { id: r.request_id, payloadHash: r.payload_hash, outcomeHash: hashBytes(r.result_json) }; | ||
| }); |
There was a problem hiding this comment.
Consumed request IDs become reusable
This query reserves only IDs from command_receipts, but S14 consumes takeover.input IDs separately in computer_inputs. Settled succeeded or unknown inputs pass adoption checks. After release, POLICY can accept those old IDs as fresh work requests because its guard checks only source_requests. Reserve the consumed input IDs too, without copying private input.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/s15/adoption/source.ts
Line: 192-195
Comment:
**Consumed request IDs become reusable**
This query reserves only IDs from `command_receipts`, but S14 consumes `takeover.input` IDs separately in `computer_inputs`. Settled `succeeded` or `unknown` inputs pass adoption checks. After release, POLICY can accept those old IDs as fresh work requests because its guard checks only `source_requests`. Reserve the consumed input IDs too, without copying private input.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| unchanged(stage); const p = currentPolicy(actor); | ||
| if (!p || !p.budget || p.expiresAt <= now() || p.budget.expiresAt <= now() || hashValue(p) !== stage.policyHash) adoptionFail('transition_policy_install_required'); | ||
| begin(); | ||
| stage.state = 'complete'; stage.revision++; finish(stage, actor); checkpoint('workers_released'); |
There was a problem hiding this comment.
Revoked policy still permits release
If the owner revokes the validated policy before release, currentPolicy() still returns the same stored policy. Revocation changes the separate policyRevoked flag, so the hash check passes and adoption reports workers released. New work then fails with authority_revoked. Check that the bound policy is still usable before reporting release complete.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/s15/adoption/service.ts
Line: 284-287
Comment:
**Revoked policy still permits release**
If the owner revokes the validated policy before `release`, `currentPolicy()` still returns the same stored policy. Revocation changes the separate `policyRevoked` flag, so the hash check passes and adoption reports workers released. New work then fails with `authority_revoked`. Check that the bound policy is still usable before reporting release complete.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.e95f4a1 to
fb2afd1
Compare
Stage explicit S14 adoption into a separate S15 store without rewriting original state, publishing private Bot data, or importing execution authority. S14 remains the default and omitted-runtime service configs remain legacy.
Closes #336
Implemented
adoption --s15 --report --config <plan.json>and installer report paths plus owner JSON/i18n status data. Existing adoption/runtime-selection APIs are retained for [Frontier] [EVID15-01] Prove the protected boundaries and the GitHub scenario on the tested build #340 composition. The read-onlys15AdoptionPortconnects authenticated real status to the merged UX recovery page without adding control authority or recording activity.Supported refusals and pending work
Review fixes
rolling_back, including interrupted discard before a policy store exists.takeover.inputIDs from input/audit continuity with null payload/outcome digests; never import private input, process metadata or its HMAC.Validation
Tested commit:
fb2afd15c5e20a9e91b85583fabb5bc37b14e6c6, based on merged main8244179e69dd2ba41ff60237a51448d10c9dd778.bun install --frozen-lockfile— passed.bun run typecheck— passed.bun run test— 4,054 passed, 9 skipped, 0 failed; 1 snapshot and 257,074 assertions across 211 files (748.79s).git diff --check— passed.Contract:
s15-state-transition/v1executable revision 2, with unchanged FR/NFR requirements and S15-AC16. No live or paid tests were run.