feat(cli): health-aware list/stop, startup failure diagnostics, port retry, doctor --probe

[AbirAbbas]

Problem

A live field test of the sub-harness flow hit four operational failures in one session, all of the same class: the platform knew something and didn't say it.

1. af stop silently killed in-flight work. A graceful stop terminated a node that had a long-running execution in progress, with no warning — even though running executions are queryable from the control plane.
2. af run startup failures were opaque and non-recoverable. When a node was assigned a port that looked free but lost the bind race (a just-stopped node's port lingering under mirrored networking), the SDK exited with the strict-port "assigned port unavailable" error and nothing retried. The CLI only printed "did not become ready within 30s"; the real traceback lived in a separate af logs.
3. af list trusted the local registry blindly. Registry status is a claim, not a fact — a node the registry calls "running" can be dead on the control plane (or vice versa) and nothing reconciled the two views.
4. af doctor reported providers as healthy from a PATH check alone. A provider CLI whose binary exists can still return instant empty completions (broken auth, model outage); doctor never actually probed provider health.

What changed

Four focused commits, one per failure class:

• feat(cli): warn before af stop interrupts running executions — before signalling the process, af stop <node> queries the control plane for executions still running on the node. Found + TTY → list count/ids/age and prompt to confirm; found + non-TTY → warn and proceed (prior behavior); no executions or --force → stop immediately; control plane unreachable → note it and proceed (best-effort). Warnings go to stderr so --json stdout stays a clean envelope.
• feat(cli): surface af run startup logs and retry once on strict-port bind conflict — on any startup failure the last ~15 lines of the node's log are printed inline plus a Full logs: af logs <node> pointer. When the failure is the SDK's strict-port bind conflict (detected from the node log), the run path retries exactly once on a fresh port (the failed port is reserved first so it is never reused), logging Port <p> unavailable, retrying on a fresh port. The port-alloc/start/wait section is refactored into attemptStart + startWithPortRetry so the retry decision and port-change logic are unit-testable without the real health poll.
• feat(cli): reconcile af list registry status with control-plane health — af list gains a HEALTH column that fetches GET /api/v1/nodes?show_all=true (so inactive nodes are included) and reconciles it with each node's registry status. Agreement shows plain health; disagreement is marked (mismatch) with an explanatory footer; a node absent from the control plane while the registry calls it running reads not on control plane (mismatch); an unreachable control plane yields unknown (control plane unreachable) and never errors. The same health / health_discrepancy fields are added to af list --json.
• feat(cli): add af doctor --probe to smoke-test detected provider CLIs — new opt-in --probe flag runs a minimal one-shot prompt against each detected provider CLI (claude -p, codex exec, gemini -p, opencode run) with a per-provider 60s timeout, classifying the result as ok (non-empty), empty (exit 0 but no completion — the silently-broken case a PATH check misses), error (non-zero exit, stderr head captured), or timeout. Probes run only for providers doctor already detects; without --probe the command is unchanged. Help text and output note that a probe consumes a trivial amount of provider quota.

Validation Contract

Tests are derived from behavior, not implementation. Each contract item maps to named tests:

Contract item	Tests
Stop with running executions: non-TTY warns with count; --force does not warn (and never queries); no executions is silent; unreachable proceeds; interactive decline aborts	TestConfirmStop_NonInteractive_WarnsWithCount, TestConfirmStop_Force_NoWarnNoQuery, TestConfirmStop_NoRunningExecutions_Silent, TestConfirmStop_ControlPlaneUnreachable_Proceeds, TestConfirmStop_Interactive_DeclineAborts, TestConfirmStop_Interactive_AcceptProceeds, TestQueryRunningExecutions_ParsesEnvelope, TestQueryRunningExecutions_EmptyServerURL, TestReadAffirmative, TestFormatExecutionAge
Startup-failure path prints the tail of the node's log and the af logs pointer	TestPrintStartupFailureDiagnostics, TestReadLogTailLines
Strict-port failure triggers exactly one retry on a different port; non-conflict failures do not retry; the failed port is excluded	TestStartWithPortRetry_RetriesOnceOnPortConflict, TestStartWithPortRetry_NoRetryOnNonConflictFailure, TestStartWithPortRetry_SuccessRunsOnce, TestStartWithPortRetry_NoDistinctPortDoesNotRetry, TestFreshRetryPort_ExcludesFailedPort, TestLogIndicatesPortConflict
af list merges the registry with a mocked /api/v1/nodes: agreement renders plainly, discrepancy is flagged, unreachable yields unknown without erroring	TestReconcileHealth, TestResolveNodeHealth_Merge, TestResolveNodeHealth_Unreachable, TestFetchControlPlaneNodes_Reachable, TestFetchControlPlaneNodes_Unreachable
Doctor probe classification of ok/empty/error/timeout from (exit code, stdout, timed-out) tuples; probes only detected providers; af doctor without --probe performs no probe	TestClassifyProbe, TestProbeHarnessProvider_RealProcesses, TestRunHarnessProbes_SkipsUndetected, TestDoctorCommand_NoProbeByDefault

Test results

• npm ci && npm run build (control-plane UI): pass
• GOFLAGS=-buildvcs=false go build ./...: pass
• go vet ./...: pass
• go test ./internal/cli/... ./internal/core/services/...: pass (all new spec tests green)
• gofmt -l on changed files: empty
• golangci-lint run: changed files introduce zero new findings (errcheck counts unchanged vs. base for doctor.go and agent_service.go)

One unrelated test, internal/server/TestStartAndStopCoverAdditionalBranches, fails locally in this sandbox because the admin gRPC listener cannot bind here; it fails identically on the base commit and this branch touches no code in internal/server, so it is a pre-existing environment artifact, not a regression.

🤖 Generated with Claude Code

[github-actions]

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface	Current	Baseline	Δ
control-plane	86.90%	87.40%	↓ -0.50 pp	🟡
sdk-go	92.50%	92.00%	↑ +0.50 pp	🟢
sdk-python	93.82%	93.73%	↑ +0.09 pp	🟢
sdk-typescript	91.05%	90.42%	↑ +0.63 pp	🟢
web-ui	84.75%	84.79%	↓ -0.04 pp	🟡
aggregate	85.54%	85.75%	↓ -0.21 pp	🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

[github-actions]

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface	Touched lines	Patch coverage	Status
control-plane	401	83.00%	✅
sdk-go	0	—	➖ no changes
sdk-python	0	—	➖ no changes
sdk-typescript	0	—	➖ no changes
web-ui	0	—	➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

[AbirAbbas]

🔴 PR-AF Review — Merge Blocked — Must-Fix Found

🚫 Merge blocked. 1 must-fix issue found by automated review.

Automated multi-agent code review · PR-AF built with AgentField

19 findings · 🚫 1 blocking · 💬 18 advisory · 🔴 0 critical · 🟠 14 important · 🔵 4 suggestions · ⚪ 1 nitpicks

PR Overview

Problem

A live field test of the sub-harness flow hit four operational failures in one session, all of the same class: the platform knew something and didn't say it.

1. af stop silently killed in-flight work. A graceful stop terminated a node that had a long-running execution in progress, with no warning — even though running executions are queryable from the control plane.
2. af run startup failures were opaque and non-recoverable. When a node was assigned a port that looked free but lost the bind race (a just-stopped node's port lingering under mirrored networking), the SDK exited with the strict-port "assigned port unavailable" error and nothing retried. The CLI only printed "did not become ready within 30s"; the real traceback lived in a separate af logs.
3. af list trusted the local registry blindly. Registry status is a claim, not a fact — a node the registry calls "running" can be dead on the control plane (or vice versa) and nothing reconciled the two views.
4. af doctor reported providers as healthy from a PATH check alone. A provider CLI whose binary exists can still return instant empty completions (broken auth, model outage); doctor never actually probed provider health.

What changed

Four focused commits, one per failure class:

• feat(cli): warn before af stop interrupts running executions — before signalling the process, af stop <node> queries the control plane for executions still running on the node. Found + TTY → list count/ids/age and prompt to confirm; found + non-TTY → warn and proceed (prior behavior); no executions or --force → stop immediately; control plane unreachable → note it and proceed (best-effort). Warnings go to stderr so --json stdout stays a clean envelope.
• feat(cli): surface af run startup logs and retry once on strict-port bind conflict — on any startup failure the last ~15 lines of the node's log are printed inline plus a Full logs: af logs <node> pointer. When the failure is the SDK's strict-port bind conflict (detected from the node log), the run path retries exactly once on a fresh port (the failed port is reserved first so it is never reused), logging Port <p> unavailable, retrying on a fresh port. The port-alloc/start/wait section is refactored into attemptStart + startWithPortRetry so the retry decision and port-change logic are unit-testable without the real health poll.
• feat(cli): reconcile af list registry status with control-plane health — af list gains a HEALTH column that fetches GET /api/v1/nodes?show_all=true (so inactive nodes are included) and reconciles it with each node's registry status. Agreement shows plain health; disagreement is marked (mismatch) with an explanatory footer; a node absent from the control plane while the registry calls it running reads not on control plane (mismatch); an unreachable control plane yields unknown (control plane unreachable) and never errors. The same health / health_discrepancy fields are added to af list --json.
• feat(cli): add af doctor --probe to smoke-test detected provider CLIs — new opt-in --probe flag runs a minimal one-shot prompt against each detected provider CLI (claude -p, codex exec, gemini -p, opencode run) with a per-provider 60s timeout, classifying the result as ok (non-empty), empty (exit 0 but no completion — the silently-broken case a PATH check misses), error (non-zero exit, stderr head captured), or timeout. Probes run only for providers doctor already detects; without --probe the command is unchanged. Help text and output note that a probe consumes a trivial amount of provider quota.

Validation Contract

Tests are derived from behavior, not implementation. Each contract item maps to named tests:

Contract item	Tests
Stop with running executions: non-TTY warns with count; --force does not warn (and never queries); no executions is silent; unreachable proceeds; interactive decline aborts	TestConfirmStop_NonInteractive_WarnsWithCount, TestConfirmStop_Force_NoWarnNoQuery, TestConfirmStop_NoRunningExecutions_Silent, TestConfirmStop_ControlPlaneUnreachable_Proceeds, TestConfirmStop_Interactive_DeclineAborts, TestConfirmStop_Interactive_AcceptProceeds, TestQueryRunningExecutions_ParsesEnvelope, TestQueryRunningExecutions_EmptyServerURL, TestReadAffirmative, TestFormatExecutionAge
Startup-failure path prints the tail of the node's log and the af logs pointer	TestPrintStartupFailureDiagnostics, TestReadLogTailLines
Strict-port failure triggers exactly one retry on a different port; non-conflict failures do not retry; the failed port is excluded	TestStartWithPortRetry_RetriesOnceOnPortConflict, TestStartWithPortRetry_NoRetryOnNonConflictFailure, TestStartWithPortRetry_SuccessRunsOnce, TestStartWithPortRetry_NoDistinctPortDoesNotRetry, TestFreshRetryPort_ExcludesFailedPort, TestLogIndicatesPortConflict
af list merges the registry with a mocked /api/v1/nodes: agreement renders plainly, discrepancy is flagged, unreachable yields unknown without erroring	TestReconcileHealth, TestResolveNodeHealth_Merge, TestResolveNodeHealth_Unreachable, TestFetchControlPlaneNodes_Reachable, TestFetchControlPlaneNodes_Unreachable
Doctor probe classification of ok/empty/error/timeout from (exit code, stdout, timed-out) tuples; probes only detected providers; af doctor without --probe performs no probe	TestClassifyProbe, TestProbeHarnessProvider_RealProcesses, TestRunHarnessProbes_SkipsUndetected, TestDoctorCommand_NoProbeByDefault

Test results

• npm ci && npm run build (control-plane UI): pass
• GOFLAGS=-buildvcs=false go build ./...: pass
• go vet ./...: pass
• go test ./internal/cli/... ./internal/core/services/...: pass (all new spec tests green)
• gofmt -l on changed files: empty
• golangci-lint run: changed files introduce zero new findings (errcheck counts unchanged vs. base for doctor.go and agent_service.go)

One unrelated test, internal/server/TestStartAndStopCoverAdditionalBranches, fails locally in this sandbox because the admin gRPC listener cannot bind here; it fails identically on the base commit and this branch touches no code in internal/server, so it is a pre-existing environment artifact, not a regression.

🤖 Generated with Claude Code

Key Findings

1 issue(s) should be addressed before merge:

• 🟠 Explicit --port is silently reassigned on bind failure (control-plane/internal/core/services/agent_service.go:112) — A caller that explicitly passes --port P can receive a successful run on an unrelated port. Gate: Silent reassignment of an explicit --port violates the CLI contract and can break caller firewall rules or service discovery.

18 advisory finding(s) surfaced as non-blocking:

• 🟠 Failed probes do not affect harness recommendation (control-plane/internal/cli/doctor.go:148)
• 🟠 Execution warning silently undercounts results beyond the fixed limit (control-plane/internal/cli/stop.go:316)
• 🟠 Classify OpenCode's exit-zero stderr failures as errors (control-plane/internal/cli/doctor.go:238)
• 🟠 Stale append-only logs can trigger a retry for the wrong attempt (control-plane/internal/core/services/agent_service.go:174)
• 🟠 Stop warning silently omits running executions beyond the first page (control-plane/internal/cli/stop.go:349)
• … and 13 more (see All Findings by Severity)

Files with findings: control-plane/internal/cli/doctor.go, control-plane/internal/cli/list.go, control-plane/internal/cli/stop.go, control-plane/internal/core/services/agent_service.go

All Findings by Severity

🟠 Important (14)

• Failed probes do not affect harness recommendation control-plane/internal/cli/doctor.go:148
• Execution warning silently undercounts results beyond the fixed limit control-plane/internal/cli/stop.go:316
• Classify OpenCode's exit-zero stderr failures as errors control-plane/internal/cli/doctor.go:238
• Stale append-only logs can trigger a retry for the wrong attempt control-plane/internal/core/services/agent_service.go:174
• Stop warning silently omits running executions beyond the first page control-plane/internal/cli/stop.go:349
• Do not require a global claude binary for the SDK-backed claude-code harness control-plane/internal/cli/doctor.go:88
• Explicit --port is silently reassigned on bind failure control-plane/internal/core/services/agent_service.go:112
• Retry can start a second child after first-child cleanup fails control-plane/internal/core/services/agent_service.go:177
• Equivalence lookup can conflate distinct registered nodes control-plane/internal/cli/list.go:176
• Desktop list integration names an unsupported JSON flag control-plane/internal/cli/list.go:83
• Stop guard can miss executions for manifests without node_id control-plane/internal/cli/stop.go:188
• Do not use PATH-only availability as authorization to run provider probes control-plane/internal/cli/doctor.go:175
• Do not classify legacy health_status "degraded" as alive control-plane/internal/cli/list.go:153
• Validate the agentic query envelope before treating it as empty control-plane/internal/cli/stop.go:345

🔵 Suggestion (4)

• A previous forced command can make an ordinary stop skip the guard control-plane/internal/cli/stop.go:21
• Unforced stop can inherit --force from an earlier execution control-plane/internal/cli/stop.go:51
• A failed reservation can suppress the only fresh-port retry control-plane/internal/core/services/agent_service.go:209
• Control-plane authentication and response failures are mislabeled as unreachable control-plane/internal/cli/list.go:204

⚪ Nitpick (1)

• Failed-port reservation leaks into future runs control-plane/internal/core/services/agent_service.go:208

Review Process Details

Dimensions Analyzed (6):

• Stop guard preserves its safety decision under complete and failed control-plane responses — 3 file(s)
• Strict-port retry must exclude the failed port under the real PortManager contract — 3 file(s)
• Strict-port retry neither violates requested-port expectations nor retries on stale evidence — 2 file(s)
• Stop guard must honor the agentic query response envelope — 2 file(s)
• New health/probe output does not report a usable provider or reachable control plane when its own probe disproves it — 4 file(s)
• Cobra command construction must not retain --force across executions — 3 file(s)

Meta-Dimension Lenses (3):

• Semantic — 3 dimension(s), 87% coverage confidence
• Mechanical — 3 dimension(s), 90% coverage confidence
• Systemic — 2 dimension(s), 88% coverage confidence

Cross-Reference & Adversary Analysis:

• 15 finding(s) adversarially tested: 6 confirmed, 9 challenged

Pipeline Stats

Metric	Value
Duration	796.1s
Agent invocations	32
Coverage iterations	1
Estimated cost	N/A (provider does not report cost)
Budget exhausted	No
PR type	feature
Complexity	medium

Review ID: rev_cf4d09b58367

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: Bug in new --probe feature does not break existing contracts or cause security/data issues; can be fixed in a follow-up.

Make --probe recompute Recommendation.HarnessUsable, HarnessProviders, and the harness note from providers whose probe status is ok.

_{🟠 IMPORTANT · confidence 99%}

A CLI that returns empty, error, or timeout during a probe does not update recommendation.harness_providers, recommendation.harness_usable, or the harness note—so zero successful probes still recommend a harness, and mixed results include failed providers. Only harness_probes surfaces the actual status, breaking consumers that rely on the recommendation fields.

Evidence

Step 1: NewDoctorCommand calls buildDoctorReport at line 145, which builds Recommendation from PATH detection: HarnessUsable: len(availableHarness) > 0 and HarnessProviders: availableHarness (lines 328-333). Step 2: for af doctor --probe, line 148 only assigns report.HarnessProbes = runHarnessProbes(report). Step 3: runHarnessProbes records statuses such as empty, error, and timeout (lines 172-180), but no code recomputes Recommendation or its notes. Step 4: JSON encoding at lines 151-154 and human recommendation rendering at lines 458-466 therefore expose the stale PATH-based recommendation even when every probe failed.

💡 Suggested Fix

When --probe is requested, derive Recommendation.HarnessUsable, Recommendation.HarnessProviders, and the harness-related note from providers whose probe status is ok (while retaining DoctorReport.HarnessProviders as the raw detection map). Preserve the PATH-based behavior when --probe is absent, and add zero-success and mixed-success JSON assertions.

---

Machine-consumer semantics · confidence 99%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The warning undercounts but does not cause data loss, security issues, or broken contracts; it is a UX improvement opportunity.

Return the decoded total alongside the results and make the warning state the real total, including that only the first 100 IDs are shown.

The guard requests at most 100 executions but discards the server-provided total. The query handler computes total before applying the limit, so a node with 101+ running executions returns 100 records and total:101. The prompt then says only 100 will be interrupted, hiding the remaining execution and undermining the acknowledgement.

Evidence

Step 1: queryRunningExecutions sends limit: 100 (stop.go:316-320). Step 2: the control-plane query implementation calculates total := len(execs) before truncating execs to req.Limit, then returns both values (internal/handlers/agentic/query.go:134-149). Step 3: the CLI response type decodes Data.Total but returns only parsed.Data.Results (stop.go:297-305, 345-349). Step 4: confirmStopWithRunningExecutions uses len(execs) in both its warning and TTY confirmation (stop.go:375, 386), so 101+ actual executions are represented as 100 and the stop then proceeds after that incomplete confirmation.

💡 Suggested Fix

Return the total alongside results and update the warning to show the real total, noting that only the first 100 IDs are displayed. Alternatively, page until all executions are fetched with a bounded timeout; add a test for total > len(results).

---

Stop execution warning correctness · confidence 98%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: Diagnostic tool misreporting does not break build, security, data, or API contracts.

Classify OpenCode's exit-zero stderr failures as errors

_{🟠 IMPORTANT · confidence 99%}

Pass stderr into classification and treat OpenCode's exit-zero stderr errors as errors.

classifyProbe ignores stderr, so a zero exit with blank stdout returns empty even when OpenCode's stderr contains known error patterns (e.g., “Model not found”). This causes af doctor --probe to misreport a real provider failure as empty instead of error.

Evidence

Changed code: control-plane/internal/cli/doctor.go:187-188 calls classifyProbe(exitCode, stdout, timedOut) and drops stderr; doctor.go:238-247 returns "empty" for exitCode == 0 and blank stdout. Other end: sdk/python/agentfield/harness/providers/opencode.py:325-336 checks result_text is None, nonempty clean_stderr, and _OPENCODE_STDERR_ERROR_PATTERNS, with the comment opencode sometimes exits 0 even on hard failures like "Model not found", then sets is_error = True and failure_type = FailureType.CRASH.

💡 Suggested Fix

Pass stderr into classification and, before returning empty for a zero exit with blank stdout, recognize OpenCode's known stderr error patterns (or use a provider-specific probe/result parser) and classify that outcome as error.

---

Consistency Verifier · confidence 99%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The described race from stale logs can cause a spurious retry but does not concretely break builds, security, data integrity, or a public contract; safe to fix later.

Record the log offset before Start and classify only output written by the current child. Also match the attempted port in the strict-port message.

The conflict classifier reads the last 40 lines of an append-only log without an attempt boundary, so a stale "assigned port ... unavailable" line from a previous run can trigger a retry for an unrelated failure (e.g., import error, health-check timeout). This burns the sole retry and may launch the node on a different port despite the current child never reporting a bind conflict.

Evidence

Step 1: The real process manager opens config.LogFile with os.O_CREATE|os.O_WRONLY|os.O_APPEND in internal/infrastructure/process/manager.go:46-61, preserving output from prior runs.
Step 2: A prior failed run can therefore leave "assigned port ... unavailable" in the final 40 lines.
Step 3: On any current readiness failure, attemptStart reads those final 40 lines at agent_service.go:174-176; it does not record a pre-start offset or otherwise limit the read to current-process output.
Step 4: logIndicatesPortConflict returns true for any line containing the two substrings (:255-261), even when it names another port.
Step 5: startWithPortRetry receives conflict=true and executes a fresh-port retry (:190-203) even if the current failure was, for example, an import error or a non-port health-check failure.

💡 Suggested Fix

Capture the log file size immediately before Start and inspect only bytes appended after that offset (or use a fresh per-attempt log). Require the detected strict-port message to identify the attempted port. Add the requested stale-line-plus-non-port-readiness-failure test.

---

Startup retry behavioral contract · confidence 98%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: Incomplete warning is a UX issue, not a security vulnerability, data loss, or contract break.

Fix the stop guard to check total and handle pagination so operators see all running executions before confirming.

_{🟠 IMPORTANT · confidence 98%}

The CLI requests only 100 executions and ignores total. If there are more than 100, the warning/prompt is incomplete, and an operator may approve the stop without realizing all running executions will be interrupted.

Evidence

Changed CLI (control-plane/internal/cli/stop.go:316-320,345-349): "limit": 100, then json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&parsed) and return parsed.Data.Results, nil; it does not inspect parsed.OK or parsed.Data.Total. Serializer (control-plane/internal/handlers/agentic/helpers.go:37-48) sends successful replies as AgenticResponse{OK: true, Data: data} with HTTP 200, so the success envelope matches. But executions query (control-plane/internal/handlers/agentic/query.go:134-150) sets total := len(execs), then truncates with if len(execs) > req.Limit { execs = execs[:req.Limit] }, and returns both "results": execs and "total": total. Thus total can exceed 100 while the CLI only returns the first 100 results.

💡 Suggested Fix

After decoding, reject !parsed.OK and either page through executions until all total results are collected or treat parsed.Data.Total > len(parsed.Data.Results) as an incomplete check and warn/proceed according to the existing best-effort failure path.

---

Consistency Verifier · confidence 98%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The issue requires an unusual embedding pattern and does not demonstrate a concrete production impact; it is a code quality improvement.

Fix the --force flag to not persist across command instances so ordinary stop prompts are never bypassed.

Because every command shares the same package-level stopForce variable, executing a forced stop sets it true, and a later ordinary stop reads that value instead of its default, skipping the execution guard at stop.go:361-364.

_{🔵 SUGGESTION · confidence 95%}

Evidence

Step 1: NewStopCommand binds every command instance's --force flag to the single package variable stopForce (stop.go:21-24, 51). Step 2: an embedding can construct forced := NewStopCommand() and ordinary := NewStopCommand(), then execute forced with --force; Cobra/pflag writes true through the shared pointer. Step 3: executing ordinary without --force does not parse a value that resets that shared pointer, and runStopCommand reads it into AgentNodeStopper.Force (stop.go:55-66). Step 4: confirmStopWithRunningExecutions returns immediately when Force is true (stop.go:361-364), so the ordinary invocation neither queries nor warns before the verified node is stopped. resetLifecycleFlags also resets stopJSON but omits stopForce (node_json_test.go:18-26), leaving this lifecycle behavior untested.

💡 Suggested Fix

Make force and json command-instance state (for example, local variables captured by the command's RunE), or reset/read flags from cmd.Flags() per execution. Add a lifecycle test that constructs both commands before executing the forced one, then verifies the ordinary command queries/prompts; also reset stopForce in the shared test helper if globals remain.

---

Stop execution warning correctness · confidence 95%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The flag leak requires programmatic reuse of command instances, not standard CLI usage; no concrete production impact.

Make the force flag local to each command instance to prevent state leaking between invocations.

_{🔵 SUGGESTION · confidence 95%}

When NewStopCommand binds the force flag to a package-level variable, reusing the command or constructing multiple command trees before execution causes a later unforced stop to inherit --force from a previous invocation. This silently bypasses the running-execution query and confirmation.

Evidence

Step 1: An embedding can call cmd := NewStopCommand(), execute it with cmd.SetArgs([]string{"node-a", "--force"}), then call cmd.SetArgs([]string{"node-b"}) and execute the same command again (or create two NewRootCommand/NewStopCommand values before executing the first).
Step 2: NewStopCommand binds every command's force flag to the single package variable via cmd.Flags().BoolVar(&stopForce, "force", false, ...).
Step 3: The first parse sets stopForce to true; pflag's Parse only calls Set for flags present in the new argument list, so the second invocation's absent --force leaves that value true. With two preconstructed command trees, the second tree's flag also points at the same variable.
Step 4: runStopCommand copies stopForce into AgentNodeStopper.Force, and confirmStopWithRunningExecutions immediately returns when Force is true. Therefore the unforced second stop skips both the control-plane query and its safety warning/prompt.

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The finding describes a race condition that may prevent retry on a different port, but does not demonstrate a concrete production impact such as data loss, security vulnerability, or broken API contract.

freshRetryPort doesn’t exclude the failed port when reservation fails, so the retry can pick the same port and bail out

_{🔵 SUGGESTION · confidence 94%}

freshRetryPort calls ReservePort(failedPort) but ignores the error and immediately scans from 8001 via FindFreePort. If the port was system-busy at reservation time but frees up before the scan, FindFreePort returns it—there’s no reservation and no listener. startWithPortRetry then sees retryPort == port and returns the original error without trying any other port, even when one is available.

The startup tests only cover the case where ReservePort succeeds; they inject the same return value but never exercise the production path where reservation fails and the port later becomes free during FindFreePort.

Evidence

Step 1: RunAgent calls runAgentGuarded, which invokes startWithPortRetry(port, ...) at agent_service.go:112 after attemptStart reports a strict-port conflict. Step 2: startWithPortRetry calls freshRetryPort(failedPort) at line 195. Step 3: freshRetryPort ignores ReservePort(failedPort) at line 209 and immediately calls FindFreePort(8001). Step 4: the PortManager contract permits ReservePort to return an error when a port is in use, and DefaultPortManager.ReservePort does exactly that before adding nothing to reservedPorts (port_manager.go:69-76). Step 5: if that system-busy port becomes available before FindFreePort scans it, FindFreePort sees neither a reservation nor a busy listener and returns it (port_manager.go:34-36). Step 6: startWithPortRetry detects retryPort == port and returns the original error at lines 196-199, without searching for any distinct available port. The only production constructors use process.NewPortManager, and no other production caller invokes freshRetryPort, so this concrete behavior applies to the runtime path.

💡 Suggested Fix

Make exclusion independent of a successful reservation—for example, add an exclusion-aware port-manager operation that skips failedPort while scanning, and use it here. Add a startup test where ReservePort(failedPort) returns the concrete system-busy error and the subsequent scan would otherwise return failedPort, while a distinct port is available.

---

Runtime contract of strict-port retry · confidence 94%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The finding describes a UX/observability improvement for automation consumers, not a security vulnerability, data loss, API break, or regression.

Control-plane authentication and response failures are mislabeled as unreachable

_{🔵 SUGGESTION · confidence 99%}

Return a typed outcome from fetchControlPlaneNodes so af list exposes authentication and response failures separately from unreachable errors.

Currently, resolveNodeHealth receives only a boolean reachability flag, which bundles 401/403, 5xx responses, malformed JSON, and empty/invalid URLs into unknown (control plane unreachable). This prevents automation from distinguishing configuration errors (credentials, URL) from a transient network outage, despite the control plane having responded in those cases.

Evidence

Step 1: both runListCommandJSON (line 72) and runListCommand (line 261) call resolveNodeHealth. Step 2: resolveNodeHealth receives only the boolean returned by fetchControlPlaneNodes (line 118) and passes it to reconcileHealth (line 128). Step 3: fetchControlPlaneNodes returns reachable=false for a non-200 HTTP response (lines 204-211) and JSON decoding failure (lines 213-217), exactly as it does for transport errors (lines 204-206) and an empty URL (lines 190-193). Step 4: reconcileHealth maps every cpReachable=false case to the literal unknown (control plane unreachable) (lines 138-141), which is serialized as JSON health at lines 78-85 without any separate outcome field.

💡 Suggested Fix

Return a typed reconciliation/fetch outcome (for example unreachable, unauthorized, http_error, and invalid_response) in addition to the nodes. Keep af list non-failing and health unknown, but render/serialize a distinct reason (ideally a stable JSON status/reason field) so consumers can act on authentication and response failures correctly. Add 401/403, 5xx, and malformed-body cases to list_health_test.go.

---

Machine-consumer semantics · confidence 99%

_{🤖 Reviewed by AgentField PR-AF}

[AbirAbbas]

Note

Advisory — non-blocking. Safe to merge and address in a follow-up.

Why non-blocking: The leaked reservation only affects in-memory state for the lifetime of a single control-plane process and does not cause data loss, security issues, or break a public contract.

Failed-port reservation leaks into future runs

_{⚪ NITPICK · confidence 82%}

Release the reserved failed port after freshRetryPort selects a retry port.

freshRetryPort reserves the failed port at agent_service.go:209 but never calls ReleasePort. A successful ReservePort marks the port in the in-memory map (port_manager.go:67-78), which FindFreePort rejects (port_manager.go:32-37). This makes the port permanently unavailable for future allocations until the process restarts, even after it becomes system-free. Release the reservation (including on error paths) once FindFreePort returns, and add a test that a later independent run can allocate the previously failed port.

Evidence

Step 1: A strict-port failure reaches startWithPortRetry, which calls freshRetryPort (agent_service.go:195).
Step 2: freshRetryPort calls ReservePort(failedPort) at agent_service.go:209 and has no corresponding ReleasePort call.
Step 3: Under the real implementation, a successful ReservePort sets pm.reservedPorts[port] = true (internal/infrastructure/process/port_manager.go:67-78).
Step 4: Every future FindFreePort rejects any port present in that map (port_manager.go:32-37).
Step 5: Thus, if the failed port becomes system-free, subsequent af run calls still cannot allocate it until process restart, despite no node owning it.

---

Startup retry behavioral contract · confidence 82%

_{🤖 Reviewed by AgentField PR-AF}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ AbirAbbas
❌ swe-AF
_{You have signed the CLA already but the status is still pending? Let us recheck it.}