Skip to content

feat(launch): add Atomic Agent as a one-click Launch-page assistant - #258

Merged
Vect0rM merged 3 commits into
AtomicBot-ai:mainfrom
plombeer31:feat/launch-atomic-agent
Aug 27, 2026
Merged

feat(launch): add Atomic Agent as a one-click Launch-page assistant#258
Vect0rM merged 3 commits into
AtomicBot-ai:mainfrom
plombeer31:feat/launch-atomic-agent

Conversation

@plombeer31

Copy link
Copy Markdown
Contributor

What

Adds Atomic Agent (AtomicBot-ai/atomic-agent) to the Launch page as a one-click assistant, next to Hermes and OpenClaw — and to atomic-chat-cli launch with it, since the two catalogs share the same configure_* writers.

Everything else on that page already works this way; our own agent was the one missing, so running it on Atomic Chat's models meant hand-editing its config file.

How

Step What happens
Install Spawns the project's official bootstrap script — curl -fsSL https://atomicagent.io/install | sh on Unix, irm https://atomicagent.io/install.ps1 | iex on Windows. It ships as a Node SEA binary from GitHub Releases, not as an npm package, so the prerequisite is curl / powershell — same shape as Goose, Hermes and Poolside. Neither script prompts, so there is no wizard to skip.
Run configure_atomic_agent upserts one provider into <state dir>/config.json and sets llm.activeTextProvider to it.
Terminal A bare atomic-agent opens the TUI, so no run_args are needed.

The entry it writes:

{
  "id": "atomic-chat",
  "kind": "openai-compatible",
  "baseUrl": "http://127.0.0.1:1337/v1",
  "apiKey": "atomic",
  "defaultChatModel": "<running model>",
  "supportsTools": true,
  "requestTimeoutMs": 300000
}

The state dir is resolved the way the agent's own loadConfig() resolves it: ATOMIC_AGENT_STATE_DIR first (read from HKCU\Environment before std::env::var on Windows, for the stale-snapshot reason in the 2026-07-01 Hermes ADR), else ~/.atomic-agent.

Design calls worth reviewing

  • The write is a merge, never a replacement. config.json is the agent's own trust surface — it holds agent.approvalLevel and is guarded by its approval ladder — so only llm.providers[atomic-chat] and llm.activeTextProvider are rewritten. Other providers, their API keys, unknown top-level blocks and the version field survive verbatim. We deliberately do not stamp a schema version: the agent fills every block it does not find with its own defaults on the next start, so there is nothing here to keep in sync.
  • activeEmbeddingProvider is load-bearing. The agent rejects the whole config when it names a provider that is not in llm.providers, so a block created from nothing also carries the local-llama entry the agent otherwise synthesises for itself, pointed at whatever localModels.url says. Embeddings drive memory recall rather than chat, so Run never repoints them — the key is only filled when absent or dangling.
  • Only the text provider is switched outright, because pressing Run is an explicit "use this" — same contract as OpenCode's model key.
  • requestTimeoutMs is a tightening, like Hermes'. The agent's OpenAI-compatible provider defaults to 600 s, long enough that a wedged local turn looks like a hang. 300 s is seeded, and a value the user already tuned on our entry is preserved.
  • endpointWithPrefix: true. The stored baseUrl reads as the base URL a user would paste; the agent's normalizeOpenAiBaseUrl strips the trailing /v1 itself, so requests land on /v1/chat/completions, not /v1/v1/... (verified below).

Verification

Rust, on this branch:

cargo test --lib --no-default-features --features test-tauri
  → 493 passed; 0 failed; 1 ignored

cargo test --lib --no-default-features --features test-tauri,cli -- cli::integrations
  → 4 passed, incl. catalog_matches_the_typescript_source (the Rust/TS drift guard)

Six new unit tests cover the merge: seeding a self-consistent block from nothing, following the user's localModels.url, preserving foreign blocks/providers/keys, idempotent re-runs that keep a tuned timeout, repairing a dangling activeEmbeddingProvider, and rejecting a non-object file.

End-to-end against atomic-agent v0.4.1, with the exact config.json this writer produces and a stub OpenAI-compatible server on :1337:

activeTextProvider: atomic-chat
built provider    : atomic-chat {"toolTransport":"native_tools", ...}
completion content: "pong"
model echoed back : qwen3-4b-instruct

stub saw → POST /v1/chat/completions   Authorization: Bearer atomic
           {"model":"qwen3-4b-instruct","messages":[...],"stream":false}

So the file loads through the agent's own parser, builds the provider, and serves a completion — no /v1/v1 doubling, no config-validation error.

Known limitation

The Unix installer drops the binary in ~/.local/bin and appends to a shell rc file, which the memoised login-shell PATH in the running app cannot see — so the "Installed" chip catches up only on the next app start. Same as Goose, Poolside and Zed today; the terminal Run opens is a fresh login shell, so the launch itself works. Not addressed here.

Notes

  • ADR: docs/decisions/2026-08-25-add-atomic-agent-as-a-one-click-launch-page-assistant.md, indexed under Launch page & external coding agents.
  • The card icon is Atomic Agent's own mark (assets/logo.svg in that repo), inlined as SVG rather than added as an image asset.
  • web-app lint/typecheck were not run here (they need the full make dev toolchain); the two TS changes are a catalog entry and one invoke case, and both files parse clean.

🤖 Generated with Claude Code

Atomic Chat can already install fifteen external agents and point them at
the local OpenAI-compatible server, but AtomicBot-ai/atomic-agent — our own
local-first operator agent — was not one of them, so running it on Atomic
Chat's models meant hand-editing its config file.

Install spawns the project's official bootstrap script (curl | sh on Unix,
irm | iex on Windows) because it ships as a Node SEA binary from GitHub
Releases, not as an npm package. Run then merges a single
`openai-compatible` provider into `<state dir>/config.json` and selects it
as `llm.activeTextProvider`.

The write is a merge, never a replacement: config.json is the agent's own
trust surface, so other providers, their keys, unknown top-level blocks and
the `version` field survive verbatim. A block created from nothing also
carries the `local-llama` entry the agent otherwise synthesises for itself,
because `llm.activeEmbeddingProvider` must name a provider that exists or
the agent rejects the whole file — and embeddings drive memory recall, not
chat, so Run never repoints them.

Verified end-to-end against atomic-agent v0.4.1: the written config loads
through its own parser, builds the `atomic-chat` provider, and completes a
request against `http://127.0.0.1:1337/v1/chat/completions` with the
configured model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@plombeer31
plombeer31 requested a review from Vect0rM as a code owner August 25, 2026 16:10

Vect0rM commented Aug 26, 2026

Copy link
Copy Markdown
Member

Thanks for this, @plombeer31 — our own agent being the one missing from that page was a real gap, and the way you closed it is careful in the places that matter. Two things stand out: treating config.json as the agent's trust surface and merging rather than replacing, and working out that endpointWithPrefix: true is correct here. That second one is easy to get backwards, and the reasoning holds — I checked.

Rather than take the description on trust I cloned AtomicBot-ai/atomic-agent (0f68964) and read the parser. Every claim in the PR body checks out:

  • normalizeOpenAiBaseUrl is called in the provider constructor (openai-provider.ts:96), not just the TUI wizard, and apiPathPrefix defaults to /v1 — so a stored .../v1 really does land on /v1/chat/completions. No doubling.
  • activeEmbeddingProvider naming an unlisted provider throws ConfigValidationError and takes the whole file down (llm-config.ts:639). The seeding is genuinely load-bearing, not defensive padding.
  • Not stamping version is safeconst version = obj.version ?? USER_CONFIG_VERSION (config-schema.ts:2906). An absent version defaults to current rather than being rejected.
  • Default requestTimeoutMs really is 600_000 (openai-provider.ts:100), so 300 s is a tightening as described.
  • The install commands match the agent's own README verbatim, ~/.local/bin is the real install dir, and atag is a real alias the installer creates (scripts/install.sh:481).
  • openai-compatible is a registered kind, PROVIDER_ID_RE accepts atomic-chat, supportsTools is a boolean on the entry, and localModels.url defaults to http://127.0.0.1:8080 with mode: "external".

Verified on your branch merged onto current main (96a13ac):

  • Fast-forwards — no conflicts.
  • cargo test --lib --no-default-features --features test-tauri — 491 passed, 2 failed. The main baseline is 485 passed, 2 failed, and it is the same two (skill_run_script::tests::cancellation_terminates_descendant_processes and timeout_terminates_descendant_processes) — container can't reap descendant process groups. The delta is exactly +6: your new tests, all green.
  • cargo test --features test-tauri,cli -- cli::integrations — 4 passed, including catalog_matches_the_typescript_source. The Rust and TS catalogs agree on order.
  • tsc -b — exit 0. eslint on both changed TS files — clean.
  • Full vitest run — 229 files, 1835 passed / 15 failed / 16 skipped. Byte-identical to the main baseline, so nothing here is disturbed. (Those 15 fail on main too, in this environment.)
  • clippy — no warnings anywhere in the added range.
  • prettier --check flags both changed TS files — and flags them identically on main. Pre-existing drift; I diffed the formatter's output and none of it touches your added lines. Please don't reformat them here, it would bury the diff.

Three things. Only the first needs a diff.

1. Three rustfmt violations, all new

commands.rs is currently fmt-clean on main (0 diffs). Your branch introduces 3, all inside the new code:

4530  the obj.entry("llm").or_insert_with(...) chain fits on one line
4556  the providers.iter().position(...) closure wants the multi-line form
4675  stray double blank line after configure_atomic_agent

cargo fmt fixes all three. There's no fmt job in CI and make verify-fast doesn't run it, so nothing else will catch this — but this file is clean today and this would be the first thing to break that.

2. llm.toolTransport is an undocumented third write, and it's a no-op

The ADR says the writer touches "only llm.providers[atomic-chat] and llm.activeTextProvider" (plus the embedding key it discusses). But the code also does:

llm.entry("toolTransport")
    .or_insert_with(|| serde_json::json!("auto"));

I checked what the agent does with an absent toolTransport: parseUserLlmFileConfig defaults it to exactly "auto" (config-schema.ts:3087). So the insert can only ever write the value the agent would have chosen for itself — it changes nothing and adds a key to the user's file.

My preference is to drop those two lines. If you'd rather keep them, the ADR should list toolTransport alongside the other writes, so the "only two keys" claim stays true.

3. The embedding repair points at atomic-chat — the one target the ADR rules out

The local-llama seed only fires when providers is empty. When the array is non-empty but contains no local-llama, the fallback chain falls through to our own id. I ran it rather than reading it:

in : llm.providers = [{ id: "groq", ... }], no activeEmbeddingProvider
out: llm.activeEmbeddingProvider = "atomic-chat"

That's the outcome the ADR explicitly argues against — "pointing embeddings at Atomic Chat instead would silently repoint the agent's memory recall, which is not what Run asked for" — and the comment directly above the code says the same.

Honest scoping: this is narrower than it first looks. A valid config must already have activeEmbeddingProvider in providers, and an absent key defaults to local-llama, which the agent rejects when it isn't listed. So the only files that reach this branch are ones the agent already refuses to load. It's a repair path, not a silent repoint of a working setup — but it repairs toward the target the ADR rules out, and your test (repairs_an_embedding_provider_that_no_longer_exists) asserts only that the value is listed, so nothing pins the intent. Seeding the local-llama entry in the repair path as well as the empty-array path would make the code match the reasoning, and the test could then assert which provider it lands on.

One related detail while you're in there: the seeded local-llama URL always follows localModels.url, but the agent's own default is mode-aware — under localModels.mode: "managed" it uses http://127.0.0.1:{managed.port} (default 19091), not localModels.url. Reachable for a config that has a localModels block in managed mode and no llm block at all.

Nits

  • The provider JSON in the PR body shows "defaultChatModel": "" where the ADR has <running model> — looks like the angle brackets got eaten as HTML. Worth fixing, since the body is what ends up in the merge commit.
  • A contract note rather than a bug: if model were ever empty, the writer emits "defaultChatModel": "" and the agent rejects the entire fileparseOptionalString throws expected non-empty string on "" (llm-config.ts:171), it doesn't treat it as absent. Both callers guard it today (launch/index.tsx:743, jan-cli.rs:1142), so nothing is broken; a filter(|m| !m.is_empty()) would just make the writer's own contract explicit instead of inherited.

Fix 1, tell me which way you want 2 and 3, and I'll take it. The end-to-end run against v0.4.1 was the right call — it's the only way to know the file actually loads, and it's why the /v1 question is settled rather than argued 🧪


Generated by Claude Code

…ddings to local-llama

Review follow-ups on the Atomic Agent Launch integration.

rustfmt: the three new violations are gone, so commands.rs is fmt-clean
again. Only that file was formatted — src-tauri has pre-existing drift
elsewhere that is not this PR's to fix.

toolTransport: dropped. The agent defaults an absent one to "auto" itself
(parseUserLlmFileConfig), so the insert could only ever restate the agent's
own choice while adding a key to the user's file. That also makes the ADR's
"only two keys plus the embedding repair" claim true again.

Embedding repair: the local-llama seed used to fire only when `providers`
was empty, so a non-empty list without local-llama fell through to our own
id — the one target the ADR argues against. The seed now runs in the repair
path too, and the repair target is always local-llama. The test asserts
which provider it lands on, not merely that the value is listed.

The seeded local-llama URL is now mode-aware, matching the agent's own
default: under localModels.mode "managed" it follows managed.port (19091 by
default), not localModels.url.

An empty model is refused before the write instead of relying on the
callers: the agent's parseOptionalString rejects "" rather than treating it
as absent, so an empty defaultChatModel would take the whole file down.

Verified: 497 passed / 0 failed (was 493, +4 new tests); clippy clean in the
added range; rustfmt clean on commands.rs. Re-ran the end-to-end against
atomic-agent v0.4.1 for three shapes — fresh install, managed mode, and the
repair path over a real provider list — all three load through the agent's
own parser and build the atomic-chat provider, with toolTransport resolving
to "auto" without us writing it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@plombeer31

Copy link
Copy Markdown
Contributor Author

Thanks — you read the agent's parser rather than my description of it, and items 2 and 3 are both real. All three are fixed in dfe2732, plus both nits.

1. rustfmt

Fixed. commands.rs is fmt-clean again (rustfmt --check on it: no diffs).

I formatted only that file, deliberately. cargo fmt across src-tauri rewrites a dozen unrelated files — lib.rs, proxy.rs, mcp/helpers.rs, downloads/* and others all carry pre-existing drift, and lib.rs alone produces a few hundred lines. Same reasoning you applied to prettier: not this PR's to fix, and it would bury the diff.

2. toolTransport — dropped

You're right that it's a no-op, and I'd rather delete two lines than widen the ADR to cover them. Gone.

Confirmed empirically rather than only by reading config-schema.ts:3087 — loading a written config through the agent now reports toolTransport: "auto" with the key absent from the file. The ADR's "only llm.providers[atomic-chat] and llm.activeTextProvider, plus the embedding repair" claim is true again, and a new test (never_writes_tool_transport) pins both halves: never written when absent, never touched when the user set "grammar".

3. Embedding repair — now always local-llama

Fixed, the way you suggested: the local-llama seed runs in the repair path as well as the empty-array path, so the repair target is local-llama unconditionally and our own id is never a fallback. Ran your exact input:

in : llm.providers = [{ id: "groq", ... }], no activeEmbeddingProvider
out: llm.activeEmbeddingProvider = "local-llama"
     llm.providers = [groq, atomic-chat, local-llama]   ← seeded

Your scoping note was right and I kept it in mind: this only ever reaches configs the agent already refuses to load, so it stays a repair path. But the code now matches the comment sitting above it, which is the actual complaint. The test asserts which provider it lands on and that the entry is listed, over both shapes (dangling id, and absent key with a non-empty list) — so the intent is pinned rather than implied. A second test (leaves_a_working_embedding_selection_alone) locks the other side: a valid selection is untouched and nothing gets seeded.

Mode-aware URL: also fixed. atomic_agent_local_llama_entry now mirrors the agent's own default — http://127.0.0.1:{managed.port} under localModels.mode: "managed" (falling back to 19091), localModels.url otherwise. Good catch; I'd read parseUserLlmFileConfig's defaults and missed that the url there is a ternary.

Nits

  • Empty model: taken. The writer now returns Err before touching the file rather than emitting "defaultChatModel": "" and letting parseOptionalString reject the whole config at next start. Its own contract now, not an inherited one.
  • <running model>: the raw PR body does contain "defaultChatModel": "<running model>" inside the ```json fence — I re-checked via gh pr view --json body. Whatever ate the brackets happened downstream of the body itself, so there's nothing to edit; flagging it back rather than silently doing nothing.

Verification on dfe2732

  • cargo test --lib --no-default-features --features test-tauri497 passed, 0 failed, 1 ignored (was 493; +4 net new tests). Neither of your two container failures reproduces here, which fits your descendant-process-group diagnosis.
  • cargo clippy --lib --all-targets → no warnings in the added range (the ones that fire are pre-existing dead-code in tauri-plugin-llamacpp-upstream / tauri-plugin-vector-db).
  • rustfmt --check src/core/system/commands.rs and src/core/cli/integrations.rs → clean.
  • End-to-end against atomic-agent v0.4.1, re-run for three shapes now rather than one — fresh install with no config, mode: "managed" with no llm block, and the repair path over a real [groq] list:
case a  text=atomic-chat embed=local-llama toolTransport=auto  ids=[local-llama, atomic-chat]
        local-llama url = http://127.0.0.1:8080
case b  text=atomic-chat embed=local-llama toolTransport=auto  ids=[local-llama, atomic-chat]
        local-llama url = http://127.0.0.1:19091   ← managed.port, not localModels.url
case c  text=atomic-chat embed=local-llama toolTransport=auto  ids=[groq, atomic-chat, local-llama]
        groq apiKey preserved verbatim

All three load through loadConfig() and build every provider through ProviderRegistry.fromConfig.

web-app is untouched by this commit, so your tsc -b / eslint / vitest results still stand.


Generated by Claude Code

Vect0rM commented Aug 27, 2026

Copy link
Copy Markdown
Member

Thanks for the quick turnaround, @plombeer31 — all three items and both nits are closed, and I checked each against the agent's own parser rather than against the commit message. Doing that turned up one new thing in the same area, and it's the only reason I'm not merging today.

Verified on your branch — it's based on 96a13ac, so it fast-forwards, no conflicts:

  • rustfmt is clean. rustfmt --check on commands.rs and integrations.rs: no diffs. Formatting only that file was the right call — cargo fmt across src-tauri still rewrites a dozen unrelated files here too.
  • toolTransport is genuinely gone, and never_writes_tool_transport pins both halves. The default it was restating is obj.toolTransport ?? defaults.toolTransport (llm-config.ts:645), with "auto" supplied at config-schema.ts:3087.
  • The embedding repair lands on local-llama unconditionally, and the test asserts which provider rather than merely that the value is listed. Both shapes from last round now come out local-llama.
  • The mode-aware URL is right. USER_CONFIG_DEFAULTS (config-schema.ts:1718) carries localModels.url: "http://127.0.0.1:8080", mode: "external", managed.port: 19091 — your three constants match, and the ternary mirrors config-schema.ts:3092-3095.
  • Empty model: defaultChatModel goes through parseOptionalString (llm-config.ts:231), which throws on "" rather than treating it as absent (:170-173). Refusing before the write is correct.
  • cargo test --lib --no-default-features --features test-tauri — 495 passed, 2 failed. The main baseline in the same container is 485 passed, 2 failed, so the delta is exactly +10 — the whole atomic_agent_tests module, all green. The two failures are environment rather than diff, and the pair isn't even stable between runs: main failed cancellation_terminates_descendant_processes + timeout_terminates_descendant_processes, your branch failed filesystem_trash_moves_directories_through_the_native_trash_api + timeout_…. Descendant process groups and a native trash API — neither is anywhere near a JSON writer.
  • cargo test --features test-tauri,cli -- cli::integrations — 4 passed, including catalog_matches_the_typescript_source. I also diffed the catalogs by hand: 18 ids, identical order, atomic-agent at position 16 in both.
  • tsc -b — exit 0. eslint on both changed TS files — clean.
  • Full vitest run — 196 files, 2063 passed, 11 skipped, zero failures, matching the main baseline exactly.
  • prettier --check flags both changed TS files — and flags them identically on main. Pre-existing drift; please don't reformat them here, it would bury the diff.

The seeded local-llama entry is missing baseUrl, and that moves embeddings

This is the one thing I'd like fixed before merge. It lands in exactly the area the ADR is most careful about, so I ran it rather than argued it.

atomic_agent_local_llama_entry writes id, kind and url. The entry the agent synthesises for itself writes a fourth field (config-schema.ts:3089-3097):

{
  id: "local-llama",
  kind: "llama-server",
  url: localModelsMode === "managed" ? `http://127.0.0.1:${managed.port}` : localModelsUrl,
  baseUrl: embeddingsDaemon.url,     // ← not written here
}

That fourth field is the one the embedding path actually reads. resolveEmbeddingLlmConfig has two branches (src/memory/embeddings/embedding-provider-registry.ts):

  • no llm block → a synthetic local-llama-embed at embeddings.enabled ? embeddings.url : localModels.url (:62-64)
  • an llm block → your entry, resolved as baseUrl ?? url (:46)

Pressing Run always creates the llm block, so it always moves a config from the first branch to the second. With no baseUrl on the entry, the ?? url fallback sends embeddings to the chat daemon.

I ran the agent's own loadConfig() + resolveEmbeddingLlmConfig() over the file before and after the writer touches it (agent checkout 86321a4), rather than reasoning from the source:

case                                   before          after (this PR)
external, embeddings OFF (defaults)    :8080           :8080
external, embeddings ON                :19092          :8080     <- moved
external, embeddings ON, port 20500    :20500          :8080     <- moved
external, custom url, embeddings OFF   :9000           :9000
managed, embeddings OFF                :19091          :19091
managed, embeddings ON                 :19092          :19091    <- moved

The default row is safe, which is why the end-to-end run didn't catch this. But for anyone who has turned the embeddings daemon on — and that is precisely the user who cares about memory recall — pressing Run silently repoints embeddings from the embeddings daemon to the chat daemon. That's the outcome the ADR rules out, arriving by a different route than last round's.

Seeding baseUrl the way the no-llm-block branch computes it restores every row. I verified this exact rule against the parser — all six cases come back byte-identical to before:

/// Where the agent would look for embeddings if we were not writing an `llm`
/// block at all — `resolveEmbeddingLlmConfig`'s no-block branch. `chat_url` is
/// the mode-aware URL already computed for this entry.
fn atomic_agent_embedding_base_url(
    root: &serde_json::Map<String, serde_json::Value>,
    chat_url: &str,
) -> String {
    let embeddings = root.get("localModels").and_then(|v| v.get("embeddings"));
    let enabled = embeddings
        .and_then(|v| v.get("enabled"))
        .and_then(serde_json::Value::as_bool)
        .unwrap_or(false);
    if !enabled {
        return chat_url.to_string();
    }
    embeddings
        .and_then(|v| v.get("url"))
        .and_then(|v| v.as_str())
        .filter(|s| !s.is_empty())
        .map(str::to_string)
        .unwrap_or_else(|| {
            let port = embeddings
                .and_then(|v| v.get("port"))
                .and_then(serde_json::Value::as_u64)
                .unwrap_or(ATOMIC_AGENT_DEFAULT_EMBEDDINGS_PORT); // 19_092
            format!("http://127.0.0.1:{port}")
        })
}

One judgement call I'd leave to you, because it cuts against "mirror the agent": the agent's own synthesised entry sets baseUrl: embeddingsDaemon.url unconditionally, even when the daemon is disabled. Copying config-schema.ts:3096 literally would therefore point a default install at :19092, where nothing is listening — worse than today. The agent's two branches disagree with each other on this; the one that governs the file you're converting is the no-llm-block branch, so matching that is what preserves behaviour. Worth a sentence in the ADR either way, since "the entry the agent synthesises for itself" is currently doing more work in the prose than in the code.

Smaller

  • activeEmbeddingProvider is still written when it was merely absent. obj.activeEmbeddingProvider ?? defaults.activeEmbeddingProvider (llm-config.ts:629-631) defaults to "local-llama" (config-schema.ts:3086), and the writer already guarantees local-llama is listed — so in the absent case the insert can only restate the agent's own choice, which is the toolTransport argument again. Only the present-but-dangling case is load-bearing. I'm genuinely undecided: being explicit in a file people hand-edit has some value. But if "we don't write what the agent would pick for itself" is meant as a rule, this is the remaining exception, and it should be either narrowed or named in the ADR.
  • model is validated trimmed but written untrimmed. model.trim().is_empty() guards, then "defaultChatModel": model stores the original — " qwen " passes and is written with the spaces. One .trim() on the way in closes it.
  • Re-running the writer replaces our whole provider entry, preserving only requestTimeoutMs. That's a fine policy for an entry we own, but the ADR's "any value the user already tuned on our entry is preserved" reads wider than what the code does — worth narrowing that sentence to the timeout.

Fix the baseUrl seed and tell me which way you want the activeEmbeddingProvider write, and I'll take it. Everything else here is settled, and the four items from last round were closed properly rather than papered over — the never_writes_tool_transport test in particular pins the absence of a write, which is the hard half to test 🧪


Generated by Claude Code

Creating the `llm` block moves the agent from the config branch that reads
`localModels.*` directly to the one that resolves a provider entry as
`baseUrl ?? url`. The seeded `local-llama` entry wrote only `url`, so for
anyone running the embeddings daemon, pressing Run silently repointed
embedding requests at the chat daemon — the outcome the ADR rules out,
arriving by a different route than the dangling-provider one.

Seed `baseUrl` the way the no-`llm`-block branch computes it:
`localModels.embeddings.url` (or `http://127.0.0.1:<embeddings.port>`) when
that daemon is enabled, the entry's own mode-aware chat URL when it is not.
Copying the agent's own synthesised entry literally was the other option and
is worse: it sets `baseUrl` unconditionally, which would point a default
install at :19092 with nothing listening.

Verified against the agent's own parser (`loadConfig()` +
`resolveEmbeddingLlmConfig()`, atomic-agent v0.1.67) rather than by reading
the source: all six cases — external/managed × daemon off/on/custom port —
resolve byte-identically before and after the writer touches the file.

Also write `defaultChatModel` trimmed, since it is validated trimmed, and
narrow the ADR's preservation claim to `requestTimeoutMs`, which is what the
code actually preserves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@plombeer31

Copy link
Copy Markdown
Contributor Author

You beat me to it — I had the same fix in flight when your 2be039e landed on the branch. Same helper, same no-llm-block branch as the model, same reasoning about why copying the agent's synthesised entry literally is worse. I've dropped my duplicate rather than force-pushing over yours; the branch is at 2be039e and there is nothing of mine left to merge.

Since you verified my commits against the parser rather than the commit message, here's the same treatment of yours — I re-ran it rather than reading it.

Your fix, verified

Same six-case matrix, driving the agent's own loadConfig() + resolveEmbeddingLlmConfig() before and after the writer (agent 86321a4):

case                                   before      after
external, embeddings OFF (defaults)    :8080       :8080     same
external, embeddings ON                :19092      :19092    same
external, embeddings ON, port 20500    :20500      :20500    same
external, custom url, embeddings OFF   :9000       :9000     same
managed, embeddings OFF                :19091      :19091    same
managed, embeddings ON                 :19092      :19091 →  :19092  same

And the harness isn't vacuous — I ran it against dfe2732 as a control, and it reproduces your three MOVED rows exactly (embeddings ON:8080, port 20500:8080, managed + ON:19091). So the matrix is measuring the thing it claims to measure.

Also on 2be039e:

  • cargo test --lib --no-default-features --features test-tauri499 passed, 0 failed, 1 ignored (497 + your two tests). No environment failures on this host.
  • rustfmt --check on commands.rs and integrations.rs → clean.
  • clippy --all-targets → nothing in the Atomic Agent range (4459–4790 and the test module). The one hit in commands.rs is bool::then at :5115, inside the pre-existing appimage test module.
  • Parse + build end-to-end still green for the four config shapes: fresh install, managed mode, repair over a real [groq] list, and a dangling selection — all load and build every provider, groq's key preserved verbatim.

activeEmbeddingProvider

You asked which way I wanted it and then made the call yourself in the same commit — I'm taking yours, and not just to avoid churn. Your argument is the one that actually decides it: the absent case is already a write, because the seeded provider has to exist or the agent's own default dangles. Once we're writing the entry anyway, naming it next to the entry costs nothing and leaves a hand-edited file self-describing. The toolTransport analogy breaks precisely there, and the ADR bullet now says so. I'd implemented the narrowing locally and I think it's the weaker option for that reason.

One thing I found while probing, not asking for a change

The fix preserves the endpoint at the moment Run is pressed, which is the right guarantee and the one the matrix measures. What it can't do is track a later change. Measured:

no llm block, embeddings daemon OFF        embeddings → :8080
  press Run                                embeddings → :8080   (preserved ✓)
  user then enables the embeddings daemon  embeddings → :8080   (was :19092 pre-Run)
  press Run again                          embeddings → :8080   (entry not refreshed)

Two separate reasons, neither a regression from this commit:

  1. Once an llm block exists, the agent stops consulting localModels.embeddings for the endpoint at all — baseUrl ?? url on the entry wins. That's inherent to creating the block, so it's inherent to the integration existing.
  2. The seed only fires when local-llama is missing, so a second Run doesn't refresh baseUrl on an entry that's already there.

I'm deliberately not proposing a fix for (2): refreshing an entry the user may have tuned is the same move the ADR argues against for everything except requestTimeoutMs, and I'd rather not trade one silent rewrite for another. If it's worth anything, it's a qualifier on the ADR sentence — "mirrors the no-llm-block branch" is true at write time, and after that the entry is the source of truth. Your call entirely; I'd merge as-is.

Thanks for pushing the fix instead of just describing it — that turned a round-trip into a review.


Generated by Claude Code

@Vect0rM

Vect0rM commented Aug 27, 2026

Copy link
Copy Markdown
Member

Took the baseUrl fix myself rather than sending it back for another round — pushed as 2be039e on top of your branch, so the PR is now three commits.

The seed follows the no-llm-block branch, which is the one that governs the files this writer converts:

fn atomic_agent_embedding_base_url(root, chat_url) -> String  // embeddings.enabled ? embeddings.url : chat_url

with ATOMIC_AGENT_DEFAULT_EMBEDDINGS_PORT = 19_092 for a file that names a port but no url, matching how the agent derives embeddings.url itself. Copying config-schema.ts:3096 literally was the other option and I left it alone for the reason you gave: it sets baseUrl unconditionally, so a default install would point at a port with nothing listening.

Verified the same way you did — through loadConfig() + resolveEmbeddingLlmConfig() on agent v0.1.67, not from the source. All six rows come back byte-identical before and after the writer touches the file:

case                   before                        after
external, off          local-llama-embed -> :8080    local-llama -> :8080
external, custom url   local-llama-embed -> :9000    local-llama -> :9000
external, on           local-llama-embed -> :19092   local-llama -> :19092
external, on, :20500   local-llama-embed -> :20500   local-llama -> :20500
managed, off           local-llama-embed -> :19091   local-llama -> :19091
managed, on            local-llama-embed -> :19092   local-llama -> :19092

local-llama is a llama-server entry, so it survives the defaultEmbeddingModel || kind === "llama-server" filter; our own atomic-chat entry does not, which is the right way round.

On activeEmbeddingProvider: keeping the write, and named in the ADR. Absent is not the toolTransport case. parseUserLlmFileConfig defaults an absent one to local-llama and then rejects the file unless that id is listed (llm-config.ts:183-198), so the absent branch is already a write — the seeded provider — and not a restatement. Naming the entry we just seeded keeps a hand-edited file self-describing, and by construction it matches what the parser would have picked anyway. The ADR now says that outright, as the one deliberate exception.

Also in: defaultChatModel written trimmed, and the ADR's preservation sentence narrowed to requestTimeoutMs with the wholesale-rewrite policy stated explicitly.

rustfmt --check clean on commands.rs; cargo test --lib --no-default-features --features test-tauri499 passed, 0 failed here (the two you saw are environment; neither reproduced in this container). Two new tests: seeded_llama_entry_keeps_embeddings_on_the_embeddings_daemon pins all six rows, writes_the_model_trimmed pins the trim.

Merging.


Generated by Claude Code

@Vect0rM
Vect0rM merged commit 073c885 into AtomicBot-ai:main Aug 27, 2026
@Vect0rM Vect0rM mentioned this pull request Aug 28, 2026
3 tasks
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