Fix: Skip the transparent listener for a local install, whichever flag starts it

[huang195]

Fixes #1254.

Problem

The enforce-redirect transparent listener was skipped only under --local. Every service install runs cortex --supervise --config ~/.cortex/config.yaml (launchd) or cortex --config … (systemd), so the supervised proxy bound 127.0.0.1:47603 — a port nothing on a laptop redirects to, that the installer's preflight never probes, and whose bind failure is fatal.

Reproduced on macOS against main, with a relabelled, port-shifted build so the live service was not touched. With only the transparent port occupied, the outcome is worse than the issue describes:

• agentop service install waits 15s for health, gets nothing, and exits 1.
• install.sh's service_install_action uses the same four-port list, so it reads that as an unusable supervisor and takes fallback. It starts cortex --local --supervise, which skips the listener and serves. install.sh exits 0 and says the environment "has no usable OS service supervisor".
• The launchd job stays loaded. Its child dies on 47603, then on 47600 once the fallback proxy holds it, every 30s, into the same proxy.log.
• Re-running the one-liner prints Already current: … running under launchd user agent and healthy, because launchd sees the --supervise parent and the fallback proxy answers health.

Fix

Key both redirect-only listeners (outbound transparent, inbound transparent) on the install instead of the flag: skip them when the config is inside ~/.cortex. That is the location test ledgerDefaultOn already uses for the same --local/--config split, and --local points --config at that same file. localMode had no other reader, so it is replaced by localInstall.

A config anywhere else, a pod's included, binds and fails exactly as before. The 127.0.0.1:47603 pin stays, so a copy of the file started from elsewhere still binds loopback only. The built-in config's comment and agentop's migration note said --config binds the listener; both are reworded.

Accepted: a service installed with agentop service install --config <path outside ~/.cortex> still binds it, on the pinned loopback address. The ledger has the same boundary.

Not changed: install.sh's four-port preflight and its fallback classification. The thin-installer series replaces both with agentop setup, which already rolls back a failed service install instead of falling back. With this change, the four-port lists in both are correct as they stand.

Verification

• New TestStartedFromLocalInstall (local install, a pod's config, leftover ~/.cortex with the config elsewhere, a .cortex-old sibling, no home, no config). It fails before the change and passes after.
• cmd/cortex: go vet, plus go test -race untagged and with the local, full and lite profiles. cmd/agentop: go vet, go test ./.... gofmt is clean.
• End to end on macOS (relabelled job, ports 5860x, transparent port occupied), install.sh from this branch:
  • Before: exit 0 via fallback, two supervisors, launchd job crash-looping.
  • After: Running as a launchd user agent, healthy, one supervisor and child on four ports, no errors, no restarts, no fallback.
• Same binary, config outside ~/.cortex: binds the transparent port when it is free, and exits 1 with the bind error when it is taken.
• Not run: the Linux/systemd install. The change has no platform-specific code.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 53 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c9a6b203-237c-4f0e-854e-08c7b9683292

📥 Commits

Reviewing files that changed from the base of the PR and between 3f8f779 and a1eaea8.

📒 Files selected for processing (5)

• cmd/agentop/cmd_config_migrate.go
• cmd/cortex/local.go
• cmd/cortex/local_test.go
• cmd/cortex/main.go
• cmd/cortex/main_test.go

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mrsabath]

Correct, minimal fix that replaces a flag check with the install-location check the codebase already uses for the identical --local/--config split. The localMode -> localInstall rename with the "NOT --local alone" counter-note is the right documentation choice: it records the trap rather than just the rule. TestStartedFromLocalInstall's six rows cover the real boundaries, including the .cortex-old prefix case and the no-home path. The accepted limitation (service install --config <outside ~/.cortex> still binds, on a pinned loopback address) is stated explicitly and matches the ledger's boundary, so the two gates stay consistent.

What I verified

Against the latest main via the GitHub API, not a local clone:

• --local behaviour is preserved. main.go sets *configPath = writeBuiltinConfig(absCortex, ...) -> ~/.cortex/config.yaml, so startedFromLocalInstall returns true on that path. The new gate is a strict superset of the old one, adding only the --config ~/.cortex/... service case from #1254.
• No silent behaviour flip when $HOME is unresolvable. --local already fatals on defaultCortexDir() erroring, before the new call, so startedFromLocalInstall's return false is unreachable from --local. A pod with no home reaches it and binds, as before.
• Assignment ordering is sound. localInstall is set after the --local block and before both read sites. No read-before-write.
• .cortex-old cannot pass for .cortex, since pathUnder uses filepath.Rel. The test's sibling-prefix row asserts exactly that.
• localMode had no other readers. The core/config/config.go hit is a historical comment, not code, so replacing the variable is complete.
• The premise holds. cmd/agentop/setup_probe.go is {"47600", "47601", "47602", "47604"}, commented "47603 is not probed, as install.sh does not". Leaving the four-port lists alone is right: with this change they are correct as they stand.

Findings not anchorable inline

Both are on lines this PR does not touch, so GitHub rejects an inline comment on them. Keeping them here rather than misfiling them onto a nearby changed line.

core/listener/forwardproxy/transparent.go:163 -- nit

A third site still carries the old framing:

// ...this listener is off by default (--local skips it) and every reason it could report is already correct in the log.

The PR body says the two stale claims (the built-in config's comment and agentop's migration note) are reworded. This one survives, and now describes the gate incorrectly in the same way those two did: it is off by default for a local install, --local or --config. Not inline-able because nothing here touches that hunk, which is also why it was missed.

cmd/agentop/setup_probe.go:17 -- suggestion

The comment reads "47603 is not probed, as install.sh does not", which described a stated gap. After this change it is no longer a gap but a consequence: a local install never binds 47603, so there is nothing to probe. Worth upgrading from "we match install.sh" to "nothing binds it" -- the current phrasing invites a future contributor to "fix" the list by adding 47603 back, reintroducing a false preflight failure.

Summary

• Areas reviewed: Go (cmd/cortex, cmd/agentop), tests, embedded YAML config, commit/PR conventions, security (no secrets; loopback pins preserved)
• Commits: 1, signed off: yes
• CI status: passing (27/27, Spellcheck skipped)

Three nit/suggestion-level documentation items, no must-fix.

[mrsabath]

nit: startedFromLocalInstall and ledgerDefaultOn now duplicate the defaultCortexDir + pathUnder pair with the same semantics. Not worth extracting a third helper, but a one-line cross-reference in each direction would stop them drifting.

Worth noting why they will not stay literally identical: ledgerDefaultOn short-circuits on DirSet() ahead of the location test, and returns a reason string. That asymmetry is the thing a future reader needs told, otherwise the obvious-looking cleanup is to collapse them into one function and quietly give the listener gate a cost_ledger.dir escape hatch it should not have.