fix: reject param-file keys that look like spawn settings (#526)

[scttfrdmn]

Closes #526.

Stacked on #529 (which is stacked on #528). Merge order: #528 → #529 → this.

The bug

cmd/sweep.go's default: arm sent every unrecognised param-file key to config.Parameters, i.e. a spawn:param:<key> tag and a PARAM_<key> env var. That passthrough is the feature — it is how a sweep hands values to a workload — but it also meant the parser could never say "that isn't a thing". A key the user meant as a spawn setting and a key they meant for their own program were indistinguishable:

written	believed	actual
ttl_hours: 4	a 4h TTL	PARAM_ttl_hours=4, no TTL
on-complete: terminate	terminate on done	PARAM_on-complete, instance never terminates
budget: 50	a spend cap	PARAM_budget=50, nothing
max_concurrent: 3	a concurrency cap	PARAM_max_concurrent=3, nothing (CLI-only)

The first two leave a running instance. That is the version of this that costs money.

The fix

Passthrough is unchanged for real parameters. A key is rejected — before anything is launched or priced — under three rules, checked in this order:

B. A recognised key, misspelled. Hyphens or wrong case: on-complete, instance-type, cost-limit, TTL, Instance_Type. The error names the exact spelling.

C. A curated denylist of CLI-only flags and near-misses: ttl_hours, walltime, max_runtime, max_cost, budget, max_concurrent, launch_delay, instance_types, image_id, spot_price, … Each entry carries the correct spelling, or says the control is CLI-only.

A. Not a valid shell identifier. This one is mechanical rather than a matter of taste, and pkg/launcher/bootstrap.go:534 is why:

echo "export PARAM_${param_name}=\"${value}\"" >> /etc/profile.d/spawn-params.sh

export PARAM_on-complete="terminate" is a "not a valid identifier" error in every login shell. A hyphenated key was never going to reach the workload no matter what it meant.

All offending keys are reported in one error rather than one per run — a 30-row sweep with three typos should be one round trip. The same check also runs at the buildLaunchConfigFromParams seam, which spawn resume and the quota preflight reach without going through launchParameterSweep, so resume cannot rebuild an instance from a file the launch path refused.

Option (3) from the issue is in too: the PARAM_* variables a sweep will set are listed in the sweep header, so an unintended passthrough has somewhere to be noticed.

The escape hatch, and why the denylist would otherwise be a bug

A reserved name with no way through is not a fix, it is a hard block on a legitimate parameter. budget is an optimiser's evaluation budget as often as it is a dollar figure; time_limit is a standard solver option (Gurobi, CPLEX). Rejecting those with no alternative would break the feature this check exists to protect.

So param:<name>: passes any name straight through as PARAM_<name>, and every rejection message names it:

"budget" is not a spawn setting: spawn has no per-sweep budget key; bound each row
with ttl: (worst case) and cost_limit: (spend cap). If you really do mean a parameter
for your workload, write param:budget and it will be passed through as PARAM_budget

Rule A still applies to whatever follows the prefix, since that is what becomes the variable name.

What is deliberately not on the denylist

timeout is the load-bearing example. The first draft rejected it and TestBuildLaunchConfigFromParams_WorkflowStep failed — correctly: examples/workflow-ci-pipeline.yaml sets timeout: 10m per step and pkg/queue.JobConfig has a Timeout field. It is documented vocabulary, not a near-miss.

Also excluded, as plausible sweep parameters: image (a container image), type, count, steps (next to spawn's step), instance (an optimisation problem instance), runtime (a container runtime), idle, instances. When in doubt the key passes through. A denylist that swallows legitimate parameters is a worse bug than the one it fixes, and TestClassifyRowKeyAllowsWorkloadParams pins all of them.

Tests

cmd/sweep_keys_test.go:

• TestRecognizedRowKeysMatchesSwitch — the registry of recognised keys is a hand-written duplicate of the switch's case labels, so this test parses cmd/sweep.go with go/ast, extracts the labels, and requires equality in both directions. A missing map entry means a real spawn key gets treated as a near-miss and a working file breaks; a missing case label means a key spawn ignores is advertised as recognised, which is this bug wearing a different hat. It also fails if the AST walk finds zero labels, because a broken walk would make every other assertion in it pass vacuously.
• TestClassifyRowKeyRejects — 19 cases from the issue's table, asserting on the guidance text, not just that an error happened.
• TestClassifyRowKeyAllowsWorkloadParams — 25 legitimate parameter names, including the ambiguous ones above.
• TestNoRecognizedKeyIsAlsoReserved / TestReservedKeysAreNormalized — two ways a denylist entry could become unreachable dead guidance that still reads as active.
• Escape-hatch tests, plus TestValidateSweepParamKeysReportsEveryProblem (all three typos named, the legitimate alpha row not named).

test/e2e/tier0_sweep_param_keys_test.go (Substrate, zero spend), 4 tests: ttl_hours: rejected with nothing launched, on-complete: rejected with nothing launched, workload params + an escaped param:budget reaching the instance as spawn:param:* tags, and the header listing PARAM_isa.

The rejection tests pass a real --ttl on the command line so the launch would otherwise succeed — without it a non-zero exit would prove only that the sweep was unbounded, not that the key was caught. All 4 verified failing at 7f13678 and passing here.

Cross-refs: #524/#528, #525/#529 — same launch path, same shape. Two further findings from this work are filed separately: #530 (unknown keys at the top level are dropped entirely, including in spawn's own examples) and #531 (param values are interpolated unquoted into /etc/profile.d, so a $ or a quote silently changes what the workload sees). This PR fixes the key side of that export line; #531 is the value side.

[codecov]

Codecov Report

❌ Patch coverage is 73.11828% with 25 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/sweep_keys.go	75.29%	21 Missing ⚠️
cmd/launch_sweep.go	0.00%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!