Conversation
ops/RUNTIME-STATUS.md records that the local SDK refuses the old swarm/workflows schema, which reads like a migration chore. It is not. Two runtimes consume these files and they disagree. The local SDK refuses 1.0 outright (five errors, rc=2). Cloud accepts it: on 2026-09-07 the review gate ran `agent-relay cloud run workflows/review-swarm.yaml` against the unmodified 1.0 file, got run 04da7e48 and sandbox b5f3b344, and failed later on Daytona quota rather than on schema. Three of the seven legacy files are consumed by cloud right now - review-swarm.yaml on every PR, watchdog.yaml as the live `flows-watchdog` schedule, drive.yaml via `agent-relay cloud schedule`. Migrating those would make them parse locally and might make them unrunnable in cloud, taking out the review gate and the watchdog together. So this PR migrates nothing. It provides the tool and the analysis, and leaves the question that has to be answered first: does cloud accept 0.1.0? The migrator refuses rather than guesses, because a migration that silently drops a field is worse than one that fails - the flow runs, looks fine, and means something else. Verified against the real SDK: all seven convert and compile. Losses are recorded as comments in each output rather than found later as bugs, and the one judgement call is documented - a named agent reference becomes the step's `cli`, since 0.1.0 requires a `model` the legacy schema never carried and inventing one is exactly the guess this refuses to make. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…igrating nothing Had all seven files converted and compiling before checking who runs them — three are live cloud consumers including the review gate and the watchdog. Reverted all seven. The open question is whether cloud accepts 0.1.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Review swarm: maintainabilityNo fresh transcript was produced for run |
Review swarm: historyNo fresh transcript was produced for run |
Review swarm: structureNo fresh transcript was produced for run |
Review swarm: FAILED
Cloud run: |
Answered: cloud requires the 1.0 schema. The two are mutually exclusive.The PR left one open question — does cloud accept 0.1.0? — and said it needed a cloud run. That was wrong; it needed three reads. Cloud's HTTP route validates nothing. The answer is client-side in the CLI, before anything is submitted. From the installed function validateYamlWorkflow(content) {
if (!hasField('version')) throw new Error('missing required field "version"');
if (!hasField('swarm')) throw new Error('missing required field "swarm"');
if (!hasField('agents')) throw new Error('missing required field "agents"');
if (!hasField('workflows')) throw new Error('missing required field "workflows"');
}
What followsReverting the seven files was necessary, not cautious. Migrating It also means this is not a migration problem at all. The convergence work is The tool and analysis here stand on their own — the migrator is what you would want the day a converged runtime exists, and it refuses rather than guesses until then. But nobody should migrate a cloud-consumed file on the strength of it. |
maintainability lens — FAILMaintainability review — PR #238 (
|
history lens — FAILBlocker — unrecorded losses contradict the commit’s stated scope (criterion 3). In This also echoes the conversion mistake recorded in Literal reproduction against the PR’s committed script: python3 - <<'PY'
import subprocess, tempfile, yaml
ns = {'__name__': 'review'}
exec(subprocess.check_output(['git', 'show', '0510fae:ops/schema-migration/migrate-legacy-workflow.py'], text=True), ns)
raw = {'version': '1.0', 'name': 'probe', 'description': 'probe', 'cli': 'codex', 'budget': {'maxTokens': 1}, 'workflows': [{'name': 'probe', 'steps': [{'name': 'one', 'type': 'deterministic', 'command': 'true'}]}]}
with tempfile.NamedTemporaryFile(mode='w+', suffix='.yaml') as f:
yaml.safe_dump(raw, f); f.flush()
spec, problems, notes = ns['migrate'](f.name)
print('problems:', problems)
print('notes:', notes)
print('output keys:', sorted(spec))
print('cli preserved:', 'cli' in spec)
print('budget preserved:', 'budget' in spec)
PYCaptured output: Preserve these fields or explicitly refuse them. The commit and README should describe the resulting supported scope accurately. Concern: Notes: The cloud compatibility deferral in REVIEW_FAILED |
structure lens — MISSING |
|
🎯 review-swarm: FAILED (M:fail H:fail S:missing) Lens transcripts posted as sibling comments above. |
…urce versions Three cubic findings on #238, all on the migrator I wrote to prevent exactly this class of bug. P1 is the one that matters. `cli`, `triggers` and `budget` are valid 0.1.0 fields, so they passed the unknown-key check and were then dropped, because the return statement emitted a fixed four-key dict. A file declaring a global CLI, a trigger registration or a budget limit would have migrated cleanly and silently lost it - the precise failure this script's own docstring refuses to commit. Absent optionals were also written as explicit nulls, which is not a valid value for them. Output is now built conditionally from what the source set. P2: the source version was never checked, so a 0.2.0 file would have been restamped 0.1.0 and migrated by guesswork. Non-1.0 input is refused. P3: the README presented a reformatted multi-line list as a verbatim transcript. The script emits one JSON diagnostic on stderr and exits 2. Replaced with the literal captured output and a note about the earlier paraphrase, per the repo's evidence rule. Verified: 0.2.0 input refused rc=1; cli and budget survive and an absent description is omitted rather than nulled; all seven legacy files still convert and compile; nothing under workflows/ is touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ll as absent Two cubic findings on #238. The version guard recorded a mismatch and then continued into the legacy loops. A future-version file using the 0.1.0-style `agents` MAP would reach `a.get(...)` and raise a traceback instead of the clean refusal this script promises. It now returns immediately. An explicitly null top-level field was treated as absent and silently dropped. An absent key and a key set to null are different facts, and dropping the second is exactly the failure this script refuses everywhere else. Missing keys are skipped; explicit nulls are refused with a message that says to remove the key or give it a value rather than guessing which was meant. Verified both: a 0.2.0 source carrying an agents map exits 1 naming the version, and `cli: null` exits 1 naming the field. All seven legacy files still convert and compile, and nothing under workflows/ is touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
ops/schema-migration/migrate-legacy-workflow.py compiles to __pycache__ when the review job runs it. Nothing tracks the .pyc, but the untracked file lands in the swarm's working tree and breaks patch application: error: cannot apply binary patch to 'ops/schema-migration/__pycache__/migrate-legacy-workflow.cpython-314.pyc' without full index line Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
`dest = sys.argv[2] if len(sys.argv) > 2 else src` meant a single-argument run overwrote the input file. The review swarm called this out as a blocker, and it is right: silently rewriting the author's source is exactly the guessing this tool's docstring refuses to do everywhere else. An in-place migration is still possible -- pass the same path twice -- but only when asked for explicitly. Verified: a single-argument run now exits 2 and leaves the file byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
The refusal path lives in the `if __name__` block, so `return` raised SyntaxError: 'return' outside function and the script would not run at all. I validated the syntax before writing the file rather than after, which is exactly the mistake the assert-the-mutation rule exists to catch. Verified after the change: single-arg exits 2 with the refusal and leaves the input byte-identical; two-arg still migrates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
AGENT_DECL accepts `model` and a named agent's model was stored in agents[name], but lowering an agent reference copied only `cli`. A 1.0 file whose agent declared a model migrated with no problems reported while the model was discarded, and the loss note then claimed the legacy file "never had" one, which was false precisely when it mattered. Dropping it is not cosmetic: PR #136's resolveNamedAgent carries declaration.model so a named selection cannot inherit the host's model. Removing the selection lets a migrated run choose a different model, with different behaviour and cost. This is the same accepted-then-dropped failure this script repairs for top-level cli/triggers/budget, so it now refuses rather than guessing which model the author would have accepted. Where a declaration genuinely carried no model, the note says so truthfully. Verified against workflows/watchdog.yaml: the unchanged file still migrates (exit 0) and its header note now reads "carried no model, so nothing was lost"; the same file with an added model is REFUSED with the agent named. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
The structure lens raised two P1 blockers and both were right. 1. Malformed input escaped as a traceback. `yaml.safe_load(open(path))` assumed a mapping, so an empty document (None), a scalar document, invalid YAML, or an unreadable file crashed before the promised REFUSED result was printed. The same unchecked assumption sat at `agents` and at `workflows[0].steps`. An author could not tell an unsupported source from a bug in this tool. 2. An empty workflow migrated successfully. Only the workflow COUNT was checked, so a missing or empty `steps` became `steps: []` and returned MIGRATED -- an artifact the SDK then refuses, per the refusal recorded in this directory's README. That is the guessing this script exists to refuse. Both now return a diagnostic and write no output file. Verified: valid watchdog.yaml exit 0 MIGRATED (1 steps, 4 recorded losses) empty file exit 1 "is empty; there is nothing to migrate" scalar document exit 1 "must contain a mapping at the top level, found str" steps: [] exit 1 "0.1.0 requires a non-empty steps array" invalid YAML exit 1 "is not valid YAML: while parsing a block mapping" Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
|
Both blockers were found against an earlier commit and no longer reproduce at the current head ( History blocker — top-level They are preserved, not omitted. The blocker's premise — "lines 106-110 omit them without refusal or loss notes" — does not hold at this head. Maintainability blocker — if len(sys.argv) > 2:
dest = sys.argv[2]
else:
... f"refusing to migrate {src} in place: pass an explicit destination, "
sys.exit(2)An implicit in-place rewrite is refused with exit 2. A note on why this was not obvious, since it bit me twice today. I tried to judge verdict freshness by comparing the head's commit timestamp to the verdict timestamp. That is unsound: commit date is not push date, so a verdict can postdate a commit and still have reviewed an earlier revision. Here the arithmetic said "fresh" while the lens was demonstrably reading The reliable signal is the one this lens supplied: name the reviewed SHA and show literal output. Both lenses that gave me usable verdicts today did that. Not claiming #238 is now correct — only that these two blockers no longer reproduce. Worth a re-review at |
… by repro Ran the history lens's own reproduction against the head: cli/budget/triggers come out preserved=True, and the maintainability blocker's cited line now refuses in-place rewrite with exit 2. Both were found against 0510fae. Also invalidated my own freshness test: commit date is not push date, so timestamp arithmetic said "fresh" for a verdict that had read an earlier commit. The reliable signal is the lens naming its reviewed SHA. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
At a462407 the dispatch accepts only select|report while the untouched test drives apply three times. Those cases are a crash-injection + idempotency suite, so deleting the test would silently drop real coverage. Did not pick a fix: apply and its test are bound to one hardcoded backlog item, which this PR exists to generalise. Posted three non-equivalent paths and the evidence instead of guessing at scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
…gate #257 timed out at exactly 65.0 min and mislabeled it `running`; #238's swarm genuinely failed and the gate reported it correctly. Different causes -- the gate is not uniformly broken as I implied on #255. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
|
Closing: the actual defect this PR touched (migrate-legacy-workflow.py raising AttributeError on scalar steps instead of REFUSED) is now tracked as its own issue at #270, with the exact repro captured. The PR's doc change alone did not fix the bug. Reopen or open a fresh PR against #270 with the guard on |
…ct (#255) (#378) `opencode` was mapped to the structure lens in both `ops/preswarm-check/ lens-runner.sh` and `workflows/review-swarm.yaml`, but the binary was absent on both runners for every sampled PR. That made structure MISSING on every review comment (byte-identical stub across #238, #240, #242, #244, #250, #251, #252), which in turn made the aggregate `review` check unpassable by construction: `swarm-post.sh` gates on `overall = PASSED`, and one lens returning no verdict makes PASSED impossible. Two PRs (#250, #252) were merged with `--admin` for exactly this reason; the corrosive effect the issue calls out is that a gate that can never pass trains everyone to override it, so a *real* FAILED becomes noise. Move `structure` to `codex` — installed and authenticated in both runners so it can actually produce a transcript. Model diversity across the swarm is preserved by `maintainability = claude`; the two `codex` lenses remain independent because their prompts differ, not because their CLIs differ. Add `ops/preswarm-check/lens-cli-parity-check.sh` and wire it into the base-owned `review-swarm-wrapper-guard.yml` alongside the prompt-parity check from #218. The two checks are the same shape of drift on two axes: prompt text and CLI mapping. The new check also refuses if a mapped CLI is not on PATH, since that reproduces the exact failure this fix exists to prevent — a lens whose CLI call fails silently produces NO_VERDICT on every PR and buries real findings. `PRESWARM_ALLOW_MISSING_CLI=1` opts a partial environment out; the guard workflow does not set it. Verified with a mutation: pointing the swarm YAML back at `opencode` while leaving the runner on `codex` makes the parity check fail on both `history` and `structure` with the specific divergent CLIs printed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 Co-authored-by: kjgbot <kjgbot@agentrelay.dev>
The finding
ops/RUNTIME-STATUS.mdrecords that the local SDK "refuses its oldswarm/workflowsschema before execution." That reads like a migration chore. It is not — two runtimes consume these files and they speak different schema versions.The local SDK refuses 1.0 outright (rc=2):
Cloud accepts it. On 2026-09-07 the review gate ran
agent-relay cloud run workflows/review-swarm.yaml --sync-code --jsonagainst the unmodified 1.0 file; it returned run04da7e48-87ec-4c7a-a1ee-22fd482e1cd1and was given sandboxb5f3b344-64cc-434d-97f8-f5da71ba4517. It failed later on Daytona CPU quota — not on schema.Why this PR migrates nothing
Three of the seven legacy files are consumed by cloud right now:
workflows/review-swarm.yaml.github/workflows/review-swarm.yml:145, every PRworkflows/watchdog.yamlflows-watchdog, cron0 8 * * *, activeworkflows/drive.yamlagent-relay cloud schedule(ops/AUTONOMY.md)Migrating those makes them parse locally and may make them unrunnable in cloud — which would take out the review gate and the watchdog together. I converted all seven and confirmed they compile, then reverted every one. The diff touches no file under
workflows/.The question that has to be answered before anyone migrates: does cloud accept 0.1.0? I could not answer it from this machine without spending a cloud run, and the Relayfile database is at its 10 GiB cap.
What is here
ops/schema-migration/migrate-legacy-workflow.py— converts one file and refuses rather than guesses. A migration that silently drops a field is worse than one that fails: the flow runs, looks fine, and means something else.Verified against the real SDK, all seven convert and compile (9, 7, 7, 6, 1, 2, 1 steps). Losses are written into each output as comments:
swarm: {pattern, channel, timeoutMs}— no run-level slot in 0.1.0preset— persona surface, RFC-0001 decision 9timeoutMs— 0.1.0 bounds deterministic steps onlycli— 0.1.0 requiresmodelin a named declaration and the legacy schema never carried one. Verified both ways: an agent step withcliand no agents map compiles; an agents map withcliand nomodelis refused. Inventing a model is exactly the guess this refuses to make.Lossless:
version1.0 → 0.1.0, stepname→id, agenttask→instruction,workflows[0].stepslifted to top level. Every legacy file declares exactly one workflow, so that is a lift and not a split — and the script refuses a multi-workflow file rather than guessing how to divide it.Not merging this myself
No independent signoff is available — the review swarm is one of the things blocked.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR