Skip to content

fix(hub,web): apply selected permission mode when resuming inactive sessions#540

Merged
tiann merged 1 commit intotiann:mainfrom
junmo-kim:fix/resume-permission-mode-minimal
Apr 28, 2026
Merged

fix(hub,web): apply selected permission mode when resuming inactive sessions#540
tiann merged 1 commit intotiann:mainfrom
junmo-kim:fix/resume-permission-mode-minimal

Conversation

@junmo-kim
Copy link
Copy Markdown
Contributor

@junmo-kim junmo-kim commented Apr 28, 2026

Problem

When a session is inactive (the CLI is not running), toggling the permission mode in the web UI has no visible effect: the POST /sessions/:id/permission-mode endpoint returned 409 because it required the session to be active (requireActive: true), so the in-memory cache was never updated. As a result, resuming the session always spawned the CLI with the stored default mode, ignoring the user's selection.

Solution

Three coordinated changes close the gap:

  1. POST /sessions/:id/permission-mode – remove the requireActive guard so inactive sessions are accepted. In SyncEngine.applySessionConfig an inactive-session branch is added that updates the in-memory cache directly (skipping the RPC call, which requires a running CLI).

  2. POST /sessions/:id/resume – accept an optional { permissionMode } body. The value is validated against the session flavor before being forwarded to resumeSession. This lets the web client pass the currently displayed mode at resume time as a safety net.

  3. Web clientapi.resumeSession passes session.permissionMode in the request body; SyncEngine.resumeSession accepts an optional opts.permissionMode that takes precedence over the cached value when building the spawn payload.

The in-memory-only approach is intentional: persisting the value across Hub restarts is a separate concern tracked elsewhere.

Tests

New cases in hub/src/web/routes/sessions.test.ts:

  • POST /sessions/:id/permission-mode returns 200 for inactive sessions and calls applySessionConfig with the requested mode.
  • POST /sessions/:id/resume passes permissionMode from the request body through to resumeSession.
  • POST /sessions/:id/resume returns 400 when the requested mode is not supported by the session flavor.

…essions

Previously, toggling the permission mode on an inactive session had no
effect on resume: the /permission-mode endpoint rejected inactive sessions
(HTTP 409), so the cache was never updated, and the spawned CLI always
received the stored default value.

- Remove the `requireActive` guard from POST /sessions/:id/permission-mode
  so inactive sessions can have their in-memory permission mode updated.
- In `SyncEngine.applySessionConfig`, skip the RPC call for inactive
  sessions and update the in-memory cache directly; the value is then
  available when the session is resumed.
- Accept an optional `{ permissionMode }` body in POST /sessions/:id/resume
  and forward it to `resumeSession` (takes precedence over the cached value),
  with flavor-compatibility validation.
- Extend `SyncEngine.resumeSession` with an optional `opts` argument so
  callers can supply a permission mode override at resume time.
- Update the web client (`api.resumeSession`) and `router.tsx` to pass
  `session.permissionMode` in the resume request body.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Findings

  • None.

Summary
Review mode: initial
No issues found in the changed lines. Residual risk: coverage is focused on route behavior; I did not execute the test suite because this review treats PR code as untrusted automation input.

Testing

  • Not run (automation)

HAPI Bot

@tiann tiann merged commit 04fbc0d into tiann:main Apr 28, 2026
2 checks passed
@junmo-kim junmo-kim deleted the fix/resume-permission-mode-minimal branch April 28, 2026 09:50
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