One button on a session row, and archive as the way out - #86
Conversation
6b8c956 to
efba67e
Compare
The sidebar row carried up to four controls. It keeps one: `×`, which archives. **Start** was the row's own `onClick` spelled a second time — `onOpenSession(...)` either way. **Stop** went because an idle CLI costs nothing, so "pause this" is not a real case; the real one is a runaway agent, which is urgent and rare, and in that moment you open the pane, where Ctrl-C interrupts with the CLI's own semantics. Its last UI consumer is gone, but the capability stays: archiving calls it. **Archiving is now stored, and it stops the agent.** It had to become stored: "I am finished with this one" cannot be expressed as an absence of activity, which is all the idle window can measure. `archivedAt` lives on the session record, so it survives a reload and means the same thing on every device. The idle window stays exactly as it was — still derived, still hiding a quiet session from the working list. Both roads lead to the archived view, but they are NOT the same road, and the PR argues why: only the deliberate one unlocks delete. **Delete only exists in the archived view**, and only for a session the operator archived. The server enforces it (409 `not-archived`) rather than the sidebar alone, so the rule holds for any caller, and it leaves an obvious hook for #76's `?ifNeverStarted=1`. **Trace actions moved to the trace.** #84 did not remove the per-row trace buttons — its own body says so — so this does. Share and "continue in a new agent" are now on the trace pane's header, which is the thing they act on. Not the reader's `i`: that panel is `TraceInfo`, and a trace pane does not use it. Reading a session's own transcript is what reader mode already is, so `openTrace` and its dead pane-spawning path go. The remote reconnect/disconnect pair is untouched: it is a line to another machine, not a local process. Group rows are untouched. `server/test/archive.test.mjs`, 16 checks against a real server: a live session refuses to delete with a code and a sentence, an archived one deletes, the flag is on disk rather than in a browser, restore puts it back and makes it undeletable again, archiving a running agent stops it, and `stopped` is still the state it lands in. This completes step 7 of `docs/conversation-view.md`'s own sequence — "the sidebar loses its trace buttons and `openTrace`" — so §3.4's table now records where each control went rather than where it was going, and no longer says agent rows keep stop/play and delete. `reader-info.test.mjs` gains two checks in the block that already asserted what the sidebar no longer carries: the row holds exactly one control, and it archives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
e862607 to
8103fde
Compare
|
Rebased onto The rebase was smaller than it lookedThe eight-file conflict list is an artifact of how the rebase is run, not of the change. → zero conflicts. Files like What I checked by hand, since a clean rebase is not a correct one
|
| controls | |
|---|---|
| working list | Archive |
| archived, retired on purpose | Restore · Delete |
| archived, merely quiet | Archive |
(Port checked as free before binding, and the served bundle hash checked against dist/index.html — I measured another agent's app that way once already this week.)
Both are clean against main and collide only with each other, in the two places this branch already expected. **The delete route.** `?ifNeverStarted=1` and the archive rule answer different questions, so both guards survive rather than one absorbing the other. The flag is the CALLER's precondition — "only if this never ran", the wrong-CLI mistake — so a session that has started is refused in #76's own words even where the archive rule would have allowed it; without the flag, delete stays archived-only. They compose into the exemption the guard was written expecting: a session that never ran has nothing worth archiving, so it goes in one step. **The sidebar row.** Resolved as one row rather than a union of two prop lists. #76's `onPrepareQuickStart` / `onAbandonQuickStart` survive untouched — they are about creating a session to hang an attachment on, not about the row's buttons — and the row keeps its single archive control. Nothing of #76's needed reinstating: every reference it had to `onStopSession`, `onShareSession`, `onOpenTrace`, `onShareTrace` and `isShareable` was the pre-existing row markup this branch deletes, and its diff adds no new line touching any of them. `isShareable` and `hiddenSessionIds` lose their last users here and go with the buttons; the second was already dead on main. `archive.test.mjs` grows six checks over the seam: a never-run session deletes in one step, a started one is refused for `ifNeverStarted` in #76's words and for the archive rule without the flag, and survives both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`screenshot-input.test.mjs` clears the session list twice by calling DELETE directly. Since this branch the server refuses to delete a session that has not been retired, so both loops silently 409'd and left their fixtures behind — the "only a stopped target" and "discards across reload" checks then counted sessions that should have been gone. Both are teardowns of sessions that HAVE started, so `?ifNeverStarted=1` is not their route; they archive first, exactly as the sidebar does. The suite is back to 31/31, matching what #76 scores on its own. Worth knowing beyond this file: the guard applies to every caller, not just the UI, so anything that deletes a session programmatically now archives it first. `migration.test.mjs` has three such cleanup calls; they are `.catch(() => {})` with nothing asserted after them, so they no-op harmlessly in a throwaway DATA_DIR and the suite stays green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Merged Both conflicts resolved, plus a third thing the merge turned up that is worth reading — it is a consequence of this PR's contract, not of the merge. 1. The delete route — both guards, because they answer different questionsImplemented as the comment promised, with #76's own 409 intact: const claimedNeverStarted = req.query.ifNeverStarted === '1';
if (claimedNeverStarted && s.everStarted) {
return res.status(409).json({ error: 'session has already started' });
}
if (!claimedNeverStarted && !s.archivedAt) {
return res.status(409).json({ error: 'archive this session before deleting it', code: 'not-archived' });
}
Six new checks in 2. The sidebar row — one row, not two prop listsNothing of #76's needed reinstating, and I checked rather than assumed. Every reference #76 has to
3. What the merge surfaced: DELETE is a contract, not a UI rule
One cause, two symptoms. Both teardowns are of sessions that have started, so The general point for the dev deploy: this guard applies to every caller, not just the sidebar, so anything deleting a session programmatically now archives first. That is the intended design — enforcing it server-side is why the rule holds for a caller that is not the UI — but it is a real API change and this is the first thing to trip on it. Verified
A process note, since it nearly fooled me: my first "pass" of |
1 participant
The sidebar row carried up to four controls. It keeps one:
×, which archives.What went, and why it did not need a new home
Start was the row's own
onClickspelled a second time — both callonOpenSession(...). Stop went on the operator's reasoning, which I agree with: an idle CLI costs nothing, so "pause this" is not a real case; the real one is a runaway agent, and in that moment you open the pane, where Ctrl-C interrupts with the CLI's own semantics rather than a button's.onStopSessionloses its last UI consumer, butstop()stays — archiving calls it.The remote reconnect/disconnect pair is untouched, as asked, and I agree it should be: it is a line to another machine, not a local process. Group rows are untouched.
A correction to the brief: #84 did not remove the trace buttons
The brief said read-trace, share-trace and share-session would already be gone on this base. They are not — #84's own body says "Per-row trace actions are untouched: read-trace, share, handover, and the trace-pane rows all still work." What #84 removed was the sidebar's trace widget, not the row's buttons. So this PR removes them, and had to find homes:
ipanel. Nothing to do.openTraceand the pane-spawning path it owned are dead code and go with it.ipanel, but that panel isConversationView's and a trace pane does not use ConversationView — it renders its own view. So both now sit on the trace pane's header, which is the thing they act on. The prefilled create panel still belongs to the sidebar, so the request travels there as an id rather than the panel moving.Archiving is stored now, and it stops the agent
It had to become stored: "I am finished with this one" cannot be expressed as an absence of activity, which is all the idle window measures — an agent you retire the moment it answers is as active as it will ever be.
archivedAtlives on the session record, so it survives a reload and reads the same on every device.POST /api/sessions/:id/archivestops the agent and stamps it.POST …/unarchiveclears the stamp and deliberately does not restart: unarchiving says "let me see this again", and starting is what opening the pane does.Delete is gated server-side (409,
code: not-archived), not just hidden in the UI, so the rule holds for any caller. The refusal is a sentence worth showing, and the toast shows it.Two judgement calls
What archiving a running agent does
It kills the PTY mid-turn (
host.pty.kill()), and the honest account of what that costs is:--resumes its pinned conversation — the session picks up its thread rather than starting blank.So the cost is bounded and recoverable except for one in-flight answer. That is why I did not put a confirmation in front of archive. But it is the one place in this design where a single unconfirmed click can lose work someone is waiting on, so if you want a guard, the right one is narrow: confirm only when
state === 'working', not on every archive. Say the word and I will add it.Auto-archived sessions becoming deletable — I did not do this, and here is why
This is the part you said you were least sure of, and I think the worry is right, so the two roads lead to the same view but not the same powers:
Three reasons. First, the window's verdict is not a decision: it is a statement about the clock that flips back the moment the setting changes, and letting a config change hand out delete rights to sessions nobody considered is a strange amount of power for a dropdown. Second, it keeps one sentence true — only a thing you retired can be removed — where the union version needs a footnote about a month. Third, it costs one click in the rare case and removes a whole class of accident in the common one.
The cost of being wrong here is smaller than it looks in one respect worth stating:
store.remove()deliberately leaves the workspace folder on disk, so deleting removes the session record and its place in the list, never the work. The button says so.If you would rather have the simpler rule — everything in the archived view is deletable — it is a one-line change (drop the
retiredsplit) and I will make it.And the no-dialog question
Agreed, no confirmation for delete. The archived view is somewhere you went on purpose, the row is already out of your way, and the files survive. A dialog there would be the third time the system asked you whether you meant it.
stoppedstays a stateUntouched, and it must be: it is what every session lands in after a Space restart, and the archive route asserts it explicitly.
Verified
server/test/archive.test.mjs, 16 checks against a real server rather than mocks:code: not-archived, and a sentence with the word archive in it; the session is still there afterwardssessions.json, which is the entire reason it is storedrunning: true→running: falsestoppedis still the state it lands inAlso: web typecheck, web suite, the full server suite (12 files, no retries), and before/after screenshots of the row and the archived view against a live instance, with the button inventory read out of the DOM rather than eyeballed.
Screenshots and the button inventory →
Not verified: how this reads on a phone (checked at desktop width only), and I have not exercised archive against a remote agent with a live peer attached — it takes the no-process branch, so it files the session away and leaves the connection alone, which is what the brief asked for.
Conflicts
server/src/index.js, 1 hunk. Upload attachments immediately with progress #76 adds its?ifNeverStarted=1guard in exactly the spot this PR puts the archive guard — my comment there predicts it by name. They compose rather than compete:web/src/components/Sidebar.tsx, 3 hunks, all in the props destructure and its type. Upload attachments immediately with progress #76 addsonPrepareQuickStart/onAbandonQuickStart; this removesonStopSession,onShareSession,onShareTrace,onOpenTraceand addsonArchiveSession,onUnarchiveSession,retired,handoverFor,onHandoverHandled. Take both sides. (Upload attachments immediately with progress #76 still carriesonOpenSharedTrace, which Trace download on the session, and the reader's facts behind one i #84 removed — that resolves when Upload attachments immediately with progress #76 rebases onto Trace download on the session, and the reader's facts behind one i #84.)🤖 Generated with Claude Code