Skip to content

feat(auth): renew active paired sessions with device authority - #334

Merged
lukemaj merged 7 commits into
mainfrom
feat/323-paired-session-renewal
Oct 14, 2026
Merged

lukemaj merged 7 commits into
mainfrom
feat/323-paired-session-renewal

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What: Active direct pairings renew through device-held rotating secrets, with headless approval for existing devices.
Why: Upstream 456930a09 now closes live connections at the fixed 30-day expiry, including cavallo-to-rocky on 2026-11-08.
So what: Merged after exact-head Sol/Grok success and green CI. The planner batches deployment, approves the known cavallo desktop commitments on bestia and rocky, then proves the live session. No restart or deployment was performed here.

Refs #323. This keeps cross-machine control available for the current Objective. Upstream main, PRs and Issues were searched first; no paired-session renewal implementation was available to port. Design and bounded review history are on #323, including the accepted core, headless approval, and browser profile correction.

Renewal retains the session ID, permissions and person. The server rotates at half the original lifetime, reissues access with the same expiry on not-due requests, deduplicates exact retries, and revokes conflicting reuse. Every mint rechecks expiry and revocation. Existing pairings can submit a commitment but gain no renewal authority until t3 auth renewal approve matches the full hash independently read from the intended device. List, reject and session revoke remain available over SSH.

Client authority is separate from connection catalogs: encrypted desktop files, browser IndexedDB and mobile SecureStore. Atomic revision checks share a pending successor before transport and recover uncertain responses. Connected clients update HTTP credentials in place; the server's socket watcher rereads authoritative expiry. New mobile pairings enroll automatically; existing direct-mobile sessions retain their old lifetime. DPoP/T3 Connect and reusable bootstrap credentials keep their existing paths.

Verification: 236 focused tests across 15 files passed across server auth, HTTP handlers, real WebSocket lifetime, shared client recovery/onboarding/resolution, primary cookie bootstrap, the connection supervisor, and desktop storage races. A runtime regression also proves renewal can initialize with the desktop local environment disabled, while the tracing test remains passing. The two Sol regression findings have fixes and red/green controls recorded in the finder recheck request. The descriptor fixture was then corrected to model an undeclared HTTP failure, and its 31 bootstrap cases passed again. The unchanged short-TTL watcher test fails with the original implementation and passes with the new watcher. A real WebSocket fixture echoes past the old expiry, then closes after revocation. Affected package typechecks and targeted lint/format were run; CI supplies the full gate. Local runtime is Node 26.8.2; CI uses the repository's pinned runtime.

Pending proof: The user explicitly declined browser execution. Reader coverage uses synthetic storage, a fake browser engine, and fake IndexedDB. No real browser reopen or live browser approval is claimed. Chromium-family legacy reads have a new headless CLI; legacy Firefox, Safari and mobile-browser reads remain unsupported and keep their original expiry. The planner's post-deploy live proof is cavallo's desktop session. No live DB was opened read-write and no server was started against live state.

Follow-ups outside the agreed gate: #325, #328, #330, #336, #337. No version bump. Website impact is runtime authentication behavior and the remote-access guide; no marketing or URL changes.

Requirements and who asked: User requested in-use paired renewal, stolen/revoked-session rejection, headless one-time approval without GUI or re-pairing, focused short-TTL/revoked tests, and exact-head independent Sol/Grok success plus green CI before delegated merge. The user later required fake-storage browser tests and an explicit real-browser proof gap.
Deleted: Version bump, deployment/restart, GUI approval, approval based on labels or server-listed hashes, preview-profile reads, and browser execution.
Bottleneck: Implementation is merged with both named reviews and full CI green. Live cavallo proof follows the planner's batched deployment and trusted-device approval.
Checked myself: Source and callers across all three clients, authoritative SQL guards, original-watcher failing control, passing focused tests, fixture-only browser read, and current Git state. Deployment and real-browser behavior remain unverified.

Implementation: GPT-6.1 Sol via Codex in T3 Code. Design: Claude Opus 5.5 via Claude Code in T3 Code. Independent reviews: GPT-6.1 Sol and Grok 4.7.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: The final C design blocker is cleared, and the two CI lint errors are fixed in a new head.
Why: CI caught unsupported Hermes Array#toReversed and a manual Effect runner inside a test. The earlier fork inventory failure was a missing ownership entry for the changed user guide.
So what: CI and both required exact-head implementation statuses remain before merge. Browser execution remains skipped; real-browser proof is pending by user instruction.

Grok's final design clearance: #323 (comment) . The fixed private reader copy uses <tmp>/Default and default-port 0. Grok ran the focused fake-engine tests, not Chromium.

The client now reverses a fresh array copy, compatible with Hermes. Cookie bootstrap tests inject renewal through the existing HTTP test fixture's runtime rather than creating an Effect runner in the test. All 47 affected tests passed after those changes, as did web/client-runtime typechecks and targeted lint. CI's full unused-code check had already passed; the local scoped unused-export check also passed.

The two implementation reviewers are still reviewing f9c13fda4e8527d308d5526d28df5d77a62a189f. Their verdicts on that head will be reconciled with the later inventory and compatibility/test changes, and both named statuses must be success on the final current head. No earlier-head review success will satisfy the merge rule.

Requirements and who asked: User's all-client renewal and exact-head dual-review/green-CI gate; explicit user instruction to skip real-browser execution.
Deleted: Hermes-incompatible API, a manual test runner, and any transfer of old-head statuses to a changed commit.
Bottleneck: Independent implementation review findings and final-head CI.
Checked myself: Exact CI job errors, changed source, 47 passing affected tests, successful affected typechecks, unused-code result, and Grok's design clearance.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent implementation review of f9c13fda4e8527d308d5526d28df5d77a62a189f finds two required correctness fixes.
Why: A failed browser descriptor request silently drops new-pair renewal enrollment, and a zero-row guarded expiry update still produces a successful rotation and credential.
So what: Fix B1 and B2 before approval; the finder rechecks the fixes on the new exact candidate. This verdict applies only to the requested SHA, not subsequent pushes.

Reviewed cumulative fbbe448316be1af24aeefb3ed124b152e8817253...f9c13fda4e8527d308d5526d28df5d77a62a189f, all 43 changed files, surrounding issuance/verification, primary bootstrap, connection authorization/supervision and storage callers. Read #323's body and the named accepted core, corrections, C approval and browser-placement records. I did not author this implementation. The author worktree and main were left untouched. Behavioral checks ran in my detached /tmp/chromeria-334-review-sol-f9c13fd, with disposable test additions only and no implementation edits.

B1 [P2, required]: do not consume a new browser pairing code after descriptor failure

Location: primary/auth.ts:232-248.

Trigger: an ordinary browser submits a valid new pairing credential, but its preliminary metadata.descriptor() request fails, for example through a transient transport/proxy/server failure. The catch converts every error to null, stage never runs, and the following successful browser-session exchange sends only { credential }. Server issuance consumes the code and creates a session without renewal authority. The UI reports successful pairing.

This permanently converts that new pairing into the legacy approval path. Retrying or reloading cannot reconstruct an authorized commitment from the already consumed code. Chromium needs an unexpected administrator approval; Firefox, Safari and mobile browsers have no supported legacy headless approval path, so they keep their original expiry despite the accepted promise that their new pairings enroll automatically. This is distinct from #330, which retains existing authority through a transient startup failure.

Independent in-process HTTP fixture reproduction through the real submitServerAuthCredential:

REVIEW_DESCRIPTOR_FAILURE {"descriptorCalls":1,"stageCalls":0,"pairingRequests":[{"credential":"valid-new-code"}]}
AssertionError: expected undefined to be defined

The assertion checks that the consuming request contains a renewal commitment; it does not. The fixture returns an internal descriptor failure followed by a successful browser-session response without renewAfter, matching the missing-commitment server path. No browser was launched.

Required correction: resolve the descriptor and durably stage authority before consuming the pairing code, or surface/retry that failure without consuming it. Preserve the explicit desktop-bootstrap exclusion. Add a focused failed-descriptor/successful-exchange control asserting that new pairing cannot silently complete unenrolled.

B2 [P2, required]: a zero-row expiry update must not report successful rotation

Location: SessionRenewal.ts:302-309.

Trigger: the session passes active(sessionId) just before expiry, then time crosses expiry before the separate DateTime.now at line 261. The renewal row advances, but UPDATE auth_sessions ... WHERE ... expires_at > now affects zero rows. Its result is ignored. The function returns the hypothetical { ...row, issuedAt: now, expiresAt }, and SessionStore.renewSession signs it anyway.

Independent deterministic proof uses the existing in-memory SQLite/service fixture and an injected Clock: first renewal time read is 599999, subsequent reads are 600001, for an original expiry of 600000. Actual output:

REVIEW_EXPIRY_BOUNDARY {"timeReads":5,"originalExpiresAt":600000,"databaseExpiresAt":600000,"mintedExpiresAt":1200001,"verify":"Success","nextAttempt":"Failure"}
AssertionError: expected 'Success' to be 'Failure'

The database session stays expired while renewal returns a future credential that the real HTTP session verifier accepts. The client can promote the successor and report success, but authoritative WebSocket verification rejects the expired session and the next renewal fails. This violates the accepted active/unexpired mint guard and successful-rotation contract. The narrow boundary is intentional: no claim is made that a request entering with an already expired session bypasses active.

Required correction: bind success to an actually updated active/unexpired auth row, reject and roll back the rotation/spent mutations on zero rows, and mint from the persisted result. Add the clock-boundary negative test and retain ordinary due-rotation and replay positives.

Proof and limits

  • Independently reran 10 focused existing files: 152 tests passed, including real WebSocket lifetime/revocation, renewal protocol/history, HTTP cookie issuance, authorization recovery, onboarding/resolution/supervision, encrypted desktop CAS, synthetic browser reader and primary bootstrap. Output: /tmp/chromeria-334-review-sol-focused.log.
  • Added disposable controls for provisional unknown-secret probing followed by exchange completion and for preservation of an older unresolved exchange across newer promotion; both pass. The two existing shared-client renewal tests also pass. This does not claim exhaustive execution of every accepted adversarial schedule.
  • Added the two discriminating checks above: both fail on the unchanged exact implementation, reproduced again after tightening the descriptor fixture. Command: ./node_modules/.bin/vp test run apps/server/src/auth/SessionRenewal.test.ts apps/web/src/authBootstrap.test.ts -t 'review:'. Output: /tmp/chromeria-334-review-sol-blockers.log. The disposable tests remain in the detached review worktree for inspection.
  • Inspected the supplied 173-test, affected-package typecheck, original-watcher negative-control and real-socket logs as supplemental evidence. Full CI remains the complete gate.
  • Browser reader proof remains synthetic filesystem/fake engine/fake IndexedDB only. Real Chromium reopening/approval is pending by explicit user instruction; no browser/dev server, real profile read/copy, live DB write, deployment or restart occurred. The planner retains post-deploy cavallo desktop proof.
  • The PR advanced during review, most recently to 8548d28fb1995fa29cd1678890628704eb0a2e6a. I kept the requested SHA pinned and have not transferred this verdict to that head.

Nonblocking follow-ups

Filed after duplicate searches: #336, repeated verified approval returns an error instead of its completed decision, and #337, ahead client clock causes unpaced not-due renewal requests. These are source-established operational/performance concerns, with no measured traffic or live approval claim, and do not widen B1/B2. Existing #325, #328 and #330 remain separate.

Requirements and who asked: User requested independent exact-head implementation review of #334/#323, preserving accepted rotating authority, supported new pairing, trusted full-hash approval, explicit proof limits, and the named commit status. This supports continuous cross-machine control for the Promachos.
Deleted: Implementation authorship, deployment/restart, browser execution, real-profile access, repo-wide checks, version changes, planner messages, reopening accepted design choices, and blocking on the separately tracked CLI/clock concerns.
Bottleneck: Two observed implementation transitions contradict accepted enrollment and guarded-rotation behavior; the finder must recheck their corrections on the next exact candidate.
Checked myself: Full exact diff and named design records, actual callers and SQL guards, 152 passing focused tests, passing provisional controls, two failing deterministic reproductions, supplied raw proof logs, current PR identity and independently verified follow-up Issues.

Verdict: CHANGES REQUIRED on f9c13fda4e8527d308d5526d28df5d77a62a189f. Reviewer: GPT-6.1 Sol (gpt-6.1-sol), high effort, Codex harness in T3 Code.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Sol B1 and B2 are fixed on 6e99d976cd050808a35ccfbebeebeba1f9ee631c.
Why: New browser pairing must stage renewal authority before consuming its code, and a renewal credential must describe an auth row the guarded transaction actually updated.
So what: Sol alone rechecks its two findings on this exact head. Both named independent statuses and full CI must still pass on the final head before merge. No browser execution or deployment is included.

Response to #334 (comment) :

  • B1: descriptor failure now propagates before stage or browser-session exchange. The desktop-bootstrap exclusion remains. A regression test makes descriptor lookup fail, proves no pairing request was sent, then retries the same code and verifies the request includes the independently known commitment. Existing request assertions now protect the requested credential while allowing the new optional commitment. The fixture models the undeclared descriptor server failure through the HTTP boundary instead of inventing a declared error response.
  • B2: the guarded auth-row update uses RETURNING; zero rows fails inside the SQL transaction, rolling back rotation and spent history. Success rereads the persisted auth row and signs that result. The boundary-clock test crosses expiry between validation and update, requires rejection with unchanged expiry, then proves the old secret can still perform an ordinary due rotation with a different successor. This distinguishes complete rollback from merely suppressing the response.

Both new tests fail against the previous implementation for the intended reasons, then pass with the fixes. Controlled old-code output: /tmp/chromeria-323-sol-blocker-control-final.log. Focused suite: 234 tests passed across 14 files, including the connection supervisor; /tmp/chromeria-323-sol-final-tests.log. After correcting the descriptor fixture's type contract, its 31 bootstrap cases and web typecheck passed again. Server typechecking, targeted lint and format also passed for the fixes. Full exact-head CI remains required.

Later changes since Sol's original review also include the recorded fork user-guide ownership entry, Hermes-compatible array reversal and shared test-fixture renewal runner. The accepted core/C design is closed, including Grok's final browser-path clearance at #323 (comment) . Browser proof remains fake storage/engine only by explicit user instruction. #336 and #337 are linked nonblocking follow-ups, alongside #325, #328 and #330.

Requirements and who asked: User's renewal and exact-head dual-review gate; Sol's independently reproduced B1/B2; explicit no-browser execution and planner-owned deployment/live cavallo proof.
Deleted: Descriptor catch-to-success, hypothetical-row minting, and reuse of an earlier SHA's review status.
Bottleneck: Finder recheck on the current candidate, independent Grok implementation verdict and full CI.
Checked myself: Both red controls, passing focused proof, SQL transaction rollback and persisted-row return, actual browser exchange order, changed fixture types, and clean committed/pushed Git state.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Finder recheck clears B1 and B2 on 6e99d976cd050808a35ccfbebeebeba1f9ee631c; no blocking finding remains in this recheck.
Why: Descriptor failure now prevents consuming a new browser pairing code, and successful due rotation requires an actual guarded auth-row update with transactional rollback on rejection.
So what: review/independent-sol is success for this exact head. Grok's independent implementation review and full CI retain their own gates; deployment and post-deploy cavallo proof remain planner-owned.

Round 2, high effort, finder-only recheck of my original B1/B2 findings. Read that comment and the parent's complete response. Base remains fbbe448316be1af24aeefb3ed124b152e8817253. I inspected the full eight-file delta from the originally reviewed f9c13fda4e8527d308d5526d28df5d77a62a189f to this head and the current B1/B2 callers, persistence and issuance paths, reusing the prior review's unchanged coverage. This is not a new independent review of the accepted design or Grok's separate implementation round. I did not author the implementation.

B1: cleared

Primary browser exchange now propagates descriptor failure before staging or calling the consuming browser-session endpoint. The explicit desktop-bootstrap exclusion remains. The regression exercises the actual primary auth function through the in-process HTTP fixture: failed descriptor lookup rejects with zero exchanges; retrying the same pairing code sends the independently known renewal commitment. Existing desktop bootstrap and other primary bootstrap cases also pass.

B2: cleared

Due rotation now checks UPDATE ... RETURNING and yields RenewalRejected inside sql.withTransaction when no auth row was updated. Rotation/history writes therefore roll back. Success reads the persisted auth row, which SessionStore.renewSession uses for credential claims instead of a fabricated extended row.

The deterministic clock-boundary regression rejects the crossed-expiry request, retains the original persisted expiry, then successfully rotates with the old secret and a different successor at ordinary due time, with generation 1. That final successful retry discriminates rollback from merely suppressing the credential response. Ordinary renewal, revocation, replay and real WebSocket lifetime cases in the same focused server file remain passing.

Independent proof

All execution used detached /tmp/chromeria-334-review-sol-r2-6e99d97, pinned to the exact head, with review-local caches and shared installed dependencies. Author workspace and main files were not modified. No tests or implementation changes were authored.

  1. Exact-candidate focused run:
    ./node_modules/.bin/vp test run apps/server/src/auth/SessionRenewal.test.ts apps/web/src/authBootstrap.test.ts packages/client-runtime/src/authorization/renewal.test.ts
    Test Files  3 passed (3)
    Tests       39 passed (39)
    
    Log: /tmp/chromeria-334-review-sol-r2-tests.log.
  2. Independent negative control: temporarily substituted only the two implementation files from pre-fix 8548d28fb1995fa29cd1678890628704eb0a2e6a into my review worktree, leaving current tests/fixtures intact. Both regressions failed for the original reasons: B1 resolved instead of rejecting; B2 returned Success instead of Failure. Log: /tmp/chromeria-334-review-sol-r2-control.log.
  3. Restored the exact candidate bytes and reran both regression cases: 2 passed, 35 skipped. Log: /tmp/chromeria-334-review-sol-r2-regressions.log. Review worktree and author worktree were clean; local and live PR heads were rechecked as the requested SHA.

The remaining delta records guide ownership, replaces toReversed() with reversal of a fresh copy, and moves the bootstrap renewal fake into the existing fixture runtime, including its corrected descriptor error typing. No additional actionable regression was established in that delta. Inspected the supplied 234-test/14-file output and the later 31-case bootstrap and web typecheck logs as supplemental evidence. The earlier combined typecheck log contains the subsequently corrected web fixture error; I do not report that combined run as all green or claim a fresh independent typecheck run.

Limits and retained follow-ups

The user-defined blocking gate remains revoked/stolen continued renewal or broken supported desktop/web/mobile pairing. #336 and #337 remain nonblocking follow-ups, alongside #325, #328 and #330. Accepted core/C design stays closed.

Browser execution remains explicitly declined. Synthetic storage/fake-engine evidence is retained; real-browser reopen/approval remains pending as already recorded on #323 and this PR. No browser, dev server, real-profile access/copy, live DB write, main-checkout write, deployment, restart, version bump, memory write or planner-thread message occurred. Focused checks do not replace full CI or the planner's post-deploy cavallo desktop proof.

Requirements and who asked: User requested an exact-head, high-effort finder-only recheck of B1/B2, findings first and then the named commit status, while preserving accepted design and proof limits. Keeping cross-machine sessions working supports the current Objective.
Deleted: Implementation authorship, a fresh broad review, accepted-design reconsideration, browser execution, real-profile access, deployment, repo-wide checks and treating retained follow-ups as blockers.
Bottleneck: B1/B2 no longer block this candidate; the separate Grok verdict and full CI remain independently owned.
Checked myself: Complete eight-file review delta, current exchange order and desktop exclusion, guarded SQL transaction/persisted-row issuance, 39 passing focused tests, two independently failing old-source controls, two restored-head passing regressions, supplied raw proof logs and exact clean/local/live candidate identity.

Verdict: PASS for the B1/B2 finder recheck on 6e99d976cd050808a35ccfbebeebeba1f9ee631c. Reviewer: GPT-6.1 Sol (gpt-6.1-sol), high effort, Codex harness in T3 Code.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent implementation review of f9c13fda4e8527d308d5526d28df5d77a62a189f finds two required fixes.
Why: A failed browser descriptor request lets a new pairing code be consumed with no renewal authority, and a zero-row session-expiry update still commits a rotation and a credential.
So what: Fix both on the next candidate. This verdict is only for f9c13fda. Pushes after it, through 6e99d976cd at the time of writing, were not reviewed and do not inherit this status.

Reviewed fbbe448316be1af24aeefb3ed124b152e8817253...f9c13fda4e8527d308d5526d28df5d77a62a189f and the callers that issue, verify, bootstrap, supervise, and store renewal. Read #323 and the accepted r4 core, provisional corrections, C approval, and browser-reader placement notes. I did not author this implementation. No checkout, reset, or write in the author worktree or main.

B1 [required]: a descriptor failure consumes a new browser pairing without enrollment

Location: apps/web/src/environments/primary/auth.ts in exchangeBootstrapCredential at the review SHA.

Trigger: the user submits a valid new browser pairing code, and the preliminary metadata.descriptor() call fails (transport, proxy, or 5xx). .catch(() => null) turns every failure into "no descriptor". stage never runs. The browser-session request sends only { credential }.

EnvironmentAuth.createBrowserSession then consumes that one-time code and calls sessions.issue without renewalCommitment, because the commitment is attached only for a one-time-token grant. The response has no renewAfter, so complete does not run. The UI still treats the exchange as success.

The code cannot be presented again. This new pairing is no longer the automatic, code-authorized enrollment. A later legacy enrollExisting can only create a pending commitment, and Firefox, Safari, and mobile browsers have no supported headless read to approve it. Desktop bootstrap stays excluded by the getDesktopBootstrapCredential() check, which is correct and should stay.

Required correction: stage the commitment before the code is consumed, or fail that attempt before consume so the user can retry. Do not report a paired session that has no renewal row. Add a focused test where descriptor fails and the consuming request must not succeed unenrolled.

B2 [required]: a zero-row expiry update still reports a successful rotation

Location: apps/server/src/auth/SessionRenewal.ts renew, the due-rotation updates at the review SHA.

Trigger: active() reads the clock while expires_at is still in the future. The next DateTime.now (the value used for the rotation) is past expires_at. The renewal UPDATE still runs. The following

UPDATE auth_sessions SET issued_at = ..., expires_at = ...
WHERE session_id = ? AND revoked_at IS NULL AND expires_at > ?

changes zero rows. The result is ignored. renew returns kind: "success" with an in-memory { ...row, issuedAt: now, expiresAt }. SessionStore.renewSession signs that object.

verify accepts the new token: it checks the token exp and revoked_at, not the session row's expires_at. The database row stays expired, so awaitInvalidation still ends the socket, and the next renew fails active(). The client can store the successor and then delete it on that 401. The supplied rotation test asserts the in-memory expiresAt, so it does not catch this.

Required correction: succeed only when the active unexpired row was actually updated. On zero rows, roll back the renewal and spent writes and return RenewalRejected. Mint from the persisted row. Add a clock-boundary test that the database expiry does not move and no token is issued, and keep the ordinary due-rotation and exact-replay tests.

Checked and not blocking on this SHA

  • Exact historical (previous, previous_next) returns stale and does not revoke. A different successor revokes. Unknown is rejected. The theft test on this SHA expects that.
  • A current reply does not clear pending. Rotation commits only when outcome === "rotated", generation === local + 1, and the pending hash is the one that was sent.
  • A 401 deletes the active slot only when that same epoch is still active. A provisional probe's 401 does not remove it. complete removes only the promoted epoch, not older unresolved provisionals.
  • t3 auth renewal approve requires the full --commitment. list prints a 16-character prefix, so the server list is not a full hash to approve. The browser reader copies into <tmp>/Default, launches Chromium with that user-data directory, and uses port 0 when the URL has no port.
  • DPoP, reusable dev, desktop bootstrap, mcp-client, and managed-relay bootstrap do not receive this renewal row. Mobile uses layerMobile, so legacy enrollment stays off.
  • awaitInvalidation re-reads expires_at and races the old deadline against the session pubsub. That rearms only when the row update in B2 actually persists.

Proof and limits

  • Supplied /tmp/chromeria-323-final-tests.log reports 13 files and 173 tests passed. It has no per-test names and does not cover B1 or B2. I did not re-run it.
  • /tmp/chromeria-323-real-socket2.log reports 1 file and 5 tests passed.
  • /tmp/chromeria-323-old-deadline-control.log is the old watcher failing at minute 11 (Deferred.isDone became true). That is the negative control, not a failure of this candidate.
  • /tmp/chromeria-323-final-tc2.log shows typecheck suggestions and no error TS line. I did not see a separate success banner.
  • No browser, dev server, real profile read, live database write, deploy, or repo-wide check. Browser-reader proof on this SHA is synthetic. Post-deploy cavallo desktop proof stays with the planner.
  • Full CI remains the complete gate.

Nonblocking

#336 and #337 already cover approval retry idempotence and an ahead client clock. I am not opening duplicates. A decoded pairing 4xx still leaves that provisional epoch stored, because only promotion removes an epoch. It does not revoke another pairing. I am not blocking on it.

Requirements and who asked: User, this review: exact-head implementation review of #334 for #323, against the accepted renewal contract, with review/independent-grok on f9c13fda. This supports the Promachos staying connected across the user's machines.
Deleted: Implementation edits, version bump, deploy, browser execution, real-profile access, repo-wide checks, and transferring this verdict onto later commits.
Bottleneck: Two implementation transitions on this SHA contradict code-authorized enrollment and the active unexpired mint.
Checked myself: The exact diff, the named design comments, issuance and verify, primary bootstrap, the renewal transaction, and the supplied proof logs.

Verdict: CHANGES REQUIRED on f9c13fda4e8527d308d5526d28df5d77a62a189f. Reviewer: Grok 4.7, high effort, T3 Code. I did not author this code.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 4.9 KiB 4.9 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.8 KiB 20.8 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 01d7066 · PR result: cea2b9d · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Head 033bb2f6fa6e7b46b794e10fcf720940e53b1773 fixes client runtime construction when no primary environment exists.
Why: Sharing the primary HTTP runtime made general client startup eagerly resolve a primary target. CI caught window is not defined; a focused desktop fixture also reproduced PrimaryEnvironmentDisabledError with the local environment disabled.
So what: The shared client runtime now owns the one renewal service, used by startup and live connections. Primary HTTP initialization stays independent. Both named reviews must pass on this new head, followed by green CI, before delegated merge.

CI evidence: https://github.com/toolboxmd/chromeria/actions/runs/38647713916/job/116138894763 . The new remote-only desktop runtime test and the existing tracing test both fail with the previous runtime file, then pass with the fix. The setup still uses the two pre-existing application runtimes; it adds no feature-specific runtime. No browser or desktop application was launched.

Focused result: 236 tests passed across 15 files, /tmp/chromeria-323-final-runtime-tests.log. The prior runtime control fails both cases for the expected missing-primary dependency, /tmp/chromeria-323-runtime-control.log. Web typecheck, targeted lint/format and fork-feature ownership validation pass.

Grok's implementation review on old f9c13fda found the same B1/B2 as Sol, with no additional blocking finding: #334 (comment) . Both fixes and their red/green proof are recorded at #334 (comment) . Sol independently cleared B1/B2 on 6e99d976cd: #334 (comment) . B1/B2 code is unchanged in this new runtime commit. Grok rechecks its findings plus the intervening delta; Sol rechecks only the runtime/ownership delta while retaining its unchanged prior coverage. Neither earlier status will authorize merge of this head.

Browser-reader proof remains synthetic, and real-browser execution remains explicitly declined. The planner owns deployment, trusted cavallo approvals and post-deploy live desktop proof. Follow-ups #325, #328, #330, #336 and #337 remain nonblocking.

Requirements and who asked: User requires supported desktop/web/mobile pairing and exact-head dual-review/green-CI before merge. CI and a local reproduction identified the unsupported dependency on a local primary environment.
Deleted: Eager primary HTTP initialization from general client runtime, a second renewal owner, and earlier-head status reuse.
Bottleneck: Exact-head final delta reviews and CI.
Checked myself: Actual CI failure, the disabled-primary regression's old-source failure, restored tracing behavior, 236 passing focused tests, successful web typecheck/lint and current clean committed/pushed state.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 3 passes on 033bb2f6fa6e7b46b794e10fcf720940e53b1773; no blocking finding remains in the reviewed runtime/ownership delta.
Why: General client runtime construction no longer resolves a primary environment, and startup and live connections still share one renewal service.
So what: Publish review/independent-sol success on this exact head. Grok's named review and full CI retain their own gates; deployment and post-deploy cavallo desktop proof remain planner-owned.

High-effort independent review of exactly the four files in 6e99d976cd050808a35ccfbebeebeba1f9ee631c..033bb2f6fa6e7b46b794e10fcf720940e53b1773, with original base fbbe448316be1af24aeefb3ed124b152e8817253. I did not author this implementation. Read the original full Sol review, B1/B2 response, round-2 clearance, Grok's same-findings review, and the latest parent response.

Unchanged prior review coverage is retained. B1/B2 remain cleared from round 2; their code is outside this four-file delta and I did not rerun their review or tests. Accepted core/C design stays closed. This is a fresh exact-head verdict after inspecting the new delta, not transfer of the earlier commit status.

Inspected the complete delta and relevant runtime consumers:

  • runtime.ts: layerRenewal is constructed once in the existing general runtime. runRenewal executes there; Runtime.layer exposes that same managed runtime context to the connection runtime. Startup and connection authorization therefore use the same service owner and scope. The existing primary HTTP runtime contains only primary HTTP services and is reached by runPrimaryHttp; its context is no longer included in general runtime construction. No additional managed runtime or wire/storage contract was introduced.
  • Checked renewal/storage, HTTP/crypto and managed-relay layer construction: constructing the general context does not resolve a primary target or eagerly perform renewal storage/network operations. The disabled-local desktop case can obtain renewal, and ordinary tracing can initialize without window. Primary HTTP authentication retains its dedicated layer, including browser cookies and desktop bearer behavior.
  • The two ownership-file edits register the existing runtime test under the renewal feature and its upstream edit allowlist. node scripts/fork-features.mjs check passes: 37 features. The four-file diff also passes git diff --check.

Independent execution used detached /tmp/chromeria-334-review-sol-r3-033bb2f, pinned to the exact head, with review-local dependency links/caches and shared installed dependency contents. No implementation or test changes were authored; the old-source substitution below occurred only in this isolated review worktree.

  1. ./node_modules/.bin/vp test run apps/web/src/lib/runtime.test.ts apps/web/src/environments/primary/httpLayer.test.ts: 4 tests passed across 2 files. This covers disabled-local renewal construction, tracing delegation, browser primary cookie behavior and desktop primary bearer behavior. Log: /tmp/chromeria-334-review-sol-r3-tests.log.
  2. Replaced only runtime.ts with its exact 6e99d976cd050808a35ccfbebeebeba1f9ee631c bytes and ran the current runtime tests: both fail, exit 1, with PrimaryEnvironmentDisabledError: The local environment is disabled. Log: /tmp/chromeria-334-review-sol-r3-control.log.
  3. Restored exact-head bytes and reran the runtime file: 2 passed. Log: /tmp/chromeria-334-review-sol-r3-restored.log. A separate fresh process running only the tracing case, without executing the window-stubbing test, also passes: 1 passed, 1 skipped, /tmp/chromeria-334-review-sol-r3-no-window.log.

Inspected the supplied 236-test/15-file focused log, old-runtime control, web typecheck output and empty lint log as supplemental evidence. The fresh execution counts above are my checks; I do not claim an independent complete-suite or typecheck run. Review worktree and author worktree are clean, candidate bytes are restored, and local/live PR heads were rechecked as the requested SHA.

No additional actionable defect was established in this delta. The blocking criterion remains revoked/stolen continued renewal or broken supported desktop/web/mobile pairing. Existing nonblocking follow-ups #325, #328, #330, #336 and #337 remain retained. Mobile and renewal protocol behavior are unchanged by this delta and retain prior reviewed coverage.

Browser execution remains explicitly declined. Synthetic storage/fake-engine proof remains retained, and real-browser reopen/approval remains pending as already recorded on #323 and this PR. No browser, dev server, real-profile read/copy, live DB write, main/author-checkout write, deployment, restart, version bump, memory write or planner-thread message occurred. Focused checks do not replace full CI or post-deploy live desktop proof.

Requirements and who asked: User requested high-effort exact-head review of only this four-file runtime/ownership delta, retaining prior clearance, supported pairing, no-browser limits and findings-first named status delivery; continuous cross-machine sessions support the Objective.
Deleted: Reopening accepted design, repeating unchanged B1/B2 review, implementation authorship, browser execution, deployment, repo-wide checks and reuse of an old status as approval of this head.
Bottleneck: No Sol blocker remains; exact-head Grok review and full CI retain their independent gates.
Checked myself: Complete four-file delta and relevant consumers, one renewal owner/shared runtime context, primary HTTP separation, 4 passing focused cases, 2 failing old-runtime controls, 2 restored passing runtime cases, standalone no-window tracing pass, ownership validation, supplied raw logs and clean/local/live exact-head identity.

Verdict: PASS on 033bb2f6fa6e7b46b794e10fcf720940e53b1773. Reviewer: GPT-6.1 Sol (gpt-6.1-sol), high effort, Codex harness in T3 Code.

@lukemaj
lukemaj force-pushed the feat/323-paired-session-renewal branch from 033bb2f to cea2b9d Compare October 14, 2026 22:45
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Rebased PR #334 onto current main 01d7066bb72e195af8c51a6e38aea3aada805d2d; exact head is now cea2b9dbd70df8967fa2b51765ba00df07ab5130.
Why: Main advanced through #327 and conflicted in the fork ownership allowlist, which prevented pull-request CI from starting.
So what: The PR is mergeable again and CI has started. Sol reviews rebase compatibility on this head; Grok finishes its existing review before a fresh exact-head rebase review. Both named successes and green CI remain required before delegated merge.

git range-diff fbbe448316..033bb2f6fa origin/main..cea2b9dbd7 shows all seven implementation commits unchanged except the allowlist conflict resolution, which preserves both Great Library native-session and paired-renewal ownership sections. There was no auth, client, contract, test or runtime conflict. Fork feature validation passes with 37 features; git diff --check passes. The 236-test/15-file focused suite is rerunning at this head. Prior Sol runtime clearance: #334 (comment) . CI: https://github.com/toolboxmd/chromeria/actions/runs/38649791380 .

No version bump, browser execution, deployment, restart, live database write or main checkout modification. Real-browser verification remains pending; planner-owned deployment, one-time CLI approval and live cavallo desktop proof remain separate.

Requirements and who asked: User requested #323 merged only after exact-head Sol/Grok success and green CI, preserving supported pairing and the stolen/revoked-session constraint.
Deleted: Reuse of statuses on an earlier head and an unresolved conflict that suppressed CI.
Bottleneck: Exact-head review and full CI after the minimal rebase.
Checked myself: Current main/head identities, complete range-diff, sole allowlist conflict preserving both owners, clean worktree, live mergeability, new CI run and feature validation.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 4 passes on cea2b9dbd70df8967fa2b51765ba00df07ab5130; no blocking rebase compatibility finding remains.
Why: The rebase preserves the cleared renewal implementation and combines Great Library and renewal ownership without losing either feature.
So what: Post review/independent-sol success on this exact head; the separate Grok review and full CI retain their gates, and deployment and post-deploy cavallo desktop proof remain planner-owned.

Independent implementation review at high effort, scoped to the superseding round-4 packet: rebase of cleared 033bb2f6fa6e7b46b794e10fcf720940e53b1773 onto 01d7066bb72e195af8c51a6e38aea3aada805d2d. Exact candidate: cea2b9dbd70df8967fa2b51765ba00df07ab5130. I did not author this code. This is a fresh verdict on the rebase, retaining unchanged prior review coverage rather than reopening the accepted design or repeating cleared implementation work.

Retained records: original Sol findings, B1/B2 response, round-2 clearance, runtime response, and round-3 PASS. Also read #323's acceptance and the accepted browser-path clarification/final Grok clearance. B1, B2 and the runtime construction correction remain cleared; no renewal code changed during this rebase.

Rebase compatibility findings

No supported actionable blocker was established under the user-defined gate of revoked/stolen continued renewal or broken supported client pairing.

  • Independently ran git range-diff fbbe448316..033bb2f6fa 01d7066bb7..cea2b9dbd7. All seven commits correspond: six patches are unchanged; the first differs only in the insertion context for scripts/fork-upstream-edits.txt, after the new base's Great Library section.
  • Compared exact Git blobs for all 44 files in the previous renewal change: 42 are byte-identical at the new head, including every implementation and test file. Only docs/fork-features.md and scripts/fork-upstream-edits.txt differ from the previous head. Those two ownership files are also the entire path overlap between the renewal work and the new base. No new renewal paths appeared.
  • Inspected the ownership diffs completely. Independently parsed the feature records and verified that the renewal record is unchanged, every new-base feature record is retained exactly, and the final allowlist is precisely the union of previous-head and new-base paths. Both Great Library and paired renewal entries survive.
  • Inspected the new base's provider-session boundary changes and relevant server layer consumers. They add ContextMD instruction accounting in orchestration/provider paths; the auth, HTTP, renewal storage, web/desktop/mobile runtime and wire contracts retain their cleared bytes. The existing auth layer still supplies SessionStore and SessionRenewal; the new ContextMD dependency is supplied to the orchestration runtime. No package manifest, lockfile or configuration changed in the base delta.

Proof checked and limits

Fresh read-only checks on the exact candidate:

node scripts/fork-features.mjs check
fork-features: OK (37 features).

Ownership reconciliation: renewal entry unchanged; every new-base feature retained exactly;
allowlist equals previous-head/new-base union.

git diff --check 01d7066bb72e195af8c51a6e38aea3aada805d2d...cea2b9dbd70df8967fa2b51765ba00df07ab5130
exit 0

Read the supplied /tmp/chromeria-323-rebase-tests.log directly: its completed Vitest run reports 236 tests passed across 15 files, with no failed-test summary. Installation emits bin-link warnings before the successful test run. This is inspected parent execution, not an independent test rerun; the log does not enumerate individual cases or embed a commit SHA. Also inspected the earlier focused-test, typecheck, real-socket and old-watcher negative-control outputs as retained supplemental evidence. The old watcher correctly fails at the former deadline. Prior independent red/green proof remains recorded in the linked round-2 and round-3 reviews; no new behavioral execution was necessary for unchanged code in this scoped rebase review. Full CI remains the complete gate.

The author worktree is clean, and both local HEAD and live PR head/base match the requested full SHAs. No main/author-worktree write, browser, dev server, real-profile access/copy, live DB write, restart, deployment, version bump, memory write or planner-thread message occurred. Browser-reader proof remains synthetic; real Chromium reopen/approval stays pending by explicit user instruction, as already recorded on #323 and this PR. Post-deploy live cavallo desktop proof remains pending. Existing nonblocking follow-ups #325, #328, #330, #336 and #337 stay retained; no additional defect or duplicate follow-up was established.

Requirements and who asked: User requested exact-head independent rebase compatibility review only, retaining prior clearance, supported pairing/security gates, no-browser limits, and findings-first named status delivery. Continuous cross-machine control supports the Objective.
Deleted: Repeating unchanged cleared work, reopening accepted core/C design, implementation edits, browser execution, deployment, repo-wide checks and transferring an old status to the new SHA.
Bottleneck: No Sol blocker remains in the rebase; separate exact-head Grok review and full CI retain their gates.
Checked myself: Seven-commit range-diff, all 44 prior changed-file blob identities, new-base overlap and integration consumers, complete ownership diffs and independent union/feature-record assertions, 37-feature validation, diff check, supplied raw proof logs, clean worktree and exact local/live candidate identity.

Verdict: PASS on cea2b9dbd70df8967fa2b51765ba00df07ab5130. Reviewer: GPT-6.1 Sol (gpt-6.1-sol), high effort, Codex harness in T3 Code.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent recheck of cea2b9dbd70df8967fa2b51765ba00df07ab5130 finds no blocking defect.
Why: Descriptor failure still stops before a new browser pairing is consumed, a missed expiry update still rolls the rotation back, and general client startup no longer builds a primary environment.
So what: review/independent-grok is success for this exact head. Full CI remains the complete gate. Deployment and post-deploy cavallo desktop proof stay with the planner.

The requested candidate 033bb2f6fa6e7b46b794e10fcf720940e53b1773 was rebased onto current main while this review was in progress. The live PR head is cea2b9dbd70df8967fa2b51765ba00df07ab5130. Range-diff shows the Hermes, pairing-rejection, descriptor-fixture, and runtime commits are the same patches. The implementation blobs for apps/web/src/lib/runtime.ts, apps/server/src/auth/SessionRenewal.ts, apps/web/src/environments/primary/auth.ts, and packages/client-runtime/src/authorization/renewal.ts are identical on both SHAs. The ownership files differ only by the upstream contextmd entries that came in with the new base. This verdict is for cea2b9db and does not reuse the status on 033bb2f or 6e99d976.

Base of this head is 01d7066bb7. I did not author the implementation. Author worktree and main were not modified. Checks ran in detached /tmp/chromeria-334-review-grok-r2.

B1: cleared

exchangeBootstrapCredential still awaits the descriptor request and stages only after that request returns. A failure rejects the attempt before stage and before the browser-session exchange. Desktop bootstrap still skips the descriptor. The current regression rejects when the descriptor dies, records no browser-session call, then retries the same code with the known commitment.

B2: cleared

The due rotation still runs inside sql.withTransaction. The guarded session update requires one RETURNING row, then rereads that persisted row for the credential. Zero rows fails with RenewalRejected, so the renewal and spent writes roll back. The boundary test rejects, leaves the original expiry in place, and then rotates the old secret to a different successor at generation 1.

Intervening delta: cleared

  • Provisional renewal order uses a reversed copy, [...(record?.provisional ?? [])].reverse(), so Hermes does not need Array#toReversed and the stored array is not reversed in place.
  • Bootstrap tests drive renewal through the existing HTTP fixture runtime. The fixture supplies one mock SessionRenewal to both test runners. Production keeps runRenewal on the general runtime and runPrimaryHttp on the primary runtime.
  • docs/fork-features.md and scripts/fork-upstream-edits.txt list apps/web/src/lib/runtime.test.ts and docs/user/remote-access.md under paired renewal. node scripts/fork-features.mjs check reports 37 features on this head.
  • layerRenewal is built once inside the existing general runtime. runRenewal and Runtime.layer both use that runtime, so startup and connection authorization share one service. primaryHttpRuntime builds only the primary HTTP layer, and only when runPrimaryHttp is called. Mobile keeps its own layerMobile runtime and is outside this delta.

Proof

On cea2b9dbd70df8967fa2b51765ba00df07ab5130:

./node_modules/.bin/vp test run apps/server/src/auth/SessionRenewal.test.ts apps/web/src/authBootstrap.test.ts apps/web/src/lib/runtime.test.ts apps/web/src/environments/primary/httpLayer.test.ts packages/client-runtime/src/authorization/renewal.test.ts
Test Files  5 passed (5)
Tests       43 passed (43)

Log: /tmp/chromeria-334-review-grok-r2-tests.log.

Negative controls, current tests against the previous implementation bytes, then the candidate restored:

  • Runtime file from a79189d867: both runtime tests fail with PrimaryEnvironmentDisabledError. Log: /tmp/chromeria-334-review-grok-r2-runtime-control.log.
  • Tracing test alone on that old runtime: ReferenceError: window is not defined from readPrimaryEnvironmentTarget. Log: /tmp/chromeria-334-review-grok-r2-tracing-old.log.
  • Tracing test alone on this head: 1 passed, 1 skipped. Log: /tmp/chromeria-334-review-grok-r2-tracing-fixed.log.
  • SessionRenewal.ts and primary auth.ts from 539e06978b: the descriptor test resolves instead of rejecting, and the expiry-boundary test returns Success. Log: /tmp/chromeria-334-review-grok-r2-b1b2-control.log.

The review worktree was clean after the candidate bytes were restored. Author worktree status was empty and matched the live PR head.

Limits

No new follow-up. #325, #328, #330, #336, and #337 stay nonblocking. Accepted core and C design stays closed. Browser execution remains declined. Real-browser proof stays pending on #323 and this PR. No browser, dev server, real profile read, live database write, deploy, or repo-wide check. Focused tests do not replace CI or the planner's post-deploy cavallo desktop proof.

Requirements and who asked: User asked for this exact-head recheck of B1/B2, the Hermes copy reversal, the shared test renewal runner, fork ownership, and the runtime split. A finding blocks only continued renewal of a revoked or stolen session, or a break in desktop, web, or mobile pairing. Keeping paired sessions alive across machines supports the Objective.
Deleted: Implementation edits, reopening the accepted design, browser execution, deployment, repo-wide checks, and treating the 033bb2f or 6e99d976 status as approval of this head.
Bottleneck: The candidate SHA changed during review. The remaining question was whether the rebase and the runtime split kept the B1/B2 fixes and supported pairing.
Checked myself: Identical implementation blobs, current exchange order and desktop exclusion, transactional zero-row rejection, one shared renewal service, 43 passing focused tests, both old runtime failures, the isolated window is not defined failure, both old B1/B2 failures, the 37-feature ownership check, and the live PR head.

Verdict: PASS on cea2b9dbd70df8967fa2b51765ba00df07ab5130. Reviewer: Grok 4.7, high effort, T3 Code. I did not author this code.

lukemaj added a commit that referenced this pull request Oct 29, 2026
* feat(auth): renew active paired sessions with device authority

* docs: link paired renewal delivery in feature maps

* docs: assign renewal guide to the fork feature

* fix(auth): keep renewal compatible with Hermes and test layers

* fix(auth): reject unenrolled pairing and expired renewal races

* test(auth): model undeclared descriptor failures

* fix(web): initialize renewal without a local environment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant