fix(cli): name the owning Profile when a Session lookup misses - #406
Open
Agnik47 wants to merge 1 commit into
Open
fix(cli): name the owning Profile when a Session lookup misses#406Agnik47 wants to merge 1 commit into
Agnik47 wants to merge 1 commit into
Conversation
A Session ID is only ever resolved inside the selected Profile, so a command that omits `--profile` reports `Session not found` for a Session that does exist. Cleanup runs hit this constantly: the agent had a valid ID from `session create`, dropped the flag on the follow-up `session close`, read the error as "already gone", and retried the same command or went inspecting unrelated default-Profile Sessions. Every Profile's Sessions share one `browser-sessions.json` keyed by `profileId`, so the owning Profile is already in hand when the scoped lookup misses — no daemon or provider round trip is needed to name it. `SessionNotFoundError` now takes an optional owner and, when it is known, reports which Profile holds the Session plus the exact retry command. Both throw sites are covered: `require` and the private `requireMutable` behind `remove`/`touch`/handoff updates. The daemon provider and the CLI local fallback resolve through this same store, so both paths gain the hint from the one change. With no owner the message and hint are unchanged. `session list` also grows a `profileId` table column. The rows were always scoped to the selected Profile, but the table did not say so, which is the other half of how agents ended up acting on Sessions that were never theirs. JSON and YAML already carried the field.
Contributor
🟠 Maintainer review suggested — low confidenceThe automated review could not reach a fully supported conclusion. Limitations
This review is advisory and does not block merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #388.
Problem
A Session ID only ever resolves inside the selected Profile, so a command that omits
--profilereportsSession not foundfor a Session that plainly exists. The eval runs in the issue show the loop this creates: the agent has a valid ID fromsession create, drops the flag on the follow-upsession close, reads the error as "already gone", and then either retries the identical command or starts inspecting unrelateddefaultSessions.session listfed the same confusion from the other side — the rows were always scoped to the selected Profile, but the table never said so, so a list ofdefaultSessions read as the whole machine.Fix
Every Profile's Sessions live in one
browser-sessions.jsonkeyed byprofileId, so when the scoped lookup misses, the owning Profile is already in hand — no daemon or provider round trip is needed to name it.SessionNotFoundErrortakes an optional owner. When known, the message becomesSession not found in Profile <selected>: <id>and the hint names the owner plus the exact retry,webcmd --profile <owner> session close <id>. With no owner the message and hint are byte-for-byte unchanged.require, and the privaterequireMutablebehindremove/touch/ handoff updates — so the close path cannot fall back to the anonymous message.local-cloak/provider.ts) and the CLI local fallback both resolve through this same store, so one store-level change serves both paths. That also settles the issue's open question about local-only versus daemon lookup: it is local, and it is correct for both.session listgains aprofileIdtable column. JSON and YAML already carried the field;columnsonly affects table/markdown/csv.docs/troubleshooting.mdxdocuments the newSESSION_NOT_FOUNDbehaviour.Acceptance criteria
webcmd --profile <owner> session close <session-id>.Open questions from the issue
session listwithout--profileever show all Profiles? Left scoped, with the scope now visible in the table. Showing every Profile by default is the behaviour the issue calls a safety problem for hosted and multi-tenant use.Tests
Three store-level tests (owner named on
require, owner named on the mutating path, generic hint preserved when no Profile owns the ID) and one CLI-level test that drivessession closewithout--profileend to end.155/155 pass across
sessions.test.ts,cli.test.ts, andsession-docs-sync.test.ts;tsc --noEmitis clean.Note for reviewers on Windows: the four symlink tests in
src/site-memory/local-store.test.tsfail on a cleanmainin my environment (symlink permissions), unrelated to this change.Non-goals
No
profile createchanges, no broad profile cleanup command, and no eval-specific aliases in user-facing text.