feat(terminal): add --resolve for MAC targets over ssh (JG-01) - #44
Conversation
A MAC `<router>` pinned to `--via ssh` needs an IP. Wire terminal into the shared `resolveMacTarget` policy that `retrieve`/`execute` already use: CDB-first IP lookup, then `target/mac-unresolved` (default `--resolve none`) or a host-ARP opt-in (`--resolve arp`). terminal's tip leads with the L2 alternative (`--via mac-telnet`, no IP needed); it never resolves via ARP or swaps transport on its own. The mac-telnet default ignores `--resolve` (the MAC is the target). Closes the papercut where the inherited error suggested a `--resolve` flag terminal did not parse. Adds the `terminal` operation to `unresolvedMacError`, the flag to the CLI help + parser, unit tests (terminal + mac), a network-free smoke anchor, and README/examples docs. No transport-path change (ssh-to-IP is already TS1 CHR-passed), so lint+test+build only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 6 minutes and 19 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a ChangesMAC→IP Resolution via
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
Adds proper --resolve <none|arp> support to centrs terminal so a MAC target pinned to the IP transport (--via ssh) can be resolved (CDB-first, optional ARP opt-in) and produces actionable, terminal-specific remediation when it cannot.
Changes:
- Parse and expose
--resolveon theterminalCLI and thread it through terminal request resolution. - Reuse shared MAC→IP resolution (
resolveMacTarget) for terminal’s IP transport path, with tailored remediation for theterminaloperation. - Add unit coverage and a network-free CLI smoke test to lock in the JG-01 behavior; update terminal command docs/examples accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/terminal.ts |
Applies MAC→IP resolution when --via ssh is used with a MAC target and wires the resolved mapping into resolveTarget. |
src/resolver/mac.ts |
Extends shared MAC resolution to support a "terminal" operation variant with terminal-specific remediation text. |
src/cli/terminal.ts |
Adds --resolve <none|arp> flag support and help text for the terminal CLI. |
test/unit/terminal.test.ts |
Verifies default “no silent ARP” behavior, mac-telnet no-op behavior, and unknown --resolve rejection. |
test/unit/mac.test.ts |
Verifies terminal-specific remediation wording for both unresolved and ARP-miss branches. |
test/integration/cli-smoke.test.ts |
Adds a network-free subprocess-level regression test for the JG-01 error + L2 tip behavior. |
commands/terminal/README.md |
Documents --resolve behavior and clarifies the “no silent fallback when --via is pinned” rule for terminal. |
commands/terminal/examples.md |
Adds an executable-spec example (TS4) describing the new --resolve / unresolved behavior and its smoke-test coverage. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@commands/terminal/examples.md`:
- Around line 127-141: The TS4 example violates the contract for numbered
command examples in examples.md files, which must correspond to integration
assertions verified against real CHR. Currently, TS4 is validated only through
network-free smoke tests and unit tests, not the CHR integration path. Either
add corresponding CHR integration test assertions that validate this example
scenario, or remove the example numbering and move the content to a non-spec
note (e.g., as an unnumbered commentary or reference section) that documents
behavior without claiming executable-spec validation.
In `@commands/terminal/README.md`:
- Around line 47-67: The "MAC target over SSH (`--resolve`)" section (lines
47-67) and the section at lines 91-95 restate constitution-level target
selection and protocol policies that should not be duplicated in the command
README. Replace these detailed policy explanations with a brief summary of
terminal-specific behavior and add a link to docs/CONSTITUTION.md for the
cross-command target selection and resolution rules. At lines 47-67, keep only
the terminal-specific detail about how --via ssh interacts with MAC resolution,
then link to the constitution for the detailed CDB-first, ARP, and silent-swap
policies. At lines 91-95, similarly replace constitution restatement with a
terminal-specific summary and link to docs/CONSTITUTION.md for the full policy
context.
In `@src/resolver/mac.ts`:
- Around line 221-223: The error message returned in the "execute" case (around
line 221-223) contains outdated information stating that "mac-telnet L2 execute
is not yet available," which contradicts the current command capabilities and
misleads users. Update the return statement for the "execute" case to remove the
stale text about L2 execute being unavailable while preserving the valid
remediation suggestions about passing an IP/hostname, adding a CDB record, or
using host ARP resolution.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c131fc8d-5950-4738-a375-020855577905
📒 Files selected for processing (8)
commands/terminal/README.mdcommands/terminal/examples.mdsrc/cli/terminal.tssrc/resolver/mac.tssrc/terminal.tstest/integration/cli-smoke.test.tstest/unit/mac.test.tstest/unit/terminal.test.ts
- src/resolver/mac.ts: drop the stale "mac-telnet L2 execute is not yet available" parenthetical — execute/mac-telnet is CHR-passed. DRY the remediation into a `macTelnetCapable` helper (retrieve = IP-first; execute/terminal = L2-first), applied to both the unresolved and not-in-arp branches. - commands/terminal/README.md: trim the constitution-restating prose (CDB / ARP / no-silent-swap policy) to a terminal-specific summary + link to CONSTITUTION (Target selection grammar, Protocol selection). - commands/terminal/examples.md: de-number TS4 → a non-spec behavior note, so numbered examples stay the CHR-integration-assertion set (it is verified by the network-free smoke tier + unit tests, not real CHR). Skipped CodeRabbit's "also applies to README 91-95": that SSH-surface table row is pre-existing and out of scope for this PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Adds a real
--resolve <none|arp>flag toterminal, closing the JG-01 papercut where a MAC target pinned to--via sshhit an inheritedtarget/mac-unresolvederror that suggested--resolve arp— a flagterminaldid not actually parse (so following the tip gaveUnknown terminal flag: --resolve).Behavior (decided in the june-gloom ledger)
A MAC
<router>defaults tomac-telnet(addresses the MAC directly — no resolution). Pinning--via sshasks for the IP-level console, so the MAC must become an IP:targetIP is used (needs no flag).--resolve none, errorstarget/mac-unresolved; the tip leads with--via mac-telnet(reach it over L2, no IP needed), then IP/hostname / CDB record /--resolve arp. centrs never falls back to ARP — or to another transport — on its own (constitution: a pinned--viais never swapped).--resolve arp(orCENTRS_RESOLVE=arp) opts into the host ARP cache; a miss istarget/mac-not-in-arp(also tipping L2).--resolveis a no-op for themac-telnetdefault.How
Reuses the shared
resolveMacTargetpolicy thatretrieve/executealready use — terminal just joins it for its IP transport (ssh), with a"terminal"operation that tailors the remediation. No transport-path change.Tests / verification
test/unit/terminal.test.ts— none-path error + L2 tip, mac-telnet ignores--resolve, unknown-value rejection.test/unit/mac.test.ts—terminaloperation remediation wording (both branches).test/integration/cli-smoke.test.ts— network-free binary-level anchor (JG-01) in the fast gate.bun run lint✅ ·bun run lint:ci✅ ·bun run test(669 pass) ✅ ·bun run build✅The ssh-to-IP transport path is unchanged and already CHR-passed (TS1), and the new MAC→IP logic is pure resolution (unit + smoke verified), so this is a lint+test+build papercut with no new CHR run. MATRIX cells stay
CHR-passed(additive flag).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--resolveflag to terminal command for SSH connections to control MAC-to-IP resolution (options:noneorarp)Documentation
Improvements