[release] v0.115.3 - #6616
Open
github-actions[bot] wants to merge 81 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…overy fix(sessions): preserve active turns when watchdog checks fail
Contributor
Author
Railway Preview Environment
|
- Default the SDK binding type to env, so a binding with only a name parses. - Sync the entity ref in a layout effect instead of during render. - Follow the revision prop in the secret request dock. - Gate the queue card and jump pill on the secret dock on both hosts. - Parse the commit response with safeParseWithLogging. - Write the rotated digest to a new store path in the release gate cell. - Add the handler and desired-state paths to the custom-secret gate trigger. - Fix the checklist link, the plan status, and the storybook scenario guard. Claude-Session: https://claude.ai/code/session_01XVrWftaiLxJT5DnL7D1xjY
[fix] Update attachment acceptance test for saved chat records
feat(agents): inject custom secrets into agent runs
Contributor
Author
📘 Docs preview
Pages changed in this pull request
This comment updates in place on every push. |
… gh.ssl stacks The api calls the runner directly to cancel a turn (Stop) and to tear down a sandbox (kill). The gh.yml and dev.yml files pass AGENTA_RUNNER_INTERNAL_URL to the api service, but the gh.local.yml (oss and ee) and gh.ssl.yml files pass it only to services. On those stacks every Stop logs "cancel: no runner internal_url/token configured" and never reaches the runner. Closes #6627 Claude-Session: https://claude.ai/code/session_015akbm8MYys7pVjkhdP9R9a
…he matching troubleshooting entry The platform API calls the runner directly to cancel a turn (Stop) and to tear down a sandbox (kill). The docs described the runner hop as Services-only, claimed every Compose file set the locator on both callers, and had no troubleshooting entry for a Stop that never lands. Closes #6629 Claude-Session: https://claude.ai/code/session_015akbm8MYys7pVjkhdP9R9a
[6629] docs(self-host): the api needs the runner URL for Stop, plus the matching troubleshooting entry
…6673) * test(gate): rename a session between two turns and assert the agent was told The gate could not see a per-turn fact that never reaches the agent. Issue #6661 shipped through a green matrix because no cell renamed a session mid-conversation and no cell asserted on the session facts at all. Add matrix_n1_session_context.py. It drives one session on the path the playground posts, renames the session between turns through the route the UI uses, and asks for the name, the agent's own display name, and the name again with a forged meta.session_context in the body. Every expected value carries a random token minted for the run and spoken nowhere, so a transcript-derived answer cannot match. Two controls keep a FAIL honest: an echo probe and a read-back of the stored header after every rename. Wire it into path_triggers.py for the SDK session-context module, the platform prompt renderer, the agent handler, and the API session-context resolver, into SKILL.md as a mandatory cell, and into coverage.md. Document in the shared invoke helper that it posts to the /services path exactly like the browser, so nobody writes a cell that assumes an API-side stamp. Add an optional meta argument for the forged control, and AGENTA_SERVICE_BASE so a fix can be gate-verified before it is deployed. * test(gate): classify a failed workflow create, and document every function CodeRabbit review follow-up. create_workflow ran before the try, so a non-200 or a transport error escaped n1_for uncaught. Under --harness-all that aborted the whole run with a traceback and printed no results, which is the outcome the handler comment exists to prevent. It now runs inside the try, so the same classifier turns it into a SKIP or a FAIL. wf is bound to None first and the archive in finally is guarded, since there is nothing to archive when the create itself failed. Add one-line docstrings to stored_session_name, n1_for and main.
…ient meta (#6667) * fix(sdk): resolve the session facts in the agent service, not off the wire The playground posts a turn straight to the agent service, so the API prelude that stamps request.meta.session_context never runs on that path and every playground turn reached the agent with no session facts. The service now reads the three facts itself, with the caller's own credential, and ignores the blob on the wire, which is client input on this path. Fixes #6661 * fix(sdk): bound the session-context reads and refuse ambiguous inputs Codex review on #6667: - Give the optional resolution one TOTAL deadline covering client construction and teardown, not the tool resolver's 30 s per-operation timeout. An expired budget cancels the outstanding reads. An outside cancellation still propagates. - Tell a malformed session body apart from a legitimately unnamed session. A shape this code cannot trust reports UNKNOWN for the whole pair, because reading it as unnamed tells a named session to rename itself. - Decline to resolve an agent name when a request names competing workflow artifact families. Inline-config runs bypass the family validator. Also corrects the API comments that claimed the stamp covers playground turns. * docs(sdk): correct the session-context docstrings to what the code does From the Codex review: three requests not two reads, and the stream read touches Redis as well as Postgres. The run context is not universally populated, because inline config skips hydration. A saved artifact with unsaved config is a draft and still has a name. first_turn means no durable turn row at lookup time, so a retry or a cold resume of the first exchange reads False. The session-id shortcut is rare, because the normalizer mints an id before the handler runs. The handler is where turns pass, not every possible SDK entry point. Records why a client is built per call and why the facts must never be cached. * fix(sdk): check artifact ambiguity before the run context, and bound the unwind Codex round 2 on #6667: - The competing-families check never ran on the service path. The run context is built from the same references and prefers the workflow family, so the early return handed back the very preference the check exists to refuse. The check now runs first. A bare-handler test could not see this, because it has no ambient run context, so the cover is a service-level test that installs one. - One deadline owner, shared by the module and the handler. It runs the work as its own task, so the unwind is bounded by a grace period rather than awaited forever, and a teardown that raises can no longer replace a caller's cancellation with an ordinary exception the boundary would swallow. - Refuse a non-finite timeout override, which would disable the deadline. - Raise the default budget to 2 s. A fresh client per turn pays connection setup, and the stream read does Redis work before Postgres. - Canonicalize reference ids so two families spelling one artifact differently do not read as a disagreement. - Correct the docstring claim that the streams GET route omits null fields. * test(services): drive the session-facts tests through the real invoke route The service-level cover now posts to /invoke rather than calling the handler, so the tracing context that feeds the artifact lookup is built by the route from the request's own references instead of handed in by the test. That is the layer where the competing-families bug was reachable and where a bare handler test could not see it. Reverting the check ordering makes the competing-families cell fail with the wrong agent's name, as Codex reported. * fix(sdk): one deadline owner, and keep a raising resolver inside the boundary Codex round 3: - A resolver that raises synchronously before returning its awaitable escaped the boundary, because the call was evaluated before entering it. The typed interface admits such a factory, so run_optional now takes the callable. - The default path nested two deadlines. The outer grace period was watching an inner wrapper unwind rather than the client that holds the connections. The handler now calls the unbounded read, and the bounded entrypoint stays for a caller that reaches past the handler. - A detached cleanup is held in a set that releases on completion. A done callback is not a reference, so the loop could collect one mid-unwind and report 'Task was destroyed but it is pending!'. - Land the docstring correction that reached a test comment last round. - Initialize the SDK singleton in the service tests rather than inheriting it, which made them fail under xdist on a loaded box. - Tighten two loose assertions: all three reads must cancel, and the slow teardown is bounded by budget plus grace rather than a second. * test: own the SDK singleton, and pin the close on the abandonment path The service tests take the SDK singleton through a fixture that restores what was there, rather than initializing a process global and leaving it installed for every later test in the worker. Adds the guarantee the abandonment path actually makes: cancelling the task raises into the async with, so the client's close is entered every time, and the caller does not wait for it. If that close itself hangs there is nothing further to force, since httpx exposes no way past aclose. * test: correct two comments that claimed more than the code does Round 4 nits, both wording and no behaviour change. The one-second threshold also rejected the ten-second teardown, so the tightening is not justified by what the comment said. And host= does not isolate the exporter, because the AGENTA_API_URL the fixture sets wins; the reason its failures are harmless is that the exporter flushes off the request path. * test(services): make the forged turn position disagree with the backend The forged-meta cell sent first_turn false while the backend also answered false, so a route that trusted the forged value passed. The backend turn list is now empty, which makes this the first turn while the forgery denies it, and the rendered marker tells the two answers apart. Injecting trust of only the forged turn position now fails the cell.
…#6672) * fix(frontend): stop the Permissions dropdown calling Allow reads the default New agents are created with Allow all since #6641, but the sub-line under Allow reads still read "Reads run, writes ask; default". The word moves to the option a new agent actually gets. Both hosts read the same list, so the wording changes in one place. * fix(frontend): drop the default marker instead of moving it Codex review: "the default" is two policies here. The standard template creates an agent on Allow all, but an agent whose config names no policy runs on Allow reads, and both selectors display that fallback as the applied value. A sub-line cannot say which one it means, so it says neither. The issue allowed either fix. * test(frontend): check the label too, and shorten the file comment CodeRabbit: a future Default marker in option.label would have passed the assertion, which only read option.help.
…h hosts (#6675) * fix(frontend): refuse a chat link whose target names a host, not a path harden parses a target that starts with ../ and hands back its pathname. The pathname of ..//evil.com/x is //evil.com/x, which the browser reads as a host, so a reply could carry a link labelled like a file that opens another site. The chat anchor on both hosts now refuses any target that resolves to a host, in its encoded and backslash spellings too, and renders it the way harden renders a target it refused. * fix(frontend): drop tabs and newlines before the host check, as the URL parser does The URL parser removes every ASCII tab, newline and carriage return from the whole href, not just its ends, so a browser reads /<tab>/evil.com as //evil.com. The check now runs on the string the parser would see. * fix(frontend): narrow the file-candidate guard and test /m through the real pipeline Two findings from the Codex review. The file-candidate guard reused the anchor's predicate, which decodes. A raw filename can legitimately contain %2F or a backslash, and nothing on that path navigates, so the wide test rejected real names for no safety. It now tests the literal two-slash prefix only. The mobile suite called the anchor renderer directly, so it could not catch a missing plugin or a renderer that stopped using the component map. A second suite renders AssistantMarkdown itself and covers raw HTML, autolinks and streaming mode. Also corrects the predicate's comment: a browser does not decode %2F, so the decode rounds are belt and braces for downstream consumers, not a model of the parser. And the desktop renderer no longer claims the composer shares it. * test(frontend): isolate the resolver assertion from earlier renders CodeRabbit: the resolver array accumulates across the file, so the positive case could have passed on an entry another test left behind. It now reads only what its own render added. Also shortens two comments.
The cell pinned sandbox local. Staging enables Daytona only and answers 403 for a local sandbox, so the cell could not run on the stage that matters. Add --sandbox local|daytona, defaulting to local, the way matrix_t9 does. The value reaches the agent config and is recorded in the result. The docstring says why the flag exists, and that the claude harness on Daytona SKIPs on the credential because Daytona rejects subscription auth by design.
Four pages still called Allow reads the default. The standard template has created agents on Allow all since #6641, so the tutorial step that said the option "may already be selected" sent readers past a change they had to make. Each page now names Allow all as where a new agent starts.
…6682) * fix(sdk): stop telling agents that allow_reads is the new-agent default Two strings said allow_reads is "the default" without saying which default. It is what the runner applies when the field is absent, which is true, but the standard template has written allow since #6641, so an agent reading either string could conclude a new agent starts on allow_reads. Both now say which is which. * fix(sdk): tighten the wording and align the config example Codex review. Name runner.permissions.default explicitly rather than "the field", and say "the standard creation template" rather than "a new agent", which was broader than the code supports: a bare AgentTemplateSchema still yields allow_reads. Also set the example object to allow. A model copying the whole example got allow_reads while the sentence three lines below said the template writes allow.
…ew (#6687) * test: correct three comments that claimed more than the tests prove Follow-up to #6667, comment and name only, no behaviour change. The exporter comment described the ordering backwards. The fixture sets up before the one that sets AGENTA_API_URL, so the loopback host is used; an ambient value would still win, which is why this is not exporter isolation. The fixture docstring said it owns the SDK singleton. It restores the ag.tracing alias and nothing else, and init also replaces api, async_api and the tracer and installs a provider. The abandonment test was named for a close that always happens. What it can prove is that the close is entered and that the caller does not wait for it. * test(services): make the fixture order real instead of assumed Two function-scoped fixtures with no dependency have no guaranteed order, so a comment describing one was asserting something pytest does not promise. backend_facts now depends on sdk_singleton, which is the fixture whose environment variable the other reads. The comment no longer claims where the exporter points either. init prefers AGENTA_API_INTERNAL_URL and then AGENTA_API_URL over the host passed to it, and either can be ambient, so ordering alone settles nothing. The claim that does hold is that isolation is not needed here.
…ng its tabs (#6685) * test(frontend): wait for the Use API drawer to settle before clicking inside it * test(frontend): shorten the added comments to one line each
…write-only value (#6691) The playground's Model section decided whether the project holds a key for the selected model's provider by reading the key value off the vault row. Secrets are write-only on staging and on every dev stack, so `/secrets/` returns no value and reports presence through `value_status.configured`, which the transform surfaces as `hasKey`. Every connected project therefore read as keyless, and the amber "Connect key" badge and its "Connect the model's provider key to run this agent." tooltip stayed up across reloads while the agent ran on that same key. The rule moves into `shouldPromptForProviderKey` and asks `hasStoredKey`, the one vault presence rule. Its exemptions are unchanged: a self-managed connection signs itself in, a named connection points at a vault record this rule never looks up, and an unresolved vault prompts for nothing.
…very session surface (#6686) * fix(frontend): make the Rename menu entry open the inline editor The shared session menu offers "Rename" on every surface, but the shared router had no branch for its key: clicking it did nothing at all. It has been dead since the rename modal was deleted in v0.114.1, on the chat tab strip and on every card list. The nav rail and the sessions list were unaffected because they intercept the key themselves. Route the key back out as `onRename`, the way "open" already travels, and wire the surfaces that were missing it. The chat tab strip asks through the same one-shot request atom Alt+R uses, so the entry and the shortcut open one editor. The card list gains the in-place edit the standalone list already had, which covers the mobile chat panes, the agent overview and the workspace home. The row context menu also had to stop taking the caret back: Radix restores focus to the trigger as the menu closes, which lands after the editor's autoFocus, and the resulting blur commits and closes it. A verb that took the caret itself now says so and that one focus restore is suppressed. * fix(frontend): defer the rename editor until the menu has closed Codex review. A menu item's select runs while the menu still holds a focus trap, so an editor opened there is blurred straight back out, and a blur commits. Suppressing the later focus restore does not help, because the first blur has already happened. Replace the boolean return with a named handoff: a verb may return a function, which runs from the menu's close, after the trap is released. It lives in one hook, `useDeferredMenuSelect`, shared by the right-click menu and the row kebab so they cannot drift. Also from the review: the mobile chat session tabs had a Rename entry with no editor behind it, and the sessions page dropped the commit callback on its browse-rail branch. Both are wired. Tests now drive the real card-list row rather than the stub: pick Rename, close the menu, type, press Enter. Dropping the editor or running the deferred work early both fail the suite.
…record read (#6658) * fix(frontend): show a sent chat message at once instead of after the record read A session that advertises the durable `queue` capability routes every composer send through the server, which adds nothing to the local AI SDK chat. The user's message therefore only appears once the invoke POST, the durable event, and the records read that adopts the saved row have all landed. The composer clears in about 20 ms and the transcript then stays unchanged for the rest of that window. Measured on a local EE stack: 945 ms median on the desktop and 747 ms median on mobile, and 3.2 s when the server queues the input behind a turn it still counts as running. Remote deployments add their round trips on top, which is the 4-5 s users report. The queue hook now keeps a local echo of each durable send and renders it between the adopted transcript and the live preview. An echo retires the moment something else owns the message: the queued-messages dock on a 202, the saved user row on a 200, and the composer restore when the send is refused. Coverage is counted rather than matched by id, because the server keys saved rows by the record's own id and never echoes the client id back. Desktop drops to 62 ms median and mobile to 217 ms, with each message appearing exactly once and no stranded echo rows. * fix(frontend): drop a pending-send echo a rewind stranded below its origin Review note from the verification pass: if an adoption ever replaced the transcript with a shorter list, the user count would fall and echoes waiting on a higher count would sit on screen until an unrelated send raised it again. Each echo now records the user count it was created at. A transcript that falls below that number was rewound past the point the send belongs to, so the echo can never be covered and is dropped instead of left behind. * fix(frontend): retire a pending-send echo during render, not in an effect Review finding: the passive cleanup ran in an effect, so it fired after the paint. The frame that first carried a newly adopted durable row also carried its echo, which is one duplicate-bubble flash per send. Retirement now happens during render. React discards that pass and re-runs before painting, so no frame ever holds both rows. The new test records every render and asserts the durable count plus the echo count never exceeds one; it fails against the effect form and passes against this one. Also cuts the comments in the touched files to one line each, per the hard rule in web/AGENTS.md. * fix(frontend): retire a pending-send echo on its own turn id, not on a count Codex review, both P1s. Counting is wrong on a shared session, and Mahmoud runs /w and /m on the same one. Any foreign user row raised the count and retired this client's echo while its POST was still unresolved: another tab's message, a promoted queued input, a Steer, or a history adoption. Worse, two echoes reserving counts four and five left the second duplicated forever if the first failed, because its own record then saved as row four while the echo still waited for five. The invoke response's first frame is `data-session-accepted` and carries the `executionId`, which is the `turn_id` the saved user record carries. Verified end to end against a live stack. So an acknowledged echo now retires only when that turn id appears among the adopted user rows, and ignores the count entirely. The count remains as the fallback for the one round trip before the frame arrives, and dropping an echo renumbers the ones behind it against the current transcript. An HTTP 200 only proves the request was taken. The stream is now read for that acceptance frame rather than drained blindly, and a stream that ends without one never started a turn, so the echo is dropped instead of left on screen. Also from the review: an echo retires when the durable queue reports its id, so the dock owns the row; and `mergePendingSendRows` orders an earlier turn's live preview above the echo while keeping the echo above the preview of its own turn. `transcriptToMessages` now emits `metadata.turnId` on user rows, which is what makes the identity match possible. Assistant rows are left alone because `latestTurnId` reads that field as the Stop guard. Tests reproduce both P1 cases and fail against the count-only version. * test(frontend): pin the user-row turn id that pending-send identity depends on * fix(frontend): own send echoes in state, and retire each on evidence about itself Codex pass two, plus the remaining items from pass one. The render-phase retirement I added was unsafe. It mutated a ref during render, and React does not roll that back when it abandons a render, so an echo could be lost permanently and the next send could then reserve a count already taken. Codex reproduced it under React 19.2.6. The lifecycle now lives in usePendingSendEchoes, with state as the only source of truth. Visible rows are DERIVED by pure filtering during render, which keeps an adopted row and its echo out of the same commit without writing anything, and the stored list is pruned afterwards in an effect where a discarded render cannot do harm. Coverage is allocated inside the state updater from the list React hands it, so a burst never reuses a number and no shadow ref is needed. Every exit is now evidence about that particular send: - Accepted. The run stream's `data-session-accepted` frame names the turn, which is the `turn_id` the saved user record carries, so the echo waits for its own row and ignores the count entirely. - Parked. A 202 body names the durable input, and the echo waits until the dock is OBSERVED to list that id. A completed snapshot request is not evidence the dock has the row. - Failed. An HTTP 200 only proves the request was taken, so the stream is read for an error frame before acceptance, and an end with no acceptance is treated as a turn that never started. Ordering against an answer already streaming is fixed too: a preview belonging to an execution no echo owns is an earlier turn's answer, so a newly sent question goes below it rather than above. Tests: an abandoned-render regression test built on a suspending transition, which fails against the ref version; a StrictMode double-render case; the foreign-row, parked-dock, burst, and renumbering cases; and the no-duplicate-commit invariant. * fix(frontend): stop calling a silent stream a failure, and never delete a refused send Codex pass three. Two of its findings still applied after the state rework, and both were mine. The runner emits the acceptance frame only for a DETACHED request, and this adapter also sends ordinary ones. Treating a stream that ends without acceptance as a refusal therefore classified good turns as failures. Silence now reports nothing and the send falls back to the count, exactly as before identity existed. Only an error frame before acceptance is a failure, and a dropped connection says nothing either way, which is what the comment already claimed. Worse, a late refusal deleted the echo outright. The composer clears on submit and its restore only runs when the submit promise rejects, so by then the user's text existed nowhere: the row vanished and took the message with it. That is worse than the gap this PR set out to close. A failed send now KEEPS its row, flagged, so the text stays on screen. The `failed` field was dead until this commit. Also from the review: the stream reader handles CRLF and CR-only framing and a final frame with no trailing newline; `mergePendingSendEchoRows` no longer guesses at previews while any echo is unacknowledged, which had put an answer above its own question; and `userStop` reads `turnId` from assistant rows only, since user rows now carry one and a question and its answer would otherwise share an identity. Tests drive the reader through real Response bodies: split chunks, both framings, a missing trailing newline, an error frame, an ordinary stream that never accepts, and connection drops before and after acceptance. * fix(frontend): bound an acknowledged echo by its own turn's terminal frame The last of the six published limits, and the one that mattered: an acknowledged echo waited forever if its user record never persisted. Acceptance precedes persistence and exhausted ingest retries drop records, so that is the original vanish class wearing a new hat, a row that sits on screen forever instead of one that never appears. The adapter now reports `onSettled` after the run stream ends AND its records have been re-read. An echo still waiting at that point stops waiting silently: it is flagged, so the text stays on screen and the state is visible. Retirement precedence is reordered so this stays a guess and not a verdict. The saved row is checked FIRST, ahead of the failure flag, so a record adopted after the terminal frame still retires an echo the guess had flagged. * test(frontend): pin the pre-acknowledgement window and say why it is the safer failure The browser cell for this class reports INCONCLUSIVE on our stack, because the second tab's send returns 202 and queues, so it cannot demonstrate the race. That is a property of the stack, not evidence the race cannot happen, so the limit belongs in a unit test over the adoption path instead. Two tests: a history adoption retires an unacknowledged echo, and the same adoption leaves an acknowledged one alone. The first pins current behaviour rather than endorsing it. The comment records the trade rather than just the symptom. Holding an unacknowledged echo until its turn settles would close the window, but any send whose stream carries no acceptance frame would then sit beside its own durable row for the whole turn. A persistent duplicate is worse than a brief re-vanish that heals when the real row lands. * fix(frontend): render the failed state on a refused send, on both hosts The flag was set and nothing read it, so a send refused inside a 200 kept its text on screen looking like an ordinary turn that would never get an answer. The text was safe; the state was invisible. Three unit assertions passed the whole time, which is a good reminder that a green test can certify a value exists rather than that anyone is shown it. Both hosts now render a note under the row: "Not sent. Copy the text and try again." It carries `data-pending-send-failed` and `role="status"`, so it is detectable in the DOM and announced, rather than signalled by colour alone. NOT yet seen in a browser. The stack belongs to another agent, and the QA cell that asserts this is written and waiting. Treat the wording and placement as provisional until that pass runs. * fix(frontend): register an echo against the current transcript, and bound a parked one Codex pass four, the two findings that were new rather than carried. An echo registered against a stale user count. `add` closed over the count from the render that created it, and capability resolution is a round trip, so a send could enter the lifecycle long after that. If the transcript moved on meanwhile, the echo reserved a number already taken and was hidden the moment it appeared. The baseline is now read at call time from a ref written in an effect, so it tracks the last COMMITTED render and an abandoned one cannot move it. The updater still receives it as a plain argument and stays replay-safe. The first version of that test passed against the broken code, because `result.current` hands back the newest callback rather than the one a pending submit is holding. It now captures the handler before the adoption, and fails against the closed-over version. A parked echo could wait forever. The backend drops a pending input from its query once its execution is running, and a parked echo never receives a turn id, so dock membership was its only possible successor. If promotion beat observation, nothing could ever retire it. The count is now its fallback, exactly as it is before any send has an identity. Also aligns the failed-row wording with the composer's, word for word. Two phrasings for one event reads as carelessness; to the user a refused send is one thing, whether the stream failed before or after the promise resolved. * fix(frontend): settle only an accepted turn, and recover a late refusal like any other Three findings, two of which put a false statement in front of the user. onSettled fired at every stream end, accepted or not. On an ordinary non-shared send the runner emits no acceptance frame, so the echo had no turn id, the flag was set, and retirement's failed branch outranks the count rule that would have cleared it. The result was a permanent "wasn't sent" under a message that was sent and answered. My own comment said "the accepted turn finished" while the code settled unconditionally, the same shape of bug as the flag nothing read. The reader now reports whether it named a turn, and settlement is gated on that. The trailing-frame rescan discarded an error verdict, so the same refusal reported nothing without a newline and a failure with one. Its return value is now used. Recovery for the two refusal shapes is unified, per review. A refusal arriving after the send promise resolved now goes back to the composer through the same channel a rejected send uses, and the row is dropped. One event, one place, one wording. The flagged row survives only where no composer can take the text, which is also what closes the worst combination in the review: even if the pre-acknowledgement window has already retired the echo, the composer still gets the message back. Desktop is wired. Mobile passes the seam through but supplies no restorer yet, because its composer sits below the hook with no handle, so a late refusal there keeps the flagged row. That asymmetry is deliberate and marked in the code. * fix(frontend): do not let a stale run completion adopt over a newer transcript Review found this by testing a belief of mine rather than accepting it, and the belief was wrong. I had said an unmount mid-stream degrades to discarded rather than wrong, on the grounds that both hosts key by session. It does not: the session registry deliberately preserves the same Chat across a remount, so the old reader's completion still calls `onExecuted` on the live session and adopts its stale snapshot over a newer transcript, then persists it. That truncates a transcript rather than misplacing a row, which makes it the most serious thing found in this PR. Every continuation past an await now checks that this mount is still alive before refreshing the queue or triggering adoption. The reader itself is left running rather than aborted, because aborting the invoke body could tear down a live run on the non-shared path, which would be a worse cure than the disease. NOT covered by a unit test. Exercising it needs the request builder's jotai setup around a mid-stream unmount, and I could not get that harness right without leaving a broken test behind. The fix is small and the reasoning is in the review; it should be verified there rather than taken on trust, which is the same mistake that produced the bug. * fix(frontend): never overwrite a draft the user typed after a send was refused Review note on my own recovery change, and it was right. I hand-rolled the late refusal's restore instead of reusing the one this file already had, and lost three properties in the process. A late refusal arrives seconds after the composer cleared, so unlike a rejected send the user has had time to type something else. My version called setMarkdown unconditionally, with no guard and no undo. It also restored the text without the staged files, so a refused send with attachments came back as text alone. And it returned true whenever an editor existed, so the queue dropped the echo row even when the restore had just destroyed a draft, which is precisely the case where the row is the recovery surface worth keeping. It now calls `restoreRefusedSend`, the helper the rejected-send path already uses, which refuses a non-empty composer, puts the staged files back with the text, and reports whether it took them. Returning false leaves the flagged row in place. Two tests pin the properties the late path depends on, rather than the ones the synchronous path happened to get for free. * fix(frontend): say a file-only send failed, instead of showing cards that look fine Both hosts painted no bubble at all for a message carrying only attachments, and the failure note lived inside that bubble, so a refused file-only send rendered as attachment cards with nothing to say they had not been sent. The note is now what the bubble carries when there are no words, on both hosts. This was the second half of the reason checking metadata alone was not enough. The flag was set, the accessor was tested, and for this shape of message nobody would ever have seen it. * fix(frontend): stop settlement writing a delivered message back into the composer I introduced this with the recovery unification and it is worse than anything it replaced. Settlement called the restorer unconditionally, so on the NORMAL accepted path — row adopted, echo retired, stream ends — a message that had been delivered and answered was written back into the composer under "wasn't sent". The non-empty-composer guard does not help, because the composer is usually empty right after a send, which is exactly when this fires. Settlement now only marks an echo that is still waiting. Marking one that has already retired is a no-op, which is the behaviour a browser pass confirmed as correct before I changed it. A genuine refusal still restores, because a refusal means the message was never delivered. The lesson is the one this PR keeps teaching: I unified two paths on the grounds that they were one event, and they are not. A refusal says the message did not arrive. Settlement says the turn is over, which is usually because it arrived. * fix(frontend): never let a refused message end up with nowhere to be Codex blocker three, both halves. A refusal arriving after the count rule had already retired the echo left the message with neither a row nor a restored draft, whenever the composer also declined because the user had typed since. Both recovery surfaces were checked and both were unavailable, and nothing noticed. A refusal now re-creates the row when the echo has gone, so there is always somewhere the message can still be seen. And `setMarkdown` returns void and does nothing at all once the handle's internal ref is gone, so the restore reported success without checking. That told the queue the message was safe in the composer when it was nowhere, and the queue used that to drop the only row showing it. Success is now confirmed by reading the text back. The existing test stub never stored what it was given, so it would have passed either way. It now models an editor that keeps its text, which is what makes the read-back assertion mean anything. * fix(frontend): gate every transcript adoption on the mount that started its read A durable send's run stream outlives the mount that started it. When it completes it calls onExecuted, which starts a records read of its own, and the mount can go away while that read is in flight. The session registry deliberately preserves the same Chat across a remount, so the old adopter still holds a working setMessages and a working persistence atom, and it still holds the old mount's watermark refs. Its stale snapshot therefore passed its own guard and replaced a newer transcript, on screen and in the cache, on both hosts. The previous fix checked a boolean before the completion callback and before the queue refresh. It did not cover the read onExecuted starts, and a boolean also fails open across a StrictMode effect replay, which re-arms it after the cleanup. useMountGeneration hands each mount a monotonic generation. Every asynchronous chain that ends in an adoption or a persistence write captures one when it starts and re-checks it immediately before the write. The generation is a required argument on both hosts' adopters, so a call site that forgets one does not compile rather than failing open. Also: a refused send reports a successful restore only when the composer really took all of it. A send carrying attachments the tray has no staged entry for, and an attachments-only send whose staged entries are gone, now keep the flagged row instead of being dropped with nothing restored. * fix(frontend): stop reading a mid-run failure as a refused send An ordinary request never gets an acceptance frame, so the reader had nothing to tell a refusal from a turn that started and then failed. Every error frame in the stream was read as a refusal. Found live on the EE dev stack. The workspace model key had run out of credits, so the run failed about 13 seconds after the answer's first frame, by which time the user's row was saved, adopted and on screen. The reader called that a refused send. The composer restore was attempted for a message that had been delivered, and the echo was flagged, leaving a permanent second copy of the message under "Message wasn't sent - try again" beside the real one. Measured before: peak transcript row count 2 on the first send of a fresh browser context, and the duplicate never went away. After: peak 1 on all five sends, on both hosts. A start frame now marks the turn as begun, and an error after that belongs to the turn rather than to the send. Scanning continues past it, because a detached run names its turn after the answer's first frame and that id is what retires the echo on identity. * fix(frontend): read admission from the turn id, not the adapter's start frames The previous commit used the start frame as evidence that a turn had begun. Codex found that wrong and it was: the Vercel adapter yields start and start-step before it reads a single runner event, so a request the runner then refuses carries them exactly as a successful one does. The rule would have swallowed a genuine competing-turn refusal. The runner emits its turn event only after it admits the request, and the adapter forwards that id as message metadata. That frame is the evidence, and it is the one thing in an ordinary request's stream that proves a turn exists. Verified live on the same stack that produced the original finding. The credits-exhausted run emits start, start-step and the turn id together at 26.2 s, and its error at 39.6 s. The transcript ends with two rows, the durable user row and the assistant row, and no flagged echo. A refusal injected behind the adapter's start frames is still recovered on both hosts. Also, per the same review: the early rejection path has no row to fall back on, because its echo is already dropped, so it now places what it can rather than refusing a partial restore. The StrictMode hook-ordering contract is pinned by a test with an effect declared above the hook. The mobile unmount test flushes the useChat throttle before reading the screen, so its on-screen assertion is no longer satisfied by a value that has not settled.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| it("reads CRLF and CR-only framing", async () => { | ||
| for (const eol of ["\r\n", "\r"]) { | ||
| const w = watcher() | ||
| const frame = accepted("turn-3").replace("\n", eol) |
…ed (#6676) * fix(api): refuse an automatic session rename over a name a person typed An approval card parks the agent's rename_session call. The person renames the session by hand while it waits. The deferred call then runs with the name the agent chose before the rename, so the person's name is replaced and the agent reports the stale one. Nothing in the agent's own view separates a stale intent from a fresh one, so the row answers instead. A header edit now says who chose the name: author=user for a person typing one, author=auto for a program proposing one. A person's name is stamped on the row as a reserved ag. tag, and an auto edit that would replace it is refused with 409 carrying the current name, so the agent adopts it. author is a query parameter, never a body field. The rename_session catalog entry fixes author=auto inside its own path and the model fills only the body, so an agent cannot claim a person chose its name. The browser's auto-title sends author=auto too, so the agent can still name a session over it. override_user_name in the body is the way past the refusal, for the rename a person actually asked for. Fixes #6657 * fix(api): decide the rename guard inside the write transaction, and bind the override to a name Three changes from review. An automatic edit no longer moves the name-source stamp at all. It used to clear it on every edit that carried a name, so the agent repeating the name a person typed - which the rule allows, because it changes nothing - left the row unprotected and the next automatic rename replaced the person's name. The guard now runs inside the DAO's write transaction, against a row locked with FOR UPDATE. A check a layer up was a read-then-write: a person's rename could commit between the read and the write, and the automatic edit would replace a name that did not exist when it was checked. override_user_name is replaced by replacing_name, the exact name the edit replaces. A bare permission is timeless: an agent authorized to rename A can sit parked while the person renames to B, and approving it then overwrites B. A precondition cannot, because it stops matching. Also from review: author=user|auto is now name_source=manual|automatic, which names where a name came from rather than who called; the tag encoding moved out of the core DTOs into the postgres mapper and the rule into its own core module; the 409 carries the agent-actionable envelope from api/AGENTS.md instead of a bare string; and SessionStream no longer exposes the decoded source, since only the write path reads it. Adds a Postgres integration suite. One cell holds the row's lock and asserts the automatic write waits for it, which fails without FOR UPDATE. * fix(api): spend a rename authorization on one state, and refuse an automatic clear Two more holes from the Codex re-pass, both of which let an automatic write weaken the protection it had just been checked against. A successful authorized rename could replay. The person names the session A, asks the agent for B, gets B, then puts A back themselves. Replaying the same call took B again, because the name matched a second time. The row now carries a name revision that counts every name change, and the precondition names both the name and the revision it was read at. A restored name brings back the string, never the revision, so the authorization is spent. Both halves are load-bearing. A revision counts from one and is guessable; a name is not. A name repeats; a revision does not. An automatic caller could clear the name. On a person-named session, an authorized {name: ""} left the row with an empty name, and every later automatic rename walked past the guard, because a row with no name has nothing to protect. Only a person removes a name now. Also: FOR NO KEY UPDATE rather than FOR UPDATE, so a rename does not contend with the KEY SHARE lock a session_turns insert takes on this row; and the refusal message bounds the name it repeats, so a long name cannot push the machine-readable half of the envelope past the runner's 2000-character cut. Web: the browser auto-title now drops its optimistic title when the server refuses the write, instead of showing a name the server rejected until the next list poll disagrees. * fix(api): accept the transitional author spelling on the header route A warm agent session holds the tool descriptors it opened with, so a sandbox started under an earlier build of this change can still execute ?author=auto after the service has moved on. Reading it costs one branch and keeps that call on the guarded path; the alternative is a rename that silently claims a person made it. An explicit name_source always wins, so the alias cannot override a caller that speaks the current spelling, and an unrecognized value falls through to the default rather than failing the rename. The parameter is hidden from the schema and marked deprecated, and can be deleted once no session that old can be alive. This never shipped in a release: git log --all -S 'author=auto' returns only this PR's own commits. * fix(api): keep the precondition values whole in the refusal envelope A rename can store a name longer than the prose bound, and replacing_name has to match the stored value exactly. A caller that copied a shortened quote out of the message was refused forever with no way to tell why, so a long person-chosen name became impossible to replace legitimately. The sentences no longer repeat a name they cannot repeat whole: past the bound they stop naming the session and point at details.current_name instead. Every next_step now names details.current_name and details.name_revision rather than the prose above it. The revision comes first in details, so the runner's 2000-character cut of the serialized detail reaches the small field before a long name can crowd it out. Two follow-ups from the Codex pass, both non-blocking there. replacing_revision now accepts 0. A title filled from a session's first message carries no revision, so a person who keeps that title owns a name nobody counted; rejecting 0 made that one session impossible to rename on request. The browser auto-title clears its optimistic title only when the server REFUSED the write, not when the write merely failed. A refusal means the server holds a name a person chose. A network failure means nobody knows yet, and blanking the row then would lose a title for no reason.
…annot use it (#6694) The model picker listed every OpenAI-compatible ("custom") vault connection under Claude Code as well as Pi. Picking the Claude Code entry committed a revision the server refuses, and the Model row then read "Unavailable". `custom` is an endpoint address, not a protocol declaration, and Claude Code publishes it among its deployments for its own Anthropic gateway. So consuming the surface was never enough. A connection with no saved harness policy now admits each model per route, and the two kinds of route are not the same claim. An OpenAI-compatible route takes every model on the connection, because that surface is a wire format and a gateway serving Mistral or DeepSeek over it is ordinary. Any other route, Claude Code's Anthropic gateway being the only one today, is a claim about the endpoint's protocol, so it takes only the models that name that vendor. That is a default, not a verdict, so a saved harness policy skips it. A model's upstream vendor does not establish the endpoint's protocol: a LiteLLM gateway serves OpenAI-named models over the Anthropic Messages endpoint for exactly this harness. The tick is the user's own statement about the endpoint, and the record carries nothing that could contradict it. A model id is read past its storage namespace. The API stores a custom model as `<provider-slug>/<kind>/<model-slug>`, so a connection called "openai" holding an Anthropic model still reaches Claude Code, and one called "anthropic" holding an OpenAI model does not. The saved model slugs find that boundary, because the stored prefix is a free-text header name that may itself contain a slash. An admitted route carries the provider the SDK needs to resolve it, so a Claude Code route writes `anthropic` instead of falling back to `openai` and being refused. An OpenAI-compatible pick still writes no provider and is byte-identical to before. The technical limit still wins over everything. Fixes #6692
… message's row (#6705) * fix(frontend): drop a refused message's row only once the composer confirms it took the text On the desktop a refusal arriving as an HTTP 200 whose stream errors left the message in two places: a flagged row in the transcript saying it was not sent, and the same text back in the composer. The user could send it twice. Reproduced on staging at 66ed5a6, with the non-ok refusal and the mobile path passing as controls. The composer is Lexical, and setMarkdown schedules the write rather than applying it, so the read-back that confirms the placement runs against a state that has not changed yet. It answered no for a placement that was on its way. The caller kept the flagged row on that answer and the text arrived a moment later. The read-back stays, because reporting success without one loses the message when the editor handle is gone. It now gets the ticks it needs, and the seam reports asynchronously so a host that cannot answer in the calling tick does not have to. The queue now fails to the safe side. The row goes up first and comes down only once the composer confirms, so the message is never in neither place, and it ends in exactly one. This is the case my browser cell for PR #6658 could not see: with SHAPE=b and no DRAFT the cell served a non-ok status instead of the erroring stream, so the desktop path was never exercised. Codex raised the read-back as a P1 against that PR and I recorded it as a follow-up on the strength of that cell. * test(frontend): take the queue harness's restore type from the hook instead of restating it The harness declared the restore callback as synchronous while the seam it exercises had widened to accept a host that answers later, so the asynchronous mocks in the new tests contradicted the declared type. It also referenced QueuedMessage without importing it. Neither showed up locally: the package tsconfig includes src only, so nothing typechecks this directory. Deriving the field from the hook's own parameter type means the harness cannot drift from the seam again, whether or not anything ever typechecks it. Raised on the PR by CodeRabbit and, separately, by Codex.
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
2 participants
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.
New version v0.115.3 in