Skip to content

Setup, end to end: pick a Bot, connect a model, prove it answers - #451

Merged
davidmckayv merged 273 commits into
mainfrom
s5-mastra-bot-kind
Sep 11, 2026
Merged

Setup, end to end: pick a Bot, connect a model, prove it answers#451
davidmckayv merged 273 commits into
mainfrom
s5-mastra-bot-kind

Conversation

@davidmckayv

@davidmckayv davidmckayv commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Desktop setup and operation

Adds the OpenBot desktop setup flow for harness selection, model-provider sign-in or API keys, managed Intelligence sign-in, and starting the selected deployment. The app includes OpenBot bundle/tray icons, close-to-tray, restore, singleton handling, Stop, and Quit.

Desktop credentials use deployment-scoped files on macOS/Linux and account-bound DPAPI blobs on Windows. The app no longer requires Keychain access or migration. Saved values are not displayed again in the credential UI.

The review corrections preserve deployment ownership across failed Start retries, keep container cleanup tied to the selected runtime, stop owned Unix descendants, attribute connection diagnostics to the selected endpoint, and retain recovery and Stop failures through setup navigation. ChatGPT token refresh now preserves the existing file owner and private permissions so a root Linux container does not make the saved sign-in unreadable to the desktop user.

Quit cleanup errors now persist a bounded, structured notice for the next launch. Recovery checks prevent surviving processes from hiding that notice. If saving it fails, OpenBot stays open, shows the error, and permits another Quit attempt. Raw cleanup output is not persisted.

Validation

Earlier native macOS and Windows candidates were exercised through actual setup/sign-in, browser navigation and page results, command execution and stdout, computer-frame capture, model continuation after tool results, and saved-chat restore. Native lifecycle checks covered tray/restore, Stop/Quit, and owned-resource cleanup. Later changes have targeted source, process, IPC, UI, and regression evidence; native claims are limited to the flows actually recorded.

This checkpoint additionally validates failed native Windows menu Stop returning to the actual setup UI: the old build loses the diagnostic, and the corrected build preserves its summary and original Compose failure. Linux token refresh was tested with the supported image on an owned Linux filesystem: the old root writer prevents the desktop-user reader from opening the refreshed file, and the corrected writer preserves ownership and mode 0600. An asynchronous non-root writer control also passes. Regression tests cover both fixes.

The local branch now includes current main to match the merge commit used by pull-request CI. The full Bun test run passes with 3,039 passed, 21 skipped, and zero failures against task-owned Postgres configured like CI. Formatting, lint, TypeScript checks, root/desktop frontend builds, and affected workflow validation pass. The final Quit source correction also passes independent review, 125 executable-target Rust tests, formatting, check, clippy, and a full executable build. Its native Windows proof is still pending. The Linux Podman test fixture now handles the explicit local-runtime selector and keeps local ownership independent of remote defaults. The exact failing command was reproduced before correction; its regression and 24 related controls pass. This final fixture change does not alter production code. Platform CI must confirm this checkpoint.

Current publication checkpoint: b4491f899c895a97f7d089d8b82b4a4734cd1f04. This remains work in progress; it is not a final approval or merge request.

Remaining work

  • Complete native Windows validation of the final Quit correction: persistence before exit, actual setup display after relaunch, and failure to save the notice.
  • Connection-probe responsiveness remains unvalidated; its native validation action was blocked by automatic review.
  • Two earlier reports remain unconfirmed: copied deployment ownership and Quit/Start admission. The latter proof action was also blocked by automatic review. Neither is claimed as a demonstrated defect.
  • Confirm platform CI and the final native evidence before the final update.

S6 telemetry and S7 signing, packaging, and auto-update are separate slices in the desktop build specification. No signed/notarized release, Linux GUI validation, or actual rootless Docker daemon validation is claimed here.

…han a token

A plan token is a bearer for chatgpt.com/backend-api/codex, and langchain-openai
pins that address on purpose, so it cannot be reached by pointing OPENAI_BASE_URL
at it with the token as a key. The harness now picks the Codex chat model when a
plan is present, so the default Bot can actually answer on a subscription.

Carrying the access token alone was wrong: it expires within the hour and nothing
can renew it, which would give a Bot that works in the morning and fails after
lunch with an auth error nobody could account for. The sign-in now hands back the
vendor's whole store, the store is written beside the .env as an owner-only file,
and compose bind-mounts it read-write so the renewals the provider makes outlast
the container. The .env gets a path, never the credential.

The file is written even when no plan was chosen, because a bind mount with no
source does not fail, it silently creates a directory in its place.
A machine that ran the version before this one has a ChatGPT plan token sitting in its .env that nothing reads any more. The writer preserves lines it does not own, so it would stay there indefinitely. Clearing it costs one line and is the same reasoning the other model keys are emptied for.
Every step before this proves that something started, which is not the same as
proving the choices work. A refused key, a lapsed plan or a model the account
cannot use all give a stack that comes up clean and a Bot that cannot answer,
and handing over at that point means somebody finds out later, inside the
product, with no idea which of their answers caused it. So the wizard now ends
on a question with one checkable answer, and the handover waits for it.

This screen also owns the worst message in the product. Measured against a
deliberately invalid key: the stream opens, says RUN_STARTED, says STEP_STARTED
and then simply stops, with no error event at all, because the framework caught
its own exception and logged it. The whole 401 lives in the container's log and
nowhere else. So a run that produces no text is a failure here rather than an
empty answer, the sentence shown is OpenBot's own and names the choice to
change, and the harness's log is fetched to fill the developer half, since
otherwise there would be no developer half to show.

Two live tests are kept and ignored by default. The fixtures are transcriptions
of a real stream, and a vendor changing the events they emit should break
something.
The .env is a settings file, and a settings file is something somebody opens,
reads out to support or pastes into a chat. A model key, a plan token and the
tokens these services prove themselves to each other with are not settings.

They now go to the credential store each platform actually has: the login
Keychain on macOS through security, DPAPI on Windows through ProtectedData
encrypting to the signed-in user, and an owner-only file on Linux, which is
said out loud rather than dressed up, because no desktop Linux install can be
assumed to run a Secret Service daemon and refusing to save a credential
because gnome-keyring is missing would fail more people than it protects.

The value never goes on a command line on any of them. ps is readable by every
process the person runs, so the Keychain and DPAPI paths both write over stdin.

From the store the credentials travel to the containers and the host processes
as environment. Compose resolves an interpolation from its own environment
before it reads the .env, so a secret reaches exactly the services that declare
it and is written down nowhere. Verified against a real deployment: a .env with
no token, the token in the environment, and the container holding it.

The writer also purges what it moved. Without that, every machine that ran an
earlier version would keep its old plaintext copy exactly where it was and the
change would have bought nothing for anybody who already had OpenBot.
Three buttons asked for `secondary`, which no rule matches, so Stop OpenBot has
been rendering identically to Show OpenBot and the new last screen offered two
equally weighted actions. The stylesheet's quiet button is what they meant.

Found by looking at the screen rather than at the markup, which is the only way
a missing class shows up: nothing errors, the button just draws as the primary.
…ity command

The `security` command takes its password through a prompt whose buffer is 128
bytes, and anything longer is cut off with no error and an exit status of zero.
Probed a length at a time: 128 stores 128, 129 stores 128, 200 stores 128.

An OpenAI project key is 164 characters. Every one of them was being saved
truncated and read back truncated on the next run, while the run that saved it
worked fine, because the value it used came straight from the window. The next
launch would have been the broken one, with a key nobody had changed.

No flag raises that buffer, and the only ways past the prompt put the credential
on a command line where ps can read it. The framework has neither limit, and the
Windows and Linux paths never had one, so this is one platform's dependency
rather than a cross-platform crate and twenty transitive packages.

Round-tripped against the real Keychain at 128, 129, 164, 256 and 512.
KEY_ENCRYPTION_KEY is what every secret the server stores goes through, and
encrypt-sso-config.ts names the symptom itself: a changed key leaves stored
configuration unreadable and sign-in broken until it is registered again.

The shell minted a new one on every press of Start, so a button labelled Start
silently orphaned everything the previous run had encrypted. The rest of the
generated secrets point the same way for a smaller reason: a Bot's computer is a
container that outlives a restart holding the old COMPUTER_TOKEN, so rotating
buys nothing and can only strand it.

Two installs still do not share a key. A machine with nothing stored generates,
which is what a first run is. Verified across a real Stop and Start: the token
the second run handed its containers is the one the first run minted.
Two halves of one dead end, both found by pressing the buttons.

Stop left agent-harness running. Compose only acts on a profiled service when
the profile is named, so the one container the person actually chose stayed up
on their laptop after they had stopped the app, still holding its port.

Then the next Start refused: "something is already listening on port 4206, which
OpenBot uses for the Bot you picked" — about a container OpenBot itself had
started, which the person never saw and could not find. There was no way forward
from that screen. A port this deployment already publishes is not a stranger on
the port, and compose up reuses what is there, so the check now skips our own and
keeps its teeth for somebody else's. Host processes are reclaimed for the same
reason: a start that got as far as spawning the server and then stopped left it
holding 3001, and they are identified by working directory, so anything stopped
belongs to this deployment and no other.

Verified in the window: Stop leaves nothing running, and a Start with our own
container on the port goes straight through.
Four things, all found by signing in through the window rather than testing the
pieces.

The login program is rendered by substituting placeholders, and renaming the
marker to OPENBOT_CHATGPT_STORE made the marker contain the STORE placeholder.
Rendering rewrote the program's own print line into a syntax error, so the
container died before printing anything and the window said "the sign-in never
offered a link to open" — a sentence with no relation to its cause. The
placeholders are underscored now and the test reads the rendered program rather
than the template, which is where it broke.

That failure also had no technical half, so there was nothing to diagnose it
with. It carries the container's output now, with the store line stripped.

The window rendered a failure with String(error), which for a two-fold problem
prints [object Object]. There is one failure component now and every screen uses
it.

And the last screen swallowed its own failure entirely: it recorded that
something went wrong and threw the problem away, believing the screen around it
would show the sentence. Nothing did. A plan that could not answer produced a
"Change the model" button and no words at all — the exact silence that screen
exists to replace.

Proven in the window: consent in the browser, the store written owner-only with
its refresh token, mounted into the harness, and the Bot answering 17 x 23 = 391
through the Codex model with no OpenAI key in the container, the .env or the
Keychain.
…end it

Two defects that only the Anthropic path could show, both on the last screens a
person sees.

macOS routes the clipboard shortcuts through the menu bar, and this window had
no Edit menu, so it had no Paste. Typing into the code field worked and pasting
did nothing — on the one screen whose own instruction is "paste the code it
shows you". Everybody signing in to a Claude plan would have reached that field,
pressed the shortcut they have used all their life, and had nothing happen.

Then, with the code in: a plan is not a key, and only one Bot speaks each
vendor's subscription. Signing in to Claude and keeping the default Bot gave a
stack that came up clean and a Bot whose log read "Missing credentials. Please
pass an `api_key`". The person had answered both screens correctly and had no
way to know which answer to change. The plan now re-points the Bot, and the
model screen says which Bot that will be while there is still a screen to say it
on. Nobody is asked to know that a subscription constrains the framework.

Proven in the window on both plans: ChatGPT answers 17 x 23 = 391 through Codex
with no OpenAI key present, and Claude answers 391 on the Claude Agent SDK with
no Anthropic key present. The refusal of a stale code and the failure of a Bot
that cannot answer both render with a plain sentence and the container's own log
behind a disclosure.
… find

Signing in from the window had never been run end to end. It failed four times
in a row, each time silently or with a message that named nothing, and each fix
was only findable because the failure started carrying what actually came back.

The session is called `cliToken`, not `token`, so the very first exchange failed
with "error decoding response body" and no way to tell which field or which
endpoint. Failures here now carry the response, and that answered it in seconds.

Project ids are numbers. Requiring a string dropped every project, and the
screen said "That account has no projects yet" to somebody with ten of them. An
empty list and an unreadable one are told apart now, because one of them is a
lie a person cannot argue with.

The keys endpoint declares `project_id: z.number()` with no coercion, so the
string "7" came back HTTP 400 VALIDATION_ERROR on the last step of the flow. Read
off `api-keys-routes.ts` rather than guessed.

And the project tiles rendered as blank white rectangles: `button` sets a white
colour, `.tile` overrode the background to white and not the colour, and the
provider rows escaped it only because they are labels. Nothing errored. The
screen asked somebody to choose between six empty boxes.

A shown body is masked, because the one that diagnosed the first bug also
carried a live session token, and the shape is what a developer needs from it.

Proven in the window: sign in, choose an organisation, ten projects listed by
name, one picked, a key created, "Connected to CopilotKit". No key typed.
The compatible row is the only one that names a model, and switching away from
it kept the name. Answering with an OpenAI key after using a local endpoint left
BOT_MODEL=local-model, so the Bot asked OpenAI for a model only that person's own
server has, and the last screen said "That account cannot use the model that was
chosen" about a model this run never chose.

Exactly the failure the key-clearing exists for, with one key missed. It is
removed rather than emptied so the compose default applies, and taken out of the
file as well, because the writer keeps lines it does not own and that is what let
it survive.

Found on a full pass through the window, in the first path.
…ndow

Clicking a conversation in the rail drew the coworker's name and then nothing at
all. The rail comes from OpenBot's own database, so a channel is listed whatever
the history store says; the messages live in the Intelligence project, and
pointing a deployment at a different project leaves the platform answering
THREAD_NOT_FOUND.

That 404 is deliberately read as "no history" and must stay that way: a thread id
is minted before the thread exists, so a brand-new conversation 404s as its
normal opening move. Widening it would tell somebody their conversation was gone
and invite them to start it over.

The two cases are told apart by a fact the app already stores. lastMessageAt is
set only once something has been said, so a conversation with none is genuinely
new and silence is correct, while one that has been spoken in and comes back
empty has a history this deployment cannot reach. That one now says so, in the
notice slot beside the existing explanations for a deleted coworker and for
turns that could not be parsed.

The channel DTO carries lastMessageAt for it, which the shape tests pin, plus a
new test that the date leaves as a string and leaves at all.
…et one

Setup ended at "Install Podman Desktop or Docker Desktop first" on any machine
that had neither, which is every machine this app is for. The step existed in
the enum with nothing behind it and the screens had been reworded to stop
promising it. So the whole install stopped at a download page.

It installs one now, and the second half is the half that gets forgotten: Podman
ships no Compose implementation, so a machine with a freshly installed Podman
still cannot raise the stack and answers with seven errors naming docker-compose.
Both are fetched, each pinned to the digest of the release it was tested against
and refused if it does not match, because these are files this app then executes.
Only what is missing is added: an engine somebody already has is theirs, and a
Compose that already answers is left alone.

Windows installs unattended. macOS and Linux each raise one authorization prompt,
which is the platform's own and is not something to route around: the package
writes to /opt/podman, and on Linux Podman is a set of binaries wired to the
distribution's paths rather than one file to download.

Two things were needed to make the result usable in the session that installed
it. The MSI extends the USER's PATH, and this process was started with the old
one, so podman could not be run for the rest of the run: every engine command
now names a resolved path, found on PATH first and in the installers' own
locations second. And the Compose provider is put in front of the child's PATH
rather than written into containers.conf, which belongs to whoever else may have
configured it.

Measured on Windows Server 2022, which is also where the recovery came from: a
Podman removed by deleting its folder leaves the registration behind, so /i
becomes a repair with no source and stops with 1603. That case uninstalls and
installs cleanly instead of reporting a failure somebody cannot act on.

Image references now come from the release's manifest wherever the shell runs a
container itself, not only where Compose does. This is the same bug a third time:
first the names were built from the ids and matched nothing published, then the
version stopped being appended so an engine read the bare name as :latest, and
now openbot-agent-langgraph-agui:v0.0.8 was resolved to docker.io/library/... and
the person was told access was denied, which reads as a credentials problem for a
repository that was never pushed. No reference is built here at all any more, and
an image this release does not include is named as that.

Both plan sign-ins set the engine up rather than refusing. They run in a
container, and "No container engine is answering, so the sign-in cannot run" named
an obstacle and no way past it, on a screen whose whole purpose is to put one
there. One function does it for Start and for both of them.

A setup step that stops now carries both registers. podman machine init failing
is exactly the case the two-part failure was written for, and it was the last
place still putting an engine's own words in front of somebody as the headline.
The compatible row refused to continue without an API key, and its own summary
names Ollama and vLLM. Neither has one. So the two examples the screen offers by
name were the two it would not accept, and the way out was to invent a key and
hope the endpoint ignored it.

An address and a model name are what that row needs. The Rust side already
treated the key as optional and writes OPENAI_API_KEY only when it is given, so
the refusal lived entirely in the screen. The field says what it is now rather
than leaving somebody to find out by being stuck.

A failure also belongs to the row that produced it. A refused OpenAI sign-in
stayed on screen after switching to the endpoint row, underneath the address just
typed, where it read as a complaint about that address.

Found by driving the screen on Windows.
Making the compatible-endpoint row accept a blank key fixed one end of that
feature and exposed the other. Both bundled Bots refuse to start without
OPENAI_API_KEY, so somebody who filled in an address for an Ollama or a vLLM got
two dead containers complaining about a key their own server does not have. The
row's summary names Ollama and vLLM by name; they were the two cases it would not
serve.

A base URL is a model, and its key belongs to it. Set, it means any endpoint
speaking that API, which is what the variable's own comment has always said, so
the startup check now asks for a key only when nothing else was named. Plain
OpenAI still refuses without one, which is the case the check was written for,
and the other two providers have no base URL to be named by so neither changes.
The SDK insists on a string even when the endpoint ignores it, so a named
endpoint with no key is handed a placeholder rather than a client that cannot be
constructed.

The decision is a module in each Bot rather than a condition at module scope,
because index.ts serves as it loads and a test cannot import it without binding a
port. Same reason model-options.ts exists.

Also the model name now reaches the bundled Bot. docker-compose.yml reads
AGENT_BOT_MODEL for agent-bot, not BOT_MODEL, so that a model chosen for the
framework Bot cannot silently take its tools away: that Bot writes
/v1/chat/completions by hand and gpt-5.6-* rejects function tools there. The
reasoning is about OpenAI's own catalogue and does not survive a custom endpoint,
where the pin asked the person's own server for a gpt-5.5 it has never heard of.
The name they typed is written to both, and cleared from both when they answer
with something that names no model.
The Bots no longer demand a key when a base URL names the endpoint, but a Bot
image published before they learned that still does, and a deployment pulls the
image the release pinned. So the keyless half of the compatible row would have
stayed broken until the next release, on every machine.

The OpenAI SDK every Bot is built on refuses to construct a client without a
string, which is the whole reason a blank key kills them. Ollama, vLLM, LM Studio
and llama.cpp all ignore the value, so a placeholder is sent instead of nothing
and the endpoint that does not read it is none the wiser.

Never treated as a credential: it is written in plain sight rather than put in the
machine's store, because it is not one. A key somebody actually typed is used
unchanged.
"Show OpenBot" did nothing on a machine where the stack was up. The window said
OpenBot is running, the button was there, and clicking it had no effect at all.

Two faults, one behind the other.

The app host process was dead. It is started through the package's `serve`
script, which ran `vite preview` through `bun --bun` so that a machine with bun
and no Node could start it: `node_modules/.bin/vite` begins with a Node shebang.
But Vite's proxy calls `socket.destroySoon()` when an upstream response ends, and
bun's sockets do not implement it, so the process died with a TypeError on the
FIRST call the app made. It served its page, exited, and nothing was listening on
3010 from then on. The shell went on reporting a stack that was up, because the
containers were.

So the app is served by a small server of its own now. It serves a directory and
forwards one prefix, which is all an install needs; a development server was
never the right thing to be running in an installed application, as the shell's
own comment about this process already said. No Node, no Vite at runtime, and the
websocket upgrade the live screen needs is forwarded rather than answered with
HTML. A miss under /assets is still a 404 rather than the page, because handing a
script tag some HTML fails in the console instead of the network panel. Paths are
normalised and confined to the directory: the deployment's .env sits two levels
above it.

And the button now shows what it was told. `show_openbot` already answered with
"OpenBot is not answering on port 3010 yet, so there is nothing to show", and the
click handler dropped it with `.catch(() => undefined)`. A true sentence was
available and the window threw it away, which is why this looked like a dead
button rather than a dead process. The Ask screen's copy of the same call always
showed it.
Both plan sign-ins keep the URL they were given and put it on screen, with a
comment saying why: an open that silently does nothing, or a machine with no
registered browser, leaves somebody watching a spinner with no idea where they
are meant to go. The CopilotKit sign-in discarded it, so that case had no way out
at all.

Found while driving setup on a machine whose browser is not the one in front of
the person.
Stop took the containers down, reported success, and left OpenBot serving.
Measured on Windows Server 2022: after it, the server still answered on 3001,
the worker was still up, and both halves of the app still answered 200 on 3010.
Only the five containers had gone.

The handles this window holds cover only what this window started, and they are
gone the moment it restarts, so a window stopping a stack an earlier one started
holds nothing. That is the case `stop_processes_under` exists for, and its
Windows arm returned 0 with a comment saying the host processes end with the
session. They do not.

They are found by the ports the deployment publishes now, which the shell already
owns and already checks for clashes, and each is ended with its children:
`bun run serve` starts the real server as a grandchild, so ending the process
holding the port would leave that one behind. Only the app's and the server's
ports, because the containers are Compose's to stop and killing whatever holds a
published container port reaches into the engine's own plumbing.

Also, an unreadable app manifest is no longer reported as an old deployment. A
byte-order mark in front of package.json made serde_json refuse it, and the
refusal was rendered as "the deployment is older than this version of OpenBot",
which sends somebody looking for a newer installer over three bytes. Windows
tooling writes that mark freely: Set-Content -Encoding UTF8 does. It is skipped,
and a manifest that genuinely will not parse says so.
The port sweep freed 3001 and 3010 and left the worker running. It listens on
nothing, so a sweep cannot see it, and its command line is identical to the
server's: both are `bun --env-file=../.env src/index.ts`, differing only by
working directory, which Windows will not tell you cheaply.

So the pids are written beside the logs when the processes start, and Stop reads
them. That is also the honest fix for the case the sweep was standing in for: the
handles a window holds die with the window, and everything else about a running
stack survives it, so a restarted window Stopping a stack an earlier one started
had nothing to work with. Now it has. The sweep stays as a second pass for a
stack whose pid file is gone.

The parse is its own function with a test on real netstat output, because reading
five columns as four is what made the first attempt report success while leaving
everything running: the foreign address was taken for the state and the state for
the pid, so nothing ever matched.
Four Keychain dialogs, every time the setup screen mounted, each needing a click
before the window would go on. Navigating between setup and OpenBot asked four
more times.

macOS authorizes every individual read of a stored password unless the
application is signed with an identity the item's ACL already trusts. A
development build is re-signed on every compile, so its ACL never matches and
every read is a prompt; the wizard reads four secrets to arrive filled in, and it
reads them on mount.

The store is now asked once per name per process and the answer is held in
memory. Absence is cached too, or a machine with no stored credential is asked on
every mount for something that was never there. Writes go through the cache and
forgetting clears it, so the two cannot disagree.

This does not remove the prompts on a first run, and nothing in this process can:
the decision belongs to the operating system and to the signature. A signed and
notarised build is granted once and never asked again, which is the real fix and
belongs to the release.
Three conflicts, each a place where both sides had solved the same problem.

Minting the deployment's secrets. Both branches stopped a restart re-minting
KEY_ENCRYPTION_KEY and orphaning everything encrypted under the old one; main's
is the better half and is kept, including its judgement that a value copied out
of .env.example is present and must still be replaced. This branch's part stays
too, because it moved those secrets into the machine's credential store, so what
a previous start minted is read from there rather than from the file main reads.
One list now, MINTED, where there were two.

The audit initiator main threads into a stalled remote Bot's report. This branch
had extracted that agent's construction into `remoteTransport` so a Mastra Bot
could be built the same way, so the initiator follows it there rather than being
dropped along with the inline construction it was written against.

Migration 0028. Both branches took the number; main's audit_initiator is already
applied elsewhere and does not move, so the Mastra enum value is regenerated as
0029 against main's snapshot rather than renumbered by hand.

One of main's compose assertions counted two Bots reading OPENAI_BASE_URL. There
are three: the harness somebody picks in setup is dialled the same way, and
leaving it out would point the Bot they actually chose at OpenAI while the two
shipped ones went to their gateway.

Verified on the merged tree: 203 Rust tests, 2595 of the repo's, typecheck and
lint clean. The one remaining failure, a payload shape in
plugin-store.integration.test.ts, fails the same way on a clean checkout of main.
CI builds on Linux with `-D warnings`, and there `SERVICE` is dead: only macOS
has a service name to file a password under. Windows keys its DPAPI blobs by
filename and the Linux fallback is a file in the config directory, so both
ignored it. macOS never noticed, because there it is used three times.
@davidmckayv

Copy link
Copy Markdown
Contributor Author

I recommend changes before merging. This review covers commit 5ea56ee5386eed5d6cf6e9a6859b8d4ab36a87c2; fixes are now in progress. I rechecked all 19 findings against published OpenBot code, primary upstream documentation/source, and local reproductions. Two priorities were reduced after validation: the Mastra serialization and CrewAI findings establish lost instructions/context, without establishing an authorization failure. The resulting priorities are 10 P1 and 9 P2 findings.

The findings below separate observed results from conditional consequences. Python requirements are unpinned; version-dependent results refer to the exact versions tested, not every possible future image. The missing release images already disclosed in the PR are not counted as another finding.

1. [P1] Persist migrated secrets before purging .env. Changed code

During migration, env::write removes secrets before vault::remember_all succeeds. If persistence fails before saving the existing KEY_ENCRYPTION_KEY, its only durable copy can disappear; a subsequent launch generates a replacement that cannot decrypt existing credentials. Complete and verify persistence before purging the source. The vault's error propagation, migration writer test, and failure-order tracing support this; no real credential-store denial was induced.

2. [P1] Verify ownership before terminating Windows listeners. Changed code

The recovery sweep treats listeners on ports 3001/3010 as OpenBot and runs taskkill /PID … /T /F, including during startup before conflict detection. Another application's process tree can therefore be killed when permissions allow it. The actual parser accepts an unrelated synthetic PID; no process was terminated. Check deployment ownership and process identity first. Microsoft's netstat and taskkill documentation confirms that the PID identifies the listener and /T /F forcibly terminates its tree; it supplies no ownership check.

3. [P1] Download deployment metadata before resolving the selected harness. Changed code

A clean API-key setup calls harness::picked, which reads container-images.json, before deployment_ready downloads the deployment. The manifest reader immediately propagates the missing-file error, so retries cannot reach the download. Plan sign-in can mask this by prefetching. Reorder acquisition before resolution. This source-traced ordering bug is separate from the disclosed missing image entries in the current release.

4. [P1] Normalize an unused OpenAI base URL before constructing Python clients. Changed code

An ordinary OpenAI-key choice leaves Compose injecting OPENAI_BASE_URL="". With the HEAD LangGraph entrypoint and openai 3.8.0 / langchain-openai 1.6.0, the client keeps an empty URL and cannot send a model request. An offline probe reproduced this; unsetting the variable restored the default API URL. Compose interpolation and the versioned OpenAI constructor support the behavior. Omit the variable or normalize empty values to the intended default.

5. [P1] Carry the Anthropic API-key choice through provider/model configuration. Changed code

With the default LangGraph selection, Anthropic → API key emits ANTHROPIC_API_KEY but does not select BOT_PROVIDER=anthropic or a compatible model. The harness supplies an OpenAI prefix to the LangChain 1.4.0 model factory, and Compose supplies an OpenAI model. An offline _model() call with only a synthetic Anthropic key reproduces the missing-OpenAI-credentials error. Carry provider/model configuration through this path or restrict unsupported combinations.

6. [P1] Mount the ChatGPT token-store directory so refresh can replace the file. Changed code

The single-file bind mount makes chatgpt-auth.json a mount point. The actual langchain-openai 1.6.0 refresh writer uses a temporary file plus atomic replacement: a synthetic file-mount probe fails with EBUSY; a directory-mount control succeeds and updates the host store. This matches the Linux rename restriction on mount points. Refresh cannot complete normal persistence. Loss after refresh-token rotation is conditional; this test did not exercise live OAuth, and a temporary file may remain recoverable inside the container.

7. [P1] Register the actual Agno and LlamaIndex run routes. Changed code

Generated endpoints use the server root, while Agno's adapter exposes /agui and LlamaIndex AG-UI 0.5.0's published router exposes /run. With Agno 3.0.6 and LlamaIndex AG-UI 0.5.0, HEAD ASGI probes return 405/404 at /, healthy /health, and validation errors at the real run paths. Both desktop verification and application transport target the wrong URL. Include the run path in the generated endpoint or mount the adapter at the advertised root.

8. [P1] Apply managed and vault authentication to Mastra rows. Changed code

Both credential branches only accept remote_ag_ui; the newly supported remote_mastra receives neither its vault header nor x-openbot-agent-token. The actual loader returns no headers and performs zero vault reads for a synthetic Mastra row. The packaged Mastra guard exempts only health, so /api/agents is rejected without the token. Apply authentication to both remote kinds. Source tracing also shows eager discovery can reject the request's entire agent map when the failing Mastra bot is visible.

9. [P1] Preserve OpenBot middleware when Mastra agents are cloned. Changed code

CopilotKit clones the registered agent for HTTP runs. @ag-ui/mastra 1.1.2 constructs a fresh agent and copies headers without copying the middleware attached here. Invoking the installed runtime's clone path reproduces one middleware becoming zero, with standing-role messages, granted tools, and signed context absent from transport input. Preserve the wrapper through cloning. The bridge 1.1.2 source map (mastra.ts:629–634) and runtime 1.70.1 artifact (handlers/shared/agent-utils.mjs) byte-match installed files. This is separate from the serialization issue below; fixing either alone leaves the other.

10. [P2] Translate standing-role and OpenBot context into Mastra's wire format. Changed code

Even the original, un-cloned wrapped agent loses system messages and OpenBot-specific forwardedProps during conversion. A real client serialization probe contains the user message and clientTools, but no standing-role/holdings messages or openbotRun, openbotBotId, and openbotDeploymentTools. The published bridge 1.1.2 source (utils.ts:138–222, mastra.ts:2819–2842) corroborates the filtering; the tested native client was 1.43.0. Add an explicit translation for the required context. The probe establishes payload loss; tool schemas do survive serialization.

11. [P2] Preserve the leading standing-role message in CrewAI. Changed code

The AG-UI CrewAI 0.3.0 adapter (endpoint.py, crewai_prepare_inputs) strips the leading system message before OpenBotFlow.answer reads state.messages. OpenBot puts the configured Bot identity/role there. With CrewAI 1.15.20, the real HEAD FastAPI route and a provider-completion recorder return 200 but send only the user message to the provider. Preserve the instruction across the adapter boundary. This establishes lost configured instructions while ordinary conversation can still run.

12. [P1] Use Mastra's transport for the mandatory first question. Changed code

ask_the_bot ignores harness kind/agent ID and sends a generic AG-UI request to the selected root URL. The packaged Mastra agent uses its native /api/agents/{id}/stream transport through the bridge, consistent with Mastra's Agents API and the 1.43.0 client above. A healthy Mastra selection cannot pass this probe and reach normal handover. Reuse the transport or translate both requests and events; changing only the URL is insufficient. No live model run was used.

13. [P2] Scope saved and pending plan credentials to their provider. Changed code

After ChatGPT sign-in, switching the row to Claude retains the shared token. The UI immediately reports sign-in complete and submits the ChatGPT store as a Claude credential without performing Claude sign-in. Reset or scope both the token and pending completion when the provider changes. This transition reproduces with actual React 19.2.8 rendering and synthetic IPC; React's state-preservation rules explain why changing the row does not reset shared component state. The reverse transition also reproduces in the earlier mocked-hook probe.

14. [P2] Make “Change the model” reach the picker after Ask fails. Changed code

The action changes step while running stays true, but the picker requires !running. The actual React transition renders Show/Stop controls instead of the picker and hides the Ask error. Stop/reconfigure the stack or make the correction screen reachable in that state. React's conditional-rendering documentation corroborates the branching semantics; the component and reproduction establish the defect here.

15. [P2] Provide retry/refresh when Intelligence returns no projects. Changed code

An empty projects array enters the project-list branch and removes sign-in. The screen asks the user to create a project and sign in again, but offers neither sign-in nor refresh, while Start stays disabled. An actual React render with an empty project response reproduces the missing recovery control. Expose a retry or refresh action in this state.

16. [P2] Restore persisted plan sessions on a fresh setup mount. Changed code

already_configured omits the saved Claude plan token and ChatGPT store from readback. After Quit/relaunch into setup, a fresh App starts without a model choice and the plan picker requires a new sign-in despite stored credentials. Restore the saved credential kind/session. Stopping within the same mounted App can retain the choice and is an exception; the fresh-mount reproduction establishes this narrower trigger.

17. [P2] Collect and use an endpoint for “An agent you already run.” Changed code

The row promises an agent address and AG-UI check, but no screen collects that address. With an API-key/compatible-endpoint choice, picked(byo-url) returns None, so setup falls back to the bundled Bot and tests it instead. The endpoint model-provider field is not an agent URL. Carry a distinct agent endpoint through registration and verification. Plan choices instead override the harness, so they are outside this specific fallback trigger.

18. [P2] Forward session headers when proxying WebSockets. Changed code

The replacement app server creates the upstream socket using only a URL, dropping cookies/authorization. A synthetic upstream on Bun 1.3.14 receives the headers for HTTP but none for WebSocket. Separate execution of the actual session guard accepts the synthetic cookie and returns 401 without it; the connection between these boundaries is source-traced. Bun 1.3.14 supports client headers. This affects authenticated configurations using app/serve.ts: a configured identity provider disables single-user bypass even with OPENBOT_SINGLE_USER=true. Default unauthenticated desktop setup and direct API-server asset serving are outside this scope.

19. [P2] Distinguish failed history retrieval from a changed project. Changed code

The new notice infers a project change from a used channel with no restored messages. The actual history reader also produces empty messages for HTTP 500, network failures, and unreadable history. When the join also supplies no messages, an outage can therefore receive that diagnosis. Fetch's response/error semantics corroborate the distinction, while the actual reader probes establish that it is discarded here. Preserve the retrieval result or use neutral unavailable-history wording. This is a false diagnostic; no evidence shows that the notice itself deletes history or resets the thread.

Validation: 203 Rust library tests passed (4 live credential/store tests ignored), and 268 focused Bun tests passed during the original review. The web-validation pass additionally rechecked dependency artifacts and reran targeted synthetic reproductions, including real React rendering. Existing CI jobs passed at this commit; image/component-Dockerfile jobs were skipped. No full installer journey, real credential-store denial, real Windows termination, or live OAuth/model call was performed for this review.

Persist migrated secrets to the vault before rewriting .env with those secret keys purged. This keeps old file copies durable if any nonempty vault write fails, while preserving the successful purge path and empty-secret forgetting behavior.
Register Agno at /agui and LlamaIndex at /run when generating PICKED_HARNESS_URL, while keeping health_path as readiness metadata.

Red: cargo test --lib harness::tests and env::tests failed because PickedHarness had no run_path field.

Green: cargo test --lib harness::tests; cargo test --lib env::tests; bun test tests/compose.test.ts.
Return the compiled listener directory to its single fixture caller and remove it after successfully reaping the listener. Preserve the existing real-process ownership regression.
Persist a replacement's complete direct-child identity while its live Child
remains held, before the supervisor can announce restart success. Preserve
prior role records without re-recording dead or reaped children, allowing
server and app to restart sequentially after both have exited. Retain handles
and durable evidence on every recording failure.

Callsite audit: restart_host_process_with is the only production caller of
replace_windows_host_process_with. Its existing children lock still covers
generation/root validation, spawn, recording and publication, with the same
post-publication generation check used by concurrent Stop. The supervisor
reports started again only after Ok(true). Unix replace_host_process, initial
Windows recording, durable-write semantics, held-child cleanup supplementation,
and tray/second-instance ownership probes remain unchanged.

Validation: three ordinary replacement regressions pass, including stale PID
reuse, other-role retention, sequential dual death and recording refusal.
Source-bound Windows-arm restart-to-ownership proof fails three expected
baseline cases and passes all four final controls. The failure proof reaches
the actual Stop handle cleanup. Twenty-nine existing Windows ownership and
cleanup regressions pass. cargo fmt --check and strict Clippy all-targets pass.
Native real-listener proof is supplied separately for root-owned validation.

Finding: D89-MAIN1-001-windows-restart-records
@davidmckayv
davidmckayv requested a review from mxmzb as a code owner September 11, 2026 04:40
@davidmckayv

Copy link
Copy Markdown
Contributor Author

Published validation checkpoint 988d5646cb7cb131d84f46ebd9b5730de6676a9c to this PR. GitHub reports the branch mergeable; CI for this commit is being monitored.

The latest six commits contain five production corrections and unavailable-engine regression coverage: container ownership after failed Start, selected-endpoint diagnostics, Unix descendant cleanup, container-runtime affinity, shutdown coverage, and retained worker recovery.

Checks passed on this exact source: formatting, lint, explicit desktop/server TypeScript checks, 59 desktop App tests, 12 targeted Rust regressions, Rust check/clippy across all targets, frontend build, and desktop binary build. Earlier accepted behavioral evidence includes actual macOS and Windows setup, browser/tool/command-line use, model continuation after tool results, saved chat, tray/restore, and lifecycle cleanup. Those native claims remain limited to their recorded flows and candidates.

Still unfinished:

  • Stop failure retained through setup navigation: correction is being tested; native failure-and-recovery proof remains pending.
  • Windows Quit failure retained across exit/relaunch: native builds/proof pending.
  • Connection-probe responsiveness: draft only; prior native validation action blocked by automatic review.
  • Copied-deployment ownership and Quit/Start admission: unconfirmed reports awaiting supported proof; the latter proof action was also blocked by automatic review.

The desktop build specification places telemetry in S6 and signing, packaging, and auto-update in S7. These are outside the current S5 work. This checkpoint is not final approval or a merge request.

@davidmckayv
davidmckayv merged commit c21ba2b into main Sep 11, 2026
20 checks passed
@davidmckayv
davidmckayv deleted the s5-mastra-bot-kind branch September 11, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants