Harden macro runtime truth and execution lifecycle - #14
Merged
Merged
Conversation
Registration used to mutate the registry as it compiled: if an edit raised at module load time, the old callable was removed before the failure reached the UI. Compile candidates off to the side and swap them in only after they are complete, so a bad replacement leaves the last-known-good macro running. Source tests now have a separate bounded execution path that never enters the registry at all. That gives the GUI and CLI somewhere honest to run unsaved source without persisting it, while four disposable slots put a ceiling on deliberately hung test candidates. Unregister also retires the per-id fire worker and discards its queued work. Deleting and recreating macros no longer leaves one daemon thread per old id for the rest of the sidecar lifetime.
A timed-out register/check/parse means the Python control loop stopped servicing requests, not merely that one user macro is slow. Retire that exact session on a control-plane timeout or broken pipe so an alive-but-deaf sidecar cannot keep accepting work forever. Registration is now last-known-good: validate the id, load the candidate into the sidecar, persist only after its ack, then promote the durable manifest and option schema. Failed candidates no longer replace disk or manifest state. Add a fire_source protocol client for editor and ad-hoc CLI runs. It executes source without adding it to the registry or touching macros/scripts, including a mock-fire form for ask previews. Macro names also stop passing through a lossy filename rewrite at creation: new registrations accept only the exact filesystem-safe alphabet, so two runtime ids can no longer silently share one script path. Deletion now evicts the warm registry immediately instead of relying on a later process restart.
The macro editor's Test and Test asks controls called the durable register path. Testing unsaved text therefore overwrote macros/scripts, and an unnamed test could leave a real draft file behind. Route both controls through the new fire_source candidate path instead: the exact editor text runs, but neither disk nor the live registry changes. The SYSTEM beacon preview now follows the same rule for saved source, and the delete copy matches the corrected core behavior: deletion evicts the warm registry immediately rather than waiting for a future sidecar restart.
Running a Python file was implemented as register-then-invoke, so a one-shot CLI command silently copied that file into macros/scripts and could replace a live macro whose id matched the filename. A run command should run; macro add is the command that persists. Use the non-persistent source execution path for --file while named registered macros continue through the normal warm registry. The long beacon-friendly wait budget is unchanged.
Extend the real-sidecar e2e proof over the three state transitions that previously lied to callers: a bad replacement must leave both the old callable and old file intact, source testing must execute without creating a script, and deleting a macro must make it uncallable in the already-warm process. These are end-to-end assertions against bundled CPython rather than mocks, because the bugs all lived in the Rust/Python registry boundary.
A top-level infinite loop during registration leaves the CPython process alive, so EOF/crash detection cannot identify the failure. Drive that exact case against the real sidecar: registration must return on its control-plane deadline, the candidate must never reach disk, and the next invocation must run in a newly spawned process. This complements the existing external-kill recovery proof with the hang shape that actually motivated retiring timed-out control sessions.
A successful Python compile used to replace the live callable before its source was committed to disk. If the write then failed, a trigger could observe a revision that the durable manifest still denied existed. Reverse that order without sacrificing warm registration: prepare the namespace invisibly, write the source atomically, record its generation, then commit the prepared callable. Every durable macro now carries a monotonic generation into each fire. A press queued under revision N therefore cannot wake up after a hot reload and execute revision N+1; the sidecar refuses the stale fire instead of silently changing what the trigger meant. Macro source and option writes use the repo's atomic-write primitive, directory scans ignore filenames that are not canonical macro ids, deletes are idempotent and serialized with saves, and failed final publication rebuilds the exact session from durable truth before reporting success. The input path still performs only an in-memory generation lookup plus the existing framed write; no disk or blocking work moved onto dispatch.
The per-macro queue cap stopped one hot trigger from growing forever, but it was not a process bound: generated ids could each allocate a worker and their own 256-item queue. Add global budgets for queued/running fires and live macro workers. Refusal stays non-blocking and explicit, and unregister returns every slot for discarded work before retiring that id's worker. Persistent macro state had the inverse problem: four unrelated macros writing four unrelated JSON files all serialized behind one process-global lock, so an AV sharing retry in one namespace stalled the other three. Keep the required load-mutate-atomic-replace exclusion per macro id instead. Unregister also retires the keyed lock so churn does not monotonically grow the lock table. The existing store stress still waits on real completion signals rather than timing assumptions; its concurrency comments now describe the per-namespace contract it actually proves.
Drive the generation boundary against the real bundled sidecar: start one old revision, queue another fire behind it, publish the replacement, then prove the queued old fire is refused while new presses reach the new callable. Also pin that a manually dropped .py whose stem is not a canonical MacroId never enters the runtime scan. Clarify the deliberate exception to per-macro actor ordering: invoke(wait=True) is inline subroutine semantics and can overlap a separately-fired target; wait=False is the form that enters the target's serial queue. Do not eagerly delete a per-macro state lock on unregister. A callable already in flight can still be finishing a state operation after its registry entry is removed; replacing its lock object in that window would create two independent critical sections for one JSON file. Keep the tiny lock entry for process lifetime instead of weakening the load-mutate-replace invariant.
worflor
changed the title
1 task
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
1 participant
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.
What changed
Make the Python macro boundary tell the truth about what is live, durable, queued, deleted, and merely being tested.
macro run --fileinvoke(wait=True)subroutine semantics instead of implying it participates in the target's serial actor queueThe input hot path remains non-blocking. It gains only in-memory generation lookup plus the existing framed write; disk, policy parsing, spawning, and waits stay off dispatch.
Important semantics
invoke(wait=True)intentionally remains inline subroutine semantics and can overlap a separately-fired instance of the same target.wait=Falseis the form that enters the target's serial fire queue. This PR makes that exception explicit rather than hiding it behind the broader per-macro ordering claim.Verification
.\validate.ps1passed on the final branch, including bundled-CPython end-to-end and macro stress tests.what changedrunner: its annotation says recent account payments failed or the spending limit must be increased. The Windows and Linux jobs are skipped before tests run.Before this can merge
If the pull request contains work by more than one author, every author must
record the same acceptance in the pull request, unless the submitter has
authority to accept for them.