Repository navigation
Unified coding-agent names: one persistent session name for Claude, Codex, and OpenCode - #819
Merged
Merged
Conversation
The WS create dispatch silently swallowed freshAgent.create frames while settings.freshAgent.enabled was off (the default): no reply frame, no log, no sidecar spawn — every programmatic driver hung its pending create with zero attribution (observed end-to-end in the native session-names smoke as a created-frame timeout with no answer of any kind). Answer with the raw-layer envelope precedent instead: freshAgent.create.failed carrying the requestId, code FRESH_AGENT_DISABLED, retryable true (the legacy disabled-gate rejection parity, ws-handler.ts:3334).
…eSource projection The send-activity tests asserted the RecordingSink the instant respond_log_frames returned, but the feed lands on the spawned sidecar-notification pump — under full-crate parallelism the pump trails by seconds and the instant assert sees an empty feed (three consecutive full-crate failures of a_send_racing under host load; each passing solo). Poll with a 15s deadline instead: the protection under test is 'the feed is never silently dropped', which a poll proves honestly. Same family, the racing and post-adoption tests also hand-modeled the session state while the fake sidecar's async init adoption could still re-write cli_session_id — wait for the adoption to complete first, then hand-model the window. The post-adoption and ui_layout_sync fixtures pin the tab-row nameSource projection 4bd6e68 added (the ws crate suite was never run to completion at that commit; the two fixture asserts were stale and are now updated to pin the intended shape). The approval_respond_write_failure flake observed alongside is NOT touched: it is the ledger-receipted pre-existing family member (failed at base 6ee5cf4 under full parallelism, green solo in 0.01s, documented in the transcript-minimap baseline).
The codex server leg passes end-to-end (13 operations incl. the resume- create writeback with a verified location, the pending rename, and the zero-turn restart survival). Root causes found and fixed in the runner: - Pre-seed the server's ~/.freshell/config.json with freshAgent.enabled (like the e2e helper): the WS create gate silently swallowed creates while the flag was off — the root cause of every prior created-frame timeout, not sidecar boot slowness (a probe of the exact server-shaped env boots the ws-listen CLI in ~250ms in all variants). - The step-(6) create is the real client's resume shape (sessionRef + namingHandle): the codex native writeback is live-connection-only by design and needs the resume lane's pending-to-durable bind to establish the verified location a plain new-thread create never gets. - Register the codex CLI extension manifest in the server home (mode: codex REST creates) and carry the app-bound FRESHELL_MCP_NODE/ENTRY pair the packaged runtime expects. - Container-local server HOME + codex projectDir + the opencode contract aligned to the server's opencodeXdgRoots (step 8's same-database writeback), refusal-aware frame waits, coherent JSONL failure tails (session_names vocabulary), and the store-document/created-frame/pane- content evidence riding the opencode durable-bind timeout for the next diagnostic run.
…surface inventory The end-of-execution gate failed on six deterministic unit tests, all stale against the last two commits' own intended behavior (their receipts covered the e2e and Rust lanes but never the unit lane): - Four api-mock factories predate the with429Retry wrapper the naming read now rides: the mocked module made it undefined, the bootstrap/ conflict-refresh paths swallowed the TypeError, and the refetch-spy and rename-capture tests saw zero POSTs. Add the passthrough to each failing file's factory. - UnifiedAgentRename's conflict fixture built the legacy raw-Error-with- 'data' shape — exactly the masking bug the conflict-lane repair fixed (toRenameError reads the ApiError's parsed 'details' body). Rebuild the fixture as a real ApiError carrying details like production. - The runtime-boundary inventory: register the two new executable provider fixtures (fake-codex-terminal, fake-opencode-terminal) as provider-fixture rows; allowlist the native session-names runner's transient port probe and the unified-agent-names e2e helper's fake Gemini listener in NON_BACKEND_LISTENER_PATHS with matching manifest rows (the same class as coordinator-endpoint and harness-06/fake-ai); ignore the untracked .tmp-native-smoke run root in the walker. perWindowLayoutKeys was triaged first as possible-real-regression and cleared: it passes solo and in company once the noise above is fixed; the product files are unchanged since the lane was last green.
The sessions directory-overlay tests resolved their ClaudeSource root
through claude_home(&home), whose contract lets the process-global
CLAUDE_HOME env var win over the explicit home (the production override
parity). Parallel test modules mutate CLAUDE_HOME process-globally while
holding their own lock, so any test that does not take that lock can
resolve a FOREIGN temp home at construction and scan the wrong root
forever — deterministic in full-parallelism binary runs (the seed file
verified present on disk, the sweep succeeding with zero scan failures,
and the session still absent after 15s of explicit refresh polls), green
solo and at --test-threads=4. Pin home.join(".claude") directly in all
three tests: the env override belongs to the production boot wiring, not
to fixtures that own their temp home.
… budget write_raw_document_bytes did a bare try_lock+expect on the sidecar lock, so a transient in-flight store transaction turned the fixture into a WouldBlock panic (observed as a rotating full-parallelism flake in workspace-wide runs). The strict path's acquire_document_lock retries contention for a bounded second — mirror that discipline in the helper.
…t fixture The runtime allowlist requires claude-sidecar/session-names.mjs (the unified naming writeback's sidecar entry), but the artifact-verification fixture never wrote it — every probe-based rejection case failed with 'missing required file' instead of exercising the probe contract. The fixture must construct a complete artifact per the current allowlist; the composite runner had not reached the electron phase in recent gates (rust-phase failures stopped it first), so the drift went unnoticed.
…end wire The opencode (8b) server-writeback proof now follows the product's real lifecycle: the freshopencode create answers created with the PLACEHOLDER id by design (opencode_ws.rs: the binding row is written at materialization — the first send), so the contract sends the first message over the WS lane (the serve's session creation and the verified bind precede the model turn, which may fail provider-less in the sandbox), then derives the durable target from the naming state itself: reading the pane's PENDING handle resolves through the store's redirect to the bound durable ses_* record — the pane CONTENT's nameRef only advances when a real client syncs its layout, which a raw-WS pane never does (observed: the bind, the redirect, and the verified database location all landed while the pane content kept the placeholder).
…stly
Task-008 review I-2, M-4, and M-6 on the cross-mode spec.
M-4: the swap request sent targetPaneId, which swap_pane ignores — the
swap was silently REFUSED forever (the endpoint answers HTTP 200 with
data.message only), and the old if(swap.ok) guard kept the sub-assert
green with nothing swapped. The body now sends the real key (target) and
the test pins the success shape (data.tabId, which the refusal arm
omits), polls the client layout until A's session genuinely lives in
B's pane id, then closes exactly that pane — the old ?? a.paneId
fallback would now close the wrong session's pane. Fixing the close
poll exposed resolveSessionIdOf reading a phantom redirect.to field
that never exists in the store (the real shape is {toKey, revision}); it
always returned null, masked while the swap was always refused. RED:
the flipped targetPaneId body failed the shape pin (data.tabId
undefined); GREEN: the case passes 7 consecutive full-file runs.
I-2: the vacuous toBeAttached(...).catch(()=>{}) no-op is gone. The
case now honestly asserts what IS true at the base contract: the shell
observably exits (the server's terminal directory records status
'exited', polled through /api/terminals) and the nonagent tab keeps its
OSC-derived title. The '(exit N)' suffix presentation defect stays
unasserted and is disclosed as registered follow-up T8-F1 (pre-existing
at base 6ee5cf4; the branch left terminal-title-policy.ts untouched).
Mutation RED: the flipped exit-status poll failed at its timeout.
M-6: an excluded coding-provider terminal pane (gemini via the existing
fake-gemini.mjs fixture at GEMINI_CMD) is now exercised: the tab's
nameSource stays legacy, the rendered title keeps a legacy derivation,
and the server-side pane content carries no namingHandle and no nameRef
(a flipped toBeDefined RED proved the fields are genuinely absent). The
rendered-title assertion tolerates both pre-existing legacy derivations
(the client's cwd basename vs the server's seeded provider label, which
a machine-snapshot restore fold imports nondeterministically) — both
predate the branch and neither is a unified session name.
The ready/reconnect collector pushed every terminal/fresh pane's sessionRef into the batched name read, and parseSessionNameRef accepted any string provider through an unsound cast — so a gemini/kimi/ amplifier pane made the server's strict scope gate reject the whole 100-ref chunk with 400 on every reconnect. Parse the session provider through NamedProviderSchema so out-of-scope panes are never collected.
The ~5s auto-title sweep called hydrate_indexed (plus activity for message-carrying sessions) for every scoped session on every pass — each a strict cross-process transaction that locked/read/digested/parsed the whole session-names document even for no-ops, O(H^2) per pass forever. Adopted-view pre-checks in the needs_generation_input pattern now prove the no-op from the in-memory view first (the activity pre-check runs the same plan_activity policy the transaction applies, so they cannot drift), with a per-pass settled memo for repeated rows. The view-accuracy and cross-process staleness arguments live on the pre-check methods; a data-dir-keyed test transaction counter pins zero transactions on a second pass with a new-session convergence control.
Every Manual offer was accepted unconditionally, so repeating the exact same explicit rename on an already-manual record allocated a new revision and re-armed a fresh three-cycle/six-read native writeback series — the plan forbids a new bounded series for equal unchanged requests. The equal manual case now answers the unchanged winner (no revision, no series, original renamedAt/manualRevision retained); a same-text rename from a lower source still promotes, and a different text still gets its own bounded series.
select_next ordered due series by the stable reference key (then cycles consumed) and NativeWorkItem never carried next_due, deviating from the plan's stated "then earliest nextDue/reference key" policy — a persistently early-keyed series was served ahead of an earlier-due one. The item now carries the series' next_due and the selector orders manual-first, then earliest nextDue (a None due is immediately ready, matching select_generation), with the reference key as the tiebreak.
Every native title observation — including each OpenCode session.updated carrying a title — committed a bookkeeping-only document generation (a full-document replace plus fsync) merely to persist last_observation.at when nothing user-visible changed. Observation provenance now stages only when a real change is committing anyway (an accepted offer or a divergence rearm); an observation-only delta answers the unchanged record without rewriting the document, and its timestamp rides the next real write. The T3-M7 provenance assertions are re-contracted to the same policy: no rename, no rearm, no error, and no durable write.
native_work_snapshot filtered due series with the real wall clock while its sibling generation_work_snapshot uses the test-offset-aware effective_now_ms — the due gate ignored the ClockOffsetMs hook, a trap for any future clock-controlled native test. Aligned to the same effective clock, with a focused test advancing virtual time through the snapshot's due gate.
All eight tasks of the committed plan are implemented; the step checkboxes stayed unchecked and the committed artifact misstated its own status.
hydrate_indexed_decision aborted the whole hydration for a session whose provider-authored title failed accepted-name validation (over the 200-scalar cap or control characters), leaving the session with no canonical record while the ~5s sweep re-failed and re-logged it every pass. An unusable title is now an absent rung: the ladder falls to the first-message extraction, then the directory basename, and a valid title still wins.
Cross-process adoption published only records whose revision or source changed, so a cooperating same-home process's status-only nativeSync transition (a native fold or divergence rearm with no visible name change) never reached the adopting process's WS subscribers until a bootstrap or later change. The adoption delta now also carries records whose nativeSync projection moved, as a changed:false frame — the same status-only fold discipline clients already apply by documentGeneration.
Delta-review round 4, finding 1 (the seam half): the auto-title sweep's kilroy discrimination becomes the ONE shared predicate every server surface consults (crates/freshell-server/src/kilroy_lane.rs) — a session is kilroy-only iff the SESSION-06 metadata types it kilroy AND the naming authority holds no canonical record for it AND no live terminal runs it in a scoped mode. The new record component fixes the sweep itself: a kilroy-typed row that already holds a canonical record (a landed migration chunk, an earlier pass's hydration, a create-lane bind) now stays in the naming-authority lane — its one singular name is the supported-mode record's (the Global Constraint's never-a-competing-kilroy-record rule), and the legacy ladder stops writing it a competing settings title. RED: a_kilroy_typed_row_with_a_canonical_record_stays_in_the_authority_lane (observed failing: the ladder wrote the competing title). GREEN: the full kilroy suite + the sweep module (25/25).
Delta-review round 4, finding 1 (surfaces 1-2): PATCH /api/sessions/:key
with titleOverride and POST .../generate-title routed every provider-claude
request into the naming authority, so a kilroy-only session (no naming
record, by the sweep's own design) answered 404 NAME_NOT_FOUND on rename
and 400 NAME_RESET_UNSUPPORTED on its still-offered reset, and
generate-title lost kilroy's retained server-side AI titling. Both routes
now consult the shared kilroy-lane seam first: a kilroy-only session
(metadata-typed kilroy, no canonical record, no live scoped terminal)
keeps the legacy override ladder / the retained Gemini ladder, while a
dual-mode session (canonical record or live scoped terminal) still
renames through the ONE authority — never a competing kilroy record.
RED (provider-only routing restored temporarily): the kilroy rename
answered 404 NAME_NOT_FOUND; kilroy generate-title answered {title:null}.
GREEN: a_kilroy_only_session_rename_keeps_the_legacy_override_path,
kilroy_generate_title_keeps_the_retained_server_side_ai_titling, plus the
dual-mode guard a_dual_mode_kilroy_typed_session_renames_through_the_authority
(green before and after — it pins the never-over-exclude half). The
sessions-router test helper is now metadata-aware (wired like main.rs);
sessions:: 25/25 and session_name_routes 15/15 green.
Delta-review round 4, finding 1 (surface 3): once the legacy-name consolidation's receipt commits, the directory's settings title lane closed for ALL provider-claude rows, so kilroy rows stopped displaying the legacy-ladder titles the sweep deliberately still writes for them. The page's kilroy-only rows are now answered by the shared kilroy-lane seam (one hoisted metadata read per request, shared with the sessionType overlay; registry: None — the record component owns the dual-mode decision) and keep the lane open; a record-holding kilroy-typed row stays closed (the supported-mode record owns its one singular name). RED: kilroy_only_rows_keep_displaying_legacy_titles_after_the_migration_receipt (observed failing: the row displayed its parsed title instead of the legacy override). GREEN: it passes, plus the dual-mode guard a_record_holding_kilroy_typed_row_keeps_the_title_lane_closed (green before and after). session_directory lane 95/95 green.
Delta-review round 4, finding 1 (surface 4): the boot consolidation's gather/import/clean pass absorbed every provider-claude override row and metadata derivedTitle, minting the Global Constraint's forbidden competing Kilroy record for kilroy-only sessions and re-wiping the sweep's post-migration kilroy titles at each boot as late legacy evidence. The gather now consults the shared kilroy-lane seam and excludes kilroy-ONLY rows from import AND from the cleanup/late-evidence targets — they stay in the legacy lane, untouched, which makes the module doc's contract true. The dual-mode subtlety is reasoned in the gather's doc and pinned by test: the exclusion is per-ROW (metadata-typed kilroy AND no canonical record); the identity ledger is empty at consolidation time, so the record is the operative evidence — a session whose canonical record exists through the claude mode still imports and cleans its legacy rows exactly like any scoped row (the supported-mode record owns its ONE singular name, no second record minted). RED: kilroy_mode_override_rows_stay_legacy_never_imported_or_cleaned (observed: a canonical record was minted) and post_migration_kilroy_titles_survive_the_late_evidence_pass (observed: the fresh kilroy title was wiped). GREEN: both pass, plus the dual-mode guard a_dual_mode_session_with_a_canonical_record_still_migrates_its_legacy_row (green before and after). session_name_migration lane 27/27 green.
Delta-review round 4, finding 2 (re-found from the focused round): in observe_native_decision a diverging observation on a non-settled armed series stamped its None next_due to now even when nothing else changed, then returned the early Decision::Read — and when that same transaction was the first to adopt a newer external generation, adopt_if_newer installed the locally-mutated copy into the view while the stored digest described the unmutated disk bytes. The stamp now rides ONLY the rearm commit (changed || status_changed), restoring the only-Decision::Write-mutates invariant. Semantically inert by construction: a None due is immediately ready (the work snapshot's convention) and an unsupported series never enters the snapshot. RED: an_observation_read_path_never_mutates_the_adopted_document (observed: the adopted view carried next_due Some(..) while the freshly parsed disk document said None, at an adopted newer generation). GREEN: the byte-identity invariant holds; session_names 29/29 and session_name_native 35/35 green.
Delta-review round 4, finding 3: parse_rename_intents silently dropped a non-integer or beyond-JS-safe ifRevision (as_u64 + ceiling filter mapped every malformed value to no-CAS), degrading a malformed compare-and-set guard to an unguarded rename. Every malformed shape is now the same loud 400 the helper's own nameIntent handling and the canonical serde route already apply; omitted/null stays no-CAS and a valid integer within the JS-safe ceiling is still honored. RED: a_malformed_if_revision_is_rejected_loudly_not_silently_dropped (observed: ifRevision "12" answered 200 and renamed unguarded). GREEN: all malformed shapes 400 with the name untouched; a matching valid revision renames, a stale one still answers 409. session_name_routes 16/16 green.
Delta-review round 4, finding 4 (Nit): the Task 5 middleware-chain edit left four stray spaces between the getDefault close brace and the .concat( call. Typecheck clean; the store client unit lane (79 files, 1375 tests) stays green.
Final-gate flake fix (the recorded gate failure is the RED evidence): 2 of 1128 --bin freshell-server tests failed in the gate's rust phase — claude_unreadable_projects_root_defers and claude_unreadable_project_subdir_defers, panicking 'an unreadable projects root must DEFER (false), never delete'. Both are deterministic solo (0.00s) and the pristine base (6ee5cf4) passed the full bin lane clean, so this is not the receipted rotating load-timing family. Diagnosis: session_directory::claude_home lets a process-global CLAUDE_HOME win over a test's explicit home, so while a concurrent test in the same binary holds CLAUDE_HOME at its own temp home (whose projects tree is readable and holds no sess-1.jsonl), the deferral tests' production path (transcript_definitively_absent) resolved the FOREIGN root and answered 'definitively absent'. The branch's own native route-discovery tests are the only setters of CLAUDE_HOME (4 set/restore sites), and the ~150 added tests widened the parallel window until the race fired. Fix: one crate-wide test-only lock — crates/freshell-server/src/test_env_lock.rs (a tokio::sync::Mutex: async mutators hold it across awaits, sync readers take blocking_lock), mirroring the HOME_ENV_TEST_LOCK / test_clock_gate precedent. Every same-binary CLAUDE_HOME/CLAUDE_CONFIG_DIR mutator now holds it for the whole set…restore window (the two discovery tests; also the three claude_exact_id_fallback tests, whose CLAUDE_HOME unsets were outside it — they take HOME_ENV_TEST_LOCK first, the documented HOME→CLAUDE order), and every env-first reader that cannot pin its root directly holds it too (the transcript_definitively_absent family in main.rs — including the two gate-failing deferral tests — and the list_claude_sessions fixtures in session_directory). Readers that pin roots directly (sessions_tests, existence) need nothing; grep-verified no same-target mutator remains outside the lock. All assertions untouched; guards only. Verification: claude_unreadable 2/2 and the native lane 35/35 solo; the full --bin freshell-server lane run repeatedly under ambient host load (load average ~42, mirroring gate contention): 3 consecutive zero-failure runs (1128 passed each); the only failures observed in any run were the previously receipted rotating load-timing family (undelivered_failures… 60ms retry-floor window, worker_contention — pre-existing at base per the gate-g2 stash-compare attribution, isolation-green, deferred as a follow-up task), never the deferral pair. cargo fmt --check and clippy clean.
Two unnecessary-get-then-check lints in session_names_tests (contains_key) and one sync-MutexGuard held across an await in the rename intent test (block-scoped before the next oneshot). No behavior changes; the three covered tests stay green.
The Windows electron job failed our staged session-names.mjs execution
acceptance: the FRESHELL_CLAUDE_SDK_SESSION_NAMES_MODULE env carried a raw
absolute path, and Node's ESM loader only accepts file/data/node schemes on
Windows (import('/abs') works on POSIX, import('C:\...') does not). Convert
the env value with pathToFileURL(...).href at both test call sites (the
electron staging acceptance and the sidecar unit harness), matching the
caller-side conversion convention the other sidecar modules already rely on.
Production never sets this env. RED: CI job 107072714289 (windows-2022,
'stages the portable runtime from pnpm deploy exports'); GREEN: 15/15 and 8/8
locally on POSIX, Windows job re-verified by CI.
This was referenced Sep 23, 2026
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Sep 23, 2026
The T9 dual-manager hook rewrite (merged in danshapiro#820 as fb5d524) lost the '#!/usr/bin/env bash' interpreter line. Git executes hooks directly, and a shebang-less script is executed under sh/dash, whose parser rejects the hook's herestring at the manager-selection block: 'Syntax error: redirection unexpected' at line 180 — every push from every worktree failed. The regression was masked all run: core.hooksPath serves the MAIN checkout's working-tree hook, and local main was fast-forwarded to the danshapiro#820/danshapiro#819 era only after the migration branch had been pushed — so the first push after the fast-forward was the hook's real debut. The T9 hermetic tests invoked the hook via bash explicitly, which is why the gap survived review. Guard tests added to prepush-manager.test.ts: the hook must declare a bash interpreter (proven red against the shebang-less file) and stay parseable by bash -n. Pushed with --no-verify per the documented bypass: the broken hook is the artefact being fixed; the commit is the two-line shebang + guard tests, and CI validates the branch fully on the PR.
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
1 participant
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.
Unified coding-agent names
One persistent saved name per Claude, Codex, and OpenCode coding-agent session (CLI and fresh* modes), shared by the pane header, the tab (when it takes its name from the session), the sidebar/history rows, the terminal presentation, and every API projection. Renaming from any surface — UI editors and menus, the session/terminal/pane/tab routes, CLI verbs, MCP actions — writes the same durable record everywhere. Only Rename is exposed (no generate/reset/custom-label controls). Non-agent panes and excluded providers (Gemini, Kimi, Amplifier, and Kilroy despite its Claude runtime) keep their existing naming behavior end-to-end, guarded by a shared server-side kilroy-only seam.
Implements the committed plan
docs/plans/2026-09-16-unified-agent-names.mdin full: the durable cross-process name authority (session-names.json, strict sidecar-locked transactions with generation fencing and pre/post-replace failure classification), verified-persistence-only pending→durable binding, bounded automatic generation (3 starts, 30s/5min retries) with Freshell short AI titles over provider titles, native title ingestion + writeback for all three providers with the documented sync-status limitations, the stable tab nameSource lifecycle, and the deterministic one-time legacy consolidation with immutable backups.Verification receipts
full-suite|661c91f5ba7e5add0846bc4c6eb048dcaa45fb4a|dirty:0(the tip commit adds two mechanical clippy-lint fixes on top; same tree content otherwise).Notes for reviewers
--bin freshell-serverlane under extreme host load (passes solo) — ifrust-gatehits one, isolation reruns or the documented bypass path apply.