fix: wait for the server before running the one-time imports - #2544
Merged
Dani Akash (DaniAkash) merged 2 commits intoSep 3, 2026
Merged
Conversation
Found testing an upgrade against a real profile. The database migration worked and the extension storage was intact, yet no providers or scheduled tasks appeared, and the run looked like data loss. The imports run when the background starts, which is the same moment the server starts. They fired into a socket nothing was listening on yet and threw. Because a failed run leaves its marker unset, the next launch lost the same race, so the data never arrived at all while the database migration looked like it had succeeded. Roughly six seconds passed between the browser launching and the server answering. The existing retry did not help because it retried the wrong thing. resolveAgentServerUrlWithRetry retries getAgentServerUrl, which only reads a preference and effectively never fails, so the budget was spent before the request that needed it. The request itself had one attempt. The imports now wait on the health endpoint first, so they wait for their dependency rather than race it. Giving up returns false rather than throwing, which leaves the markers unset deliberately for the next start, and failures are reported through Sentry instead of swallowed: silence is what made this look like lost data rather than a slow start.
Contributor
✅ Tests passed: 467/467Ran 1 of 16 suites (15 not affected by this change).
|
Contributor
Greptile SummaryThis PR delays one-time local-first imports until a local health endpoint responds and reports migration failures through Sentry instead of swallowing them.
Confidence Score: 4/5The PR should not merge until the readiness check probes the same agent server that hosts the one-time import endpoints. The new wait can report readiness from the independently addressed MCP proxy while the agent server is still unavailable, leaving the original import race reachable and also deferring valid imports when only the proxy is unavailable. Files Needing Attention: packages/browseros-agent/apps/app/modules/local-first-migration/wait-for-agent-server.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant BG as Extension background
participant Proxy as MCP proxy
participant Agent as Agent server
BG->>Proxy: GET /system/health
Proxy-->>BG: 200 ready
BG->>Agent: POST provider/job imports
alt Agent still starting
Agent--xBG: Connection failure
BG->>BG: Leave markers unset
else Agent ready
Agent-->>BG: Import succeeds
BG->>BG: Set migration markers
end
Prompt To Fix All With AI### Issue 1
packages/browseros-agent/apps/app/modules/local-first-migration/wait-for-agent-server.ts:20
**Proxy readiness gates agent imports**
If the MCP proxy and agent server become ready at different times, this probe reports the wrong service's state: `getHealthCheckUrl()` uses `PROXY_PORT`, while the imports use the agent URL from `MCP_PORT`. A proxy that starts first leaves the original import race reachable, while an unavailable proxy defers imports even when the agent server is ready.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(app): wait for the server before run..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The wait used getHealthCheckUrl, which resolves the proxy port, while the imports address the agent server on the mcp port. They are separate services that can become ready at different moments, so the probe was answering a question nobody asked: a proxy up first would wave the imports through into the very race this closes, and a proxy that was down would defer imports the agent server was ready to accept. The two coincide in dev, where no proxy runs and the proxy port falls back to the server port, which is why it appeared to work. Health is now taken from the agent server's own base url, so the probe and the request address one service.
Dani Akash (DaniAkash)
merged commit Sep 3, 2026
d0f96fd
into
epic/local-first-storage
8 checks passed
Dani Akash (DaniAkash)
added a commit
that referenced
this pull request
Sep 4, 2026
…yncing them (#2542) * chore(app): add react-query-kit for the local-first storage epic The epic moves provider and schedule config from extension storage onto the local server, which means a large set of new query and mutation hooks over Hono RPC. Kit factories are the repo standard for those, and the app is on plain react-query today, so the dependency lands before the hooks do. 3.3.4 peers on @tanstack/react-query ^4 || ^5; the app is on ^5.101.4. * feat(server): add local storage for llm providers and scheduled jobs First phase of moving provider and schedule config off the cloud and onto the machine. Server side only: nothing reads or writes these tables yet, so the extension is unaffected and this ships dark. Two tables beside the existing agents, conversations and oauth. Credentials live here in the clear, next to the oauth tokens already in this database, protected by filesystem permissions and nothing more. That is the same posture they had in extension storage, and it is why the cloud copy of a provider was never usable: it deliberately never carried a key. Both tables carry a nullable profile_id, always null for now. No extension API exposes a browser profile identifier, so every profile on a machine shares one database. The column exists so isolation can be turned on later without a second migration. Upserts key on the client-supplied id and leave created_at alone on conflict. The migration that follows re-runs per profile and after partial failures, so landing twice has to be indistinguishable from landing once. The job to provider reference sets null rather than cascading. A job whose provider was deleted should surface as needing attention rather than disappearing because of a delete made elsewhere. * feat(server): add the schema and migration for the new tables The previous commit shipped the routes, stores and tests but not the tables they depend on. The server .gitignore has a bare db/ rule, meant for a runtime database directory, which also matches src/lib/db/ and silently swallowed the new schema files and the migration. The existing schema files are tracked only because they were force-added the same way. * fix(server): anchor the db and identity ignore rules to the app root A bare db/ rule matches a directory of that name at any depth, so it covered src/lib/db/ and tests/lib/db/ as well as the runtime directory it was meant for. New schema files and migrations landed ignored, and the existing ones are tracked only because they were force-added. Anchoring both rules with a leading slash keeps the runtime directories ignored while leaving source and tests alone. Nothing on disk was missing from the repository, so this closes a trap rather than recovering anything. * feat(app): stop syncing to the cloud (#2519) * feat(app): stop syncing to the cloud and say so Every write path to the cloud is gone. Providers, scheduled jobs and chat turns now stay on the machine, and a one-time notice on the settings page tells the user what changed. The chat change is the significant one. A signed-in user's turns were uploaded by the client while a signed-out user's were persisted by the local server during /chat. Everyone takes the local path now, so history lands in SQLite regardless of session. The client keeps no write of its own, which also retires the durable turn buffer that existed only to survive an interrupted upload. Incognito still persists nowhere. The legacy conversation migration is kept but no longer branches on session: it always drains pre-upgrade local:conversations into the local server, which is the direction the rest of this work moves data. The sign-in promote is deleted outright rather than disabled. It uploaded the local server's history and then deleted the rows it had uploaded, so under this model it would move data off the machine and drop the local copy. Cloud reads are untouched. Cloud history still displays and a cloud conversation can still be opened, which the next phase turns into a union with the local list. The notice is past tense because sync stops in this same release, so a warning about the future would describe something that already happened. It answers the question people actually have, which is whether they are losing anything: providers, agents and schedules keep working, cloud chats stay visible in history for now. Two consequences of the ignore fix that shipped with the tables. Biome respects .gitignore, so src/lib/db was never linted and had drift, including in the schema files added last phase. The hand-written files are formatted here; the drizzle-generated migration metadata is excluded instead, since formatting it would fight the generator on every migration. * fix(app): do not drain a legacy conversation the server did not store The import route is insert-if-absent, so an id already on the server is answered with a success that wrote nothing. The client only checked the HTTP status, reported the conversation as handled, and the caller then deleted the legacy copy from extension storage. Where the existing server row was an older, shorter version of the same conversation, the messages it did not contain were gone. The route already reported this as `imported`; the client discarded it. A skipped import is now only treated as handled once the server row is confirmed to hold every message the legacy copy has, compared by message id rather than by count so a same-length but different row is not mistaken for the same content. Anything unconfirmed stays in storage for the next attempt. The bug predates this branch, but only logged-out users reached this path before. Routing everyone through it made a latent problem universal, so it belongs here rather than in a follow-up. * feat(app): show local and cloud history together (#2520) * feat(app): show local and cloud history together History was one or the other: signed in showed only the cloud list, signed out showed only the local server. A signed-in user could not see the conversations their own machine was storing, which is now where every new chat lands. The local list is always shown and comes first. Conversations still held in the account appear beneath it under their own heading, saying what they are and that they do not live on this device. Grouped rather than interleaved: the cloud is a shelf that empties when it is retired, blending it into the local list would hide that, and merging two cursor-paginated sources by date against one scroll position buys nothing here. Deduplicated by id, local winning. The same conversation id is used by extension storage, the local server and the cloud, so anything synced before sync was turned off exists in both lists. Two composition problems came out of rendering both at once. The list owned its own scroll container and rendered a <main>, which was fine while exactly one ever rendered and would have been two competing scroll areas and two landmarks side by side; the screen owns a single one now. The empty state also read "No conversations yet" directly above a populated cloud section, which is the common case immediately after this ships, so it names the store it is talking about instead. * fix(app): keep paging the cloud past a page that is entirely local Cloud pagination is driven by a sentinel rendered inside the list, and the list is not rendered while the section has nothing to show. A page whose conversations all exist locally deduplicates away to nothing, so the section returned null, the sentinel never mounted, and the cloud-only conversations behind that page could not be reached. Two guards were involved. The section returned early when nothing was visible, and the sentinel itself sits in the branch the list renders only when it has conversations, so removing the first guard alone would not have helped. The stalling page is the ordinary one immediately after this ships. Legacy conversations are drained into the local server while the same conversations are already in the account, and being the most recent they sort onto the first cloud page. The section now pulls the next page itself while it has nothing visible and pages remain, handing back to the sentinel as soon as something renders. It terminates when the pages run out, so a user whose whole account history is duplicated locally walks the pages once and is shown nothing, which is correct. The decision is a pure function so the conditions are testable without a renderer, including that a fetch is not stacked on one already in flight. * refactor(app): drop the legacy conversation drain Conversations are already written straight to SQLite: the server persists each completed turn during /chat. The drain was a separate path that read pre-upgrade local:conversations from extension storage and posted them back to the server, which is a hop the data does not need. It also ran only for logged-out users before this epic, and widening it to everyone was not asked for. That widening was the sole way a conversation could end up in both the account and SQLite, which is what the deduplication in the history union exists to handle. Nothing writes local:conversations any more, so the leftovers stay in extension storage untouched rather than being deleted. Conversation history is the data we accept losing when the cloud is retired, so paying to move it was the wrong trade. Removes the migration module, its helpers and tests, the client import helper, and the legacy storage definition, along with a dead serial runner left behind when the sign-in promote was removed. Deduplication and the paging that goes past a fully deduplicated page stay. Overlap is now only possible from a promote that uploaded to the account and then failed to delete the local rows, which could easily cover a whole page. * fix(app): stop cloud history auto-advance after a failed page A rejected fetchNextPage leaves hasNextPage true, because it is derived from the last successful page, while the in-flight flag clears. Every input to the advance guard returned to its pre-fetch value, so the section restarted the fetch with no user interaction and a persistently failing request retried forever. Guard on isFetchNextPageError so a failed page settles instead. * feat: migrate providers and scheduled jobs into local storage (#2523) * feat(server): add insert-if-absent import for providers and jobs The import must fill gaps without replacing. The app writes to these tables directly, so an upsert would let a second run restore a stale copy over a row the user edited since. onConflictDoNothing gives the absent-or-present decision in one statement. Also guards /llm-providers and /scheduled-jobs with the app-origin check the other protected routes already use. The blanket trusted-origin middleware only rejects a request carrying a disallowed Origin, so one with no Origin passed straight through to rows holding API keys. * feat(app): migrate providers and scheduled jobs into the server once Reads extension storage and the browseros.providers pref backup, unions them with storage winning, and posts both to the import endpoints. The pref backup covers the reinstall case where extension storage was cleared but the per-profile pref outlived it. The cloud is not a source. Its scheduled jobs include every job deleted since the deletion queue lost its only reader, so importing them would bring deleted jobs back. Its providers never carried credentials and already surface through the incomplete-provider prompt in AI settings. A done marker in per-profile storage stops it repeating. The marker is set only after both imports land, so a failure retries on next startup, which is safe because the server inserts only what is absent. * fix(app): drop unimportable entries instead of failing the batch The import is one request, so a single entry the server rejects returned 400 for every provider in it, blocked the scheduled jobs behind it, and left the done marker unset. That would repeat on every startup, because the pref backup it came from has no migration path and the user cannot edit it. Providers and jobs are now checked against exactly what the server requires, and optional fields holding the wrong type are dropped so the server default applies rather than the batch failing. Filtering runs before the merge so an unusable stored entry cannot win the id and take a good backup copy with it. Removed provider types are excluded too. Storage migrations drop them, the pref backup never gets that treatment, so a stale one could import a provider of a type the app no longer supports. * feat: read and write llm providers through the server (#2537) * feat(app): read and write llm providers through the server useLlmProviders keeps its exact interface so both consumers, AI settings and chat target selection, are untouched. Underneath it is now a react-query-kit query over Hono RPC instead of extension storage. It gains an unavailable state. Previously an empty list meant the user had no providers, and the hook seeded the built-in one in response. Over HTTP a failed load looks the same as an empty one, so seeding moved into the fetcher where it can only run on a confirmed empty response, and AI settings now says the list could not be loaded rather than showing none. Saving a single-instance provider used to collapse earlier copies as a side effect of writing the whole list. That is now an explicit plan of one PUT and the deletes it displaces. The default provider id stays in extension storage. It is a per-profile preference and every profile shares one database, so a column would make them share a default too. A stale id costs nothing because it is resolved on read. Logout no longer deletes providers or scheduled jobs. That was right while they were account data synced to the cloud; they are now local data the account does not back. * fix(app): do not substitute a provider the caller did not choose An unreachable provider list returned an empty array, so a scheduled job that named a provider found no match and fell through to the built-in one. It ran on a different model with different credentials and was recorded as completed. The list being unreachable says nothing about whether that provider exists, so the two cases are now distinguished and naming a provider that cannot be loaded fails the run instead. A provider that was genuinely deleted still falls back, as before. Deleting a provider also persisted the replacement default before attempting the delete, so a failed delete left the provider configured but no longer default with nothing to show for it. The delete goes first; a default id left pointing at a deleted provider is repaired on read. * fix(app): never resolve a provider from a list that failed to load The previous guard only covered a job that named a provider, which left the same hole one step over. A job that names none still has a choice behind it: the configured default, whose id lives in extension storage but whose model and credentials live in the list. So an unreachable list sent those runs to the built-in provider and recorded them completed, which is the case this guard existed to prevent. The condition drops to the list itself, which also states the invariant plainly. An empty list keeps the fallback, because that is the server answering that it genuinely has no providers rather than not answering. * feat: move scheduled jobs and run history to the server (#2538) * feat(server): add local storage for scheduled job runs Job definitions had a table; their run history did not, so it was the one part of the domain with nowhere to live on this side. Runs cascade on job delete, unlike the job to provider reference which is set null. A job whose provider was removed is a job needing attention, whereas a run whose job was removed means nothing, and deleting a job already removed its runs before this table existed. The tool call log is a json column. Its input field is optional here where the extension has it required: an unknown already admits undefined, so the two describe the same values, and matching the validator avoids asserting the difference away at the route boundary. * feat(server): carry the per-job run cap across with the runs The extension kept fifteen runs per job, trimming as it created each one. Now that it no longer owns the history that policy has to live here, or the table grows without bound. It applies on every write rather than only on creation, which is bounded and idempotent, so it holds however the run was written. The import path does not prune, staying purely additive; the next real run trims. * feat(app): read and write scheduled jobs and runs through the server The hooks keep their shape, so the tasks page, the results view, the card and the new tab panel are unchanged apart from where they import from. Both gain an unavailable state, since an empty list and an unreachable server are now the same shape without one. The alarm runner distinguishes them everywhere it reads. Treating a failed load as an empty list would read as nothing being scheduled: alarms would not be rebuilt on startup and schedules would quietly stop firing, with no failed run to show for it. It skips the pass instead and retries on the next startup. Extension storage no longer carries the data, but it still carries the change signal. Runs are written by the background while the side panel and new tab display them, and storage watch is what kept those in step. A revision item is bumped after a write so every mounted view refetches. Run history is imported once, under its own marker. It cannot share the provider and job marker because that import must never run twice: extension storage is frozen now, so a second pass would insert back whatever the user has since deleted. Also removes the scheduled job deletion queue, whose only reader went when sync did, and the mount-time storage read that chose the opening tab, which is now derived so it settles when the history arrives. * fix(app): record a finished run against the current job Recording that a run finished wrote back the job as it was read before the run started. A run can take minutes and the job stays editable throughout, so a rename, a schedule change, a disable or a different provider chosen while it was going would be silently reverted. The old code merged into a freshly read list; passing the job object instead was an attempt to save a read and is what lost the update. It takes an id again, so a stale snapshot cannot be handed to it, and it skips the write when the job was deleted mid-run rather than resurrecting it. * chore: sync the local-first storage epic with main (#2539) * fix(server): steer ACP agents to browseros, not a co-installed browseros-neo (#2517) * fix(server): steer ACP agents to browseros, not a co-installed browseros-neo * refactor(server): replace ACP skill file with system prompt + workspace CLAUDE.md/AGENTS.md * refactor(server): slim the agent system prompt and move tool guidance into the tools (#2521) * refactor(server): slim the agent system prompt, move tool guidance into tools (TKT-947) * docs: tidy prompt comments * fix(browser-mcp): fence run structured output so untrusted values reach the model marked * test(server): expect fenced run structured output in browser + dual-era tests * feat(app): give first-run its own setup step instead of the settings page (#2511) * feat(app): give first-run its own setup step instead of the settings page Finishing the native onboarding dropped the user on the full AI settings screen: sidebar, configured list, promos, default-target control, usage and billing links. That is an administrative surface, and it was someone's first minute with the product. Adds #/onboarding/ai, a bare route beside features and outside every layout, carrying the provider catalogue and nothing else. Connecting anything hands off to #/home, which is the new tab page, so the first thing after setup is the thing the product is for. The handoff fires on the transition to connected, never on the state. A subscription template takes the user off the page and back, so success arrives as a change to the provider list rather than from a submit handler, and a user who opens the route with providers already configured has to stay on it rather than being bounced. Connected cannot mean a non-empty provider list: a built-in entry is seeded on first load, so it means any provider that is not that one, or any agent. The dialog and OAuth wiring moves out of BrowserOsAiPane into a shared hook, since the catalogue only raises intent and something has to own the four dialogs. The settings page behaves exactly as before. Includes a skip, because both onboarding exits still land here and the page has no sidebar to escape through. * fix(app): hand off when an already-configured user connects something The handoff compared a boolean: not-connected becoming connected. For anyone who already had a provider or an agent that boolean was true on arrival and stayed true, so adding another connected nothing and the page never moved. Only a profile with nothing configured could ever reach the new tab page. It now compares a count against a baseline taken when both lists settle, so what matters is whether the user connected something on this visit rather than whether they had ever connected anything. Deleting does not count: the count has to grow. Readiness now waits on the agent list too, via its flag rather than . The two lists load on separate async chains, and that hook documents that reads false for a render while the list is still empty, so a baseline taken on the providers alone could miss existing agents and fire the moment they arrived. The previous behaviour was covered by a test asserting a user who arrives already connected is not handed off. That test encoded the bug, so it is replaced by two that cover adding to an existing setup. * refactor(app): hand off from onboarding on the add callback, make added provider default * fix(app): set the added provider or agent as the active chat target before handoff * fix(app): hand off with the persisted provider id so an OAuth reconnect resolves * chore: browseros-claw update * chore: bump version * chore: bump app onboarding version to 0.0.1 (#2524) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore(release): snapshot browseros server alpha v0.0.152 Automated release snapshot update. * chore: bump server version to 0.0.152 (#2526) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore(release): update extension alpha feeds to 0.0.146.0 Automated release snapshot update. * chore: bump agent extension version to 0.0.146.0 (#2528) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: sync internal-docs submodule (#2529) Co-authored-by: browseros-bot <bot@browseros.ai> * feat: unify product installation identity (#2530) * feat(chromium): unify product installation metrics * feat(agent): share BrowserOS installation identity * feat(claw): separate installation identity from consent * fix(dev): share product state root with Chromium * test(agent): cover canonical installation identity * fix(agent): clean failed identity publishes * style(claw): satisfy analytics lint * feat: agents never steal focus in BrowserClaw (#2531) * feat(claw-mcp): tabs new and pages.newPage always open in the background Agents can no longer request a foreground tab. The background field is still accepted and ignored so clients holding the old schema keep working, but it is hidden from the tool schema. Conformance cases stop assuming an agent-opened page becomes the active tab. * feat(patches): automation never steals focus pref and Browser gates Adds browseros.automation_never_steals_focus (default on for BrowserClaw). With it on, a tab with a DevTools client attached cannot switch the user's active tab or raise the window through Browser::ActivateContents, and tabs or popups its pages open after an agent click land in the background (Browser::AddNewContents, mirroring the upstream actor gate). * feat(patches): Browser.createTab defaults to background; activate commands honour the focus pref createTab now opens tabs in the background unless background=false is passed, and an explicit false only selects the tab within its window. Under browseros.automation_never_steals_focus, activateTab stops raising the window, activateWindow becomes a no-op, and createWindow plus setWindowVisibility(activate) show windows inactive. * test(claw-mcp): retired tabs background field stays accepted but inert * chore: bump version * ci: grant nightly call sites the permissions their workflows declare (#2532) * ci: grant nightly call sites the permissions their workflows declare The nightly family workflow had never started: a called workflow can only narrow the caller's GITHUB_TOKEN, so any call site whose ceiling is below what the called workflow declares fails the entire run at validation time, before a single job is created (run 33689075661, startup_failure). Six call sites were short: - prepare/finalize-claw-server granted contents: write, but release-claw-server.yml declares publish-ota and reflect-version with pull-requests: write. Both are skipped here (publish_ota: false, state_owner: suite) but validation is static and runs before if:. - build-browseros/build-browserclaw had no permissions block, so they inherited the workflow-level permissions: {} and granted nothing to nightly-macos-product.yml, which declares contents: read. - server-ota/claw-server-ota granted contents: read to publish-server-ota.yml, which declares contents + pull-requests write to publish the feed snapshot and its reconciliation pull request. Ceilings now match what release-browseros.yml and release-browserclaw.yml already use for the same called workflows. * ci: keep the nightly ceilings minimal Narrows the previous commit to the only call sites that actually elevate. Only job-level permissions inside a called workflow are validated against the caller's ceiling; a callee's workflow-level block is a default for standalone runs and is supplied by the caller when it is invoked through workflow_call. release-claw-server.yml is the only callee here that declares job-level permissions (publish-ota and reflect-version, both pull-requests: write), so it is the only ceiling that had to widen. Reverted as unnecessary: - build-browseros/build-browserclaw: nightly-macos-product.yml declares no job-level permissions and never checks out or uses the token. - server-ota/claw-server-ota: publish-server-ota.yml declares none either, and in suite mode the writes belong to reconcile-state, so contents: read is the correct least-privilege ceiling. ci_workflow_test asserts it. * ci: cover the permissions the nightly's build and OTA callees declare (#2533) The nightly still failed validation after the claw-server fix. Bisecting with push-triggered copies of the workflow on a scratch branch localised two more call sites; each was proven in isolation: - build-browseros/build-browserclaw had no permissions block, so they inherited the workflow-level permissions: {} and granted nothing to nightly-macos-product.yml, which declares contents: read. - server-ota/claw-server-ota granted contents: read to publish-server-ota.yml, which declares contents and pull-requests write. A called workflow can only narrow the caller's GITHUB_TOKEN, and that is checked statically for the whole nested tree before any job is created, so a short ceiling rejects the entire run. With both covered, a full copy of the workflow created all 17 jobs and stopped at the intended 'must run from refs/heads/main' guard. ci_workflow_test asserted the contents: read ceiling that caused this, so it encoded the bug; updated to the ceiling that actually validates. * chore: sync internal-docs submodule (#2535) Co-authored-by: browseros-bot <bot@browseros.ai> --------- Co-authored-by: Nikhil Sonti <nikhilsv92@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: browseros-bot <bot@browseros.ai> * feat: unify the provider tables and resolve the chat provider on the server (#2540) * feat(server): merge acp agents and llm providers into one table Both are providers for the chat, and everything above the database already said so: the chat target is one union with a kind on the client, and the wire target has always been a discriminated union. Only storage disagreed, and it charged for the mismatch. Selecting an acp agent left the default provider pointing at the previously selected llm one, because the write was conditional on kind. A scheduled job could reference an llm provider and nothing else, so picking Claude Code in chat was possible while scheduling against it was not. Migration 0010 creates the unified table and copies both sources in; 0011 repoints scheduled jobs and drops the old tables. Two migrations rather than one, so the copy proves itself before anything is dropped, and because drizzle cannot diff a simultaneous add and remove without a rename prompt. Drizzle put its drops ahead of its own foreign key pragma, which fired the ON DELETE SET NULL on scheduled jobs and silently unlinked every job from its provider. Reordered so the drops happen after the rebuild, inside the block where enforcement is off. The default provider now lives on a column, and a partial unique index admits exactly one row, of either kind. Not keyed by profile: sqlite treats nulls as distinct in a unique index, so pairing it with the unset profile column would let every row be default at once. Also brings the packaged-build schema fallback current. It had drifted four migrations behind, so a build without migration files would have created a database with none of this epic's tables. The test's copy of the migration history is now derived from the journal, since a hand written duplicate is what let the drift go unnoticed. * feat: resolve the chat provider on the server The provider block is gone from the chat body. A request names an id, or names nothing and gets whichever provider is selected, and the server fills in the model, endpoint and credentials from the row. Sixteen fields of provider configuration collapse into one, and the api key, the aws secret and the session token stop crossing the wire on every message. Every one of those fields is still accepted. The extension updates independently of the browser binary, so a shipped build can be running a client that sends the whole configuration inline; the server stops requiring them, not accepting them, and a row it does not recognise leaves whatever the client sent in place. The selection moved to the server with the tables it points at, so choosing a coding agent now records it. It could not before: the default lived in extension storage and only ever named an llm provider, so picking an agent left it pointing at the provider chosen before it. The scheduled runner drops its provider lookup entirely, and the guard that came with it. That guard existed because an unreachable list and an empty one looked alike, so a job could run on the built-in provider with the wrong credentials and still be recorded as a success. There is nothing to tell apart now: the job names an id and the server resolves it. Refine prompt still resolves on the client. It posts to its own endpoint with its own schema, and giving it the same treatment is separate work. * fix(server): gate chat on trust when the server supplies the credentials A browseros chat request is deliberately allowed without the app-origin check, on the reasoning that it carries its own credentials and so can only spend what the caller already had. Resolving the provider from storage broke that reasoning: naming an id, or naming nothing and taking the selected provider, would have let any local caller spend the user's key against an external service. The check now applies exactly when the configuration came from a stored row. A request that brought its own is as unrestricted as it was before, so the capability that reasoning was about is untouched. * fix(server): gate chat on every path where the server holds the credential The previous gate keyed on whether a stored provider row was read, on the reasoning that a request naming no known row must have brought its own key. That is false for four provider types. The oauth three take a token from this machine's oauth store and browseros takes the gateway credential, so naming one with an unknown id skipped the check and had the server hand over a credential the caller never held. The predicate lives beside resolveLLMConfig, since it has to mirror those branches exactly and would drift if the route kept its own copy. This particular hole predates the change: the exemption for browseros requests and the credential injection behind it were both already there. It is fixed here because the gate added alongside it claims the ungated path is safe, and that claim has to hold. The chat integration test now sends the origin header the extension always sends. It was relying on the exemption this closes, and the routes the background alarm runner already calls carry that header today. * test(server): stop a module mock dropping the exports it does not name CI failed with `Export named 'SERVER_CREDENTIALED_PROVIDERS' not found` against a file that plainly exports it. A module factory is a total replacement, so everything it omits disappears for every file importing that module afterwards, and bun's registry is process wide. Adding an export to a module someone mocks partially is enough to break a different file entirely. The factory now spreads the real module and overrides only the function under test. This does not reproduce locally: file ordering is stable on APFS and not on ext4, which the test runner's own notes call out as the reason this class of failure kills CI while local runs pass. * fix: restore cross-surface provider sync and keep credentials off provider reads (#2541) * fix(app): restore cross-surface provider sync and stop hiding credentials Three issues from testing on a real profile, two of which share a cause. Extension surfaces are separate contexts with separate query caches, and the provider list broadcast to all of them through extension storage until it moved to the server. Nothing replaced that, so a provider added in one surface stayed invisible to the others. The scheduled tasks dialog showed only the built-in provider for exactly that reason. Writers now bump a revision in extension storage, which does reach every context, mirroring what the scheduled runs already do. The same staleness turned destructive in the sidebar. Selecting a new provider wrote the choice and the server accepted it, then the repair effect ran against a list this surface had not refreshed, failed to find it, and wrote the built-in provider back over the selection. Agents were unaffected only because the repair skips them, which is why it looked like a provider problem. Absence from a list in hand is no longer read as deletion. A scheduled job can target a coding agent since the two provider tables merged, but the dialog only ever offered llm providers, so the capability was unreachable. It now offers both. Every provider read returned the api key and the aws secret. The store gained a projection that omits them and reports only whether each is set, and the one caller that needs them, building an outbound model request, asks by name. Since a client can no longer read a credential back, an upsert that omits one keeps what is stored, or a rename would wipe the key on save. Also filters the provider list on kind rather than leaning on the unknown-type guard, which dropped coding agents by accident and said something else in its comment. * fix(server): treat a blank credential as not supplied, not as a clear The keep-existing guard only skipped undefined, but a form field the user never filled in submits as an empty string, and the schema accepts it. So the very edit the guard exists to protect, renaming a provider without retyping a key that is no longer readable, wrote an empty string over the key. The flags then reported it as stored, because an empty string is not null, so nothing looked wrong until the next request failed to authenticate. Blank now counts as not supplied on the way in, and as unset on the way out. Clearing stays deliberate: send null. The four flags share one definition rather than four copies, since a divergence would only ever show on whichever credential nobody tested. * fix: wait for the server before running the one-time imports (#2544) * fix(app): wait for the server before running the one-time imports Found testing an upgrade against a real profile. The database migration worked and the extension storage was intact, yet no providers or scheduled tasks appeared, and the run looked like data loss. The imports run when the background starts, which is the same moment the server starts. They fired into a socket nothing was listening on yet and threw. Because a failed run leaves its marker unset, the next launch lost the same race, so the data never arrived at all while the database migration looked like it had succeeded. Roughly six seconds passed between the browser launching and the server answering. The existing retry did not help because it retried the wrong thing. resolveAgentServerUrlWithRetry retries getAgentServerUrl, which only reads a preference and effectively never fails, so the budget was spent before the request that needed it. The request itself had one attempt. The imports now wait on the health endpoint first, so they wait for their dependency rather than race it. Giving up returns false rather than throwing, which leaves the markers unset deliberately for the next start, and failures are reported through Sentry instead of swallowed: silence is what made this look like lost data rather than a slow start. * fix(app): probe the agent server rather than the proxy The wait used getHealthCheckUrl, which resolves the proxy port, while the imports address the agent server on the mcp port. They are separate services that can become ready at different moments, so the probe was answering a question nobody asked: a proxy up first would wave the imports through into the very race this closes, and a proxy that was down would defer imports the agent server was ready to accept. The two coincide in dev, where no proxy runs and the proxy port falls back to the server port, which is why it appeared to work. Health is now taken from the agent server's own base url, so the probe and the request address one service. --------- Co-authored-by: Nikhil Sonti <nikhilsv92@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: browseros-bot <bot@browseros.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found running the upgrade against a real profile: the database migration worked, extension storage was intact, and still no providers or scheduled tasks appeared. It looked exactly like data loss.
What was happening
The one-time imports run when the background starts, which is the same moment the server starts.
That seven second gap is the whole bug. The UI worked because it ran later. The background did not because it ran first.
The important part is what happens next: a failed run deliberately leaves its marker unset so it retries on the following start. But the following start loses the same race, so it never completes. The data stays in extension storage indefinitely while the database migration looks like it succeeded.
Why the existing retry did not cover it
resolveAgentServerUrlWithRetryretriesgetAgentServerUrl, which is a preference read and effectively never fails. All three attempts succeed instantly, so the retry budget is spent before the call that needed it, and the request itself gets a single shot at a server that is still booting.The fix
The imports wait on the health endpoint before running, so they wait for their dependency rather than race it. Sixty seconds at one second intervals, which covers a cold start where the server has migrations of its own to apply.
Giving up returns false rather than throwing, so the markers stay unset on purpose and the next start tries again, which is the behaviour that was already intended.
Failures are reported through Sentry rather than swallowed. Silence is what made this present as lost data rather than a slow start, and a background failure otherwise has nowhere to surface.
A probe that throws counts as not reachable rather than ending the wait, since connection refused is the expected state early on.
Verification
tsc --noEmitclean on both, biome clean.Not covered here
This does not change what happens if the server is genuinely absent for a whole session: the imports defer to the next start, as before, but now say so. Nor does it retry an import that fails for a reason other than the server being down, which is correct, since a validation failure would only fail again.