api/routes: claim CLI/TUI/Desktop sessions on first POST, not just GET

[merodahero]

Restore POST-path session claim for CLI/TUI/Desktop sources; gate bare gateway/unknown; map state.db timestamps; sanitize 500s.

Why

CLI, TUI, and Desktop sessions were being recreated on every POST to /api/chat/start instead of being claimed from the on-disk state, so the WebUI lost context between requests. The denylist for gateway / unknown sources, the 500-error sanitization, and the state.db timestamp mapping are Greptile-driven hardening around the same claim path.

What changed

• api/routes.py
  • _claim_or_synthesize_cli_session (~L3355) now claims local CLI/TUI/Desktop sessions on the first POST to /api/chat/start, not just on the GET path.
  • New source denylist gate (~L3309-3320) rejects bare gateway and unknown sources at the claim path; they have no claimable backing store and would otherwise 500.
  • 500 responses from the claim path are sanitized before they go out as JSON, so internal exception text does not leak to the WebUI.
  • CLI meta is now looked up once per POST (dedupe).
  • state.db created_at / updated_at are mapped into the session metadata response (was previously missing on synthesized CLI/TUI/Desktop sessions).
  • CLI-meta enrichment is copy-on-write: it builds a new dict instead of mutating the caller's cli_meta.
• tests/test_chat_start_claim_cli_session.py (new, 29 tests, 973 lines)
  • Source denylist: test_helper_refuses_bare_gateway_session, test_helper_refuses_bare_unknown_session.
  • No-mutation guards: test_helper_does_not_mutate_callers_cli_meta, test_helper_does_not_mutate_callers_cli_meta_when_empty.
  • read_only plumbing: GET stub + POST 403 path.
  • state.db timestamp mapping.
  • Original POST-claim flow.
• CHANGELOG.md
  • Unreleased entry under [Unreleased] -> ### Fixed.

Testing

• 29/29 in tests/test_chat_start_claim_cli_session.py.
• Greptile 5/5 (no findings) on the prior api/routes: claim CLI/TUI/Desktop sessions on first POST, not just GET #3901 commit set. The branch was reauthored after a force-push, so a fresh Greptile review is expected to land on the new SHAs; the previous summary is stale.

Risk / rollback

The change is gated by the source denylist, so it only activates on write-claimable local sources (CLI/TUI/Desktop). WebUI-originated sessions are unaffected. Rollback = revert the merge commit.

Supersedes the closed #3901 (force-pushed into an unrecoverable state during reauthor; this is the same content, fresh PR).

[greptile-apps]

Greptile Summary

This PR closes the GET-vs-POST asymmetry for CLI/TUI/Desktop sessions: both GET /api/session and POST /api/chat/start now route through a shared helper, _claim_or_synthesize_cli_session, so a session visible in the sidebar no longer 404s on the first write. It also ships a source denylist for non-claimable foreign-owned sessions (messaging, claude_code, cron, gateway, unknown) and cleans up several follow-on issues from the prior Greptile review.

• The shared helper materialises a WebUI-owned sidecar on first POST (synth.save()), persists state.db timestamps into created_at/updated_at, and returns a typed reason string so the caller can give semantically correct responses (404 for deleted sessions, 403 for read-only foreign ones, 500 sanitised via _sanitize_error for save failures).
• The previously-inline WebUI-vs-foreign classifier was extracted into _session_index_marks_was_webui, and read_only is now read from the synthesized Session rather than from cli_meta directly, fixing the bug where source-refused sessions (messaging / claude_code) rendered the composer until the POST 403'd.
• A new 29-test file (tests/test_chat_start_claim_cli_session.py) pins the static structure, source-denylist behaviour, no-mutation contract, state.db timestamp mapping, and the read_only plumbing.

Confidence Score: 5/5

Safe to merge; the change is gated to write-claimable local sources only and all claim paths have defensive fallbacks.

The core claim path is well-encapsulated in the new helper with clear typed reasons, copy-on-write semantics for cli_meta, sanitised 500s, and 29 new tests covering every major branch. Both previously-flagged gaps (cron denylist, diagnostic reason label) are addressed. The two remaining observations are narrow edge cases that don't affect the common-path behaviour.

No files require special attention; api/routes.py carries all the logic but is well-commented and the test file exercises the critical paths.

Important Files Changed

Filename	Overview
api/routes.py	Core logic change: extracts _session_index_marks_was_webui, adds _is_claimable_cli_source denylist, adds _claim_or_synthesize_cli_session helper, wires helper into both GET /api/session and POST /api/chat/start; also fixes read_only plumbing on the GET stub and in _handle_session_import_cli.
tests/test_chat_start_claim_cli_session.py	New 29-test suite covering static structure checks, source denylist, no-mutation contract, state.db timestamp mapping, read_only plumbing, and the POST claim flow with a realistic in-memory state.db.
CHANGELOG.md	Adds five well-scoped [Unreleased] Fixed entries covering the claim path, read_only plumbing, epoch timestamp fix, cron denylist, and diagnostic reason label.

_{Reviews (10): Last reviewed commit: "api/routes: claim CLI/TUI/Desktop sessio..." | Re-trigger Greptile}

[nesquena-hermes]

Pulled the branch into a read-only worktree and read the full api/routes.py diff plus the new helper, both wired call sites, and cross-checked the agent's session-source values. The architecture is sound — a single _claim_or_synthesize_cli_session shared between the GET stub and the POST claim is the right way to kill the GET/POST asymmetry. One genuine inaccuracy in a test docstring, one confirmation on the denylist, and one small note.

Verified: the denylist matches the agent's actual source values

The refused families in _is_claimable_cli_source (api/routes.py, ~L3309-3320) are {claude_code, cron, external_agent, gateway, messaging, unknown}, with cli/tui/desktop falling through to claimable. I checked these against what the agent actually writes:

• tui / cli — hermes_cli/main.py:2106 (source = "tui" if use_tui else "cli")
• cron — hermes_cli/web_server.py:6798 (source='cron' for scheduled rows)
• bare gateway / unknown — gateway/platforms/api_server.py:866 (return " ".join(fields) if fields else "source='unknown'")
• desktop — surfaced via hermes_cli/subcommands/gui.py:16

So the allow-by-omission set (cli/tui/desktop) is exactly the locally-owned interactive surfaces, and every foreign-owned family the agent can stamp is in the denylist. The classification holds.

The updated_at timestamp mapping doesn't do what the test docstring claims

test_..._materialized (lines 417-427) asserts sess.updated_at > 0 with the rationale that otherwise "the synthesized Session will be written into the sidecar with a 1970 timestamp on first save." That's true for created_at but not for updated_at. Session.save() defaults to touch_updated_at=True and unconditionally stamps now:

def save(self, touch_updated_at: bool = True, skip_index: bool = False) -> None:
    ...
    if touch_updated_at:
        self.updated_at = time.time()

(api/models.py:715,735-736). So on the POST claim path (synth.save() with the default), updated_at is overwritten with wall-clock now regardless of whether the state.db mapping ran. The updated_at mapping is genuinely load-bearing only for the GET read-only stub (which never calls save()), where it sets the pre-claim sidebar timestamp. created_at is preserved by save() (it's not in the touch list), so that half of the mapping is essential on both paths.

Net: no bug, the mapping is still worth doing for the GET stub — but the docstring overstates it. If you want the original last-activity time to survive the actual claim into the persisted sidecar, the save would need synth.save(touch_updated_at=False); otherwise a freshly-claimed TUI session jumps to "just now" in the sidebar the moment it's continued. That may even be the desired behavior (it was just touched), so this is a product call, not a defect — just flagging that the persisted updated_at ≠ the state.db value the test is pinning.

Confirmed: the not_claimable arm correctly skips save()

_handle_chat_start returns 403 (not 404) on not_claimable and never reaches synth.save() — test_not_claimable_returns_403_not_404 pins both the 403 and the absence of synth.save() in that arm via regex over the source. Good: that's the ownership-boundary guard, and 403-vs-404 matters because the frontend's 404 handler (messages.js) runs the empty-state self-heal that strips the URL. A read-only-but-listed session should refuse in place, which is what this does.

Small note: 500-path sanitization

The _sanitize_error(_save_err) wrap on the save-failure 500 is the right call — a raw OSError str would carry the absolute sidecar path. Worth a one-line assertion in the existing path-leak test that the sanitized string doesn't contain a / segment from the session store root, so a future refactor of _sanitize_error can't silently regress it.

Reads clean overall. I didn't execute anything (review is read-only), so the 29-test green is on the author's CI, not re-run here.

[merodahero]

@greptile review

[merodahero]

@nesquena-hermes addressed — docstring updated to match what the code does (created_at is preserved by save(), updated_at is touched to now on the POST claim path, which is the desired UX per your lean), and added the path-leak regression assertion to _sanitize_error's test (feeds an OSError carrying the real SESSION_DIR path, asserts the sanitized string doesn't contain "//"). Also rebased onto current master (ebf778e) to pick up #4155; CHANGELOG conflict resolved by keeping both your v0.51.403 stamp and our [Unreleased] entries. New head SHA: da87a73. Re-ran Greptile, 5/5 holds with no new findings. Thanks for the read-through.

[merodahero]

@greptile review

[merodahero]

@nesquena-hermes squashed the 13 PR commits into 1 and rebased onto current master (1643d66) to reduce future conflict surface — the diff is identical to what you reviewed. New Greptile review on the squashed SHA confirms 5/5. No production code touched since your sign-off.