fix(run_agent): gate concurrent checkpoint preflight on block_result (fixes #34827)#35255
Merged
Conversation
…ixes #34827) In the concurrent tool-execution path, checkpoint preflight (write_file, patch, destructive terminal) fired BEFORE plugin guardrail block_result was computed. A blocked write_file could still dirty checkpoint state (doc_modified_this_turn, _last_write_file_call_id, turn_counter). Move checkpoint preflight to AFTER block_result computation, gated on `if block_result is None:` — matching the invariant the sequential path already enforces.
Contributor
🔎 Lint report:
|
Collaborator
|
Salvage of #34856 (by @beardthelion). Both fix #34827 — concurrent checkpoint preflight ordering. If this merges, #34856 should be closed as superseded. |
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
3 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
hermes updateon Windows reliably stops self-flagging its own launcher shim as a concurrent instance, closing the recurring "Another hermes.exe is running" false positive (#29341, #34795).Root cause: the distlib
Scripts\hermes.exelauncher spawnspython.exeand waits; detection runs in the python child, so the launcher shim appears inprocess_iter. The prior fix walked ancestors with per-hopcurrent.parent()insideexcept: break— the first psutilAccessDenied/NoSuchProcess(common on Windows across session/elevation boundaries) bailed the walk early, leaving the launcher in the candidate set.Changes
hermes_cli/main.py_detect_concurrent_hermes_instances: useproc.parents()(whole ancestor list in one call) and evaluate each ancestor independently, so one unreadable hop never strands the launcher. Only exclude ancestors whose exe is itself a shim — a genuine secondhermes.exeunder a non-Hermes parent (Desktop backend child) is still flagged._format_concurrent_instances_message: print a copy-pasteabletaskkill /PID … /Ffor the exact stale PIDs, so a user who already closed everything can self-remediate before retrying.parents()/exe(); add a regression test for the one-bad-hop case; assert the taskkill remediation line.Validation
parents()/ a hop raises AccessDeniedhermes.exetaskkill /PID … /Ftests/hermes_cli/test_update_concurrent_quarantine.pyConservative shim-only ancestor approach credited to the parallel attempts in #29358 (@xxxigm) and #31808 (@jquesnelle). Closes #29341, #34795.
Infographic