Remove the dead grok composer backend from swarm#37
Merged
Conversation
grok CLI 0.2.101 dropped `grok-composer-2.5-fast` (`grok models` lists only grok-4.5; `-m grok-composer-2.5-fast` errors with "unknown model id"), so the composer voice was dead code that failed at runtime. grok-4.5 is now the sole grok model and the ensemble is three voices. - agents.sh: drop GROK_COMPOSER_MODEL, run_grok_composer() and its defensive parser, the composer branch/preflight/effort note, and the header paragraph. - Make grok readiness model-aware, mirroring work-system's agent-registry.sh: `ready`/`list` now also require grok-4.5 in `grok models`, so a dropped or renamed model reads as not-ready with an actionable hint instead of failing mid-review. Degrades to the auth check when the probe is unusable: an empty or unparseable list (offline, renamed subcommand), or no coreutils timeout to bound the call — `ready`/`list` were purely local before this probe, so it must never be the thing that hangs them. - swarm-review.js: drop the composer voice and its FAMILY/MODEL_LABEL entries. The FAMILY map itself stays: the fan-out is many-voices-per-family (one Claude finder per lens), so "same vendor agreeing with itself is one vote" remains load-bearing even though backend→family is now 1:1. - Update docs, READMEs, plugin/marketplace descriptions and knowledge entries; the schema drift warning now names two mirrors, not three. - Bump swarm to 0.4.3 + CHANGELOG entry. Verified live against grok 0.2.102: probe unit-tests (offered / dropped-model / empty-list), `list` + `ready` green, composer rejected with exit 2, an end-to-end `run grok` returning a schema-valid finding, and `ready` on a PATH without timeout/gtimeout answering in ~25ms instead of calling out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gering
force-pushed
the
task/remove-swarm-composer
branch
from
July 17, 2026 12:43
33b6897 to
3351458
Compare
Swarm-reviewed its own diff (PR #37) and found seven issues, all in the probe added by the previous commit. Fixes, most severe first: - Never lie about the model check. With no coreutils timeout the probe is skipped, but it did so SILENTLY while the header/README/CHANGELOG promised `ready` requires grok-4.5 in `grok models` — the same "promise that doesn't hold at runtime" bug this branch exists to remove. It now warns on stderr, and the docs state the degrade. - Jail the probe. It runs a networked third-party binary on a path that was previously local-only, so it now goes through the same env-secret filter + read-deny jail as every other external call. `sandboxed()` hard-wires the review-length cap, so the jail prefix is split into `_build_jail` and shared; the probe keeps its own short bound. - Bound it with `SWARM_PROBE_TIMEOUT` (10s) instead of hardcoding 10 while ignoring the documented `SWARM_TIMEOUT`. Separate knob on purpose: the review cap is 600s and `0` disables it — neither suits a probe `list` blocks on. - Parse model ids by shape, not position. `$2` turned a reworded line (`* default: grok-4.5`) into garbage tokens, which read as "model gone" and failed closed. Match `grok-*` anywhere in a bullet line; no id-shaped token now lands in the trust-auth degrade. - Fix the memoization, which was a no-op: `$(grok_model_list)` ran in a subshell, so the cache globals never reached the parent and the comment's "fetch at most once" was false. `grok_model_fetch` fills the globals directly. - Update `/swarm:agents` notes (still taught grok Ready as auth-only) and the fan-out banner (still said "4 voices"). - Knowledge: record the probe footguns, incl. the subshell-memoization trap. Verified live against grok 0.2.102: cache honored on a second fetch (no second network call), drift format `* default: grok-4.5` now parses to grok-4.5, an id-less bullet degrades to trust-auth, the no-timeout path warns and answers in ~25ms, the jailed probe strips AWS_SECRET_ACCESS_KEY/GH_TOKEN, and an end-to-end `run grok` still returns a schema-valid finding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swarm loop round 1 over the previous commit — 3 findings, all applied. - Failed probe degraded silently (consensus: claude+codex+grok). The last commit warned only when the timeout binary was MISSING, while `|| true` still swallowed a probe that ran and failed (offline, non-zero, garbage output) — and it had tightened the docs to "with a warning, never silently", so the strengthened promise was false on two of three routes. All degrade routes now share one exit (`_probe_degraded`) and name the cause. "The probe answered honestly" (model gone → not ready + hint) stays distinct from "the probe never answered" (→ trust auth + warning). - Drop `_grok_probe_skipped`: set, never read — it implied a skipped-vs-empty distinction no code made. The warning is the channel. - Key the sandbox memo on the backend. `_init_sandbox` cached on a bare "already built" flag, safe while only run_* built a jail; the probe added a second entry point (`_build_jail grok`), so a later same-process `sandboxed codex` would have reused grok's profile — denying codex its own ~/.codex and leaving ~/.grok readable, the cross-backend theft the denylist exists to prevent. Latent today (no path reaches both), fixed structurally. - Knowledge: record that this fix was made three times and shipped wrong twice — N degrade routes need one shared audible exit, not N hand-written warnings. Verified: probe rc=1 → warns + trust-auth; garbage list → warns + trust-auth; valid list without grok-4.5 → honest not-ready, NO warning; grok and codex jails now differ (each denies the sibling's cred dir, keeps its own). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swarm loop round 2 over the previous commit — 6 findings, all applied. Three came out of that commit's own fixes, so this round collapses the branching instead of patching it again. - CRITICAL: rc was only read when the parsed list came back EMPTY. A probe killed mid-stream (timeout) or erroring late can flush PARTIAL output — a non-empty list then skipped the degrade entirely and was treated as authoritative, reporting "this grok CLI does not offer grok-4.5" and telling the user to update an already-current CLI. rc is now checked first and partial output discarded: only a clean exit is an answer. - The no-jail warning lived in sandboxed(), but the probe reaches the jail via _build_jail — so on a host without sandbox-exec/bwrap it ran a networked binary unjailed AND silent. Moved the warning into _build_jail, where the condition arises, so both callers announce it. (The previous commit wrote "N routes need one shared exit" into the knowledge base and then broke it.) - Match the model id as a SUBSTRING, not a whole field: `grok-4.5,` / `` `grok-4.5` `` / a trailing dot survived the field match and broke the exact comparison, failing closed on a CLI that does offer the model. The pattern ends on alphanumerics so no separator rides along. - `_sandbox_ready` sentinel: `""` collided with an empty backend argument and would skip jail construction — failing open, the one direction a sandbox must never fail. Use a sentinel no backend name can equal. - Bump knowledge frontmatter (updatedAt/updatedFrom) on both rewritten entries, per the beaf2b7 precedent — stale stamps make /reindex skip the entry it should re-verify. - Knowledge: record that this spot took four rounds, and why it converged only once the shape changed (rc-first + one exit) rather than on another patch. Verified: live grok ready; model-gone → honest hint with NO degrade warning; partial+timeout → warning + trust-auth; no timeout binary → warning + trust-auth; jail rebuilds per backend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swarm loop round 3 — 3 findings, all in the probe. Rather than patch a probe that had a security jail bolted on (rounds 1-3 all fixed jail-adjacent bugs), this removes the jail: a readiness check passes no untrusted diff, so it never needed one — the sibling `codex login status` in ready_check is unjailed too. - Run `grok models` directly, not through sandboxed(). This roots out the round-3 CRITICAL (#18): the jail's `_init_sandbox` builds its deny profile with python3, so routing the probe through it made python3 a hard dependency of the formerly-local `ready`/`list` paths — and a missing python3 then misreported as "grok models failed (rc=65)". No jail, no python3 dep, honest reasons. - Bound with `timeout -k 3` (#17): plain `timeout` only SIGTERMs, so a grok that ignores SIGTERM (or forks a stdout-inheriting child) kept the command substitution blocking past the deadline — the "must never hang" hole. `-k` SIGKILLs after a grace period. rc 124 (SIGTERM at the deadline) reports as "timed out"; rc 137 (SIGKILL) reports as "killed" — almost always our own `-k`, but an OOM/external kill shares the code, so the message doesn't assert a timeout that may not have happened. - Take only the FIRST id per bullet line (#19): scanning the whole line also matched a grok-4.5 mentioned in prose on another model's line ("* grok-5 (successor to grok-4.5)"), reporting a retired model as offered. - Fold _build_jail back into sandboxed(). It existed ONLY to share the jail with the probe; with the probe unjailed, the split — and the shared-warning (#12) and per-backend-memo (#15) fixes it forced last round — are moot. sandboxed() is back to its pre-split shape; _init_sandbox keeps the backend key (harmless, strictly more correct). - Fix a stale swarm-review.js input-contract comment ("subset of the three" → codex, grok). - Docs (header/README/CHANGELOG/knowledge) drop the jail claim. The knowledge entry records the real lesson: five rounds of patching a jailed probe ended the moment the jail was deleted — a feature added to be safe that generates every round's bugs is a shape problem; cut it, don't harden it. The composer removal (the PR's subject) drew zero findings across all five rounds. Verified: live grok ready; SIGTERM-ignoring grok bounded by -k in ~5s, reported as killed; SIGTERM-obeying grok reported as timed out; model-gone → honest hint, no warning; probe rc=1 → warn + trust-auth; NO python3 invoked on the ready path; review path still fully jailed (sandbox-exec + AWS/GH secret stripping); end-to-end review returns a schema-valid finding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
grok-composer-2.5-fast(grok modelslists only grok-4.5;-m grok-composer-2.5-fasterrors withInvalid params: "unknown model id"), so swarm's composer voice was dead code that failed at runtime.grok-4.5becomes the sole grok model; the ensemble is three voices (Claude lenses + codex + grok-4.5).agent-registry.sh: a dropped or renamed model reads as not-ready with an actionable hint instead of failing mid-review.Changes
Adapter (
scripts/agents.sh)GROK_COMPOSER_MODEL,run_grok_composer()and its defensive parser (~130 lines), the composer branch/preflight/effort note, and the header paragraph. Drops the now-unusedeffort_setvariable.ready/listrequiregrok-4.5ingrok modelson top of auth. Three deliberate details: parse the* grok-4.5 (default)bullet form; match via a newline-fencedcaserather than piping togrep -q(an early-exitinggrep -qcan SIGPIPE the writer, and underset -o pipefaila hit would report failure); and an empty/unreadable model list falls back to the auth check rather than silently dropping grok from every fan-out (offline, timeout, or a future CLI renaming the subcommand).Pipeline (
workflows/swarm-review.js)FAMILY/MODEL_LABELentries.FAMILYmap itself stays, deliberately. The task called for removing the family layer as a one-member special-case, but the fan-out is many-voices-per-family (one Claude finder per gated lens), so "same vendor agreeing with itself is one vote, not a cross-check" is still load-bearing — even though backend→family is now 1:1 and the mapping is a no-op for consensus counting today. Collapsing consensus onto backends would silently re-introduce correlated self-agreement the day a second same-vendor voice returns. Rationale captured in the knowledge base.Docs / metadata
pipeline-blueprint.md,SKILL.md, both READMEs, plugin + marketplace descriptions,CLAUDE.md; the schema drift warning now names two mirrors, not three./querystops resurfacing dead composer guidance; the composer facts are replaced by the lesson that survives (probe models, don't trust them).Readiness
mainplugin.json+marketplace.json(in sync)check-structure.py: 0 errorsTest plan
Verified live against grok 0.2.102:
agents.sh list/ready grokgreen; grok reportsgrok-4.5onlyrun grok --model grok-composer-2.5-fast→ rejected with exit 2run grok --prompt-file→ schema-valid finding in ~8scheck-structure.pygreen (JSON validity, version sync, shell syntax, plugin tests)FAMILY-map decision above is the call you want🤖 Generated with Claude Code