fix: a sweep honours --ttl/--idle-timeout/--cost-limit instead of logging them (#525)

[scttfrdmn]

Closes #525.

Stacked on #528 (fix/524-estimate-only-sweep). Review that one first; this branch's diff against main will include it until #528 merges.

The bug

buildLaunchConfigFromParams starts from an empty aws.LaunchConfig, and the sweep dispatch copies only Region/InstanceType/Name off the base config. So --ttl, --idle-timeout and --cost-limit were parsed onto a config nothing ever read: every row launched with no TTL and no cost limit.

The param-file route did not work either — cost_limit: had no case in the buildLaunchConfigFromParams switch, so it fell through to the unknown-key arm and became a PARAM_cost_limit env var and a spawn:param:cost_limit tag. A spend control that parsed, launched, tagged, and capped nothing.

Both routes failed, which left a sweep with no per-instance dollar cap at all.

The worst part is not the drop, it is the log line. Pre-fix stderr on --ttl 1h --cost-limit 8:

⚠️  WARNING: --no-detach specified
   Using safeguards: ttl=1h, idle-timeout=

⚠️  Auto-setting --idle-timeout=1h for all sweep instances

It affirmatively reports the safeguard as active at the instant it is discarded, and then substitutes an idle timeout in its place — which never fires on a compute-bound row while still displaying as a bound. A user reading that output has every reason to believe the sweep is capped.

The fix

Defaults is the one seam both orchestration paths read: the foreground path merges it per row, and the detached path uploads it to S3 for the Lambda orchestrator. So the CLI flags are folded into paramFormat.Defaults right after the param file is parsed, before anything else looks at it:

appliedControls := applyCLISpendControlsToSweep(paramFormat)

Precedence is most-specific-wins: a row's own params: > the CLI flag > the file's defaults:. That is what lets a matrix put one expensive GPU row on a shorter leash than the sweep it belongs to.

cost_limit: now has a real parser case (parseCostLimit), accepting every numeric shape YAML/JSON/CSV can produce — 8, 8.50, "8.50" — and erroring on anything else rather than leaving the field at 0. Unlike the string cases around it, a value it cannot read fails the launch: a mistyped ttl costs you the difference between two timeouts, a mistyped cost limit silently removes the only per-instance dollar cap on the path. Disabling the cap has to be spelled cost_limit: 0.

Two knock-on fixes fall out of injecting before the other consumers of Defaults:

1. --estimate-only now prices the TTL you passed. EstimateSweepCost derives per-row hours from row ttl → defaults["ttl"] → a "1h" fallback, so --ttl 4h on a sweep was previously estimated at 1h — a 4× understatement on the command whose entire job is to tell you the number before you spend it.
2. The --no-detach zombie guard is per row. It used to read the CLI ttl variable, so a param file carrying its own ttl: was refused even though that value is the one that actually reached the instances — the only way through was to pass --ttl to satisfy the guard and have its value discarded. One flag to pass the check, another to take effect: the mirror image of this bug. The guard now checks each row's merged bound and names the unbounded rows, because "some row is unbounded" is not actionable on a 30-row sweep.

The safeguards log now prints the merged sweep defaults and says where they came from, instead of echoing variables it is about to drop.

Tests

Regression coverage is on EC2 tags, not on any in-process value, because the tags are what spored and the out-of-band reaper actually enforce. A test asserting "the flag was parsed" would have passed throughout the bug's life — the flag was parsed.

test/e2e/tier0_sweep_spend_controls_test.go (Tier 0, Substrate, no AWS account, zero spend), 7 tests:

test	pins
SweepAppliesCLISpendControls	spawn:ttl=1h + spawn:cost-limit=3.0000 on every row; no idle timeout substituted behind an explicit --ttl
SweepRowTTLBeatsCLI	row ttl: outranks --ttl
SweepCLIBeatsFileDefaults	--ttl outranks the file's defaults:
SweepParamFileCostLimit	cost_limit: alone works, and no longer leaks to spawn:param:cost_limit
SweepFileTTLSatisfiesNoDetach	a file-provided bound satisfies the guard, and reaches the instance
SweepNoDetachNamesUnboundedRows	partly-bounded file refused, refusal names c5.xlarge, does not name the bounded row
SweepRejectsUnusableCostLimit	cost_limit: eight dollars fails the launch, message names cost_limit, nothing launched

Plus unit tests in cmd/sweep_spend_controls_test.go for the precedence table, sweepRowsWithoutBound, and parseCostLimit.

All 7 fail against the pre-fix tree (verified in a worktree at da8e0dd) and pass here. The last one is worth calling out: as first written it passed pre-fix, because the old CLI---ttl guard rejected the file before anything looked at cost_limit, so the non-zero exit proved nothing. It now passes --ttl on the command line and asserts the message names cost_limit. A check that cannot fail is worse than no check.

Caveat, stated rather than implied

All the e2e tests take the foreground path (--no-detach), which is the one that provisions in-process and is therefore the one Tier 0 can observe end to end. The detached path is covered at the same seam by construction — both paths read the same defaults: map — but the sweep-orchestrator Lambda does not live in this repo, so its half of the fix is not verifiable here. If it reads ttl/cost_limit out of the uploaded defaults it now gets them; that needs confirming wherever that function is deployed.

Cross-refs: #524 / #528 (same launch path, same "the flag lies" shape), #526 (unknown param-file keys become PARAM_* silently — the general form of the cost_limit: failure), #527.

[codecov]

Codecov Report

❌ Patch coverage is 81.08108% with 14 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/launch_sweep.go	72.09%	12 Missing ⚠️
cmd/sweep.go	93.54%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!