Skip to content

refactor: unify session catalog activity authority - #3619

Merged
Astro-Han merged 17 commits into
apache:mainfrom
Astro-Han:refactor/session-catalog-single-authority
Aug 23, 2026
Merged

refactor: unify session catalog activity authority#3619
Astro-Han merged 17 commits into
apache:mainfrom
Astro-Han:refactor/session-catalog-single-authority

Conversation

@Astro-Han

Copy link
Copy Markdown
Contributor

Summary

Make session_catalog_projection.activity_at the single Session catalog recency authority across Runtime Host, CLI/TUI, and Desktop.

  • publish one canonical activityAt projection and reuse its mapper across clients
  • preserve complete authoritative catalog snapshots in Desktop instead of locally merging, upserting, or patching Session rows
  • remove the redundant Session lastUsedAt model/protocol field and drop its legacy SQLite column through schema migration 29
  • retain only view-level transformations such as pinned grouping, current-directory partitioning, revision projection, and cross-Host composition using the canonical timestamp

The Runtime Host compatibility epoch is 43 because strict Session continuity and inspection projections changed. Schema migration 29 temporarily restores the legacy column only for synthetic or interrupted pre-29 schemas that lack it, lets historical migrations complete, then removes it.

Verification

  • npm run build --workspace @maka/core
  • npm run build --workspace @maka/storage
  • npm run build --workspace @maka/runtime
  • npm run build --workspace @maka/runtime-host
  • npm run build --prefix packages/cli
  • npm run build --workspace @maka/ui
  • npm run typecheck --workspace @maka/desktop
  • focused Storage tests: 69 passed
  • focused Runtime Host continuity and CLI Session driver tests: 95 passed
  • focused Desktop catalog, settings, revision, and UI tests: 36 passed
  • post-rebase Runtime Host protocol tests: 43 passed
  • Biome passed for all changed TypeScript, JavaScript, and JSON files
  • git diff --check origin/main...HEAD

The full repository test suite was not run locally; CI should provide repository-wide coverage.

Migration

Schema 29 removes session_metadata.last_used_at after preserving the canonical catalog projection. Historical migration references remain intentionally so databases at older schema versions can advance safely before the column is dropped.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Maka assisted with architecture analysis, implementation, tests, migration work, and PR drafting. The contributor must review the final diff, provenance, and commit messages before merge.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Carry the storage catalog's materialized activity timestamp through Runtime Host projections and cursors. This removes the protocol's dependence on the redundant last-used timestamp and gives all clients one ordering fact.

Generated-by: Maka
@Astro-Han
Astro-Han marked this pull request as ready for review August 23, 2026 13:54

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on — the authority convergence itself holds up under scrutiny, and I want to say that first because the two findings below are about collateral state, not about the central idea.

What I verified about the convergence. SessionHeader.lastUsedAt is gone from current production. Schema 29 treats the historical column purely as migration input and drops it at the end. session_catalog_projection.activity_at is the single ordering and cursor fact, Runtime Host is required to send activityAt, CLI and Desktop Main share one mapper, and a Desktop catalog snapshot missing activity fails closed. The same-named fields in historical migrations and in the project catalog are not a second authority for the current Session. I also went through both heavily net-deleted test files line by line: the read-boundary second projection is genuinely removable, since a Host read-marker commit flows back through refreshCanonical → session.catalog.changed → sessions:changed → renderer refresh; and the three live-run merge tests are removable because mutation upserts are gone and only a complete catalog reaches the list. All 37 non-trivial files were classified and the mechanical ones were checked hunk by hunk with nothing smuggled in.

[P1] Plan transient state was removed as if it were a second catalog authority

app-shell.tsx:1049-1056 now discards the successful mutation's projection and waits only for a full refresh, and :1076-1083 then decides settled state from the stale sessionsRef. The required run 32643871031 (job 97205001842) lands exactly here: 54 pass, 2 fail, 1 skip, and both failures are the Plan contracts, each waiting for aria-checked=true on the first click and receiving false for a continuous 10s.

The two broken contracts are rapid on→off latest-wins, and deletion while a toggle is pending.

I want to be explicit about the fix direction, because the obvious repair would undo the point of this PR: do not put the mutation summary back into the catalog list. The catalog must stay the single authority. What needs restoring is a separate transient pending intent/view — including its latest-wins resolution and its cleanup on deletion. That is a distinct concern from catalog authority, and deleting it was collateral damage rather than a consequence of convergence.

[P1] Promise.allSettledPromise.all lets one failing Host block every healthy one

At preload.ts:777-787, any rejecting sessions:list now discards the fulfilled snapshots from every other Host. The refresher keeps the previous full list, so while one Host keeps rejecting, updates from all healthy Hosts never reach the UI at all.

This contradicts a documented contract. docs/runtime-host-remote-access.md:151 states that Desktop keeps every Host connected independently and that "a failed remote connection remains visible without interrupting the other Hosts."

Removing the cross-Host stale cache was the right call, and I am not asking for it back. Settling this round's requests without a cache is enough: keep the Hosts that succeeded, and throw only when none succeeded. A two-ready/one-rejecting regression test would pin it.


ran: Core, Storage, Runtime Host and Desktop Main+Preload builds (green); focused Desktop 55/55 and Storage/Runtime Host 168/168; diff-check clean; independent verification of the hosted run and job outcome for this exact head.
did-not-run: the full Desktop E2E lane locally — the hosted required run above is the evidence for the Plan finding.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technical incremental review of exact head 44f3cfadb88edc9231094f061476199f1ca11235, acting on behalf of @WAWQAQ under the standing Kabi review/publisher authorization.

Conclusion: COMMENT — one P2 remains. No APPROVE and no merge.

Both earlier P1 mechanisms moved in the right direction:

  • Plan's committed value now lives only in a per-session transient presentation map. It is read by the active control and cleared by session renderer cleanup; it never inserts, upserts, removes, or reorders catalog rows. The old duplicate catalog-mutation authority is gone.
  • collectRuntimeHostSessionCatalogs() preserves every fulfilled Host group and throws only when a non-empty request set has no fulfilled group. Both new regressions are mutation-sensitive: adding an "any rejection throws" branch makes the healthy+reject case fail; deleting the all-reject guard makes the second case fail. The focused tests are green on the restored head.

[P2] A failed catalog refresh discards the committed Plan presentation and falls back to stale state

At apps/desktop/src/renderer/app-shell.tsx:1054-1063, the Host mutation can succeed and persist plan; the renderer then records that committed fact in transientPlanModeBySession and awaits refreshSessions(). But the shared refresher deliberately does not reject when the catalog read fails: apps/desktop/src/renderer/session-read-state.ts:46-60 reports the read error, keeps the old sessionsRef, and resolves normally. The existing renderer session read state test explicitly locks this behavior in as "keeps the current list when the latest list refresh fails."

Because app-shell.tsx:1074-1075 unconditionally removes the transient in finally, that failure path immediately makes the control fall back to the old catalog value. The Host is now in Plan while the UI shows Agent. applyPlanMode() also returns true, because the refresher swallowed the list failure; the generic refresh error does not reconcile the setting.

This is the same authority split the new transient was meant to bridge, now on the failed-refresh boundary rather than the ordinary delayed-refresh boundary. Keep the committed transient until a successful catalog snapshot for that session contains the same collaboration mode (or the session is deleted/cleaned up); add a regression where the Host write succeeds, the following list read fails and retains the stale catalog, and the Plan control must remain on the committed value. A later successful matching refresh should then retire the transient.

Validation: incremental surface is 3 commits / 4 Desktop files / +105/-15 over reviewed head 3dac6d9a4; Desktop 1260/1260 passed, both helper mutation runs failed at their intended assertions, restored focused tests 2/2 passed, and the worktree is clean. Three discussion endpoints were reconciled after provisional sealing: one old-head COMMENT, no inline comments, no issue comments. Fresh state is OPEN, non-draft, MERGEABLE/BLOCKED solely by review; exact-head hosted test is completed/success. No GitHub action beyond this COMMENT was taken.

Keep the latest Plan and orchestration intent in one setting owner until the Host commit is observed in a catalog snapshot. Catalog refresh failures no longer roll presentation back or prevent a queued latest intent from reaching the Host.

Generated-by: Maka

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed on behalf of WAWQAQ under the standing Kabi review authorization, scoped to exact head 77c594b10eb7baf65ba534cd36d20138a56f41ed.

NO-GO: 1×P2. The prior failed-refresh finding is closed: a successful Host write now retains its presentation overlay when catalog refresh rejects, and the latest-intent worker continues. The new inline P2 is a different convergence boundary: a newer Host-owned value can never retire an older committed overlay.

Independent machine blockers also remain on this exact head: GitHub reports zero check-runs, and the PR is DIRTY / not mergeable against current main. A fresh merge-tree reports conflicts in apps/desktop/src/renderer/app-shell.tsx, packages/cli/src/runtime-host-session-driver.ts, packages/cli/src/__tests__/runtime-host-session-driver.test.ts, and packages/runtime-host/src/protocol/index.ts. Local full build and both new focused E2E cases passed, but absent hosted checks are not green. No approval or merge performed.

Comment thread apps/desktop/src/renderer/use-session-setting-intent.ts Outdated
Fence committed mode overlays by successful catalog observation revision instead of value equality. Runtime-owned transitions such as Plan approval can now supersede an older renderer commit without letting failed reads discard it.

Generated-by: Maka
Resolve the Desktop shell, shared Session catalog mapper, CLI mapper tests, and Runtime Host compatibility comment against current main while preserving the single catalog projection authority.

Generated-by: Maka
Current main already owns epoch 43 for shell-run poll correlation. Advance the retired Session timestamp wire change to epoch 44 and pin that compatibility floor.

Generated-by: Maka

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review at exact head 5fde3d4951d5bfbcd91518ebc9e96b33616f4d13.

GO. No open P0–P2. The prior overlay P2 is closed, not relocated.

useSessionSettingIntent now fences the committed overlay on successful catalog observation revision (use-session-setting-intent.ts:70-76, 109) instead of requiring the catalog value to equal intent.committed. A failed refresh still keeps the overlay (refreshCatalog is a nudge; list commit only happens on success in use-app-shell-session-list.ts:54-72,79-88). A later successful snapshot with a higher catalogRevision retires the overlay even when Runtime has already left Plan for Agent.

Covered by session-setting-intent.test.ts (“Runtime leaving Plan after approval supersedes the committed Plan overlay”): write succeeds, refresh throws, then revision 1 with Agent value clears the Plan overlay.

Path-filtered checks on this head: test completed+success. audit/package did not run (no lockfile/package.json change). No merge from this seat.

@Astro-Han
Astro-Han merged commit 43e96c6 into apache:main Aug 23, 2026
1 check passed
@Astro-Han
Astro-Han deleted the refactor/session-catalog-single-authority branch August 23, 2026 19:17
mikemikimike pushed a commit to mikemikimike/maka that referenced this pull request Aug 23, 2026
* refactor: publish canonical session catalog activity

Carry the storage catalog's materialized activity timestamp through Runtime Host projections and cursors. This removes the protocol's dependence on the redundant last-used timestamp and gives all clients one ordering fact.

Generated-by: Maka

* refactor: share canonical catalog projection

Generated-by: Maka

* refactor: accept complete desktop catalog snapshots

Generated-by: Maka

* refactor: retire session last-used authority

Generated-by: Maka

* refactor: remove session settings list patches

Generated-by: Maka

* refactor: remove desktop catalog upserts

Generated-by: Maka

* refactor: delete session summary merge layer

Generated-by: Maka

* refactor: drop legacy session activity column

Generated-by: Maka

* refactor: remove remaining desktop catalog patches

Generated-by: Maka

* chore: restore canonical ASF source header

Generated-by: Maka

* fix(desktop): repaint committed session settings

Generated-by: Maka

* fix(desktop): keep plan state outside catalog snapshots

Generated-by: Maka

* fix(desktop): retain healthy host catalogs

Generated-by: Maka

* fix(desktop): reconcile mode intents with catalog snapshots

Keep the latest Plan and orchestration intent in one setting owner until the Host commit is observed in a catalog snapshot. Catalog refresh failures no longer roll presentation back or prevent a queued latest intent from reaching the Host.

Generated-by: Maka

* fix(desktop): retire settings on newer catalog snapshots

Fence committed mode overlays by successful catalog observation revision instead of value equality. Runtime-owned transitions such as Plan approval can now supersede an older renderer commit without letting failed reads discard it.

Generated-by: Maka

* fix(runtime-host): advance catalog protocol epoch

Current main already owns epoch 43 for shell-run poll correlation. Advance the retired Session timestamp wire change to epoch 44 and pin that compatibility floor.

Generated-by: Maka
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.

2 participants