feat(api,core): multi-target fan-out — shared foundation + api consumer - #114
Merged
Conversation
Make fan-out a first-class, uniform capability. Phase 1 extracts the shared foundation; Phase 2 makes `api` its first new consumer (the deferred item from the `centrs api` work). Both CHR-passed on 7.23.1. Foundation (resolver/selection.ts, cli/selection.ts, core/fanout.ts): - expandCdbSelection: the full target-selection grammar (positionals + repeatable --group/--where + --all + --default), de-duped by record index, ordered by record index; --where matches raw comment facts + core fields (core wins); --all excludes __default__; allowAdhoc surface policy (MCP keeps the CDB allowlist); an absent default CDB is an empty registry, never created. - parseRawCommentFacts (allowlist-free fact reader). - shared CLI selection parser + isFanoutMode (keyed on intent) + strict --concurrency; selectionCommandOptions for command help. - generic runFanout + fanoutExitCode (granular 0/2/1) + promoted envelope helpers. - usage/fanout-not-supported + cdb/empty-selection codes + pages. - retrieve-fanout refactored onto the shared core; retrieve exit code now 0/2/1. api fan-out (api-fanout.ts, cli/api.ts): - apiFanout on runFanout + expandCdbSelection; resolveApiRequest gained a pre-resolved-CDB override (load once, no __default__ synthetic collision). - api positional boundary (final positional = endpoint), fan-out-mode dispatch, request-shape validation before expansion. - --yes-once write confirm naming the blast radius; __default__ deterministic target/unresolved guard; --listen/--stream → usage/fanout-not-supported and --raw → usage/conflicting-flags in fan-out mode. Docs: constitution Target-selection grammar updated to the implemented contract (FanoutData, 0/2/1, --yes for RouterOS writes / --force scoped to devices CDB); api README + examples (F1-F9) + MATRIX; devices README reconciled. CHR-passed 7.23.1: test/integration/api-fanout.test.ts (F1-F9) + fanout-retrieve regression; hermetic cli-smoke guards. Unit: selection, fanout, api-fanout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds multi-target fan-out orchestration to the ChangesAPI multi-target fan-out
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Implements the shared multi-target fan-out foundation (selection grammar + core orchestration helpers) and makes centrs api the first new consumer, including the locked FanoutData envelope shape and the granular 0/2/1 exit-code contract.
Changes:
- Added shared target-selection parsing + CDB selection expansion (
--group/--where/--all/--default) and generic fan-out runner helpers. - Implemented
apifan-out mode (CLI boundary, multi-target dispatch, write confirmation via--yes, and fan-out guards for--raw/--listen). - Updated docs + tests to reflect/validate the uniform fan-out contract (including retrieve’s exit code on partial failure).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/selection.test.ts | Unit coverage for selection expansion (--group, --where, --all, literals, empty selection, absent default CDB). |
| test/unit/fanout.test.ts | Unit coverage for new core fan-out helpers (runFanout, fanoutExitCode, buildFanoutEnvelope, etc.). |
| test/unit/api-fanout.test.ts | Unit tests for api fan-out behavior + CLI positional boundary + concurrency parsing strictness. |
| test/integration/fanout-retrieve.test.ts | Updates retrieve fan-out integration expectations to use exit code 2 on partial failure. |
| test/integration/cli-smoke.test.ts | Adds hermetic CLI guards for incompatible fan-out flags (api + --listen / --raw). |
| test/integration/api-fanout.test.ts | New CHR-backed integration coverage for api fan-out scenarios (F1–F9). |
| src/retrieve-fanout.ts | Refactors retrieve group fan-out to the shared runFanout + shared envelope helpers. |
| src/resolver/selection.ts | New shared selection expansion engine (expandCdbSelection) implementing the fan-out grammar. |
| src/resolver/index.ts | Re-exports new selection and raw-comment-facts utilities. |
| src/resolver/comment-kv.ts | Adds parseRawCommentFacts for --where device-class selection. |
| src/resolver/cdb.ts | Exports resolutionFromEntry for reuse by selection expansion. |
| src/index.ts | Public exports updated to include new fan-out/selection APIs. |
| src/core/fanout.ts | Adds generic runFanout, fanoutExitCode, commonVia, and shared envelope builders. |
| src/core/error-catalog.ts | Registers cdb/empty-selection and usage/fanout-not-supported. |
| src/cli/selection.ts | New shared CLI selection flag parsing (consumeSelectionFlag, isFanoutMode, help rows). |
| src/cli/retrieve.ts | Uses fanoutExitCode for retrieve group fan-out. |
| src/cli/common.ts | Adds parseStrictInteger utility for strict numeric flag parsing. |
| src/cli/api.ts | Adds selection parsing, fan-out dispatch path, and fan-out output + exit code behavior. |
| src/api.ts | Adds resolveApiRequest override seam + exports request-summary builder for fan-out meta. |
| src/api-fanout.ts | New apiFanout orchestrator (selection expansion, write confirmation, per-target envelopes). |
| docs/MATRIX.md | Updates api cell status narrative to include fan-out + associated tests. |
| docs/errors/usage/fanout-not-supported.md | New error page for fan-out-incompatible modes (single-session surfaces). |
| docs/errors/cdb/empty-selection.md | New error page explaining empty selection warning semantics. |
| docs/CONSTITUTION.md | Updates the authoritative target-selection + fan-out envelope/exit-code/write-confirmation rules. |
| commands/devices/README.md | Aligns devices README fan-out description with the locked FanoutData contract. |
| commands/api/README.md | Documents api fan-out behavior, guards, and exit-code semantics. |
| commands/api/examples.md | Updates examples header + adds fan-out examples section (F1–F9) aligned with tests. |
| { cdbResolution: member.resolution }, | ||
| ); | ||
| } | ||
| return resolveApiRequest({ ...request, targetInput: member.input }, env); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
commands/devices/README.md (1)
187-207: 📐 Maintainability & Code Quality | 🔵 TrivialAvoid restating the constitution's fan-out envelope and exit-code contract.
Lines 193–202 repeat the generic
FanoutDataenvelope shape and exit-code mapping that already lives indocs/CONSTITUTION.md. Per the path instructions, command READMEs should link to the constitution for envelope and target selection rather than restate them. Keep only devices-specific details here (CDB de-duplication,--forcegating for CDB mutations).Also, the list of rejected commands is incomplete: it omits
api --raw, which the constitution documents as rejected withusage/conflicting-flags.As per path instructions,
commands/*/README.mdshould not restate the constitution for envelope, errors, settings precedence, target selection, and protocol selection; link todocs/CONSTITUTION.mdinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@commands/devices/README.md` around lines 187 - 207, Remove the duplicated FanoutData envelope and exit-code description from the devices README and replace it with a brief pointer to docs/CONSTITUTION.md for the shared fan-out contract, target selection, and exit-code rules. Keep only devices-specific content like CDB de-duplication and --force gating, and update the unsupported-commands list to include api --raw with its usage/conflicting-flags rejection.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/errors/cdb/empty-selection.md`:
- Around line 7-10: The empty-selection error doc is currently too broad and
incorrectly suggests that bare `--group` empties map here; update the wording to
scope this page to non-group fan-out empties from the selection flow and
explicitly note that bare `--group` is handled by `cdb/empty-group`. Use
`selection.ts` and the `cdb/empty-selection` / `cdb/empty-group` symbols to keep
the error descriptions aligned with the resolver behavior.
In `@src/api-fanout.ts`:
- Around line 383-394: The failure summary in renderApiFanoutEnvelope() hides
the original target input when identity/host are missing, so resolve failures
show as “(unknown)”. Update the label fallback in the target rendering loop to
include meta.target.input after identity and host, keeping the existing
recordIndex and error formatting intact. Use the existing memberTargetMeta()
shape and the target.meta.target fields so adhoc resolve errors clearly identify
the host/path that failed.
---
Nitpick comments:
In `@commands/devices/README.md`:
- Around line 187-207: Remove the duplicated FanoutData envelope and exit-code
description from the devices README and replace it with a brief pointer to
docs/CONSTITUTION.md for the shared fan-out contract, target selection, and
exit-code rules. Keep only devices-specific content like CDB de-duplication and
--force gating, and update the unsupported-commands list to include api --raw
with its usage/conflicting-flags rejection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 77ab3073-b401-4520-836b-151a2a79973e
📒 Files selected for processing (27)
commands/api/README.mdcommands/api/examples.mdcommands/devices/README.mddocs/CONSTITUTION.mddocs/MATRIX.mddocs/errors/cdb/empty-selection.mddocs/errors/usage/fanout-not-supported.mdsrc/api-fanout.tssrc/api.tssrc/cli/api.tssrc/cli/common.tssrc/cli/retrieve.tssrc/cli/selection.tssrc/core/error-catalog.tssrc/core/fanout.tssrc/index.tssrc/resolver/cdb.tssrc/resolver/comment-kv.tssrc/resolver/index.tssrc/resolver/selection.tssrc/retrieve-fanout.tstest/integration/api-fanout.test.tstest/integration/cli-smoke.test.tstest/integration/fanout-retrieve.test.tstest/unit/api-fanout.test.tstest/unit/fanout.test.tstest/unit/selection.test.ts
Comment on lines
+7
to
+10
| A fan-out selection (`--group` / `--where` / `--all` / `--default` / positionals) | ||
| resolved to zero targets. This is reported as a warning on an `ok: true` fan-out | ||
| envelope with `summary { total: 0, ok: 0, failed: 0 }`, not a hard failure — there | ||
| was simply nothing to run. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exclude bare --group from this error description.
src/resolver/selection.ts:276-295 emits cdb/empty-group when a group-only selection matches nothing. This page currently implies that an empty --group selection lands on cdb/empty-selection, which points users at the wrong error doc. Please scope this page to non-group empties or explicitly call out that bare --group uses cdb/empty-group.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/errors/cdb/empty-selection.md` around lines 7 - 10, The empty-selection
error doc is currently too broad and incorrectly suggests that bare `--group`
empties map here; update the wording to scope this page to non-group fan-out
empties from the selection flow and explicitly note that bare `--group` is
handled by `cdb/empty-group`. Use `selection.ts` and the `cdb/empty-selection` /
`cdb/empty-group` symbols to keep the error descriptions aligned with the
resolver behavior.
Comment on lines
+383
to
+394
| for (const target of envelope.data.targets) { | ||
| const meta = target.meta; | ||
| const label = meta.target.identity ?? meta.target.host ?? "(unknown)"; | ||
| const index = meta.target.recordIndex ?? "-"; | ||
| if (target.ok) { | ||
| lines.push( | ||
| ` [${index}] ok ${label} (${target.meta.operation?.objectCount ?? 0} object(s))`, | ||
| ); | ||
| } else { | ||
| lines.push( | ||
| ` [${index}] FAIL ${label} [${target.error.code}] ${target.error.summary}`, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the operation/target meta type to confirm `input` is a valid field.
fd -t f 'envelope.ts' src/core --exec sed -n '1,200p' {}
rg -nP --type=ts -C3 'memberTargetMeta|target:\s*\{' src/api-fanout.tsRepository: tikoci/centrs
Length of output: 7665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant target-meta construction and renderer around the cited lines.
sed -n '130,260p' src/api-fanout.ts
printf '\n---\n'
sed -n '360,430p' src/api-fanout.tsRepository: tikoci/centrs
Length of output: 5579
Adhoc resolve failures should include the original input
renderApiFanoutEnvelope() falls back to (unknown) for positional targets because memberTargetMeta() only carries input and the renderer ignores it. Add meta.target.input to the label fallback so resolve errors identify which host/path failed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api-fanout.ts` around lines 383 - 394, The failure summary in
renderApiFanoutEnvelope() hides the original target input when identity/host are
missing, so resolve failures show as “(unknown)”. Update the label fallback in
the target rendering loop to include meta.target.input after identity and host,
keeping the existing recordIndex and error formatting intact. Use the existing
memberTargetMeta() shape and the target.meta.target fields so adhoc resolve
errors clearly identify the host/path that failed.
# Conflicts: # commands/api/README.md # commands/api/examples.md # docs/CONSTITUTION.md # docs/MATRIX.md # src/cli/api.ts
- selection: --concurrency rejects 0/negative, not just non-integers (the error already promised "integer >= 1"). (Copilot) - api-fanout: drop the borrowed __default__ recordIndex from ad-hoc literal members and stamp their input, so distinct literals don't collide on one index. (Copilot) - api-fanout: renderApiFanoutEnvelope falls back to meta.target.input before "(unknown)", so an ad-hoc resolve failure names the host. (CodeRabbit) - docs/errors/cdb/empty-selection: scope the page to non-group empties and point bare --group empties at cdb/empty-group. (CodeRabbit) - commands/devices/README: stop restating the constitution's FanoutData envelope + exit-code contract; link to it (commands/CLAUDE.md rule). (CodeRabbit nitpick) Unit: --concurrency < 1 rejected; literal member drops recordIndex, keeps input, renders by input not "(unknown)". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+186
to
+193
| // Validate the request shape + method ONCE, before selection expansion: a | ||
| // missing endpoint or an invalid `-X` is an outer error regardless of how many | ||
| // targets the selection resolves to (so an empty selection cannot mask it). | ||
| validateApiRequestShape(request); | ||
| const method = parseApiMethod(request.method); | ||
| const requestSummary = apiRequestSummaryFromRequest(request, env); | ||
| const selectionSummary = summarizeSelection(selection); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Fixed in 13a711a. A globally-pinned --via / CENTRS_VIA outranks every per-target CDB override (in resolveStringSetting, explicit/env beats comment-kv), so an invalid one fails every target identically — it is genuinely a pre-flight condition. apiFanout now validates request.via ?? env.CENTRS_VIA once up front via a shared assertApiProtocolSupported, throwing the same settings/invalid-via / routeros/protocol-not-implemented errors as single-target resolveApiProtocol, which the CLI catch-block renders as an outer ok:false. An invalid CDB-record via override (no global pin) correctly stays a per-target failure, since it is target-specific. Unit coverage added in test/unit/api-fanout.test.ts.
A globally-pinned `--via` / `CENTRS_VIA` outranks every per-target CDB override, so an invalid one fails every target identically. Validate it once up front in `apiFanout` (outer ok:false), matching single-target `resolveApiProtocol`, instead of degrading into N per-target resolve failures. Extracts the shared check as `assertApiProtocolSupported`. Addresses PR #114 Copilot review (src/api-fanout.ts invalid --via). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 30, 2026
This was referenced Jul 18, 2026
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
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.
Why
Fan-out (running a command across many CDB-selected routers) has been the recurring "last item" in command implementations and lived in exactly one place —
retrieve, wired only to a single--group. This makes it a first-class, uniform capability: a shared selection grammar + generic orchestrator that every command can adopt. Phase 1 extracts the foundation; Phase 2 makesapiits first new consumer — the multi-target fan-out deferred from thecentrs apiwork (#91 follow-ups).This PR is the api part. Phases 3–5 (retrieve full grammar, execute, transfer) follow in a later PR on the same plan.
Decisions (locked with the maintainer)
--where <attr>=<value>matches raw comment facts + core fields (target/identity/group/mac; core wins — no spoofing).--yes-only (no--force), confirmed once up front, with a blast-radius message naming the router count when missing.--forcestays scoped todevicesCDB mutations.fanoutExitCode.What's in it
Foundation —
src/resolver/selection.ts(expandCdbSelection: full grammar, de-dupe by record index,--allexcludes__default__,allowAdhocMCP boundary, absent default CDB → empty registry never created),parseRawCommentFacts,src/cli/selection.ts(shared parser +isFanoutMode+ strict--concurrency+ shared help rows),src/core/fanout.ts(runFanout,fanoutExitCode, promoted envelope helpers), newusage/fanout-not-supported+cdb/empty-selectioncodes/pages.retrieve-fanoutrefactored onto the shared core (retrieve now exits 0/2/1).api fan-out —
src/api-fanout.ts(apiFanout),resolveApiRequestpre-resolved-CDB override (load once, avoids the__default__synthetic-target collision), api positional boundary, fan-out dispatch, request-shape validation before expansion,__default__deterministictarget/unresolvedguard,--listen/--rawfan-out guards.Docs —
docs/CONSTITUTION.mdTarget-selection grammar updated to the implemented contract (resolves the prior conflict:FanoutData, 0/2/1,--yesfor RouterOS writes); apiREADME/examples(F1–F9);MATRIX;devicesREADME reconciled to the locked envelope.Validation (CHR 7.23.1)
test/integration/api-fanout.test.tsF1–F9 (38 assertions):--group,--where, empty selection, write-reject, write,--all(excludes__default__), positional+group union de-dupe,--concurrency,--defaultguard.test/integration/fanout-retrieve.test.tsregression (exit 2 on partial).cli-smokeguards (--raw/--listen+ fan-out).selection,fanout,api-fanout. Full suite 892 pass / 0 fail;lint+lint:ci+buildgreen.Incorporates two review rounds
Locked-envelope-only, per-command parser boundaries,
--whereraw facts + core precedence,__default__guard, no-resolveCdbcollision, fan-out-mode dispatch, MCPallowAdhoc; plus the second round: request-shape validation in fan-out, no default-CDB file creation on read, strict--concurrency, constitution updated in this phase, MATRIX claim backed by F6–F9, selection flags in CLI help, examples header refreshed.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests