Remember workspace mode across app launches#691
Conversation
Deploying maple with
|
| Latest commit: |
137ee30
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fd63a10d.maple-ca8.pages.dev |
| Branch Preview URL: | https://codex-remember-mode-across-l.maple-ca8.pages.dev |
marksftw
left a comment
marksftw
left a comment
There was a problem hiding this comment.
Code review report
Outcome: No blocking findings remain after the follow-up revision.
Scope and design
- The change is appropriately narrow: one storage service, bootstrap restoration, mode-switch persistence, and focused tests.
- Restoring the route before importing the app ensures the router's first location snapshot is correct and avoids a visible Chat-to-Agent redirect.
- Persistence now happens only from explicit mode-switch actions. Automatic authentication, callback, settings, deep-link, and history navigation cannot accidentally rewrite the user's saved preference.
- Missing, invalid, or unavailable local storage safely falls back to Chat Mode without disrupting navigation.
Finding validated and fixed
The original implementation observed the root route globally. An ordinary sign-in or automatic redirect to / could therefore save Chat Mode and erase a previous Agent selection. Commit f5a6ef1 removes that global observation and records the mode at the successful user switch instead. This is both safer and smaller: the follow-up added 4 lines and removed 49.
Considered without expanding scope
- Restoring
/agentuses the same eligibility behavior as navigating directly to the existing/agentroute. Adding a new asynchronous feature-flag or billing route guard would change existing routing policy and substantially broaden this issue. - Direct URLs and browser Back/Forward do not change the saved preference. This preserves the last mode the user explicitly selected and prevents incidental navigation from clobbering it.
Verification
- Focused persistence tests: 9 passed, 0 failed
- Full test suite: 224 passed, 0 failed
- Typecheck and production Vite build passed
- Lint: 0 errors; 12 pre-existing warnings
- Desktop restart acceptance flow passed in both directions:
- Chat → Agent → kill/relaunch → Agent
- Agent → Chat → kill/relaunch → Chat
The implementation satisfies issue #651 without introducing a broader routing abstraction or invasive authentication changes.
|
UX testing passed. The selected mode persists across launches. |
|
The implementation looks good overall, especially the follow-up that persists only a completed, explicit mode switch. I agree that this should not grow into an asynchronous routing or entitlement refactor. There is one small account-handoff edge case worth covering: This is not an Agent data-isolation problem—the native Agent state remains account-scoped—and I would not add a feature-flag or billing guard to Concretely, I would add a tiny helper beside the existing preference functions: export function resetWorkspaceModePreference(
storage: WorkspaceModeStorage | null = getBrowserStorage()
): void {
rememberWorkspaceMode("chat", storage);
}Using the existing writer keeps the same storage-failure behavior and avoids expanding Then import and call it in the successful account-exit paths: await os.signOut();
resetWorkspaceModePreference();The normal logout call sites are:
For if (!deletionConfirmed) {
await os.confirmAccountDeletion(confirmationCode, secret);
deletionConfirmed = true;
setIsAccountDeleted(true);
cleanupBlockRef.current.retainUntilNextSession();
}
resetWorkspaceModePreference();One focused test should be enough: test("resets the launch preference to Chat when an account leaves", () => {
const storage = new MemoryStorage();
rememberWorkspaceMode("agent", storage);
resetWorkspaceModePreference(storage);
expect(getStoredWorkspaceMode(storage)).toBe("chat");
});That keeps the current bootstrap approach, avoids cross-account carryover, and adds only a helper plus four small call-site changes. If Maple later decides the feature flag must act as a true revocation or operational kill switch, that should be handled separately with an authoritative |
|
responding to the feedback |
|
Implemented the account-handoff reset in What changed:
Validation:
This keeps the change limited to explicit successful account exits and does not alter entitlement or routing logic. |
|
resolving conflicts and testing |
05891e6 to
137ee30
Compare
Final validation reportPR #691 is rebased onto Automated validation
Agent-driven UI validation
Human UI validationThe requester independently tested the final dev-backed build and reports that the human UI testing succeeded as well. No blocking findings remain from the rebase, automated checks, agent-driven UI testing, or human UI testing. |
|
ready for review |
AnthonyRonning
left a comment
AnthonyRonning
left a comment
There was a problem hiding this comment.
Approved. We reviewed the final implementation and smoke-tested this exact head end to end in the packaged macOS app against the managed OpenSecret, billing, and feature-flag stack. Chat and Agent persistence, cold restarts, functional model paths, billing state, and the logout reset all passed. The code is focused, failure-tolerant, and appropriately avoids broader routing or authentication complexity. No remaining concerns.
|
Everything looks good from our side. The current head passed code review and full macOS GUI smoke testing against the managed OpenSecret, billing, and feature-flag stack, including Chat and Agent persistence, cold restarts, functional model paths, billing state, and the logout reset. The implementation is clean, narrowly scoped, and makes the right tradeoff without extra complexity. You can merge whenever you are ready. |
2 participants
Summary
Why
Maple had no durable workspace-mode preference and always initialized from the home route, so users who selected Agent Mode returned to Chat Mode after every app relaunch.
Impact
Desktop users now resume their last explicitly selected workspace mode. Missing or invalid preferences safely default to Chat Mode, while web launches, deep links, callback routes, and failed navigation keep their existing behavior.
Validation
bun test— 224 passed, 0 failedbun run typecheck— passedbun run lint— 0 errors (12 pre-existing warnings)Closes #651