Skip to content

claude: merge a caller's --settings instead of clobbering ucode's#189

Merged
Edwinhe03 merged 3 commits into
mainfrom
edwin-he/claude-settings-merge
Jul 8, 2026
Merged

claude: merge a caller's --settings instead of clobbering ucode's#189
Edwinhe03 merged 3 commits into
mainfrom
edwin-he/claude-settings-merge

Conversation

@Edwinhe03

Copy link
Copy Markdown
Collaborator

What

ucode claude prepends its own --settings <ucode-file> to every launch. Claude Code honors only one --settings flag, so a caller that also passes --settings '{…}' (e.g. an integration injecting hooks) has exactly one of the two silently dropped — either ucode's gateway config or the caller's settings. This broke omnigent's managed Claude sessions: its transcript/permission hooks were dropped and the web UI rendered empty.

Change

When the caller supplies --settings, compose instead of clobber: deep-merge the caller's settings with ucode's — ucode's gateway keys (apiKeyHelper, env) win, and hooks from both sides are unioned — then hand Claude a single merged --settings (inline JSON). The merge is per-launch and is never written back to the shared ucode-settings.json, so concurrent launches can't accumulate one another's hooks. No caller --settings → unchanged (ucode's settings file is passed directly).

This is the general "ucode should compose with any prior claude command out of the box" fix — it helps every caller of ucode claude --settings, not just omnigent.

Test plan

  • uv run pytest — green.
  • New TestBuildClaudeArgv covers: merge into a single --settings, the --settings= form, ucode-wins-on-conflict, file-path value, and unparseable pass-through.
  • uv run ruff check . + ruff format --check clean.

Related

Split out of #184 (which now carries only the codex --profile-on-app-server fix). Same underlying principle: ucode's injected flags must compose with the caller's command — merged where a flag would clobber (--settings, here), conditional where a subcommand rejects it (--profile, in #184).

This pull request and its description were written by Isaac.

`ucode claude` prepends its own `--settings <ucode-file>` to every launch.
Claude Code honors only ONE --settings flag, so a caller that also passes
--settings (e.g. an integration injecting hooks) has exactly one of the two
silently dropped — either ucode's gateway config or the caller's hooks. This
broke omnigent's managed Claude sessions: its transcript/permission hooks were
dropped and the web UI rendered empty.

Compose instead of clobber: when a caller supplies --settings, deep-merge it
with ucode's (ucode's gateway keys win; hooks from both are unioned) and hand
Claude a single merged --settings (inline JSON). The merge is per-launch and is
never written back to the shared ucode settings file, so concurrent launches
cannot accumulate one another's hooks. No caller --settings → unchanged (pass
ucode's settings file directly).

Co-authored-by: Isaac
Comment thread src/ucode/agents/claude.py Outdated
lilly-luo
lilly-luo previously approved these changes Jul 8, 2026
@Edwinhe03 Edwinhe03 marked this pull request as ready for review July 8, 2026 16:23
Per review on #189: a caller --settings value that ucode could not parse
was silently swallowed (returned None) and then passed through as a second
--settings flag. Claude Code honors only one --settings, so that collision
silently dropped either the caller's settings or ucode's gateway config
(breaking auth). Raise an actionable RuntimeError instead — malformed inline
JSON, a malformed/unreadable settings file, a nonexistent file path, or
non-object JSON — surfaced by the CLI as a clean error. Removes the
pass-through branch, which never helped (the colliding flag always loses).

Co-authored-by: Isaac
@Edwinhe03 Edwinhe03 enabled auto-merge (squash) July 8, 2026 21:00
@Edwinhe03 Edwinhe03 merged commit e42de49 into main Jul 8, 2026
2 checks passed
@Edwinhe03 Edwinhe03 deleted the edwin-he/claude-settings-merge branch July 8, 2026 21:07
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