Conversation
Contributor
|
I tested locally using the example from the README and then creating a new Nexfile. I tested updating and restarting via the CLI. Oddly, there are logs from the first Nexfile and the updated Nexfile intermingled: ❯ nats sub '$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout'
14:43:41 Subscribing on $NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout
[#1] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
[2026-08-19 14:43:58] Uptime: 14:43 up 27 days, 22:32, 5 users, load averages: 7.12 6.40 6.40
[#2] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
[2026-08-19 14:44:28] Uptime: 14:44 up 27 days, 22:32, 5 users, load averages: 6.00 6.20 6.33
[#3] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
i have been updated
[#4] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
i have been updated
[#5] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
[2026-08-19 14:44:58] Uptime: 14:44 up 27 days, 22:33, 5 users, load averages: 5.43 6.05 6.27
[#6] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
i have been updated
[#7] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
i have been updated
[#8] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
i have been updated
[#9] Received on "$NEX.FEED.system.logs.yJ08N4tBGcb9Vwtx8Wih3Y.stdout"
i have been updated |
Contributor
❯ ./nex -s nats://localhost:4222 workload list
No workloads foundbut I still see logs: The workloads appear to be orphaned: ❯ ps -a | grep sleep
81157 ttys003 0:00.07 /bin/sh -c while true; do echo "[$(date '+%Y-%m-%d %H:%M:%S')] Uptime: $(uptime)"; sleep 30; done
83841 ttys003 0:00.10 /bin/sh -c while true; do echo "i have been updated"; sleep 5; done |
Contributor
Author
|
thanks @joeriddles . Working on it. |
Contributor
Author
|
Great catch — reproduced and root-caused. You found a real dual-writer on the native-nexlet path, which our test matrix (inmem + podman) never exercised. Three composing defects, two of them pre-existing in
Fixed in
Two known residuals, tracked in our tracker: a workload that forks still orphans grandchildren on stop ( |
Contributor
|
Stopping is still not working for me but sounds like that is a separate issue. |
Contributor
|
I updated a workload but then was unable to restart it: ❯ ./nex -s nats://localhost:4222 workload update bRd7hjIhWxUvIBTMF4Ew29 -f Nexfile.updated
Workload bRd7hjIhWxUvIBTMF4Ew29 successfully updated
❯ ./nex -s nats://localhost:4222 workload restart bRd7hjIhWxUvIBTMF4Ew29
error: workload not found |
Jarema
added a commit
that referenced
this pull request
Aug 20, 2026
Three node-side defects found by running the CLI against a live node and the native nexlet (PR #523 field report), each pinned by a new test: - The stop-confirmation RequestMany inherited the NATS connection's 2s default request timeout (the node context has no deadline), while a nexlet's synchronous stop is legitimately slower (native: 5s grace + SIGKILL + 750ms confirm). UNDEPLOY replied Stopped:false and UPDATE aborted 'stop unconfirmed' while the dispatched stop killed the workload anyway: destroyed workload, no replacement, RESTART then 'workload not found'. Both waits now carry an explicit 15s budget; the inmem test agent gains StopDelay to pin it. - RESTART replied 'nothing to restart' whenever no record was stored -- which is every workload on a node without --state (the default). With nothing on file it now replays the live definition the ownership fetch already returned; the foreign-namespace-record case still refuses, since store-first would clobber the other namespace's record. - Failure messages promised 'stored definition will apply on next agent registration' even on stateless nodes where nothing was stored, and claimed nothing was torn down when the dispatched stop may still land. Messages now state the stop may complete and, on stateless nodes, that the update is lost and needs a redeploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Stopping and updating look to be working: Screen.Recording.2026-08-26.at.9.36.10.AM.movTesting with KV-backed state Screen.Recording.2026-08-26.at.9.38.36.AM.mov |
Contributor
|
Restarting with KV looks good Screen.Recording.2026-08-26.at.9.40.25.AM.movWithout KV: Screen.Recording.2026-08-26.at.9.41.19.AM.mov |
joeriddles
previously approved these changes
Aug 26, 2026
joeriddles
left a comment
joeriddles
left a comment
Contributor
There was a problem hiding this comment.
I focused on reviewing at a high-level and running it locally, with and without KV-backed state. Claude's comments are overwhelming.
Something that would be good to improve is what happens if you update a workload and it fails. If the stop succeeds but the update fails, e.g. a workload config that passes the schema validation but fails for some other reason, you cannot re-try updating the workload since it's now gone.
I tested manually and also let Claude test. This comment it has about the reaper is interesting:
2. The reaper is a no-op for any workload that writes to stdout (worth knowing, probably fine).
Not a bug in the reaper — the code is correct and I verified it fires and kills. But its practical reach is narrower than it
looks. Workload stdout goes through a pipe the node owns, so on a hard node crash most workloads die of SIGPIPE on their next
write before the reaper ever sees them. I had to build a deliberately silent workload to produce an orphan at all.
That's arguably fine — SIGPIPE does the job for free in the common case, and the reaper covers the rest. But it means the
crash-orphan path is thinly exercised in practice, so bugs there would stay hidden. If you want it genuinely covered, a test
with a silent workload is the one that matters; a noisy one passes for the wrong reason, which is exactly the trap I fell
into twice.
joeriddles
approved these changes
Aug 26, 2026
handleStopWorkload initialized ret={Stopped:false, WorkloadType:""} and, on
the fallthrough path, unconditionally called
state.RemoveWorkload(ret.WorkloadType, workloadID). When RequestMany errored
it returned before purging at all; when agents answered but none confirmed
the stop, it purged key "_<id>" (zero-value WorkloadType) -- a silent no-op
against a nonexistent key, since KV Purge of a missing key succeeds. Either
way the real record ("<type>_<id>") survived, and resume-on-registration
would resurrect a workload the caller explicitly stopped.
Semantics decision: an unconfirmed stop deliberately keeps the record --
truthful, because the workload may still be running -- and the response's
Stopped:false is the caller's signal to retry, not to fire-and-forget.
Purge only runs when a stop was actually confirmed (ret.Stopped == true).
Test scaffolding added along the way:
- _test/nexlet_inmem/inmemagent.go: FailStops bool field, checked at the top
of StopWorkload, to simulate a nexlet that cannot confirm a stop. Exposed
via a new NewInMemAgentWithHandle constructor (NewInMemAgent's existing
signature only returns the opaque *agent.Runner, with no way for a test to
reach back into the concrete agent after wiring).
- _test/helpers.go: StartNexus is now a thin wrapper around a new
StartNexusWithOptions, which additionally accepts extraOpts
([]nex.NexNodeOption) applied after the existing state/runner wiring.
Needed to inject a state-recording decorator via nex.WithState without
touching StartNexus's signature (15+ existing call sites).
- node_undeploy_state_test.go: TestNodeUndeployUnconfirmedKeepsState, the
first test in the repo to pass state=true to StartNexus, exercising its
real NATS-KV-backed state wiring end to end. It is an external test
package (nex_test), not node_test.go's internal one (package nex): an
internal test file importing the _test helper package -- which itself
imports the root nex package -- is an import cycle disallowed for test
files. Being external means the decorator is injected through the
exported nex.WithState option rather than by reaching into NexNode's
unexported state field directly.
The load-bearing assertion is on a recordingState decorator's captured
RemoveWorkload calls (none on unconfirmed stop, exactly one -- with the
correct type and id -- on confirmed stop), not on KV contents alone: KV
Purge of a missing key succeeds silently, which is exactly how the old
wrong-key purge went unnoticed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Stamp the minted workload's public NATS user nkey into the stored
StartWorkloadRequest's Metadata (key "nex_minted_nkey" -- a cross-repo
contract name, do not rename) before StoreWorkload, on both the deploy
path (handleAuctionDeployWorkload) and the resume-on-registration path
(handleRegisterAgent). The node already mints a distinct credential per
workload but previously discarded the nkey, making future credential
fencing (revocation) unbuildable: nothing recorded which nkey belongs to
which workload.
On resume, creds are re-minted per persisted record, so the record is
re-stamped and re-StoreWorkload'd so the persisted nkey tracks the live
credential rather than going stale.
The persisted value is the PUBLIC nkey, not a secret -- the seed is the
secret half, and it is never persisted here or elsewhere.
Also fixes internal/credentials/signing_key.go: SigningKeyMinter.Mint
computed the user keypair's public key (pubKp) for the JWT claims but
never wrote it into NatsConnectionData.NatsUserNkey, so the field this
task persists was silently always empty in production. Populate it
directly; nothing else changes about the minted JWT/seed.
models/schema/start-workload-request.json's "metadata" property declares
additionalProperties:false, but this is inert: the file is only consumed
by `task gen-schema` for codegen, no runtime path validates a
StartWorkloadRequest against it, and the generated Go type
(StartWorkloadRequestMetadata = map[string]interface{}) is already
free-form and accepts arbitrary keys via plain json.Unmarshal. No schema
change needed; verified via the new test's KV round-trip.
Test: node_metadata_nkey_test.go (external nex_test package, following
node_undeploy_state_test.go's StartNexusWithOptions/state=true pattern).
Uses a real credentials.SigningKeyMinter injected via nex.WithMinter,
since the harness's default TestMinter stub never populates
NatsUserNkey and would make the nkey-validity assertions vacuous. Covers
both deploy (KV record carries a valid nkey) and resume (re-registering
the "inmem" agent type re-mints, and the KV record's nkey changes to
match).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Review finding on the prior commit (persist minted workload nkey in state metadata): both the deploy path and the resume-on-registration path already swallow a StoreWorkload failure at Warn and continue, so the live credential and the persisted "nex_minted_nkey" record can silently diverge. Since that field exists to enable credential fencing (revocation), a stale or missing persisted record means a future revocation would target the wrong identity (resume path) or have nothing to revoke against at all (deploy path) -- silently. Scoped fix, no retries, no semantic changes: upgrade both log lines to Error and name the consequence explicitly, plus a one-line comment at each site stating the constraint. Deploy path does not fail the deploy on this error (the caller's response was already sent beforehand) and still returns after logging, same as before; resume path still continues to hand the agent the fresh credential regardless, same as before -- only the log level and message changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Wire-level groundwork for the UPDATE control verb. UpdateWorkloadRequest
is {namespace, start_request}, where start_request $refs the existing
start-workload-request.json rather than restating it -- the same
composition clone-workload-response.json and
shared-agent-start-workload-request.json already use, so the replacement
definition is literally the same shape a deploy takes.
UpdateWorkloadResponse is {id, updated, message}. updated:false is a
first-class outcome rather than an error: the verb persists the new
definition before it touches the running instance, so a replacement that
could not be completed still has a stored definition that the next agent
registration will apply. The message field is what tells the caller
which of those happened.
Subjects follow the UNDEPLOY pair exactly: a request subject
$NEX.SVC.<ns>.control.UPDATE.<workloadId> and the wildcard subscribe
form for the node's endpoint.
Generated models/api_control.go is regenerated from the schemas via
task gen-schema (Taskfile gains the two --schema-output mappings); it is
never hand-edited.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Nex had no update verb, so callers faked one by starting a replacement
workload and then best-effort-stopping the old one. That leaves an
unbounded window in which both instances are live: both publish, both
advance the same checkpoints, and both bind the same durable consumer,
which silently splits the stream between them. UPDATE closes the window
by composing the existing agent STOPWORKLOAD + STARTWORKLOAD operations
node-side, in an order that has no dual-writer state.
handleUpdateWorkload checks subject/body namespace agreement, then
establishes ownership by asking the local nexlet for the workload's
current definition. Every node sees every control message (the micro
queue group is the node's own id), so the answer decides whether this
node acts at all: no local nexlet holds the id -> answer updated:false;
a nexlet holds it but under another namespace -> drop silently, since
answering would confirm the id exists to a caller with no right to it
(the convention handleCloneWorkload already uses). A replacement naming
a different namespace than the workload's own is rejected: UPDATE
replaces a definition, it does not relocate a workload, and honouring
the move would re-scope the credentials minted for it.
replaceWorkload holds the ordering, and is factored out because the
RESTART verb is the same operation with an unchanged definition:
validate -> mint -> [purge old-type key] -> store -> stop -> start
- validate before storing, so a rejected definition cannot displace a
good stored one;
- mint before storing, so the nkey in the record is the one the
replacement instance actually receives (a stale one would send a
later credential revocation at the wrong identity);
- store before stopping, which is the crash-safety mechanism: after
this point a crash leaves the NEW definition as the one
resume-on-registration starts, so the repair path completes the
update instead of silently reverting it;
- start only after a CONFIRMED stop. An unconfirmed stop aborts before
the start and answers updated:false with the reason -- truthful, and
self-healing via the stored definition.
Same workload id throughout: one KV key, one Put, and no crash window in
which two records exist and resume-on-registration starts both. The
handler emits no lifecycle events of its own; the agent's stop and start
paths already emit exactly one WORKLOADSTOPPED and one WORKLOADSTARTED,
which is what namespace quota accounting nets against.
No auction is involved -- UPDATE addresses a workload id directly -- and
no agent-side verb, sdk.Agent method or credential template changed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Five tests over real NATS against the in-memory nexlet, each pinning one
property the handler's step order is there to provide:
- subject/body namespace disagreement is rejected before any lookup,
mint or store;
- a run_request violating the nexlet's registered schema is rejected
AND leaves the persisted definition untouched. The store assertion
is the load-bearing half: moving validation after the Put keeps the
error response identical and only this assertion catches it;
- an unknown workload id is ANSWERED with updated:false, not dropped.
Silence is reserved for the cross-namespace case, where existence
must not leak; an id nobody holds is not a confidentiality question,
and answering lets a caller tell "nobody has it" from "the request
never arrived";
- a nexlet that cannot confirm the stop aborts the update WITHOUT
starting anything, while the newly stored record already holds the
NEW definition and a freshly minted nkey. That is store-first made
observable: moving the Put after the stop flips this test alone;
- the happy path replaces the definition under the same id, with one
replacement start and new credentials.
Two support changes. recordingState (node_undeploy_state_test.go) gains
StoreWorkload recording alongside its existing RemoveWorkload recording;
the reasoning is the same on the write side -- KV contents after the
fact cannot distinguish "never stored" from "stored then overwritten".
The inmem nexlet gains WithStartRequestSchema. It registers "{}" by
default, which accepts every run_request and would make the validation
assertions vacuous, so the tests that need the node's schema check to
actually reject something register a restrictive schema instead. The
zero value still means "{}" because several existing tests build
InMemAgent as a plain struct literal and the node cannot compile an
empty schema.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Three review findings on the UPDATE control verb, all of them cases where
the handler's own error handling worked against the property the verb
exists to provide.
1. A workload-TYPE change reopened the dual-writer window. The state key
embeds the type, so a type change wrote the replacement to a NEW key while
the old key survived, and resume-on-registration is scoped per agent type
(handleRegisterAgent -> GetStateByAgent): the two records get resumed by
two different nexlets that know nothing about each other. Let the stop go
unconfirmed and both instances run -- exactly the failure this verb removes,
reintroduced by the verb itself. The purge-the-old-key branch only narrowed
the window and added one where the workload is lost outright.
Type changes are now rejected with 403 ("update cannot change a workload's
type; undeploy and deploy instead"), symmetric with the existing
namespace-relocation rejection, and the purge branch is gone. The check
lives in replaceWorkload rather than the handler because every caller
depends on it, T6's RESTART included. The store-first doc comment claimed
the repair path completes the update; that claim is only true because the
type and the namespace are both pinned, and it now says so.
2. A start request that timed out returned an opaque 500 while the nexlet
answering with an error returned an honest updated:false -- despite the two
leaving identical state (old instance stopped, new definition stored,
nothing running). Both now take the same branch and the same message.
3. Unknown workload ids were answered with updated:false while ids owned by
another namespace were dropped silently. Since every node sees every control
message, the reply count alone separated those two cases, which turns the
verb into an existence oracle for workloads the caller may not see. Unknown
ids are now dropped silently too, matching handleCloneWorkload in both
cases. Callers read no-responders/timeout as not-found, which is already
what CLONE requires of them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
RESTART's request only needs a namespace (the definition itself comes from the stored record, not the caller) so its schema mirrors update-workload-request.json minus start_request. Reuses UpdateWorkloadResponse -- no new response shape earns its keep for a verb whose whole point is "same definition, fresh instance." Generated models/api_control.go via task gen-schema; no generated file hand-edited. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
handleRestartWorkload follows the same subject-parsing, namespace-
agreement and ownership-fetch/silent-drop conventions as
handleUpdateWorkload (unknown id and cross-namespace must stay
indistinguishable to the caller -- see its comment for why). The
definition it hands to replaceWorkload is deliberately the STORED
record, not the one the ownership fetch just returned: the fetch
reports whatever the nexlet has running now ("reality"), while the
persisted state record is "intent" (store-first, per replaceWorkload's
doc comment). They normally agree, but can diverge exactly when a
prior UPDATE didn't finish -- an unconfirmed stop or failed start
leaves the NEW definition stored while the nexlet still runs the OLD
one. Restarting from the live snapshot in that case would silently
re-apply the stale definition; restarting from the stored record
finishes the interrupted update instead. The state interface has no
per-id getter, so the handler reads GetStateByNamespace and picks the
id out of the map.
If the nexlet holds the workload but no record is persisted for it
(reachable in normal operation: handleAuctionDeployWorkload starts the
agent before its own state.StoreWorkload call and doesn't fail the
deploy if that store errors), the handler answers updated:false with a
truthful message rather than going silent -- ownership was already
established, so silence would only mislead a client that maps
no-responders to not-found.
Also hardens replaceWorkload: def.Metadata is mutated in place (the
nkey stamp), and def/current can share the same underlying map when a
caller constructs both from one value. Cloning def.Metadata up front
makes that mutation safe regardless of what a caller passes.
Registers the RestartWorkload endpoint in node.go under the node's own
queue group, like every other control verb. Bumps node_test.go's
subscription count 23 -> 24.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Reuses updateHarness from node_update_workload_test.go: namespace mismatch rejects before any lookup, an unknown id and a workload owned by another namespace are both silently dropped, and the happy path confirms the stored definition's content survives a restart unchanged (name, run_request) while the minted nkey rotates -- restart's "same definition, fresh instance" contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
NexClient gains UpdateWorkload/RestartWorkload, both scatter-gather requests via natsext.RequestMany that map no-responders/timeout to a not-found error, mirroring CloneWorkload -- RESTART/UPDATE only ever get one real responder (the owner), so a message that never arrives within the stall window means not-found rather than a real failure. Extracts StopWorkload's system-namespace ownership discovery into resolveOwningNamespace(workloadId, action) so UpdateWorkload and RestartWorkload get the same system-operator support (system hosts no workloads of its own and must discover the real owner via ListWorkloads before it can even address a namespace-scoped control subject). Pure refactor for StopWorkload: same messages, same behavior, parameterized only by the verb name used in error text. UpdateWorkload does not force req.Namespace to the resolved target -- UPDATE never moves a workload, so a caller-supplied definition naming a different namespace must fail loudly (the node's 403) rather than have the client silently correct it. CLI: `nex workload update -f Nexfile <id>` and `nex workload restart <id>`. Factors the Nexfile-parsing half of `start`'s inline logic into loadNexfile, shared by both commands; `update` has no placement step to hang client-side schema validation off of (no auction), so it relies on the server-side validation replaceWorkload already performs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Deploy, restart in place (rotated credentials), update in place (new definition), then confirm an unknown id surfaces a not-found error. Needs state=true (real NATS KV state), unlike this file's other tests: RESTART reads the persisted record back, which the NoState stub never populates. Also waits out handleAuctionDeployWorkload's respond-before-store race (same race node_update_workload_test.go's deploy() helper waits out) before the first restart attempt, and uses a separate short-timeout client for the not-found assertion so it isn't at the mercy of however much of the main client's fixed request budget the persistence-wait retries happened to spend. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
UpdateWorkload forwarded the caller-supplied StartWorkloadRequest.Namespace
into the request body unchanged. A system caller has no home namespace of
its own, so its request is commonly built with the caller's own namespace
("system") already filled in rather than the workload's actual owner --
exactly the case resolveOwningNamespace's discovery exists to handle. The
node addressed the subject correctly (at the discovered owner) but then
rejected the body's mismatched StartRequest.Namespace with a 403 "cannot
move a workload between namespaces", even though the caller never asked to
relocate anything. `nex workload update <id>` run from the default (system)
namespace against any non-system workload always hit this.
Fix: when the caller is system-namespace and the request's Namespace
disagrees with the discovered target, adopt the discovered namespace into a
local copy of the request before sending. A plain-namespace caller is
unaffected -- its target is always its own namespace with no discovery step,
so a mismatch there can only be a deliberate (or mistaken) relocation
attempt, and the node's 403 still catches it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
TestNodeRestartWorkloadReplaysStoredDefinition never makes the stored record and the nexlet's live definition differ, so a regression to replaceWorkload(id, current, current) -- restarting from whatever the nexlet reports live instead of the stored record -- would still pass it. Adds setStoredRecord (writes directly into the KV bucket the harness already reads from) to force the divergence an interrupted UPDATE leaves behind, and a test that deploys one definition, forces a different one into the stored record, restarts, and asserts the nexlet ends up running the STORED definition. Verified this actually discriminates: temporarily changed the handler to pass *current, *current into replaceWorkload -- only this new test failed (want: v2-stored-only; got: v1-live), every other RESTART test still passed -- then reverted (git diff confirmed clean). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Document the new `workload update` and `workload restart` CLI verbs in running-workloads.md (semantics, flags, updated:false outcomes, the store-first crash property) and concepts.md (verb composition, the never-moves/never-retypes rule, client method list). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
InMemAgent.StartWorkload unconditionally appended a new InMemWorkload entry regardless of whether the given id was already present in that namespace's slice. UPDATE/RESTART's node-side composition always stops the old instance before starting the replacement (handlers.go replaceWorkload), so this bug was not reachable from that path -- but resume-on-registration calls StartWorkload(existing=true) for every persisted record without a prior Stop, and a caller-chosen workload id can legitimately be reused (see the nex-workload-verbs plan's D2). Either path hitting an id this agent already holds would silently accumulate a duplicate entry, masking same-id replace semantics that real nexlets (podman, native) are expected to honor. StartWorkload now looks up the given id within its namespace's slice and overwrites the entry in place; only a genuinely new id is appended. StopWorkload already removed entries correctly (slices.Delete) and needed no change. Pinned by _test/nexlet_inmem/inmem_test.go:TestStartWorkloadSameIdReplaces, which reproduced the duplicate (len==2) against the old code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
TestNodeUpdateRestartRoundtrip is Task 7 of the nex-workload-verbs plan: the
canonical round-trip test (node_test.go TestNodeDeployCloneUndeploy) extended
to the new UPDATE/RESTART verbs, and the first test in the repo to drive the
full lifecycle -- auction, deploy, update, restart, undeploy, then a
post-undeploy update -- against the real state=true (NATS KV) substrate end
to end, reusing the updateHarness introduced for the handler-level UPDATE/
RESTART tests.
Sequence and what each phase pins:
a. deploy definition A: WPING lists it, the KV record matches A, and the
stored nkey (N1) is a valid, freshly minted public user key.
b. UPDATE to definition B: response Updated:true, the agent's live
definition is B, the KV record is B, the nkey rotates (N2 != N1), the
workload id is unchanged, and exactly one replacement start is issued.
c. RESTART: response Updated:true, the live definition is still B, the
nkey rotates again (N3 != N2, N3 != N1), and exactly one replacement
start is issued.
d. UNDEPLOY: response Stopped:true and the KV record is gone.
e. UPDATE on the now-dead id: per the controller ruling superseding the
original brief's step 5, this is a silent drop at the node (the CLONE
convention), not an updated:false reply -- asserted via the existing
updateExpectSilence helper (bounded 8s wait, not the 10s default) and
confirmed to leave the persisted-state store count unchanged.
Stable under `go test -race -count 3 -run TestNodeUpdateRestartRoundtrip`
(3/3, ~8s each, dominated by step e's bounded silent-drop wait) and both
`go test -race -count 1 ./...` (root module) and the same in cmd/nex's
module pass clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Review found two orderings where a stop still completes over a workload that a restart brought back, and my claim that none remained was wrong. A restart claim is not safe on pointer identity alone. A stop that arrives after the claim still finds the claiming generation under the id and marks it stopping, so the identity check passes and the restart spawns; the stop then confirms the process it was handed -- already dead -- and reports success while the workload runs the old definition with nothing recorded as running. The restart now also abandons when its claiming generation has been marked stopping, which the stop does under the same lock the restart re-checks it with. The exhausted-budget path went through RemoveWorkload by workload id after releasing the lock. A start arriving in that gap was allowed (the old process was already dead) and then had its brand new generation stopped and deleted in the dead one's place. That path now drops its own generation by pointer, under the lock that just confirmed its identity, and emits the stopped event itself: the process is already gone, so there was never anything to stop. Two more from the same review: - The start guard asked whether a generation was running, and a stopping one is not, so a start arriving during a stop passed the guard and spawned beside a process that was still alive. Worse, if that stop went on to fail, the new process was left orphaned. The guard now asks whether the id is occupied -- a live process, whatever its state -- and refuses with "is stopping" so the caller can retry. Adoption is unchanged and still asks whether the generation is running, so the node's resume replay is never blocked. - The stop was bounded at twice the grace period, landing on the node's 10s stop budget exactly. SIGKILL cannot be caught, so a process that outlives it is stuck in the kernel and five more seconds will not free it; the post-kill confirmation is now a few poll intervals. Also: RemoveWorkload logs when its delete finds the id already taken by another generation, because the stopped event it goes on to emit does not describe what holds the id then; waitExit confirms the exit rather than trusting the watcher's signal; and releasing a generation's command context tolerates one that never reached the spawn, which was a nil call waiting to happen on any generation seeded without a context. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
Three deterministic tests for the windows review found, in the same pointer-and-state form as the existing identity coverage: - A restart whose claiming generation was marked stopping before it reached the start path spawns nothing and leaves the entry the stop is working on untouched; the same claim without a stop against it does spawn. - A restart that exhausted its budget drops its own generation and not whatever holds the workload id by the time it gets there. - A start arriving while a workload is stopping, with its process still alive, is refused with an error naming that, does not spawn a second process, and succeeds once the process is really gone. Spawned pids are now registered for a kill at test end. These workloads sleep for half a minute, and a test that fails partway through never reaches its own stop, so failing runs were leaving them behind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co
staticcheck cannot mark Fatalf as noreturn through the testing.TB interface, so the nil-guarded proc.Pid read tripped SA5011 in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reply is the caller's durability signal; deleting after responding let a reader race the purge and still see the record. Both the roundtrip and undeploy-state tests assert reads right after the reply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The auction timeout only appears on CI runners and the node's slog output vanishes with the buffer, leaving no evidence. Mutex-wrapped buffer because node goroutines still log while t.Cleanup reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three node-side defects found by running the CLI against a live node and the native nexlet (PR #523 field report), each pinned by a new test: - The stop-confirmation RequestMany inherited the NATS connection's 2s default request timeout (the node context has no deadline), while a nexlet's synchronous stop is legitimately slower (native: 5s grace + SIGKILL + 750ms confirm). UNDEPLOY replied Stopped:false and UPDATE aborted 'stop unconfirmed' while the dispatched stop killed the workload anyway: destroyed workload, no replacement, RESTART then 'workload not found'. Both waits now carry an explicit 15s budget; the inmem test agent gains StopDelay to pin it. - RESTART replied 'nothing to restart' whenever no record was stored -- which is every workload on a node without --state (the default). With nothing on file it now replays the live definition the ownership fetch already returned; the foreign-namespace-record case still refuses, since store-first would clobber the other namespace's record. - Failure messages promised 'stored definition will apply on next agent registration' even on stateless nodes where nothing was stored, and claimed nothing was torn down when the dispatched stop may still land. Messages now state the stop may complete and, on stateless nodes, that the update is lost and needs a redeploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'workload stop' printed 'successfully stopped' without checking the response's Stopped flag, hiding exactly the unconfirmed case; and main printed errors while exiting 0, invisible to scripts and CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single NuidGen is shared by the node's control endpoints (auction, deploy, remote registration), which dispatch on independent goroutines. *nuid.NUID's instance Next() is unsynchronized (only the library's package-level Next() locks, via a lockedNUID wrapper), so the shared instance is a data race and can hand out torn -- possibly duplicate -- ids, which would put two workloads on one KV state key. Guard Generate with a mutex; a -race concurrency test pins it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Node-side robustness fixes found reviewing the workload verbs for production readiness: - RESTART's store-first CAS was made against a revision re-read just before the write, not the revision it read the replayed definition at. A concurrent UPDATE committing in that window bumped the record, the re-read picked up the new revision, and the CAS SUCCEEDED -- silently reverting an acknowledged UPDATE (definition and instance both) while reporting updated:true. The revision is now threaded from the read that produced the replayed definition, so a racing UPDATE conflicts instead. A pre-mint injection hook makes the interleaving deterministic in test. - handleStopWorkload reported Stopped:true even when the KV record purge failed (Warn only), leaving a record that resume-on-registration would resurrect. The purge is now retried; on final failure the response says so and it logs at Error. Stopped stays true -- the process IS stopped. - The node's replacement-verb timeouts are named and documented as a chain (ownershipFetchTimeout 3s + stopConfirmBudget 15s + replacementStartTimeout 60s ~= 78s worst case), and the client's shared request deadline is raised from 60s to 90s so a slow-but-succeeding update is not abandoned and misreported as not-found. Raising it only extends the MAX wait; replies still settle early. - Wire the node's second bare nuid (loggerID) to the concurrency-safe NuidGen so handlerError's error ids are race-free too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- update/restart with Updated:false printed a status line and exited 0, while stop's Stopped:false exited 1; and --json mode exited 0 for ALL outcomes, so a script adding --json silently lost failure detection. Failure outcomes now exit non-zero in both plain and --json modes via an errSilentExit sentinel that main turns into exit 1 without printing. - clone --stop dropped a Stopped:false-with-no-error stop entirely: the clone succeeded, the original was left running, exit 0, no indication. It now surfaces the unconfirmed stop. - main exited 0 after printing an error; it now exits 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The observability section did not work as written: double-quoted $NEX expanded to empty in the shell, the event subject was .event. not the actual .events., and "replace > with the workload id" is wrong for both feeds (logs are logs.<id>.<stdout|stderr>; events are keyed by event type with the id in the payload). Fixed to single quotes, the real subject shapes, and correct per-workload filtering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In a multi-node nexus every node answered UNDEPLOY: the nodes that did not hold the workload replied fast with 'not found', and those fast negatives tripped the client's reply wait before the owning node's slower stop confirmation (up to stopConfirmBudget) arrived -- so a real, successful stop was reported to the operator as 'workload not found'. Reproduced live on a 2-node nexus and pinned by a multi-node test whose negative control fails without this change. The node now does the same GETWORKLOAD ownership fetch and silent-drop that UPDATE/RESTART/CLONE use: a node that does not own the workload stays silent, so only the owner answers and its slow confirmation is never raced. Stopped:false from the owner now means 'owned, but not confirmed stopped', never 'unknown id'. The client waits for that one owner reply on a stop-sized deadline (stopReplyTimeout, 25s -- above the node's 3s+15s worst case) instead of the 90s default, and takes the owner's answer as authoritative. A genuinely unknown id has no owner, so no node replies and the caller waits this out before reporting not-found; the system-namespace path still short-circuits that via its existing list pre-check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
handleLameduck writes the lameduck flag into n.tags while PING, AUCTION, INFO and placement handlers read the same map on other goroutines -- a concurrent map read+write, which is a fatal runtime error, not merely a race. It surfaced as a -race failure in the multi-node client tests (TestNexClient_System) under CI's parallel load. Every tag access now goes through a RWMutex-guarded helper (setTag / tagValue / tagsSnapshot); snapshots are copied out so no lock is held across response I/O. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The node's stop can legitimately take the ownership fetch (3s) plus the stop-confirmation budget (15s); a 5s request timeout in the undeploy tests was under-provisioned and flaked under -race parallelism. Bumped to 20s to cover the real worst case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A restarted agent is a new instance, but it re-registered under the same id the crashed instance held. The node refuses a duplicate id and never evicted the dead entry (the health monitor only flips a status flag, with no removal), so a crashed local-binary agent (e.g. podman) could never rejoin while the node stayed up -- and the store-first resume path it gates was unreachable exactly when needed. Each (re)start now mints a FRESH instance id and matching register credentials (the watcher owns the mint, so it is automatic per restart); ap.ID stays the stable slot id for tracking and stopping the process. A fresh instance never collides, so the duplicate guard stops blocking recovery and instead does its real job -- catching a genuine same-id double-register -- and per-instance identity is what makes a stale instance independently fenceable. The health monitor now evicts a registration once it is Offline (with lastHeartbeat seeded at Add time so a just-registered agent is not evicted before its first heartbeat), so dead entries do not accumulate. Scope: this is leg A (rejoin). It does NOT kill a crashed nexlet's workload processes -- that (process-group / Pdeathsig) is the paired leg-B fix still required for full duplicate-safety on resume. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A workload that forked -- a shell wrapper and the sleep it spawns -- had only its direct child signalled on stop. The graceful SIGINT killed the shell, the node saw the leader gone and reported the workload stopped, and the forked grandchild was left orphaned, still running and still publishing (the joeriddles report: sleep processes surviving a stop). Each workload now starts in its own process group (Setpgid), the stop signals the whole group, and -- because the leader can exit on the graceful signal while a signal-ignoring grandchild outlives it -- the stop now confirms the entire GROUP has drained (sweeping it with a group SIGKILL) before reporting stopped, rather than trusting the leader's exit alone. Pinned by a test whose forked grandchild ignores SIGINT; it fails without the group sweep. Scope: this closes the STOP/UPDATE/lameduck orphan (leg B for the paths where the node is alive to do the kill). A hard node crash (SIGKILL, no cleanup) can still orphan workloads -- Pdeathsig is unsafe to use naively in Go (premature delivery on thread exit), so that residual needs a reap-on-boot approach and is left as a follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Leg B killed forked children on a clean stop, but a hard node crash (SIGKILL/OOM) leaves native workload processes running with no chance to clean up; on restart, resume-on-registration replayed their stored definitions and started SECOND copies beside the orphans -- the dual-writer state the verbs exist to prevent. The native nexlet now records each spawned workload's pid and process start time to a node-local file (<resource-dir>/<node-id>.workload-pids), and on startup -- during construction, strictly before the node's Start()/resume path -- group-kills any survivors from a previous incarnation, then clears the file. The recorded start time is the pid-reuse guard: a pid recorded before the crash can be recycled afterwards, and a blind kill(-pid) would group-kill an unrelated process (the Bazel process-wrapper hit exactly this), so a leftover pid is killed only when the live process at it still reports the recorded start time. Start time is read via gopsutil (cross-platform). The pid is forgotten when its process exits by any path, keeping the file bounded. The resource-directory flag was a dead flag (never passed to the native nexlet); it is now threaded through so the file has a durable per-node home. A disabled reaper (no resource dir, e.g. tests) is a no-op. Windows reap is leader-only (same limitation the live stop already has); macOS start-time works but is best-effort by platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The gopsutil dependency (pulled only for the process start time) dragged a heavy transitive tree -- wmi, go-m1cpu (cgo), sysconf, numcpus -- whose compile time pushed the CI test job past its 10-minute limit (it was cancelled, not failed). Replaced it with a build-tagged start-time read using golang.org/x/sys, which is already a direct dependency: - linux: field 22 of /proc/<pid>/stat (start time in clock ticks) - darwin: kinfo_proc p_starttime via unix.SysctlKinfoProc - other (e.g. windows): unavailable, which disables the reaper there (record/reap become no-ops) rather than reaping without a reuse guard. Same identity guard, no new dependency, and the CI compile weight is gone. Reaper tests pass on darwin and linux; the reuse-guard negative control still fails without the check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NativeAgent.StartWorkload checked occupancy under one lock acquisition (WorkloadOccupancy) while startWorkload inserted under another -- a TOCTOU window: the resume replay runs one goroutine per workload beside the endpoint handler, so two starts for the same id could both pass the check, and the loser's generation was overwritten in the map with its process left running untracked. The whole decision now lives inside startWorkload's critical section, atomic with the insert: AddWorkload refuses an occupied id, the new ResumeWorkload adopts a running generation (existing=true), and the stopping-generation fallthrough is unchanged. WorkloadOccupancy is deleted so the outside-the-lock check cannot be rebuilt on it. Guard refusals keep their exact wording via startRefusedError instead of being double-wrapped. Pinned by state-layer tests plus a concurrency hammer (8 racers on one id, exactly one winner). Also adds the missing !windows build tag to lifecycle_test.go, whose syscall.Kill/ESRCH use broke GOOS=windows go vet for the whole package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
The state record key is "<register_type>_<workload_id>" and
GetStateByAgent matches records on the "<register_type>_" prefix, so a
register type containing "_" makes keys ambiguous: type "a" scanning
prefix "a_" also claims type "a_b"'s keys, and the resume re-read
rebuilds the same colliding key (GetWorkloadRecord("a", "b_wl") is the
key "a_b_wl") -- so resume finds the foreign record and starts another
agent type's workload, the dual-writer the type pin exists to prevent.
The exact-prefix fix could not close this case; the separator has to be
kept out of the type itself.
Registration is the only path a type enters the key namespace (every
store path is gated on a registered type), so the check lands in
handleRegisterAgent. Workload ids containing "_" stay safe: with an
underscore-free type the key splits unambiguously on the first "_".
Built-in types ("native", "inmem") are unaffected; a third-party nexlet
using an underscore type must rename.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
replaceWorkload's store-first write reads the record's revision immediately before storing; a record purged by a concurrent confirmed UNDEPLOY reads back as revision 0 (a purged key is indistinguishable from one that never existed) and create-only succeeds over the purge tombstone -- so UPDATE, and RESTART's live-definition fallback, could persist a record for a workload the operator was just told is stopped, answer updated:false "the stored definition will apply on the next agent registration", and have resume-on-registration resurrect the undeployed workload. Leaving the stored definition for resume to finish is only sound when the write REPLACED a committed record. A record the verb CREATED proves nothing was supposed to exist, so an unconfirmed stop now rolls that create back: StoreWorkload returns the written revision, and the new NexNodeState.RemoveWorkloadAtRevision deletes it revision-checked -- a conflict means a newer writer owns the key (left alone), an already-gone key is the desired end state (success; the conflict branch re-reads to tell the two apart, since a tombstone also bumps the key's last sequence). This branch is also every stateless node's unconfirmed stop (NoState reads every record as revision 0); there the rollback is a no-op and the message is now the accurate one -- nothing stored, workload may still be running -- instead of advising a redeploy that could start a duplicate. Known residual, documented in code: three overlapping verbs on one id can still chain a CAS-update onto an in-flight create and leave a resumable record; closing that needs provenance, not a revision flag. Both interleavings are pinned deterministically (the UNDEPLOY runs inside one-shot hooks: pre-mint for UPDATE, pre-record-read for RESTART's fallback), asserting the record is gone AND a fresh registration resumes nothing. KV semantics of the revision-checked delete are pinned against real embedded JetStream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
The health monitor evicted (unsubscribed + deleted) any registration silent for 30s. Agents register exactly once at startup and heartbeats are fire-and-forget publishes, so evicting a LIVE agent is permanent: its heartbeats land on an unsubscribed subject until its process restarts. A >30s node-side NATS outage evicted every agent at once, and a heartbeat starved behind the nexlet state lock (held across a slow synchronous artifact fetch) got a healthy agent evicted mid-deploy. Grading is now two-stage. 30s still marks AgentOffline -- which already excludes the agent from placement (GetByRegisterType filters Healthy) and heals itself, because the heartbeat subscription stays up and the next heartbeat re-grades unconditionally. Eviction happens only after agentHardEvictAfter (10 min) of silence, with a re-check under the registration lock so a heartbeat landing between the grading pass and the eviction cannot orphan a just-revived agent. The map stays bounded: a genuinely dead instance is still dropped, and its replacement always registers under a fresh id. GetByRegisterName now prefers a Healthy match, so a crashed instance's corpse lingering Offline beside its same-name replacement cannot be returned at random. Known cosmetic cost: node info shows offline entries for up to 10 min (a crash-looping agent about 20 of them). Pinned by sweep-level tests: offline-not-evicted, hard-ttl-evicts, and live recovery through the surviving heartbeat subscription. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
NewClient stamped defaultTimeout as ONE context deadline at construction -- absolute wall-clock, shared by every call. Any client older than 90s (or whose earlier calls had consumed the budget) had every RequestMany end instantly on the expired context, and because natsext swallows the expiry, requestWorkloadReplacement read the empty result as the silent-drop signal and reported a real, running workload as "workload not found". The client context now carries cancellation only; each RequestMany site derives a fresh per-request budget via requestCtx (a caller-supplied deadline still applies when shorter). requestWorkloadReplacement answers not-found only when a full fresh budget elapsed in silence -- a canceled client is reported as such, never as a fabricated negative. NexClient gains Close() so long-lived programs can release the per-client cancellation child (one-shot CLI use can skip it). Exported-API notes: a caller context WITH a deadline is now also capped at defaultTimeout per request (no in-repo caller affected; override with WithDefaultTimeout), and the CLI --timeout flag now bounds each NATS operation rather than the whole command. Pinned by a test aging a 2s-budget client past its budget and then restarting a real workload -- fails on the old code with the bogus not-found, passes with per-request budgets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
New docs/nex/control-api.md: the full control-verb surface for client and control-plane authors -- subject grammar, namespace semantics, the three reply conventions (scatter-gather, single-owner, silent drop and why it exists), per-verb request/response schemas and expected observable effects, the stateless-vs-stateful semantics table, the updated:false partial outcomes, credential minting, and how to verify an update/restart actually acted (workload id and container name are stable by design, so the honest signals are pid/container id, start time, and the stopped/started event pair -- the exact confusion a stateless restart demo produces). Running Workloads links to the verification section from the update/restart guide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
States the contract explicitly in the control-api reference: the node-minted workload ID is the only identity (stable across UPDATE/RESTART, the token every verb addresses, the container name on container nexlets); the workload name is unvalidated display text -- not unique (CLONE duplicates it by design) and changeable in place by UPDATE. Name-to-ID resolution is a client-side convenience that must handle ambiguity, not an API guarantee. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AYyDx69HUbGHyLSKv8RX9
Jarema
added a commit
that referenced
this pull request
Aug 26, 2026
Three node-side defects found by running the CLI against a live node and the native nexlet (PR #523 field report), each pinned by a new test: - The stop-confirmation RequestMany inherited the NATS connection's 2s default request timeout (the node context has no deadline), while a nexlet's synchronous stop is legitimately slower (native: 5s grace + SIGKILL + 750ms confirm). UNDEPLOY replied Stopped:false and UPDATE aborted 'stop unconfirmed' while the dispatched stop killed the workload anyway: destroyed workload, no replacement, RESTART then 'workload not found'. Both waits now carry an explicit 15s budget; the inmem test agent gains StopDelay to pin it. - RESTART replied 'nothing to restart' whenever no record was stored -- which is every workload on a node without --state (the default). With nothing on file it now replays the live definition the ownership fetch already returned; the foreign-namespace-record case still refuses, since store-first would clobber the other namespace's record. - Failure messages promised 'stored definition will apply on next agent registration' even on stateless nodes where nothing was stored, and claimed nothing was torn down when the dispatched stop may still land. Messages now state the stop may complete and, on stateless nodes, that the update is lost and needs a redeploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Summary
Adds two node-side workload verbs —
UPDATE.<workloadId>(replace a workload's definition in place) andRESTART.<workloadId>(bounce with the stored definition) — plus the prerequisite fixes that make same-id replacement safe, and revision-CAS for the workload state record closing the lost-update races between its writers. Client methods and CLI commands included.Motivation: nex had no update verb, so callers (notably the connector control plane) improvised start-new-then-best-effort-stop-old with swallowed stop timeouts — an unbounded dual-writer window. This puts the verb where the workload, the persisted definition, and the credential live.
Design
sdk.Agentchange, no new agent-side subjects, no credential-template change — no nexlet breaks.updated:falsewith the stored definition applying on next agent registration.nex_minted_nkey) — the prerequisite for credential fencing.NexNodeStategainsGetWorkloadRecord(record + revision) and revision-CASStoreWorkload(expectedRevision, 0 = create-only) withmodels.ErrStateConflict; blind Put is gone. Per-writer policy: deploy = create-only (a late deploy Put can no longer clobber a fast update); resume = fresh read per record, ExistingState built from the fresh definition, CAS re-stamp with one retry (a concurrent update can no longer be silently reverted); update/restart = read-revision-then-CAS, conflict →updated:false"modified concurrently". Also fixes RESTART pinning the stored record's namespace, an exact-prefix bug inGetStateByAgentkey matching (typedockermatcheddockerx_*keys), and resume no longer hands an agent a definition that lost the CAS.Prerequisite fixes
handleStopWorkloadpurged state with the wrong key when no agent confirmed the stop, leaving records that resume-on-registration would resurrect after an explicit stop. Now purges only on confirmed stop.SigningKeyMinter.Mintnever populatedNatsUserNkey(always empty in production); fixed, behaviorally inert for connection auth (JWT+seed branch always won).Verification
go test -race -count 3 ./...green at the final head (11 packages, no failures, no races);cmd/nexmodule green.-race -count 3— the first tests in the repo to exercisestate=true.running-workloads.md+concepts.mdupdated; semantics reviewed against code.Known follow-ups (tracked in our beads)
NexNodeStatesignature,Delete-vs-Purgenot-found mapping, and it carries the same bare-prefixGetStateByAgentbug fixed here.updated:falsestart-failure branches are correct but untested (needs a FailStarts knob on the inmem test agent).run_requestremains a JSON-escaped string by design; migration tojson.RawMessageis deferred to the config-as-data redesign.🤖 Generated with Claude Code
https://claude.ai/code/session_01EAhmZ7td1YA9Gp9cA9Y9co