fix: idleTimeout counts as a bound, costLimit alone warns (#55) - #62
Merged
Conversation
The unbounded-launch guard accepted the weakest of the three bounds and refused a stronger one: bound Go accepts spawn-ts accepted --ttl yes yes --idle-timeout YES NO --cost-limit no YES Both halves were backwards. costLimit is a SOFT limit: spored polls accumulated compute-seconds against spawn:cost-limit, so if spored never starts (failed bootstrap, wrong instance profile, crash-loop) nothing enforces it. Only the TTL is enforced from outside the box — the ttl-reaper Lambda reads spawn:ttl-deadline without the instance's cooperation (lambda/ttl-reaper/main.go:688), which is why types.ts calls it "the hard cost backstop". Worse, findOrphans skips any instance whose deadline is 0 (src/core/orphans.ts:46), so a cost-limit-only instance is invisible to orphan detection too. The guard was waving through exactly the launches nothing downstream can catch, and saying nothing while it did — a guard that passes must not imply a guarantee it isn't making. New src/core/bounds.ts holds one pure predicate, with the enforcement distinction (external vs on-instance) as the type that was missing. idleTimeout now counts; costLimit alone still permits the launch — refusing it would be a new, harsher divergence from Go — but emits a warning naming the consequence and the orphan-detection blind spot. Both launch paths now share it. The CLI's `launch` calls provider.launch directly rather than going through SpawnClient, so its check was not a friendlier restatement of the client's, it was an independent second implementation of a cost-safety predicate. Two copies drift, and the drift is silent because the lenient copy is the one that lets a launch through. Also adopts Go's acknowledgement step: --no-timeout now requires a confirm (or --yes), matching cmd/zombie_guard.go:58 — "disabling the cost guardrails is an explicit, acknowledged choice". A flag alone can be a typo or a copied command line. Fixes a latent parser bug found while testing that: --no-timeout was not in BOOLEAN_FLAGS, so `launch --no-timeout job` consumed "job" as the flag's VALUE (parseArgs takes the next non-dash token for an unknown flag). The instance name vanished and flagBool read false. It failed safe — the guard refused rather than allowing an unbounded launch — but the flag was silently inert in that word order. The deliberate divergence from Go is documented rather than removed: Go applies a 1h idle default and proceeds, spawn-ts refuses. Refusing is right for a browser (nothing launches, so nothing bills) — it just shouldn't read as the same guard. Tests: 420 passed. The 6 new assertions covering the inversion were proven to fail against the old condition before being kept; the existing guard test only exercised the mock path, which is why the inversion was never asserted either way. Part of #57.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #55. Part of #57.
The inversion
The guard accepted the weakest of the three bounds and refused a stronger one:
--ttl--idle-timeout--cost-limitBoth halves were backwards, and the
costLimithalf is the one that bills money.costLimitis a soft limit: spored polls accumulated compute-seconds againstspawn:cost-limit. If spored never starts — failed bootstrap, wrong instance profile, crash-loop — nothing enforces it. Only the TTL is enforced from outside the instance: the ttl-reaper Lambda readsspawn:ttl-deadlinewithout the box's cooperation (lambda/ttl-reaper/main.go:688, "the authoritative, launch-anchored deadline"), which is whytypes.tscalls TTL "the hard cost backstop".The compounding part, which the issue didn't have:
findOrphansskips any instance whose deadline is0(src/core/orphans.ts:46). So a cost-limit-only instance isn't merely softly bounded — it's invisible to orphan detection as well. The guard was waving through precisely the launches nothing downstream can catch, and saying nothing while it did.idleTimeout, meanwhile, was refused although it's the bound Go itself auto-applies as its 1h default (cmd/zombie_guard.go:20, triggerTTL == "" && IdleTimeout == "").The fix
New
src/core/bounds.ts— one pure predicate, with the distinction that was missing as an actual type:idleTimeoutcounts as a bound (matching Go).costLimitalone still permits the launch — refusing it would be a new, harsher divergence from Go — but returns awarnnaming the consequence and the orphan blind spot. Silence would read as "bounded", which is the wrong answer: the fix(extend): add the safety floor, write both TTL tags, nudge spored (#54) #63 invariant applied to a safety check.Both launch paths now share it
A correction to the issue's premise, which said the CLI "duplicates the check only to produce a friendlier message before the throw": the CLI's
launchcallsprovider.launchdirectly (src/cli/commands.ts:211), notclient.launch. So its check wasn't a friendlier restatement — it was an independent second implementation of a cost-safety predicate, and the two could drift apart indefinitely. The drift is silent because the lenient copy is the one that lets a launch through. Both now callevaluateBounds.Go's acknowledgement step, adopted
--no-timeoutnow requires a confirm (or--yes), matchingcmd/zombie_guard.go:58— "disabling the cost guardrails is an explicit, acknowledged choice". A flag on its own can be a typo or a copy-pasted command line; a confirmation can't.Latent parser bug, found while testing that
no-timeoutwas not inBOOLEAN_FLAGS, soparseArgstook the next non-dash token as its value:The instance name vanished and
flagBool()readfalse. It failed safe — the guard refused rather than allowing an unbounded launch — but the flag was silently inert in that word order. Fixed, with a regression test.The divergence from Go is documented, not removed
Go applies a 1h idle default and proceeds; spawn-ts refuses. Refusing is the better choice in a browser — nothing launches, so nothing bills, and there's no daemon here to fall back on. It just shouldn't read as the same guard, so
evaluateBoundssays so.Verification
The new assertions were proven to fail against the old condition before being kept — restoring the inverted logic fails 6 of them (4 in
bounds.test.ts, 2 inclient.test.ts). Worth noting why nothing failed on the first run: the existing guard test (client.test.ts:55) only exercised the mock path, where the guard doesn't engage, so the inversion was never asserted in either direction.Unrelated flake seen once during a full-suite run and filed separately as #61 (
ssm/session.test.tswaits on a fixed 5 mssetTimeout; reproduced on unmodifiedmain, file untouched here).