mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
460308241d61adfcbb6ed9469b236dec31ae4164
1659 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
460308241d |
fix(tools): lower working directory and workspace into fs tool descriptions
The process cwd was nowhere in the model's context: shells start in the inherited process cwd (spawn_group_leader passes no cwd), relative file paths resolve against it, but nothing told the model where it was standing — in stock Docker every shell ran in /data while user files sat in the /workspace mount, and the model's only recourse was to probe with pwd (#857, #833). Lower both facts into the tool schemas, where they gate intrinsically on tool availability (a persona without fs tools carries no note, and coordinator envelopes are untouched): - tools/*.json: cwd_note/workspace_note metadata templates on bash, read_file, write_file, edit_file, search, diff_file; bash also states the fresh-shell-per-call semantics (cd does not persist) and drops a stale reference to the removed man tool. - tools.apply_cwd_context(): renders the notes into descriptions; deep-copies noted tools (the fs dicts are shared across TOOLS/INTERACTIVE_TOOLS/TASK_AGENT_TOOLS and aliased through merge_mcp_tools), passes note-less tools through by reference. - ChatSession._apply_cwd_notes(): wraps every fresh interactive build of _tools AND _task_tools (construction, MCP catalog change, MCP disconnect) — assignment-time, so the wire tools block stays byte-stable for provider prompt caches. os.getcwd() is OSError-guarded (MCP rebuilds run on a background thread; eval tears down its workdir); the workspace hint drops when the dir is missing or equals the cwd. Task-agent sub-agents carry their own notes via _task_tools, independent of parent persona visibility. - config.get_workspace_dir(): [tools] workspace_dir with TURNSTONE_WORKSPACE env fallback (searxng pattern), informational only — no chdir, no path confinement (per-workstream working-dir grants are a separate planned feature). - Dockerfile: ENV TURNSTONE_WORKSPACE=/workspace so stock deployments surface the mount with zero operator config. - docs/docker.md: document the /data working directory, the working_dir: /workspace compose override as the operator-level fix, and the SQLite-fallback-DB-in-cwd caveat. Closes #857 |
||
|
|
83277a4d17 |
fix(console): validate project pick against rebuilt choices
The launcher project picker restored `previous` unconditionally after a choices rebuild: a since-deleted project landed the select on a blank selectedIndex=-1 instead of the "No project" placeholder (submit was safe — getOptionValue returned "" — but the select looked broken). Route it through _restorePick like the other three pickers; the "+ New project…" sentinel stays excluded (it is a command, not a state, and it IS in choices so validity alone would not exclude it). |
||
|
|
1d144c331b |
fix(ui): apply round-3 review findings (composer caches)
- ui: _paintFromCache returns its async-repaint promise; _paintProjectPicker routes through it (fork/hint stay bespoke) and the dashboard chains an Options-chip recompute on EVERY paint — an async repaint can drop a server-removed pick (or revert persona to its kind default) without firing 'change', and the chip must always name what submit will send - ui/console: the false "never worse than the pre-cache behavior" claim replaced with the accepted-tradeoff ruling for module-load failure (no per-picker retry — cache-busted re-imports split-brain the cache; no inline-fetch fallback — that resurrects the deleted dual path) - console: _paintHomeFromCache collapses the four verbatim _refreshAndPopulate* wrapper bodies; _restorePick collapses the four preserve-pick blocks (persona keeps its kind-default revert, now pinned by a test) - models/skills: drop the consumer-less loaded/error readers from the modules + bridges (same omitted-not-exposed doctrine as onChange; projects/personas keep theirs as pre-existing public surface) - tests: boot-order guard pins ALL FOUR data-layer module tags before shell.js (the boot anchor) in both index.html; wrapper/project-picker guards redirected to the chokepoints; chip-recompute chains asserted |
||
|
|
0d8be572c7 |
fix(ui): apply PR #869 review findings (turnstone + copilot)
- list_cache: null-prototype _byKey — a row keyed "__proto__" swapped the
map's prototype via the inherited setter, and getByKey of inherited
members ("toString", "constructor") resolved them as rows; + guard test
- list_cache: document why _pending clears BEFORE the trailing refresh
(a .finally clear would coalesce a late force onto a stale fetch —
declines the reviewer's .finally suggestion with the ruling in-code)
- list_cache: extra() accessor doc reflects the conditional reset;
resetExtraOnError @param notes it is moot without extraDefaults
(declines per-module knobs in personas/skills, which have no extra)
- ui: extract _paintFromCache — sync-mirrors-freshOnOpen /
async-always-fresh:false now encoded once for the model/skill/persona
wrappers and asserted at the chokepoint
- ui: replaceChildren() for the model/judge/skill picker clears
(consistency with the persona/project populates)
- console: reword the skills fail-open comment to unambiguous past tense;
drop the orphaned _resolveModelLabel docstring
- tests: fork-gate asserts require each paint to open its own
`if (!_forkFromWsId)` block (the rfind+50 window false-passed a closed
gate; the model first-gate check was vacuous; the persona gate was
unasserted); drop one redundant `0 <=` (kept where it guards find()==-1)
|
||
|
|
c3beb202eb |
fix(ui): apply round-2 review + fix-sanity findings (composer caches)
An unprimed convergence re-review found a real login-recovery seam gap plus cleanups (round 1's fix round manufactured one of them); fix-sanity vetted the plan. - console onLoginSuccess recovery seam [0]+[2]: it re-warmed only skills+models after an in-place login; projects+personas (same pre-auth-401 gap) stayed empty (rail group-by-project flat, saved-coordinator raw slugs). Now force-refreshes ALL FOUR caches on login — force so a still-in-flight failing pre-auth fetch yields a trailing AUTHENTICATED refetch rather than coalescing onto the 401 (skills/personas have no *_changed event to recover). Threads an optional callOpts through the four cache modules + console wrappers (backward-compatible; every non-console caller passes nothing). - fork skill paint [4]: the round-1 wrapper extraction left the modal skill paint unconditional on a fork (wasted GET /v1/api/skills + hidden-select rebuild); fork-gate it like model/persona/project. - persona wrapper [5]: extract _paintPersonaSelect so all four composer pickers share the sync-then-refresh wrapper instead of persona being inline-duplicated. - dead machinery [6]: remove the zero-subscriber onModelsChange/onSkillsChange and the models fpExtra fingerprint fold (and the now-orphaned core fpExtra branch). The console repaints models via its direct models_changed handler, not a subscription; the fold only fed the subscriber-only fingerprint. - O(1) modelLabel [7]: index the models cache by alias (keyField) so modelLabel is a getByKey, not a per-paint scan. Declines documented in-code: forks-inherit-model [1] (deliberate) and the fail-open cache [3] (intended, same policy as projects/personas). Deferral comment at the ui onLoginSuccess twin (recovers on dashboard re-focus; follow-up). Tests: rewrote the 7 guards the code changes moved (persona relocation, callOpts threading, force, fpExtra removal) preserving their ordering intent, and added fork-skill-gate, persona-wrapper, all-four-force, keyField, and onModelsChange-removed coverage. 106 pass; ruff + mypy green. |
||
|
|
8d57697b2a |
fix(ui): apply max-effort review + fix-sanity findings (composer caches)
A max-effort review of the composer-cache branch found 3 correctness + 2 cleanup
issues; fix-sanity refined the plan before implementing.
- Modal select stickiness [0]: the reused new-ws <dialog> kept the last open's
model/judge/skill pick and silently applied it to the next chat (sharp for a
fork — model/judge were sent unguarded). The composer selects now render fresh
each open (a fresh open has no `previous` selection to preserve) across ALL
five selects, while a within-open async repaint still preserves a mid-window
pick. The modal now shows the resolved default ("Default — gpt-5"). A fork
INHERITS its source's model + judge (hidden + submit-gated on !_forkFromWsId,
matching skill/persona/project).
- models default-alias reset [1]: the shared core's extra-reset-on-failure is
now opt-in (resetExtraOnError). projects keeps it (require_project gates the
picker, must fail open); models opts out, so a transient failure keeps the
last-known resolved-default annotation instead of blanking it.
- models_changed coalescing race [2]: an opt-in trailing refresh in the core —
a force caller (models_changed) awaits a refetch chained after the in-flight
one and converges to the latest state instead of a response predating the
change; startup/open callers stay coalesced.
- cleanups: the 4x paint-then-refresh block collapses into _paintModelSelects /
_paintSkillSelect [6]; the "alias (model)" label centralizes into models.js
modelLabel (registered on the window bridge) [7], deleting both local copies.
Tests: rewrote the 5 guards that pinned pre-fix literals + added fresh-matrix,
fork-inherit, bridge-registration, both-error-branch reset, and trailing-refresh
guards. 106 pass; ruff + mypy green.
|
||
|
|
71b1365e7b |
refactor(ui): shared list-cache core + models/skills caches (no composer FOUC)
The model and skill composer pickers had no client cache: the new-ws modal, the dashboard quick-create, and the console launcher each re-fetched /v1/api/models and /v1/api/skills inline on every open, flashing an empty dropdown for the round-trip even though the data was usually already in memory. Add shared caches (models.js, skills.js) the composers read SYNCHRONOUSLY, then refresh-and-repaint — the pattern the project/persona pickers already use. The coalescing / fail-open refresh / change-detection / window-bridge machinery was ~70% duplicated between projects.js and personas.js. Extract it once into list_cache.js (makeListCache) and retrofit projects.js + personas.js onto it, preserving their full public surface byte-for-byte (rail.js + project_creator.js import them by name; the classic bundles read the window bridges). The require_project advisory rides projects.js as fail-open `extra` state; personas keep their kind-filtered choices and name->label map. models.js carries BOTH server schemas (the node sends default_alias, the console sends coordinator_default_alias; both send judge_default_alias) so each app reads its own, and folds them into the fingerprint so a role-alias change still fires onChange. skills.js returns raw rows (the ui pickers add a " [MCP]" suffix the console omits). Selection is preserved across the sync->async repaint on every select, including model + judge independently. Also: the dashboard model/skill fetch-once guard is dropped (refresh-on-open now, matching project/persona); the console re-warms models on login too (the boot pass runs pre-auth, so the dropdown used to stay empty until a reload); and models_changed repaints via the single refresh wrapper (no double path). |
||
|
|
a23cc2c25e |
ci: remove claude workflows
The @claude mention responder (claude.yml) and the automatic PR review (claude-code-review.yml) have been unreliable and are a frequent source of CI breakage. Drop both; core CI (ci.yml, docker-publish, publish, understone-example, vendor-js) is untouched and nothing else in the tree references them. |
||
|
|
079257967d |
refactor(ui): extract _paintProjectPicker (dedupe modal/dashboard)
Review of #868 flagged the sync-paint + refresh + required/optional hint block as copy-pasted between showNewWsModal and _loadDashboardOptionsLists, already diverging structurally, so a future tweak could drift and silently re-introduce the FOUC on the missed surface. Collapse both into a shared _paintProjectPicker(sel, hint, {fork}) -- the modal passes the fork flag, the dashboard never forks. Guards re-pointed at the helper + a new one pins its sync-before-async pattern. |
||
|
|
4079542447 |
fix(ui): paint composer project/persona pickers from warm cache (no FOUC)
The new-workstream modal, the dashboard composer, and the console launcher painted their project and persona <select>s only inside the async refresh().then(...) callback, so each open flashed an empty/stale dropdown for a network round-trip even though the client caches are already warmed at startup. Paint synchronously from the warm cache first, then refresh-and-repaint (still catches items created elsewhere). On a cold cache the sync paint is a no-op the async fills, so it is never worse than before. Both project paints reuse the same _populateProjectSelect + reconcile, so the require_project strict-picker invariant (never auto-select a real project into a possibly-shared one) is unchanged; persona reuses _populatePersonaSelect, which preserves a mid-window pick and only applies the kind default when nothing valid is selected. Also folds in two deferred require_project polish items in the same code: the dashboard Project label now shows the "required"/"optional" hint (parity with the modal), and _reconcileRequiredProjectSelection reuses the projectChoices() list its caller already built instead of recomputing it. Models/skills selectors are a separate follow-up (no client cache today). |
||
|
|
8ce94360ae | docs(projects): requireProject() advisory is safe to read synchronously | ||
|
|
c019ab41d7 |
feat(server): opt-in server.require_project gate
Add an opt-in, default-off `server.require_project` setting. When an admin enables it, creating an interactive chat is refused unless it is filed under a project. The feature is inert and byte-identical when off, and can only ever fail toward "off" (a missing config store or unset key reads as disabled). - settings_registry: server.require_project (bool, default False, live read). - auth: require_project_enabled + require_project_denies_create predicates (service scope / coordinator token_source exempt; NOT admin.coordinator), plus REQUIRE_PROJECT_ERROR / REQUIRE_PROJECT_CODE. - node create gate via a declarative cfg.create_gate_require_project (wired True on the interactive mount only; coordinator spawns stay ungated). - fork/resume: a fork's project is structurally its source's. Any explicit project_id is discarded, so a fork can never be re-filed under an unrelated project (which would move its copied history across a tenancy boundary). Inaccessible / projectless / nonexistent sources are uniform on body and status, so there is no cross-tenant oracle. - console cluster-create proxy surfaces only the coded require_project 400 and masks every other node outcome (401/429/3xx/5xx, un-coded 400) to a sanitized 502, guarding both body reads. - list_projects advisory field + projects.js requireProject() (fail-open). - fresh-create project picker requires an explicit project choice under the flag (no silent auto-select); forks hide the picker (inheritance is server-enforced) and get an accurate refusal message. - tests: predicate matrix, resume-inheritance oracle discriminators, console masking, and end-to-end node-gate mount wiring. |
||
|
|
cdc360bd98 |
fix(server): sanitize the retry closure's error display
The retry (_run) closure emitted the raw str(exc) to ui.on_error, so a credential-bearing base-URL in a backend ConnectError (https://user:pass@host) crossed into the dashboard SSE — the confidentiality floor _record_fatal_error enforces, bypassed here. Sanitize the display inline with the same sanitize_error_text redactor. This is separable from the reused-session stale-flag hazard that keeps _run off ensure_error_recorded: that hazard is about recording / idempotency (deferred to #865); this is only the display string. The double state emit and the pre-try no-persist remain in #865. Adds a focused test that a retry-error's on_error is redacted. Flagged by review on #866. |
||
|
|
3af80907c7 |
fix(server): init-message worker exits to error, not idle, on first-turn failure
The initial-message worker (_run_initial) collapsed both cancel and backend-error exits into one `except (Exception, GenerationCancelled)` arm that always stamped state=idle, clobbering the state=error that session.send's _record_fatal_error had persisted+emitted. A spawned child's first-turn backend failure (unreachable model server, exhausted quota, auth error) therefore read as an empty, successful turn — the coordinator's wait/inspect surface reads last_error only for state=='error' — and the real error surfaced only after a manual nudge re-ran the turn synchronously. Split the arm: cancel -> idle, exception -> error. The failed child now settles at state=error and the first wait_for_workstream returns the enriched backend error inline. Also fixes the same latent bug for scheduled tasks, which dispatch through the same endpoint and closure. A failed first turn is deliberately terminal for automated wakes: it settles to a non-ready error terminal, not the idle ready-set that timer/watch wakes recur to, so explicit user/coordinator action reactivates it rather than a silent auto-retry (a self-healing wake-from-error would be a separate wake-gate change). The exception arm routes through a new ChatSession.ensure_error_recorded: a no-op when send already recorded the error in-line (the common backend-boundary path — no duplicate state emit), and the recorder when a pre-try exception (model-registry refresh, user-turn append, system-message recompose) bypassed send's own handler, so state=error always carries a meaningful last_error. Its idempotency guard (_has_persisted_error) is session-lifetime, so ensure_error_recorded is scoped to _run_initial's FRESH first-turn session only; the docstring spells out why a session-reuse caller (retry, /send, coord send, wake) must not route through it until the per-turn error-recorded signal of #865 lands. The other half of making an errored workstream cheap for a model to handle is a stable identifier: the enriched backend error now leads with the model ALIAS the coordinator references everywhere (list_nodes, spawn) and annotates the backend id for the operator — "model=DeepSeek-V4-Flash (id=deepseek-v4-flash)" — so a model routing around a failed model correlates it against those surfaces without a lookup, instead of burning reasoning tokens reconciling the alias against a backend id it never sees anywhere else. Collapses to one token when the alias and id coincide. Tests (TestInitialWorkerFailureState) assert the coordinator-visible manager state and the persisted last_error across the matrix — common- backend and pre-try errors both settle error with a readable last_error; cancel-to-idle settles idle with no error recorded. De-forks the create-app fixture and uses the shared monotonic wait_until helper. The completion-notification honesty surface and the error-recording hygiene of the other send-worker closures (retry / main send / coord send / wake) are deferred to #865. |
||
|
|
9dea2c89f1 |
chore(sdk): regenerate openapi-console.json
The console OpenAPI spec had drifted from build_console_spec(): the committed file was last generated at 1.7.0rc1 and was missing the persona and project_id workstream-creation fields (Personas and Projects, both 1.7) plus the version bump to 1.8.0a2. Regenerate via sdk/typescript/scripts/generate-types.py to resync. Spec-only; no console API behavior change (openapi-server.json was already current). |
||
|
|
515d372a14 |
fix(compaction): validate retry_in backoff before rendering the retry note
updateCompactionProgress coerced evt.retry_in with Number() and rendered it unguarded, while the sibling part/total path two lines below is finiteness- validated — a malformed backoff would render "retrying in NaNs". Validate retry_in the same way (finite, non-negative), and keep the error text regardless: the error is the load-bearing half of the note, so an unparseable duration drops to "retrying (error)…" rather than suppressing the whole arm. Addresses PR review feedback on the compaction reducer. |
||
|
|
abe053f507 |
fix(compaction): review round 11 — settle-helper null-guard to the chokepoint
The settleSendResponse extraction left the two panes' call sites diverging on
the null-guard: interactive passed bare `data`, the coordinator passed
`data || {}` — reintroducing the copy-paste variation the shared helper existed
to erase. If a /send 2xx body were ever non-object JSON, the unknown/"ok"
fall-through would deref `data.attached_ids` and paint an already-delivered
message as a connection error; the endpoint always returns an object, so this
is a latent divergence, not a live bug.
Normalize the body once at the helper entry (`data = data || {}`) so both call
sites pass bare `data` and stay byte-identical, and every internal deref plus
any future caller is covered by the single chokepoint. The node settle-harness
gains a null-body case — red without the fix, since the call-arg evaluation
throws before the stub runs.
|
||
|
|
1e86f068cd |
chore(compaction): review round 10 — cleanups from the first correctness-clean round
- INTERJECTION_CAP_CHARS joins PENDING_SENDS_MAX in workstream.py: the 2000-char interjection cap was triplicated (queue_message's truncation, the defer-fidelity refusal, the test fake) and already drifting in measurement — the defer check deliberately measures RAW text (raw >= cleaned since parse_priority only strips, so it can only over-refuse into a full-fidelity fresh spawn, never admit a truncation), now stated in a comment. The four unrelated 2000s (notify tool, recall preview, summary formatting, agent step cap) stay deliberately unlinked — they are different contracts. - The changelog's ~110-line compaction bullet is split into six per-seam bullets matching house style, and the Breaking (1.8) compaction-event notice moved under "### Changed" where integrators scanning bullet heads will actually see it (cross-referenced both ways with the pre-1.8 embedder compat bullet). - SpawnMetricsHook takes (ui) only: the request parameter was threaded through the whole dispatch-attempt path solely to be ignored by both installed impls; the stale "coord wires None" claims in the rewritten comment blocks are corrected too. - The attachments tests' Mock-hardening block lives once in _harden_ws_mock() — deliberately excluding _worker_running, which each fixture chooses per scenario (one relies on the truthy auto-Mock). - Two hand-rolled poll loops become wait_until (file convention, diagnostic timeout) and the orphaned time import goes with them. |
||
|
|
d280db514e |
fix(compaction): review round 9 — drain-exit ownership, missed-edge settle, pre-turn hook guard
Three point-guards from the ceiling round (no primitive took a hit;
correctness yield halved at identical review sensitivity):
- The drain's clean-exit wake moved OUT of the function-level try: it
runs after the drain has already retired its slot, so a raise out of
the wake (the dispatcher re-raises Thread.start failures) could reach
the last-resort handler and clear a slot this thread no longer owned —
nulling a successor drain's live registration and letting two drains
service one list. The wake now runs post-try under its own guard
(mirroring _retry_pending_wake), only on the clean-exit path, and the
last-resort slot-clear is identity-guarded like every sibling exit
seam. The except arm needed a function-local threading import: the
module-top import is TYPE_CHECKING-only, so the guard would have
NameErrored inside the handler with strict mypy fully green.
- The shared settle helper promotes a non-deferred chip that binds onto
an already-idle pane: its only sweep fired mid-POST (unbound then) and
no message_dispatched ever comes for non-deferred sends, so the chip
stayed a permanently retractable "queued" bubble for a delivered
message. Keyed on post-bind chip state (also catching a raced folded
settle bind just reconciled) and skipping dismiss-in-flight chips —
the sweep's own aria-busy discipline. Pinned behaviorally: the helper
now executes under node (a 4-row missed-edge matrix), possible since
the consumer-less window bridge is gone.
- _claim_generation's on_generation_claimed emission is call-guarded:
it sits on send()'s pre-turn path, before the user turn is appended
and before the fatal handler's coverage, so a raising override
degrades to a lost latch-break instead of silently dropping every
user message on that session.
Cleanups: /command's transport catch and status-less non-2xx bodies are
loud now (threading {ok, status} through the parse — deliberately no
throw-on-!ok pre-gate, since the busy and error arms ride 409/503);
PENDING_SENDS_MAX lives in workstream.py and ChatSession._QUEUE_MAX
aliases it (one backpressure bound, structurally incapable of
diverging); the send handler's not-ok arm uses _queue_full_response();
the dead window.createQueueController bridge is deleted and the file
header's consumer map corrected.
|
||
|
|
1224b02d03 |
fix(compaction): review round 8 — seam obligations become primitives
Eight rounds of findings against the defer-and-drain seam shared one
generator: N sites each hand-copying M obligations (spawn discipline,
the order-barrier pair, backpressure, best-effort emission, the client
settle matrix), with every review finding an empty (site x obligation)
cell. This round makes each obligation a single primitive:
- The order barrier is Workstream.send_barrier_active() — one
definition of the two-term pair (pending entries OR drain alive),
consulted by the /send route, the coordinator adapter, and the
queued-nudge wake gate, which previously carried only the list term
and let a synthetic wake jump an acknowledged send during the
claimed-entry window. _PendingSend moved to workstream.py beside the
invariant that justifies the drain-alive term; the pending fields got
precise types and worker_kind became a Literal, so a typo'd
"command" comparison is now a type error instead of a silently
never-firing defer guard.
- _defer_send probes the barrier before constructing anything, bounds
acceptance at 10 pending (the interjection queue's own backpressure
contract — unbounded acceptance pinned message + attachment bytes
per entry for a whole command window and then ran one unattended
turn each), and spawns the drain with rollback: a Thread.start
failure pops the just-accepted entry and answers the retryable
queue_full instead of 500ing after registration (a phantom the
client could neither see nor retract, dispatched later as duplicate
turns). start() deliberately stays inside the lock, unlike
session_worker's outside-lock discipline: this slot is
is_alive()-gated, false for a constructed-but-unstarted thread, so
an outside-lock start would open a double-drain window.
- A /command whose worker never spawned answers 503
{"status": "error"} (spec + docs + a pane error arm) instead of the
generic 200 ok that told SDK callers their /clear ran.
- The compaction lifecycle emitter is raise-proof at its single
dispatch tail: a raising duck-typed hook degrades to a lost render,
never a lost end event — previously a raising on_error or a raising
failed-end emit left every pane a frozen progress bar, and a raising
SUCCESS end after the committed swap fabricated a failed end.
- The client settle matrix lives once: composer_queue's
settleSendResponse owns every /send response arm for both panes
(the near-verbatim twins were already drifting), parsePriority is
shared, and the busy stamp is centralized in setBusy(b, source) with
"server" as the fail-safe default. Deferred sends release the
composer (no worker exists for them; retracting the chip no longer
strands the pane in Stop mode), queue_full on an idle-looking pane
removes the optimistic bubble and restores busy (the refusal can now
fire with no worker and no drain to ever emit a state event), and
the pre-bind settle buffer is TTL-based — a burst of deferred
dispatches parked this tab's own raced settle first, where the old
size cap evicted exactly it.
- The command backstop / console proxy timeout inequality is enforced
by a test importing both named constants (both proxy_client
constructions, startup and the mTLS re-create); the compaction card
wears blue (magenta is reserved for the MCP surface); the redundant
TerminalUI.on_compaction override is gone (the inherited protocol
default is the policy site).
|
||
|
|
5511ab9a35 |
fix(session-worker): release the slot claim when Thread.start itself fails
If thread creation raised (thread exhaustion, MemoryError), the dispatcher had already claimed the worker slot under ws._lock — but the flag's only clearer is _runner's finally, on a thread that never started. The workstream then looked idle forever (no state change ever fired) while every subsequent dispatch took the reuse path into a queue no worker would drain, until an operator force-cancel. Roll the claim back under the lock (identity-guarded, like _runner's own clear, so a concurrent force-cancel's successor is never clobbered) and re-raise. Re-raise rather than return False: callers' crash paths — the deferred-send drain's per-iteration handler with its backoff — are shaped for exceptions, and a False would masquerade as queue-full backpressure and mislabel the wake gate's refusal log. worker_kind is left stale, as documented (every reader conjoins _worker_running). Affected every dispatch path: sends, wakes, retries, the deferred-send drain, and workstream init. |
||
|
|
fd5d3efb43 |
fix(compaction): review round 7 — drain crash/order/settle rows, protocol-default fallback, bool event-id guard
Completes the defer-and-drain seam against the matrix rows round 6 never
enumerated (the defer contract itself took no hits):
- Crash row: a claimed entry survives a dispatch crash — the
per-iteration handler re-inserts it at head (claim-flagged so a
claim-section failure can't duplicate it), backs off ~1s, retries.
The last-resort handler spawns no successor (Thread.start fails under
the exact exhaustion that reaches it): the route's ensure-drain stays
the single spawn site, so single-flight is structural and a dead
drain revives on the next defer.
- Order row: the pending list is the order authority. The /send route
pre-checks pending/drain-alive under the same lock acquisition that
appends (one _defer_send helper serves the barrier and command-window
triggers); the coordinator adapter refuses via its return value; the
queued-nudge wake gate yields to pending sends and is re-armed by the
drain's clean exit — which covers lists emptied by pure retraction —
as well as every deferred turn's exit; retry-after-rewind is a
documented accepted overtake; init/create is fresh-ws-by-construction.
- Client settle row: queued responses carry "deferred": true
(SendResponse + regenerated openapi-server.json, status enumeration
completed); bind(el, msgId, {deferred, attachedCount}) replaces the
_deferredAttachments expando; the idle sweep skips deferred and
unbound chips; the shared dispatch attempt emits pane-tier
message_dispatched (folded: true for interjection fold-ins — the chip
clears only its deferred flag and keeps a live x while DELETE still
genuinely retracts); settles that beat bind() park in a bounded
buffer; an idle-thinking pane retro-converts its optimistic bubble
into a real queued chip instead of presenting a parked message as
sent. Rejection polling waits on the slot flags — one dispatch
attempt per slot-state change, not 4 Hz.
- SessionUI.on_compaction's protocol stub became a real default body
(the classic on_info rendering): explicit subclasses inherit protocol
members as real methods, which defeated _compaction_event's getattr
fallback for exactly the pre-1.8 embedders it serves.
- _coerce_event_id() rejects bools (isinstance(True, int) is True) at
all three duck-typed event-id coercions: the compaction marker stamp,
on_system_turn's persisted return, and _ui_event_id.
- Quick-command backstop 60s -> 25s, under the console proxy's 30s so
the degraded "running" answer can traverse a proxied pane (which now
surfaces it); /resume docs drop the fictional history SSE event
(clear_ui + REST re-fetch is the contract); /send response docs match
the wire.
|
||
|
|
e99673eb0c |
fix(compaction): review round 6 — defer-and-drain send windows, workstream-scoped notify, ERROR badge survives /compact
Replace park-and-abandon /send semantics with defer-and-drain: a send
landing in a command window is answered {status: queued, msg_id}
immediately and dispatched full-fidelity by a per-workstream drain
thread when the window closes. Parking encoded client disconnect as
message retraction — true only for the composer's ✕-abort; every
bounded caller (coordinator client and console proxy at timeout=30,
SDKs, stock proxies) timed out and lost its message for the whole
window, and the compensating client machinery was racy (one-shot
sendAbortMs sample) and over-broad (_sendAbort fired on the
interjection path, dispatching dismissed messages while showing a
connection error). Dismissal is now uniformly bind() → DELETE, with a
fall-through that retracts pending entries; retracting an
attachment-bearing deferred send surfaces the discarded-attachments
consequence. The drain claims entries under ws._lock immediately
before dispatch (DELETE can never remove an in-flight message),
refuses the truncating interjection fallback for oversized or
attachment entries atomically inside the enqueue callback, and never
gives up while the workstream lives; durability is documented as
node-local at-most-once. sendAbortMs, _sendAbort, the 600s bound and
the park loop are deleted; route and drain share one dispatch
implementation (spawn metrics included).
Also: the initial-send completion notify is un-gated from slot
ownership (_fire_notify_targets has exactly one call site — successor
turns never notify, so the round-5 guard prevented a duplicate that
cannot exist while converting force-cancel into permanent notification
loss for scheduled workstreams); /compact on an ERROR workstream
restores the badge instead of stamping idle over it; duck-typed
SessionUIs without on_compaction get the classic on_info lines back
via a shared renderer (superseded OK ends included — a committed swap
must never be silent; pre-1.8 SSE clients are deliberately not
dual-emitted, documented as a 1.8 breaking change); failed-end notice
suppression is computed once by the emitter as a notice bool on the
end event (SDK py+ts), replacing the hand-synced cli/JS policy while
the panes keep their pane-local card-ownership clause.
|
||
|
|
1dbf7f410c |
feat(compaction): lifecycle events, web progress card, history re-render
Compaction becomes visible: a first-class 'compaction' SSE lifecycle (start/progress/end, compaction_id-correlated, superseded-flagged ends) replaces the loose info lines; both web panes render a progress-bar card that settles into a persistent result card, re-rendered after reload via the /history projection of the compaction marker row. Slash commands echo as command chips instead of fake user turns. The enabling rework: /command dispatches onto the workstream worker slot (the old inline path blocked the node's event loop for whole compactions and let /clear interleave with live turns). Busy refusals answer 409; quick commands are awaited loop-natively with a 60s backstop; /compact is fire-and-forget. Sends during a command window park in the /send route and dispatch full-fidelity afterwards — the interjection queue (length cap, cross-user guard, identity-swap hazards) is unreachable there — with a compaction-aware client abort bound shared by both panes. compact_now() carries send()'s full generation discipline; Stop aborts the in-flight summary HTTP stream via a generation-scoped cancel ref; force-abandoned compactions retire at their next checkpoint and their stragglers are fenced off every surface (panes, pill latch, CLI). Every session retry backoff is cancel-aware via one shared helper. Docs, OpenAPI spec, and both SDKs updated. Verified: 9457-test non-live suite, JS pin suites, headless-Chrome reducer harness; five unprimed multi-agent review rounds (correctness trend 15/6/6/4/4) with plan-level design passes on every fix round. |
||
|
|
82676080a4 |
fix(session): describe skill selections accurately
Use neutral "set" wording for operator skill markers so re-selecting the current skill does not falsely claim a change. Update the regression expectation for the persisted marker. |
||
|
|
a64cd25807 |
fix(session): record operator skill changes
Operator-driven /skill changes were only shown in the UI, leaving no trajectory marker for the model. Persist a system turn for named skill changes and clears, with regression coverage for both paths. |
||
|
|
8240d2c00d | chore(deps): update helm release postgresql to ~18.8.0 | ||
|
|
8f8c2f4ca3 | chore(deps): update github actions | ||
|
|
7b1f77dda7 | chore(deps): lock file maintenance | ||
|
|
72229cac26 | chore(deps): update ghcr.io/astral-sh/uv docker tag to v0.11.29 | ||
|
|
09c5475b0e | chore(deps): update actions/setup-node action to v7 | ||
|
|
0e6a99e0f1 | chore: bump version to 1.8.0a2 v1.8.0a2 | ||
|
|
84577ee530 |
fix(mcp): PR #844 review — correct success docstring, back out the dead admin pill
Copilot review feedback (all three valid): - _record_refresh_success docstring still claimed the push-driven single-kind refresh calls it — round 8 deliberately stopped that (a single kind can't declare a server-scoped 'ok'). Docstring now states the full-pass-only contract and points at the push path's _record closure for why. - The admin refresh pill's skipped-tint logic (admin.js) and its .mcp-refresh-pill-skip CSS were dead code: /v1/api/_internal/mcp-status strips last_refresh_at/last_refresh_outcome via the read-scope projection, so admin.js never sets newestRefreshAt and the pill block never runs. Backed both out; the whole pill fix (whitelist the fields with a read-scope-coarsened outcome, THEN the color logic + CSS) now lives in #843. The CHANGELOG's false 'the admin console's refresh pill paints…' claim is dropped — the /mcp refresh CLI and the 202-skipped endpoint (which read last_refresh_outcome directly, not via the strip) still work and remain documented. The 5 github-code-quality 'statement has no effect' comments are the known PR #840 false-positive class (the scanner reads 'await <name>' as a valueless expression); each flagged await is load-bearing (drains a parked runner so the next assertion is non-vacuous, delivers a cancellation, or awaits a _noop to fabricate a done owner_task) — no code change. Refs #839, #843 |
||
|
|
80e7b9e9ca |
fix(mcp): review round 8 — push-success can't declare health, first-notify never debounced
- A single-kind push SUCCESS no longer clears the server error pill or stamps 'ok': _last_error / _last_refresh are server-scoped but a push refreshes only ONE kind, so a tools-failing server must not go green because its prompts push succeeded (a wrong-healthy window, bounded by the health tick — but a real 200-OK lie). Only a full pass declares 'ok'; the failure's armed health-tick retry runs it. This reverts the over-reach of round 7's push-success outcome write (a self-inflicted regression) — net simpler. - The (server, kind) debounce uses a None sentinel, not a 0.0 default: time.monotonic() counts from boot, so on a node whose process started < _NOTIFICATION_DEBOUNCE (5s) after boot, the 0.0 compare would debounce the VERY FIRST push — dropped with no recovery on the pool path. Absent stamp = never refreshed = always admit. - _record_refresh_skipped completes the outcome-helper set: the three inline 'skipped' stamps now share one config-gated helper (with _record_refresh_success / _record_refresh_failure), and the reconnect-success branch routes through _record_refresh_success — no more hand-copied gates to drift. - The per-message refreshers dict + on_debounce_drop closure are built ONCE per handler (both static and pool), not on every server->client message before the isinstance/debounce/coalesce early-returns. Accepted (documented): an operator /mcp refresh that finds the connect lock busy skips + arms the retry rather than waiting (waiting re-introduces the refresh-budget exhaustion busy-skip exists to prevent). 4 findings refuted. Suite 9408 green. Refs #839 |
||
|
|
52dcb6a47b |
fix(mcp): review round 7 — consolidate the refresh-outcome write path
All three round-7 findings shared one root cause: last_refresh_outcome (the single source of truth for the CLI / endpoint / admin pill) was written inconsistently — ungated writes scattered across _refresh_server and _refresh_all, never written by the push path, never popped on removal. Consolidate every static outcome write through two config-gated helpers so the invariant holds: _last_refresh[name] exists IFF the server is configured and has a real outcome. - _record_refresh_failure now stamps the (config-gated) error:<Class> outcome; _record_refresh_success is its twin (gated ok stamp + pill clear). The ungated writes inside _refresh_server (both the internal error write and the success write) and _refresh_all's except are removed — routed through the helpers. A failure observed for a just-removed server no longer leaves a permanent stale error: row. - The push-driven refresh path (_run_static_notification_refresh._record) now records the outcome on BOTH success and failure, not just the error pill — a green 'ok' outcome no longer persists under a red error row after a push fails, and a successful push clears a prior error. - remove_server_sync pops _last_refresh (via _clear_static_push_state markers=True); a session drop KEEPS it (the outcome persists across a reconnect — only removal clears it). The removed-mid-pass branch drops any stale row too, so last_refresh_outcome doesn't report a departed server's prior 'ok'. _reap_bounded's pending-task concern was reviewed and REFUTED (a pending child on external cancel during shutdown is correctly left to loop teardown). Declined the per-notification refreshers-dict allocation cleanup: trivial (a 3-entry dict on a rare debounced path), and the late binding is deliberate for test overrides + mypy attribute checks. Tests: push-refresh success/failure write the outcome, removal pops it, session drop keeps it, failure for a removed server leaves no stale row; the 3 TestLastRefreshTracking tests updated to the split contract (_refresh_server propagates, the caller records). Suite 9407 green. Refs #839 |
||
|
|
86aeb43120 |
fix(mcp): review round 6 — close the refresh-outcome reporting residuals
Three residual gaps in the round-5 skip-outcome threading, all in _refresh_all's other reconnect branches plus the endpoint ordering: - The disconnected-server reconnect DEFERRAL (_ensure_static_connected returns None: a sibling call in flight on the old stack, lock not held) returned None without stamping 'skipped', so the endpoint and pill read the STALE prior 'ok' and reported a never-run refresh as current. Now stamps 'skipped' like every other skip branch. - A server removed from config between the top-of-loop session check and the cfg lookup fell through to with results[name] UNSET, omitting it from the returned dict — an operator refreshing that one server saw a bare 'refresh complete' with no line. Now reports None so it renders. - internal_mcp_refresh_one checked 'skipped' BEFORE the error pill, so a skip on a server carrying a live error returned a benign 202 instead of 500 — a status-code-keyed caller would treat an erroring server as healthy-but-busy. Error is now checked first. - _reap_bounded swallowed an external CancelledError (shutdown / an operator cancel of the refresh runner) — it now re-raises after a best-effort exception retrieval, honouring the cancel. Dropped the unneeded asyncio.shield in the process. Tests: deferral stamps skipped, removed-mid-pass reported not omitted, endpoint error-beats-skip → 500, reap re-raises external cancel. Suite 9403 green. Refs #839 |
||
|
|
748f670fe8 |
fix(mcp): review round 5 — thread the refresh outcome to every operator surface
The 'skipped'/None refresh sentinel added in round 4 was only half
threaded: consumers still misreported it. Unify all operator surfaces
on ONE source of truth — the per-server last_refresh_outcome ('ok' /
'skipped' / 'error:<Class>') — exposed via a new last_refresh_outcome()
accessor:
- _refresh_all returns None (not ([], [])) for a FAILURE too, so a
failed refresh is never rendered as 'no changes' (the pre-#839 lie
the sentinel exists to close); None is disambiguated skipped-vs-failed
by the outcome. ([], []) now strictly means 'ran, no changes'.
- /mcp refresh renders skip ('skipped — retry scheduled') and failure
('refresh failed (error:X)') distinctly from 'no changes'.
- The node-internal refresh endpoint returns 202 'skipped' instead of a
misleading 200 'ok' for a refresh that never ran (the busy-lock skip);
it reads the outcome from the manager accessor because the public
status projection deliberately whitelists last_refresh_outcome out.
- admin.js paints 'skipped' with a neutral info pill
(.mcp-refresh-pill-skip), not the error-red any-non-'ok' used to get.
- _admit_list_changed rolls back BOTH the coalesce marker and the
debounce stamp when scheduling raises, so a same-kind push in the
window afterward isn't debounced against a refresh that never spawned
(the pool path has no on_debounce_drop recovery).
Tests: endpoint 202-skip, CLI skip/failure render, _refresh_all
failure→None + outcome, spawn-failure stamp+marker rollback. Suite
9399 green.
NOTE filed #843: the admin refresh pill's data (last_refresh_at/outcome)
is stripped by BOTH status projections and never reaches admin.js — a
pre-existing latent bug (the pill has never rendered); the admin.js
color fix here is correct-when-reachable. Out of #839 scope (the read
projection strips it for a privacy reason that needs its own coarsening
decision).
Refs #839
|
||
|
|
1a80466369 |
fix(mcp): review round 4 — removal/reconcile lifecycle, honest skip reporting, same-kind debounce recovery
reconcile_sync no longer abandons a DB-driven removal that timed out: both the removal loop and the config-update loop keep the name in _db_managed (and skip the follow-on add) when remove_server_sync returns its mutated-nothing False, so the next pass retries instead of the deleted/reconfigured server serving stale tools until restart. remove_server_sync is now cancel-safe end to end: it FORCE-drops the session before queueing (parked push runners bail at their session gate instead of serializing ≤30s list calls ahead of the removal — the noisy #839 server was exactly the one whose runners could starve its own removal), and wraps the post-lock cleanup in try/finally so a caller-timeout cancel landing mid-teardown still completes the state pop, catalog rebuild, and lock retirement rather than stranding a config-gone ghost catalog. Config survives a park-cancel, so the health loop recovers it. _refresh_all reports None (not a fake ([], [])) for a busy-skip or supersede, stamps a 'skipped' status row, and /mcp refresh renders it distinctly — the operator is no longer told a never-refreshed server is current. A same-kind push lost to the debounce window (the prior runner already finished; the server won't re-announce) arms the health-tick retry, closing the one staleness hole the per-kind debounce still had; a push covered by a queued runner does not arm (no lost change). Static resource/prompt catalogs are capped at connect discovery and every refresh. _list_resource_pair's reap is bounded so a future SDK cancel-regression can't wedge the lock. Cleanups: _arm_refresh_retry (retry-arm gate, ×3), _spawn_full_refresh (discard+spawn, ×3), _popen_mcp_server (live-server spawn, ×2), the tautological stamp-arithmetic TestNotificationDebounce deleted. Suite 9395 green. Refs #839 |
||
|
|
53f11454ad |
fix(mcp): review round 3 — cap static catalogs, atomic removal, unify the list_changed protocol twins
- Static resource/prompt catalogs are now size-capped at connect
discovery AND on every refresh (mirrors the pool twins and the static
tools path): a misbehaving server's push ran uncapped through the new
spawned refresh path and could balloon the shared node's merged
catalogs on every notification.
- remove_server_sync mutates NOTHING outside the per-name lock: the
up-front config pop meant a removal cancelled while parked (behind
the push-refresh runners that now share this lock) left a
half-removed server — config gone, session and published catalogs
alive, no driver able to reconnect or cleanly re-remove. A timed-out
removal is now honestly retryable.
- _refresh_all's DISCONNECTED branch busy-skips too (parking inside
_ensure_static_connected burned the pass's 30s budget on one
mid-reconnect server), and a busy-skip on either branch ARMS the
health-tick retry — an operator-requested refresh can no longer be
silently dropped with output indistinguishable from 'no changes'.
- reconnect_sync drops the session before queueing on the lock (FORCE
semantics already rebuilt live sessions): parked push runners bail
at their session gate instead of serializing up to one 30s list call
per kind ahead of the operator's recovery action. Residual: one
mid-list holder can still precede the 45s attempt; a timed-out
reconnect is honest and retryable.
- _refresh_server's supersede check gains the session arm: a spawned
retry/post-reconnect pass racing an eviction skipped instead of
manufacturing a false 'not connected' error pill (and a re-arm loop)
for a self-healing condition.
- The list_changed protocol twins are UNIFIED (Closes #842): the
admission half (_admit_list_changed) and the runner half
(_run_list_changed_refresh) each exist once as plain parametrized
methods — values and small closures, no factory layer (mcp v2 drops
the factory pattern; the two thin message_handler closures remain
only as SDK-v1 bindings). The one true asymmetry — coalesce-marker
ownership on the superseded path — is a documented boolean: pool
markers are only ever cleared by their runner; static markers are
cleared by remove_server_sync, so a present marker belongs to the
re-added generation. Both runners keep their names and signatures;
the notification suites pass unchanged.
- Cleanups: per-kind staleness rechecks stripped from the static
refreshers (unreachable under the lock discipline — the MUST-hold-
lock contract is documented instead); _run_hl (5th run-on-loop copy)
replaced at 44 call sites; _poll_until centralizes the live-test
wait loops; docs no longer describe the periodic refresh tier
removed in
|
||
|
|
f8f191686f |
fix(mcp): review round 2 — busy-skip the refresh pass, fail-fast list pairs, health-tick refresh retry
- _refresh_server never parks on a held connect lock: the holder is itself a catalog publisher whose publish supersedes the pass, and parking burned refresh_sync's whole 30s budget on ONE busy server (a reconnect attempt holds the lock up to 45s), failing the operator pass for every healthy server queued behind it. Busy → skip (None), no publish, no status writes; the identity/state recheck stays as belt-and-braces for the one-tick check→acquire race. - _list_resource_pair: the ONE copy of the paired resources/templates list protocol (both twins). Fail-fast — a fast real error (auth / method rejection) surfaces as ITSELF instead of being masked behind a hung sibling's eventual 30s TimeoutError — with the survivor CANCELLED and REAPED inside the timeout scope, never left detached on the shared session. - Health-tick refresh retry: there is NO periodic refresh pass (removed in eb2a119d; the docs still claimed the 4h tier — fixed), so a push refresh that failed while the transport stayed up had no automatic recovery and the shared catalog stayed stale for every user until an operator intervened. Failures and busy-skips arm _static_refresh_retry via the shared recorder; the health tick drains it with one bounded, lock-serialized full pass per tick; success, session drops, removal, and the post-reconnect spawns clear it. This also un-latches the error pill: the retry's completion clears it within a tick. - _record_refresh_failure: the bearer-redaction policy (type + message, never exc_info) lives exactly once; all three refresh-failure sites route through it. - Static runner discards its coalesce marker only AFTER the lock-identity check: on the superseded path a marker present in the set belongs to the re-added generation's parked runner, and discarding it would mint duplicates past the one-parked-runner bound (the pool runner deliberately differs — nothing else clears pool markers, so its marker is its own to release). - _clear_static_push_state: the ONE (server, kind) keyspace walk for stamps + retry flag (+ markers on removal). - Tests: busy-skip, superseded-no-status, fail-fast + reap (<5s bound), retry arm/drain/re-arm/clear quartet, logged-wrapper contract updated to the shared recorder's arg shape; vacuous stamp-math test deleted (behavioral per-kind coverage retained); _free_port/_wait_tcp_ready/_wait_session_live hoisted to conftest for both live tests. Refs #839 |
||
|
|
aefcf53405 |
fix(mcp): review round 1 — supersede retired-lock refreshes, per-kind debounce, complete gather pairs
- _refresh_server: post-acquire lock-identity + state-existence recheck; a pass superseded by remove (or remove + re-add) returns None and writes NO status — it must not run its list calls as a second, unserialized publisher against the re-add's discovery wiring, resurrect status rows for a removed server, or stamp a false "ok" over a generation it never refreshed. _refresh_all treats None as a deliberate skip (no breaker success record). - Debounce stamps are per (server, kind) on BOTH paths: refreshes are kind-scoped, so a server-scoped stamp dropped a different-kind notification inside the window outright — a tools push swallowed the prompts push 100ms behind it, and nothing observed the prompt change until the server pushed that kind again. Teardown pops loop the kinds; remove_server_sync also discards the server's coalesce markers so a parked old-generation runner's marker cannot coalesce away a re-added server's first push. - Resource refreshers (static + pool) gather with return_exceptions=True: fail-fast gather left the surviving list call running detached — outside the timeout scope and the lock serialization — as an unbounded in-flight request on the shared session. - Spawned post-reconnect refreshes route through _refresh_server_logged: the re-raise escaped into _spawn_background's done-callback, whose exc_info log serializes the chained httpx.Request carrying the configured bearer for auth_type=static servers; _refresh_all's except drops exc_info for the same reason. Failure diagnostics widen to "Type: message" in logs and the error pill — the message text is header-free; only the serialized chain leaks. - Accepted + documented: connect-lock contention on dispatch reconnects is bounded to one in-flight list call (parked runners bail instantly post-eviction); the error pill persists until the next COMPLETED refresh (a notification's arrival proves nothing about whether the failure resolved). - Tests: per-kind debounce independence, superseded-pass writes nothing, gather-sibling completion, logged-wrapper swallow with the exc_info channel asserted SILENT, remove clears markers; _run_on_loop/_drain_background hoisted to conftest (4 drifted copies); proc.kill() portability in the live push test. Runner-twin dedup (static/pool protocol duplication) deferred to #842. Refs #839 |
||
|
|
37144991c9 |
fix(mcp): spawn static list_changed refreshes off the receive loop
The static-path notification handler awaited its catalog refresh inline in the SDK's receive loop, but the refresh issues a request on the same session — a request whose response only that (now parked) loop could route. The refresh never completed, and every user's calls on the shared per-node session stalled behind it, unbounded, until the health loop's ping timeout tore the transport down — which was also the only way a pushed catalog change ever landed. Port of the pool-path protocol (#836) onto the static primitives: - Refreshes are debounce-gated, coalesced per (server, kind), and spawned as tracked tasks; the runner serializes on the per-name connect lock so a refresh, a connect's discovery wiring, and the manual/periodic _refresh_server pass can never publish out of order (the remove -> re-add race is closed by lock identity, the static twin of the pool's entry-identity check). - The coalesce marker is cleared at lock-acquire so a change the in-flight list missed spawns exactly one successor; the finally discard is gated on non-acquisition so it never clobbers that successor's marker. - The debounce stamp survives a failed refresh (throttle over lost window) and every teardown/eviction path now pops it via the paired _drop_static_session_and_stamp, so a reconnected transport's first notification refreshes immediately. - All three static list calls are bounded by _CONNECT_TIMEOUT and discard their result if the state entry was replaced mid-flight; the resource pair rides one gather (mirrors the pool sibling). - Failure logging is (Exception, BaseExceptionGroup) type-name-only: an escaping group reaches _spawn_background's exc_info log, which serializes the chained httpx request carrying the configured bearer for auth_type=static servers; the recorded operator error string is type-name-only for the same reason. Non-list-changed notifications no longer clear the server's error pill (that pop was accidental — only a completed refresh proves anything). Includes a live end-to-end repro (FastMCP subprocess pushing tools/list_changed through a real receive loop): pre-fix the triggering call itself deadlocks (verified against main), post-fix it completes with the catalog landing on the original session, no teardown. Closes #839 |
||
|
|
b2f53d329b |
chore(ci): drop review-event triggers from claude.yml
Bot PR reviews (Copilot, code-quality) fired pull_request_review and pull_request_review_comment runs that always gate out but pile up as awaiting-approval clutter. @claude stays invocable via issue and PR conversation comments, the only path actually used. |
||
|
|
6f991d6aff | chore: bump version to 1.8.0a1 v1.8.0a1 | ||
|
|
7d4d76e097 |
fix(providers): PR review — orphan deltas arm the finish shim, test style
- Orphan argument deltas count as delivered output for the finish_reason_optional shim, exactly as they count as a streamed signal for the terminal harvest: a lax Responses server that never announces items AND never sends a terminal event still delivered its tool call — with the tolerance declared that is a completion, not an IncompleteStreamError. (Review caught the shim/harvest inconsistency the round-9 fix introduced.) - Test style: single import style for the model_turn module, assert on a local instead of a call expression, drop a pass-through lambda. |
||
|
|
747177a76c |
fix(providers): review round 9 — orphan/harvest collision, shared shim gate, retired-id rationale
Correctness: - Responses: orphan argument deltas (streamed without any output_item.added) now count as a streamed tool-call signal, so the terminal harvest stands down instead of re-emitting the same call onto the same slot — the reproduced collision concatenated the arguments JSON into an unparseable double copy. Cleanup / documentation: - finish_shim_due in _protocol is THE gate for the lax-server finish shim — one predicate (and one definition of 'delivered output') for all three adapter families, so the same capability flag cannot acquire per-family completion semantics. - The Responses error/response.failed branches share one failure tail (only code/message extraction differs) — the same server failure can never become retryable through one event type and fatal through the other, pre- or post-terminal. - _format_refusal pins the refusal rendering the streamed event and the terminal harvest both use. - The capability-table floor comment and CHANGELOG Removed entry now state the real rationale: OpenAI has RETIRED the pruned ids from the API — the rows described unreachable contracts, not unpopular ones. - CHANGELOG names the stream-entitlement break class (verified-org streaming, pre-stream_options gateway api-versions) with its serving-side remediation; deliberately no non-streaming fallback. - docs/architecture.md retry section describes the collapsed transport: the two stacked retry ladders, IncompleteStreamError / ResponsesStreamFailedError retryability, finish_reason_optional remediation; stale non-streaming mentions updated (+ puml). - Anthropic whole-block emission carries its residual hybrid-gateway bet as an explicit comment. Held on standing rulings: post-finish usage forfeiture (keep result + warn, rounds 4/8), session merge_usage twin and StreamAbortRef twin (#832), stream_options wire delta (round 2, caveat now names Azure). |
||
|
|
49d33d8594 |
fix(providers): review round 8 — under-streaming gateway parity, abort-race close, usage-blip visibility
Correctness: - Responses: output that exists ONLY in the terminal payload (buffering gateways that never fire output_text.delta / output_item.added) now reaches CompletionResult.content and tool_calls — the retired non-streaming _parse_response read this same payload, so the drain must too instead of returning a clean-looking empty success (blank compaction summary, silently-skipped tool call). Gated on nothing of that kind having streamed; refusal parts render as the streaming branch does. - Anthropic: content pre-populated inside content_block_start (whole- block lax-gateway emission — the real API sends start blocks empty) is emitted for text, thinking, and tool_use input, type-guarded like _reasoning_text so duck-typed blocks can't leak non-strings. - Responses: a response.completed payload that OMITS status maps to "stop" via the event type, matching the payload-less branch — the empty-string status read as 'length' and fired truncation policies on complete output. - model_turn: the drain-retry loop re-checks cancel_ref.aborted after the backoff sleep — an abort landing mid-sleep now kills the abandoned worker with the original failure instead of issuing one more full request behind the deadline's back. - drain_stream: the post-finish transport-blip tolerance logs a warning naming whether usage was captured — the kept result may report usage=None (chat-lane usage trails the finish reason) and that spend was vanishing from usage accounting with no signal. Cleanup: - ChatSession's inline tool-call fold adopts accumulate_tool_call_delta (drop-in — same ToolCallDelta semantics), so THE merge rule now has one implementation across the chat loop, drain_stream, and the Google capture; the helper's mirror-mandate docstring is retired. - The task-agent _api_call contract comment reconciles the two retry layers (sub-harness owns request-level policy; model_turn owns drain-time re-issue) instead of claiming model_turn is policy-free. - Anthropic's three terminal-emission sites share one _attach_terminal_blocks helper — replay fidelity can't depend on which terminal path a stream took. - Responses create_streaming resolves capabilities once. Held on standing rulings: o-series capability-row removal (4th report; deliberate break, release-noted), StreamAbortRef/_CancelRef unification (#832; docstring mirror-mandate). |
||
|
|
8fa0e7a29e |
fix(providers): review round 7 — id-disciplined slots, all-lane finish tolerance, retry backoff
Correctness:
- ToolCallSlotter: a slot whose id is KNOWN never splits on an id-less
delta — on an id-disciplined server new calls arrive with ids, so an
id-less fragment (the call's FIRST name announcement included) is
always a continuation. Round-6 regression: {id} → {name} → {args}
emission split into an unnamed id-bearing call plus a nameless twin.
Also: a name arriving for a slot with no name yet never splits
(args-first emission), and a bare same-name delta after complete
arguments merges as a redundant footer instead of minting a phantom
zero-argument call that would re-run a side-effecting tool.
- finish_reason_optional is honored on every drained lane, not just
Chat Completions: Anthropic shims a missing message_delta
stop_reason + message_stop pair, Responses a missing terminal event
(both with collected blocks riding the shimmed finish) — the
documented capabilities-JSON remediation now works on the
anthropic-compatible/responses-compat gateways it was written for,
matching the retired non-streaming paths' tolerance.
- Responses: an in-band error/response.failed frame arriving AFTER the
terminal event is teardown noise — log and end the stream instead of
raising away a generation already in hand (the in-band twin of
drain_stream's post-finish transport-blip tolerance).
- model_turn drain retries pace like the SDK request retry they
replace: 0.5s base, doubling, ±50% jitter — instant re-issues
re-hit the still-active rate limit/overload and synchronize into
fleet-scale retry bursts.
- Responses slot bookkeeping survives lax servers: slots minted by a
counter (len(dict) collided calls after a duplicate/empty item-id
overwrite), orphan argument deltas route to the most recently
announced call instead of hardwired slot 0.
Cleanup:
- on_tool_call_delta now receives the normalized ToolCallDelta plus the
raw SDK delta — Google's capture accumulates the exact bytes the
mirror sees (the byte-identical extraction no longer exists twice).
- _ArgsScanner feeds only fully id-less slots (its verdict is never
consulted for id'd slots — dominant-case hot path).
- Anthropic retryable set hoisted to a class constant (per-access
frozenset allocation, same pattern already fixed on Responses).
- GoogleProvider class docstring names the hook-based capture instead
of the deleted _extract_tool_calls override.
|
||
|
|
16647db1b0 |
fix(providers): review round 6 — strict-by-default finish gate, slotter v3, drain retry
Correctness: - The chat-lane finish shim is now armed only by an operator-declared finish_reason_optional capability (model-definition capabilities JSON). Default lanes treat a clean finish-less end as died-mid-generation (retryable) — SSE cannot distinguish lax-server completion from a worker dying behind a clean-closing proxy, and the default must catch truncation rather than bless it. When armed, reasoning-only output counts as a completed generation (parity with the retired non-streaming path's finish_reason-or-stop default). - ToolCallSlotter v3: id-less call-boundary decisions now consult argument JSON completeness (incremental scanner) and name identity instead of a boolean has-args gate. Fixes both residual id-less ambiguities: two zero-argument whole-delta parallel calls no longer fuse (silently dropping an action), and redundant per-fragment name headers no longer split one call into malformed half-JSON calls. - model_turn re-issues transient mid-stream deaths (provider's retryable_error_names, raised while draining) up to twice — the new home of the SDK request-level retry the non-streaming transport gave every single-shot lane (judge, title, perception, compaction). Request-time failures keep the SDK's own policy; an aborted cancel_ref suppresses re-issue (StreamAbortRef gains .aborted). Cleanup: - One slotter drives both the normalized mirror and Google's raw fidelity capture via an on_tool_call_delta hook — raw/mirror slot parity is structural now, not a maintained invariant. - accumulate_tool_call_delta in _protocol.py is THE tool-call merge rule; drain_stream and the Google capture use it (session's copy is #832's tracked adoption). - Responses terminal rebuild only runs when the terminal payload can disagree with the .done-collected items (truncation or count mismatch); on rebuild, annotations are replaced, not re-extended. - Responses retryable set precomputed at class creation. Held on standing rulings: o-series capability-row removal (deliberate, release-noted with remediation), StreamAbortRef/_CancelRef unification (#832; docstrings mandate mirroring until then). |
||
|
|
91c46051d9 |
feat(providers): drop o-series and pre-5.4 GPT-5 capability rows
The OpenAI commercial capability table floor is now gpt-5.4: o1, o1-mini, o3, o3-mini, o3-pro, o4-mini, gpt-5, gpt-5-mini, gpt-5-nano, gpt-5-pro, gpt-5.1, gpt-5.1-codex-max, gpt-5.2, gpt-5.2-pro, and gpt-5.3 are effectively unused in the field. The gpt-5-search-api row (different product surface) and the audio/STT/TTS rows stay. A legacy id now resolves to OPENAI_DEFAULT (temperature sent, no declared effort vocabulary, 200K window) — which those models may reject; the remediation is the model definition's capabilities JSON or a current model, release-noted under Unreleased → Removed. This also retires the transport-collapse review's thrice-reported "stream-rejecting o1-era models are stranded" finding by removing its subject: no row in the table describes a non-streaming model anymore. Tests migrate to 5.4-era equivalents that pin the same behaviors: always-reasoning temperature suppression and off-list effort snap (gpt-5.4-pro for gpt-5-pro/o3), explicit-none forwarding (gpt-5.4 for gpt-5.1), empty-effort-vocabulary knob drop (gpt-5-search-api for o1-mini), and the longest-prefix shadow hazard (gpt-5.4-pro vs gpt-5.4 for codex-max vs gpt-5.1). |