Repository navigation
fix(update): a killed Windows updater no longer strands paused gateways, and never resumes them onto a torn tree - #132338
Conversation
૮ >ﻌ< ა ci reviewran on 6bb7daa — test(update): the post-commit ZIP resume test's git-command debug infoCI timingsCI timings · View report · View jobWall time 30m16s vs 25m26s (+19.0%). 11 job(s) slower, 17 faster,
|
alt-glitch
added
type/bug
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Review of 1b2348d302e00fa2e5b3cfb0ebea59772d156b71
Four P1 findings and one P2 finding are attached inline. The failure class is not closed yet. The write-before-pause record addresses the original missing state, but that state must also survive recovery, an in-flight drain, and adoption rollback. Separately, adopting an orphan currently replaces the evidence used by the whole-tree gate and can authorize an unrepaired checkout.
The relevant boundaries are recovery claim and cleanup, live-incarnation filtering, orphan adoption and re-stamping, rollback discharge before adopted entries are merged, and runtime-host argument parsing.
Executed evidence and its limits
I ran an isolated source-level regression harness against this pinned head's implementations. The complete 279-line update_pause_record.py was copied from the GitHub response and its Git blob hash independently matched 8fd60dfeb6d1e2bf6320aec58be4358a37a8da93. The Windows pause/rollback/resume functions and runtime classifier were loaded as source excerpts. Windows discovery, liveness premises, gateway launch and SCM operations were stubbed; record IO/rename, filtering, adoption, rollback control flow and Git checkout inspection were real. The recovery-death check hard-exited a disposable Python child at the resume boundary.
Result: 7 failing assertions across these five defects; 4 control cases passed. Three failures are option-value variants of the same classifier defect, not three separate findings. These are assertions for the desired corrected behavior, not claims that seven existing repository tests failed. I did not run the Windows E2E suite here.
Observed regression receipts
crash: canonical_exists= False claim_count= 1 next_launch_orphan= None
FAIL: gateway obligation exists only in an undiscoverable .claim
draining: launches= [] record_after_recovery= None
FAIL: live-but-exiting gateway lost both restart and durable obligation
rollback: restored_services= ['svc-beta'] resumed_profiles= ['current-profile'] record= None
FAIL: rollback deletes an adopted obligation it never resumed
adoption: old_dirty= [] new_dirty= ['a.py'] old_gate= False new_gate= True
FAIL: adoption reclassified a torn file as a pre-existing user edit
['hermes', '--profile', 'work', 'gateway', 'run'] -> False
['python', '-m', 'hermes_cli.main', '-p', 'work', 'gateway', 'run'] -> False
['hermes.exe', '--reasoning', 'high', 'serve'] -> False
FAIL: valid runtime hosts are misclassified
7 failed, 4 passed
Controls: ordinary recovery resumed its recorded profile and discharged successfully; bare hermes gateway run, module-form serve, and --profile=work gateway run were recognized as runtime hosts.
The new Windows journey usefully asserts that the original process has already stopped before testing recovery, but consequently does not exercise the still-draining boundary. The new unit coverage also does not cover death of the recovering launcher, adoption after a torn pull, or adopted-state retention during SCM rollback. These findings are separate from the completion-child marker and PID-reuse limitations already disclosed in the PR description.
The regression contract needs to include interruption of recovery itself, recovery between pause ACK and process exit, retry adoption followed by an early update error, current-pause rollback with an older obligation still outstanding, and supported top-level option/value forms in runtime ancestry. Only verified completion or explicitly verified cancellation should remove an outstanding restart obligation; adoption alone must not turn failed tree verification into success.
f0e6a55 to
ebb4cb3
Compare
There was a problem hiding this comment.
Re-review of ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0
Five P1 blockers remain. Three findings from the earlier review are repaired in the tested paths; two remain reproducible. The additional findings below concern content evidence, partial restart settlement, and recovery racing checkout mutation. The current implementation can still strand a gateway or admit a restart onto an unfinished checkout.
This review covers the current stacked diff, including its dependency on #132365. The current Job Object flags include JOB_OBJECT_LIMIT_BREAKAWAY_OK. GitHub's Git commit objects confirm that this head and the prior ebb4cb3dc6c6b844ba80d7f221c36541f3a556ca have the identical tree 3a3174535327b0e0dede41ecd4e5537a24c75832; the newer commits retrigger CI. All source links and inline anchors below bind to the current head, and the behavioral probes were rerun after verifying that tree equality.
Reconciliation of the earlier findings
The same behavioral experiments were run against the complete pinned modules at 1b2348d302e00fa2e5b3cfb0ebea59772d156b71 and this head.
| Earlier finding | Current disposition | Before → current evidence |
|---|---|---|
| Killed recovering launcher strands its claim | Repaired in this probe | A separate recovery process hard-exits after claiming. The old head leaves an undiscoverable .claim; the current head rediscovers it, resumes once, and then has no remaining debt. Dead-claim enumeration is now present. |
| Live original process can already be draining | Still broken: F1 | Both heads discard the live incarnation's debt; the process then exits with no launch and no discoverable obligation. |
| Adoption replaces the orphan's safety baseline | Still broken: F2 | Both heads change the gate from false to true on the same unrepaired checkout after adoption. |
| Current SCM rollback discharges an older orphan | Repaired for controlled SCM rollback | Both runs restore svc-beta and restart current. The old head loses orphan; this head retains it unowned through abandon_pause. This closes that debt-retention finding, not F2's separate evidence-loss problem. |
| Runtime-host option values confuse classification | Repaired for all five tested forms | Module/path invocation, --profile, -p, --reasoning, and dashboard rows change from five false results to five true results through the canonical matchers. |
RCA / SSA: durable obligation, admission, effect, settlement
Writing before the first stop repairs the original absence of durable state. The remaining failures occur where a later transition substitutes a weaker fact for the original obligation or its evidence:
| Boundary | Required fact | Current substitute and consequence |
|---|---|---|
| Pause intent → recovery | A live process has not accepted a stop, or its pending drain remains owed | Same-incarnation liveness is treated as proof that no stop happened: F1. |
| Orphan → next update | Preserve each pending runtime and the evidence authorizing its restart | Merge only some token fields, then stamp the current tree: F2. |
| Checkout → launch admission | Same HEAD still has safe contents after any failed mutation | A pathname present in the old dirty list authorizes any later bytes at that path: F3. |
| Launch → durable retirement | Every owed logical runtime has a matching readiness acknowledgement | Watcher creation plus any ready fleet PID retires the entire set: F4. |
| Recovery → concurrent update | Checkout safety remains valid through the actual start and handoff | Record-claim custody and a momentary update-liveness check provide no checkout exclusion: F5. |
Both original diagrams were rendered with Mermaid CLI and visually inspected before submission. The flowchart has since been split into linked ownership and control panels with embedded semantic legends; evidence confidence and native-platform limits remain explicit in the prose.
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TB
subgraph CORE[" "]
direction TB
P["Durable pause token and<br/>checkout evidence"]
C["Recovery claim custody"]
A["F2: adoption loses<br/>evidence and cold starts"]
G["Whole-tree admission"]
F["F1: live draining PID<br/>loses restart debt"]
B["F3: dirty path accepts<br/>different bytes"]
W["Restart watchers created"]
V["Some fleet PID becomes<br/>ready"]
T["F4: entire claim retired"]
P -->|next launch| C
P -->|next update| A
C --> G
C --> F
A --> G
G --> B
G --> W
W --> V --> T
end
CORE ~~~ QA_LEGEND
QA_LEGEND["Legend<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Teal: durable state<br/>Green: success<br/>Red: failure"]
style CORE fill:#0d1117,stroke:#30363d,color:#e6edf3
style QA_LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
classDef teal fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
class P,Q_teal teal
classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
class C,G,Q_purple purple
classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
class W,Q_amber amber
classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
class V,Q_green green
classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
class A,F,B,T,Q_red red
click P "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L301-L307" "Pinned source"
click C "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L379-L391" "Pinned source"
click A "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L339-L354" "Pinned source"
click G "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1342-L1358" "Pinned source"
click F "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L330-L336" "Pinned source"
click B "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L141-L149" "Pinned source"
click W "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1239-L1267" "Pinned source"
click V "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1300-L1323" "Pinned source"
click T "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L367-L376" "Pinned source"
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TB
subgraph CORE[" "]
direction TB
P["Durable pause token and<br/>checkout evidence"]
C["Recovery claim custody"]
A["F2: adoption loses<br/>evidence and cold starts"]
R["Verified: dead claim<br/>rediscovered"]
U["Verified: failed resume<br/>stays owed"]
S["Verified: rollback keeps<br/>older profile debt"]
P -->|next launch| C
P -->|next update| A
C -. killed claimant .-> R
R -. retry .-> C
C -. zero ready .-> U
A -. current SCM rollback .-> S
end
CORE ~~~ QA_LEGEND
QA_LEGEND["Legend<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Teal: durable state<br/>Green: success<br/>Red: failure"]
style CORE fill:#0d1117,stroke:#30363d,color:#e6edf3
style QA_LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
classDef teal fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
class P,U,S,Q_teal teal
classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
class C,Q_purple purple
classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
class Q_amber amber
classDef green fill:#052e16,stroke:#4ade80,color:#f0fdf4,stroke-width:2px
class R,Q_green green
classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
class A,Q_red red
click P "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L301-L307" "Pinned source"
click C "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L379-L391" "Pinned source"
click A "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L339-L354" "Pinned source"
click R "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L240-L255" "Pinned source"
click U "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L367-L376" "Pinned source"
click S "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L320-L327" "Pinned source"
F1 — [P1] Keep restart debt while an acknowledged gateway is still draining
drop_never_stopped() deletes a profile or unmapped entry whenever its original process is still the same live incarnation. The socket pause path explicitly accepts pausing / already_stopping and then waits for a graceful drain. An updater dying between that acknowledgement and process exit therefore leaves a live gateway that is already committed to stopping.
Reproduction: persist a pause token naming a real, still-live fixture process; recover before that process completes its simulated drain; then let it exit. The actual record/recovery code reports launches=[], obligation_retained=false, gateway_exited=true on both heads. The live fixture represents the acknowledged/draining state; no Windows socket ACK was generated in this probe. Because empty work clears resume_needed, recovery deletes the claim before the gateway finishes exiting. Subsequent launches have nothing left to recover.
Repair and regression: retain unfinished pause debt until drain/stop completion or positive evidence that the request was never sent. PID liveness alone cannot settle it. Add a barrier after pause acknowledgement but before exit, kill only the updater, recover during that barrier, then complete the drain and verify the profile is eventually restored. Keep a control for death before any stop request to avoid unintended duplicate starts.
F2 — [P1] Preserve the orphan's complete obligation and original checkout evidence during adoption
merge_into() copies profiles, argv, and services, but omits pre_sha, dirty_at_pause, process identities, and cold-start authority fields. record_pause() consequently stamps the current checkout after adoption. The no-running-gateway path reaches exactly this operation.
Reproduction: record a clean disposable Git checkout, leave HEAD unchanged, and change tracked a.py as the failed-pull state. The original token correctly yields old_gate=false. Invoke the actual next-update pause/adoption path with no running gateways and no repair: the adopted token now has dirty_at_pause=['a.py'] and yields adopted_gate=true. This reproduces on both heads. A retry that fails source resolution before applying an update can immediately resume using that newly permissive token; a later recovery can also consume it.
A second experiment of the same lossy projection starts with only cold_start_profiles={'secondary':'generation-old'} and no newly discovered attestation. Adoption produces a token with no cold-start obligation on both heads. That field is executable restart work: its consumer starts the named profile and consumes the recorded generation only after readiness. Fresh discovery cannot be assumed to reconstruct every persisted obligation.
Repair and regression: preserve the full pending contract, including its original safety evidence and generation/identity bindings. Keep separate evidence for obligations from different pauses where a single baseline cannot represent them safely. Do not certify an adopted checkout by restamping it. Cover unrepaired adoption followed by an early update abort, and adoption of a cold-start-only token with fresh discovery returning nothing.
F3 — [P1] A previously dirty pathname does not prove its later contents are safe
This is independent of F2: it occurs with the original token preserved. At the original HEAD, tree_is_whole() checks only current_dirty_paths - dirty_at_pause. Pause recording precedes the real autostash. A previously edited a.py can therefore be stashed clean, then partly rewritten by a failed checkout while HEAD stays unchanged. The same pathname is still allowed even though its contents no longer match the pre-pause edit. The failed-Git exception path removes the interrupted-pull marker, so that marker does not necessarily block this state.
Reproduction: in a real disposable Git repository, edit a.py, stamp the token, run git stash push, then write different partial-update bytes to a.py without moving HEAD. Both heads report dirty_at_pause=['a.py'], now_dirty=['a.py'], stashed_user_edit=true, head_unchanged=true, and gate=true. Git/stash and the gate are real; the failed Windows file replacement is represented by the resulting partial bytes, not exercised natively.
Repair and regression: retain a durable incomplete-mutation fact until successful completion or verified restoration, or validate original index/worktree content rather than allowing arbitrary bytes by pathname. Extend the held-file failed-pull test so a file partially written by Git was already dirty before the update and got autostashed. Also assert that an unchanged legitimate user edit remains admissible.
F4 — [P1] Retire only the runtime obligations whose readiness was actually verified
_relaunch_paused_gateways() removes each entry when its watcher was created. The subsequent verifier accepts any nonempty fleet-wide PID list; the poll itself returns on any stable hit. It does not establish which planned profile/home or argv-backed runtime is ready. The verifier predates this round, and this PR now uses its result to permanently unlink the durable claim.
Reproduction: record two independent profile gateways, alpha and beta. Both launch boundaries return watcher-created success; inject readiness for beta alone into the actual Windows resume/recovery control flow. Both heads print that alpha and beta are restarting and remove all debt: watchers_created=['alpha','beta'], ready_profiles=['beta'], retained_profiles=null. On this head, the zero-ready control retains both entries and the all-ready control retires them, isolating the partial-readiness hole. The readiness vectors are explicit substitutes, not real Windows processes.
Repair and regression: match readiness to each owed logical runtime, preserving failed or missing entries and their retry state. A multiplexer can satisfy several profiles if its served-profile evidence establishes that relationship; a PID count alone is insufficient. Test independent alpha failure/beta success, a ready SCM gateway alongside a failed manual gateway, and matching all-ready completion.
F5 — [P1] Serialize recovery's actual start with checkout mutation
orphans() checks for another updater, and recover() claims the record. Neither holds checkout exclusion through the tree gate and resume. The startup caller runs outside the update command's checkout lock. Claim rename arbitrates record custody, not checkout mutation. A subsequent updater ignores the live recovery claim and may see no gateway to pause yet. The detached watcher performs the later spawn without another checkout check.
Reproduction: a separate recovery process claims a record and passes the real tree gate. Pause it at the launch boundary. The parent then acquires the real UpdateLock, calls the actual pause path (adopted_token=null because the child owns a live claim and no gateway is running), writes the pull marker and partial checkout bytes, and releases recovery. Importing the complete pinned gateway module and calling its unchanged restart watcher starts a harmless fixture program. That program independently probes the kernel lock and observes:
{"updater_acquired":true,"adopted_token":null,"effects":[{"case":"effect","pull_marker_exists":true,"update_live":true}],"recovery_claim_still_present":false}This is a real two-process/kernel-lock and detached-process witness on POSIX. The full Windows orchestration functions use explicit external discovery/service/launch boundaries; it is not native Windows Job Object evidence. The control with an updater already holding the lock before recovery preserves the record unchanged, so the initial liveness guard works and the later window remains open.
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph FLOW["Observed order — native Windows interleaving not run"]
direction TB
R["1 · Recovery claims debt<br/>Tree gate passes"]:::purple
U["2 · Updater acquires checkout lock"]:::purple
S["3 · Updater skips live recovery claim"]:::purple
T["4 · Updater writes tree<br/>Creates pull marker"]:::amber
W["5 · Recovery resumes beyond earlier gate"]:::amber
G["6 · Watcher spawns fixture runtime"]:::amber
F["7 · Fixture observes held lock and pull marker<br/>Demonstrated unsafe overlap"]:::red
E["8 · Updater releases checkout lock"]:::amber
R --> U --> S --> T --> W --> G --> F --> E
end
LEGEND["Legend<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Red: failure<br/>Gray: context / transient"]
style FLOW fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
FLOW ~~~ LEGEND
classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
classDef gray fill:#1e293b,stroke:#94a3b8,color:#f8fafc,stroke-width:2px
click R "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1342-L1358"
click U "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/main.py#L2504-L2521"
click S "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L240-L255"
click T "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd.py#L910-L958"
click W "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1342-L1358"
click G "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/gateway.py#L1085-L1154"
click F "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/gateway.py#L1085-L1154" "Source: runtime spawn boundary; fixture observation is documented in the adjacent review evidence"
click E "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/main.py#L2555-L2558"
Repair and regression: define one exclusion/handoff protocol covering the final safety check, actual watcher/runtime start, and visibility to the updater's pause/discovery phase. Make a concurrent update account for unfinished live recovery work; a second unlocked check still leaves the same race. Add the barrier-controlled interleaving in both orders and port the actual start witness to the native Windows lane.
Validation and remaining limits
The offline stdlib behavioral harness used Python 3.12.14 and complete Git-blob-verified source snapshots, with no project-source edits. Twelve assertions ran per head: current: 6 pass, 6 fail, 0 errors; earlier head: 1 pass, 9 fail, 2 skip, 0 errors. The two skips are the checkout-lock controls, whose API is absent on the earlier reviewed head. Two current failures are separate symptoms of F2, hence five findings rather than six. Failures assert the required corrected behavior; they are not results from the repository's CI suite.
Record JSON/atomic rename, real Git repositories and stash, real process exit/liveness, and POSIX checkout locking were exercised. Windows discovery, SCM, socket/launcher outcomes, readiness lists, fresh-attestation discovery, and the Windows façade selector were explicitly substituted; unrelated import-only/UI boundaries fail if reached. sys.platform was not changed. The F5 watcher and subprocess compatibility implementation were imported from the complete source and used their actual POSIX subprocess path. No live Hermes gateway or service was touched.
The repository's canonical runner and native Windows E2E were not run locally: this workspace has pinned source snapshots rather than the bootstrapped project environment, and pytest is unavailable. The new Windows kill-after-pause cell kills after the pause report and asserts the old process already stopped; it does not exercise F1's earlier acknowledged-drain window. Its torn-pull cell also does not cover adoption or the already-dirty/autostashed pathname case.
CI status at submission is recorded below; no CI failure is attributed to these findings without a matching failure trace.
Current-head check snapshot at 2026-10-03T22:18:04.450Z: 67 checks, 24 unfinished, 0 failed so far. The retriggered CI run has not established a final verdict.
Previous run on the identical code tree (ebb4cb3) |
Status |
|---|---|
| Desktop install/update E2E (Linux, app-update) | Failed |
| Windows install.ps1 + hermes update E2E | Passed |
The earlier Desktop failure's cause remains unestablished here: its step/log reads timed out. The passing Windows job does not cover the untested interleavings above.
There was a problem hiding this comment.
Re-review: five additional failure boundaries at ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0
Five additional P1 findings (F6–F10), plus two more failure branches of F4. The previous five blockers still reproduce. This head has not changed since the previous re-review; this pass follows the other consumers of the protocol: lazy completion, claim transfer, checkout identity, SCM failure isolation, and Electron's marker reader. Some are pre-existing consumers left outside the new protocol, as identified below.
Current-head CI now reports 48 successful checks, 23 skipped, no unfinished or failed checks. The required-check gate, Windows install/update job, and Desktop app-update job are green. The earlier Desktop CI failure is no longer a current-head blocker. The independent failures below are outside those passing cases.
F6 — [P1] Lazy and standalone completion still enter a checkout owned by another updater
The new lazy-admission check samples update_in_progress(root) and then takes UpdateLock() without install_root. That lock protects the invoking home's marker; it does not acquire the checkout lock. With two homes sharing a checkout, an updater can acquire the checkout between the sample and the lazy tail. The unchanged standalone completion caller has the same missing checkout ownership and can enter even when the other updater already holds it.
Executed reproduction: use one disposable source-root fixture and homes A/B. In a child running actual prepare_launch, present a self-managed install with a pending completion and current dependencies. Barrier immediately after the real busy probe reports false. The parent acquires actual UpdateLock(install_root=root) under home B; release the barrier. The actual admission reaches its completion-effect boundary under home A. An independent kernel-lock probe there reports:
lazy: busy=false → other updater acquires checkout → tail enters, checkout_busy=true
control: updater held before sample → refused, no tail
standalone complete_source_checkout: checkout already busy → body entered, result=true
Locking, admission and the two-process interleaving are real POSIX behavior. PM/steward discovery, Python selection and build effects are substituted; no build ran. This is a remaining defect in the checkout-lock migration, not a claim that marker-only completion callers were previously correct.
Required repair: acquire checkout ownership atomically at every completion admission, and carry it through the tail's entire process tree. Updating only those two constructors is insufficient: the lazy child spawn and the standalone second-interpreter handoff also need ownership propagation. Otherwise POSIX can self-refuse or lose custody; Windows needs its own child-custody path. Regression: both arrival orders, homes A/B sharing a checkout, standalone completion, and death of the launching process while its tail survives. Assert no completion effect occurs beside another mutation owner.
F7 — [P1] Claim transfer can create two executable copies of one restart obligation
There are two demonstrated transfer windows in the new durable protocol.
First, claiming an existing abandoned claim renames it into the new live claimant's filename, but the body retains the old dead/unowned claimer until the rewrite. _claimer() prefers that stale body identity over the live filename PID. Another recovery therefore treats the live in-flight claim as orphaned and renames it again. The first claimant's later atomic write recreates its old pathname. Both processes return with alpha's obligation.
Second, record_pause() publishes the merged canonical record before deleting the source claims. Killing the adopter between those operations leaves canonical and claimed copies of alpha. The next recovery enumerates both and executes both.
Executed reproductions and controls: pause a real claiming child before _atomic_write; a second claimant succeeds while the first child's real liveness is true; release it and observe two claims with alpha. A fresh canonical-record control leaves one claim and the second claimant cannot take it. Separately, hard-exit an adopter with status 71 at release_claims after the real canonical write, then call actual recovery. It produces two real harmless fixture-runtime starts, both with the actual generated --profile alpha gateway run --replace arguments. Completing the transfer without interruption produces one start.
reclaim live window: claimant_alive=true, second_claim_succeeded=true, claims=2
publish then die: executable copies=2, alpha restart calls=2, fixture starts=2
controls: single custodian; completed publication → one fixture start
This cannot be dismissed as an idempotent replay: profile restart supplies --replace, and gateway startup can terminate the already recovered same-profile instance through its existing-instance consumer. Repeated replacement is the source-derived consequence; a full live Hermes takeover was not run.
Required repair: make ownership publication and source-to-canonical transfer recover as one custody transition, with a stable obligation identity that prevents two independently executable copies. Recovery must not trust a previous body owner after a new rename, and copy-then-delete needs durable transfer recognition. Regression: barriers before owner publication, after canonical publication, and after source retirement; concurrent recoveries and process death in each window; assert one custodian and one effective replacement for each owed runtime.
F8 — [P1] The record writer bypasses checkout identity and reassigns a foreign obligation
orphans() deliberately filters by install_root, but write() reads the same canonical path and merges any dead owner's token without checking that field. It then writes the current checkout's install_root. The single root-home record path makes this reachable when two checkouts/worktrees use the same root home.
Executed reproduction: under one disposable home, record checkout A's alpha debt. Switch the composed updater to checkout B and discover no running gateways. B correctly discovers no A orphan. The actual pause → failed cold-start → sync/write path nevertheless imports alpha and rewrites A's record as B. After the child exits, B discovers the reassigned debt. A separate-home control preserves A's original record and leaves B alpha-free.
A second real-Git case binds an owed service to A and leaves A's interrupted-pull marker present. A's gate is false; B's orphan scan excludes it; B's record write imports the service and stamps B's healthy gate true while A's marker remains. This demonstrates the wrong checkout being used as restart evidence, conditional on the shared-home/two-checkout topology. The SCM effect itself is substituted.
A gate=false; B sees zero A orphans
B write: service-fixed-to-A retained, install_root=B, B gate=true, A marker still present
composed alpha case: foreign obligation becomes discoverable by B
separate-home control: A preserved, B contains no alpha debt
This is distinct from F2's lossy adoption: even if merge retained every field, the writer would still bypass the install-scope exclusion and destroy A's record identity.
Required repair: key durable custody by checkout identity, or refuse a foreign canonical record without overwriting or merging it. Carry that identity through all writes, claims, recovery and discharge; test path canonicalization where aliases represent the same checkout. Regression: A→B→A using a shared home, with A interrupted and B healthy; assert B cannot re-certify or retire A's runtimes and A remains recoverable.
F9 — [P1] One refusing SCM service starves all independent manual gateway recovery
_resume_windows_services() retains failed service debt and raises. _resume_paused_set() calls it before reaching any profile/unmapped relaunch. The new startup recovery owner retries that whole token on each launch. A permanently failing or removed service therefore prevents unrelated, recoverable manual gateways from ever reaching their restart boundary.
Executed reproduction: persist one token containing SCM service fixture-refuses and independent manual alpha. Substitute a permanent SCM start refusal and successful alpha launcher/readiness boundaries. Run actual recovery twice. Service attempts are ['fixture-refuses','fixture-refuses']; alpha launch attempts are empty, and alpha remains stopped/owed. With the failed service absent, the same alpha case is launched and discharged.
This ordering predates the durable record. The new recovery route preserves debt but inherits the failure coupling, so its repeated retries do not restore the independent part of the paused set. The record is not lost in this case; progress is blocked by an unrelated runtime's failure.
Required repair: attempt independent runtime obligations with per-entry failure isolation, persist only unresolved entries, and report the aggregate failure after the other entries have had a chance to recover. Do not mark a failed service successful to unlock progress. Regression: failed SCM service plus healthy manual profile, failed manual profile plus healthy service, and multiple services with one refusal. Assert healthy entries recover exactly once while failed entries remain durably owed.
F10 — [P1] Electron deletes claims that the new owner/delegate protocol considers live
The stack introduces creation-time-bound owner/delegate lifetime and in-flight publication and delegate publication. The unchanged Electron marker reader still parses only two lines, applies the 20-minute ceiling to a live owner, and deletes a marker when its original PID died. It also deletes the fresh empty file during Python's publication grace. It does not understand the live delegate.
Executed cross-language reproduction: write each marker fixture into a disposable home and call actual Python read_live_update, then actual TypeScript readLiveUpdateMarker, updateGateReason, waitForUpdateClearance, and updateHandoffConflict under Node. Use the real live parent PID and a real exited/reaped child PID; preserve a recent live-v2 owner as the control.
| Marker fixture | Python | Electron file effect | Desktop admission |
|---|---|---|---|
| Recent live v2 owner | Live | Retained | Marker gate closed; handoff conflict present |
| Fresh empty claim | Publication in flight | Deleted | clear; no conflict |
| Live v2 owner, 21 minutes old | Live | Deleted | clear; no conflict |
| Dead owner, live delegate | Live delegate | Deleted | clear; no conflict |
The result reaches Desktop's actual gate dependencies, the pool's clearance-to-runtime path, and handoff conflict checking before another updater is spawned/prewritten. The concrete witnessed effect is deletion of live evidence and reopening those gates. The checkout kernel lock still excludes ordinary competing Python updaters; this is not evidence that two Python checkout writers actually ran.
File IO, Node parsing/deletion, gate decisions and signal-based PID liveness are real. The Python creation-time seam supplies the parent's actual /proc/self start time because this workspace's PID namespace/procfs numbering differs. No full Electron window or native Windows process ran.
Required repair: migrate all marker readers and writers with the producer: Electron, Rust, Python and handoff paths must share owner incarnation, delegate, publication grace and v1 compatibility rules. Regression: common cross-language fixtures for these three failure cells, plus recent-v2, dead-v2 and stale-v1 controls; a live v2 owner/delegate or publishing claim must remain present and preserve marker-derived startup/handoff admission. Desktop's separate 20-minute wait-budget policy is outside this assertion; these probes demonstrate marker deletion and immediate clear, including fresh publication and live-delegate cases unaffected by that age ceiling.
F4 is wider than the earlier partial-readiness case
The earlier case had both watchers created, only beta ready, and all debt retired. Two additional branches now have executed evidence:
| Branch | Observed result | Violated settlement fact |
|---|---|---|
| Alpha watcher created; beta watcher creation fails | readiness_calls=0; durable residual contains beta alone |
Alpha was removed before any runtime readiness evidence. |
SCM-only recovery, wrapper reports running, no gateway ready |
readiness_calls=0; durable record deleted |
Service-wrapper liveness was accepted as gateway readiness. |
In the first case, the relaunch loop writes only launch failures back to the token and raises, bypassing the verifier call. Residual claim persistence faithfully persists that already incomplete token. Controls: all watchers created/no ready retains alpha+beta; all created/all ready discharges.
In the second case, SCM start establishes service state, service bookkeeping removes the entry, then the no-manual-work branch sets resume_needed=False. There is no service-to-profile readiness acknowledgement before the claim is deleted. SCM state/launcher/readiness boundaries are explicit substitutes; record retirement is real.
These are extensions of F4, not extra finding-count inflation. Settlement must be per logical runtime, after matched readiness, including error unwinding and service-only tokens. A successful watcher creation or a running SCM wrapper must remain pending until its runtime is verified.
Reconciliation: none of the earlier five blockers disappeared
I reran the prior 12-case behavioral harness against the same Git-blob-verified current modules: 6 pass, 6 fail, no errors. Two failures remain separate manifestations of F2. These are assertions of required behavior, not failing repository-CI tests.
| Earlier blocker | Current evidence, consequence and repair boundary |
|---|---|
| F1: live-but-draining process loses debt | Real fixture is live during recovery and exits afterward; launches=[], no retained obligation. Liveness filtering still substitutes “alive” for “no accepted stop.” Retain acknowledged drain debt; regress recovery between stop ACK and process exit, with a never-requested-stop control. |
| F2: adoption loses original evidence and cold-start work | Real-Git unrepaired adoption flips gate false→true; cold-start generation becomes absent. Merge projection plus restamping still loses the original contract. Preserve all pending work and its original per-pause evidence; regress early-aborted retry and cold-start-only adoption. |
| F3: old dirty pathname allows different partial-update bytes | Real stash, unchanged HEAD, same dirty pathname with different bytes; gate=true. Path-set subtraction still does not prove content restoration. Keep incomplete-mutation evidence or validate content/index identity; regress already-dirty/autostashed failed checkout and unchanged-user-edit control. |
| F4: unrelated/partial readiness retires all debt | Both watchers created, beta alone ready, alpha obligation absent. Fleet-wide truthy PID acceptance remains insufficient. The additional error/service-only branches above require the same per-runtime settlement repair. |
| F5: recovery start races checkout mutation | Real two-process/checkout-lock witness still starts a harmless fixture with update_live=true and pull marker present. Recovery admission does not exclude mutation through the later watcher spawn. Hold one exclusion/handoff protocol through actual start and update discovery; regress both barrier-controlled arrival orders. |
The initial review's single killed-recovery rediscovery, controlled SCM rollback-retention, and runtime-host argument forms still pass. The basic dead-claim repair works; F7 exposes other custody transitions it does not close. F9 concerns failure isolation, not the repaired rollback-retention case.
RCA / SSA: the same obligation crosses different owners
The durable state must conserve what is owed, which checkout/home owns it, who can execute it, and what evidence settles it. The missing sides are separate consumers of those facts: kernel exclusion for build tails (F6), transfer identity and replay (F7), install identity at the writer (F8), independent progress across runtime kinds (F9), and cross-language marker interpretation (F10). F1–F5 still cover stop intent, safety evidence and launch/settlement. A record-file claim is not checkout ownership, and a launcher/service success is not matched runtime readiness.
The embedded legends distinguish durable state, ownership/policy, transitions and failure paths; evidence remains explicit in the review. The original diagram, before this presentation update, was rendered with Mermaid CLI in dark mode and visually inspected. It maps the pinned record/claim owners, completion admission, runtime consumers, and Electron reader.
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph FLOW["Restart debt, custody and settlement"]
direction TB
O["Outstanding restart debt"]
C["Claim transfer"]
D["F7: two copies and<br/>replacement replays"]
B["F8: foreign checkout relabelled"]
R["Runtime resume"]
S["F9: one SCM start refuses"]
M["Independent manual gateways stay stopped"]
V["F4: debt removed before<br/>readiness"]
O --> C
C --> D
C --> R
O --> B
B --> R
R --> S
S --> M
R --> V
end
LEGEND["Legend<br/>Purple: ownership / policy<br/>Amber: transition / pending<br/>Teal: durable state<br/>Red: failure"]
style FLOW fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
FLOW ~~~ LEGEND
classDef teal fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa,stroke-width:2px
class O,L_teal teal
classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
class C,L_purple purple
classDef amber fill:#451a03,stroke:#fbbf24,color:#fffbeb,stroke-width:2px
class R,L_amber amber
classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
class D,B,S,M,V,L_red red
click O "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L301-L307" "Pinned source"
click C "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L263-L279" "Pinned source"
click D "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L263-L307" "Pinned source"
click B "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_pause_record.py#L181-L193" "Pinned source"
click R "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1361-L1380" "Pinned source"
click S "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1204-L1230" "Pinned source"
click M "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1361-L1380" "Pinned source"
click V "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_cmd_windows.py#L1300-L1323" "Pinned source"
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph FLOW["Cross-language marker ownership"]
direction TB
U["Marker v2 owner and<br/>delegate"]
X["F10: Electron deletes live marker"]
G["Desktop admission gate opens"]
U --> X --> G
end
LEGEND["Legend<br/>Purple: ownership / policy<br/>Red: failure"]
style FLOW fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
FLOW ~~~ LEGEND
classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
class U,L_purple purple
classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
class X,G,L_red red
click U "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_lock.py#L474-L510" "Pinned source"
click X "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/apps/desktop/electron/update-marker.ts#L143-L156" "Pinned source"
click G "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/apps/desktop/electron/update-marker.ts#L143-L156" "Pinned source"
%%{init: {'theme':'dark','themeVariables':{'background':'#0d1117','primaryTextColor':'#e6edf3','lineColor':'#94a3b8'}}}%%
flowchart TD
subgraph FLOW["Lazy completion and checkout ownership"]
direction TB
T["Lazy completion admission"]
L["F6: per-home marker only"]
K["Another process holds checkout lock"]
T --> L
L -->|tail still enters| K
end
LEGEND["Legend<br/>Purple: ownership / policy<br/>Red: failure"]
style FLOW fill:#0d1117,stroke:#30363d,color:#e6edf3
style LEGEND fill:#1e293b,stroke:#94a3b8,color:#f8fafc,font-size:11px
FLOW ~~~ LEGEND
classDef purple fill:#3b0764,stroke:#c084fc,color:#faf5ff,stroke-width:2px
class T,K,L_purple purple
classDef red fill:#450a0a,stroke:#f87171,color:#fef2f2,stroke-width:2px
class L,L_red red
click T "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/venv_sync.py#L398-L420" "Pinned source"
click L "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/venv_sync.py#L398-L420" "Pinned source"
click K "https://github.com/NousResearch/hermes-agent/blob/ac7fdce8c9b3c4dd7c10f4dfc8660980bcac99b0/hermes_cli/update_lock.py#L725-L754" "Pinned source"
Evidence limits and remaining coverage
Python 3.12.14 on Linux; actual pinned TypeScript imported under Node; no project-source modifications. Complete reviewed Python modules and the additional completion/Electron sources were verified against Git blob hashes. The prior harness, claim/custody probes, lazy-lock barrier probe, runtime-error probes, and cross-language marker probe were rerun for this review.
Real behavior exercised: JSON/atomic rename and deletion, disposable Git/stash/markers, process exit, detached harmless fixture starts, POSIX kernel locks, TypeScript reader/gate IO, and their control flow. Windows discovery/SCM/socket/launch/readiness and PM/build boundaries were explicitly substituted; sys.platform was not changed in the retained verification. The canonical repository runner/native Windows E2E/full Electron session were not run locally. CI results above are GitHub observations, not claims that those jobs contain these probes.
Native Windows confirmation remains outstanding for the source-traced default-profile home-overlay risk and for Job assignment before/after completion-child startup; neither is counted as an additional reproduced finding. Unmapped argv capture failure still follows the existing explicitly reported manual-restart fallback, so it is not counted as a new automatic-recovery defect here.
2915b58 to
286b70a
Compare
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Deep code + security review — 286b70a0135cbb0250c13c79d409f5100b706348
Eight P1 correctness / hardening blockers remain at this head. Four are newly demonstrated below; four are prior findings that still reproduce. I did not establish a credential leak, unauthorized external-surface access, or OS-isolation escape in the changed paths. The confirmed security-relevant failures are update-integrity and availability failures inside the local updater trust envelope.
The review is bound to live base 8b66a51036c1e20920a17cdd049fdf55c968d683, merge base 343500b3547e12530457c2fda60ec687e25118b4, and head 286b70a0135cbb0250c13c79d409f5100b706348.
New current-head findings
1. [P1] The Windows Node-build custody path neither preserves installation nor kills the mutation tree
The new contained_command() launcher hands an inheritable job handle to a Python wrapper, and that wrapper calls AssignProcessToJobObject on itself before starting Node. This is the actual path used by run_source_script().
Two independent native-Windows effects fail:
- The exact local repository runner collected three live custody cells: Git and the direct child passed, but
test_killed_owner_takes_its_node_build_downleft five descendants alive after the checkout-lock owner was killed (python.exe,cmd.exe, and the build child chain). Result: 2 passed, 1 failed. - GitHub's x64 and arm64 Windows jobs reproduce that same survivor failure. Separately, the HEAD → NEXT installer journey aborts the fresh install while building the TUI: the wrapper prints
could not join the update job: 5, thensource_completionexits 1.
So the new custody route currently has both failure modes it was meant to exclude: a normal Windows installation can fail at its first Node build, and a killed owner can free the checkout lock while descendants still write. The complete repair needs a parent-controlled spawn boundary: create the actual build root suspended, bind it before any code runs, then resume it; or extend run_contained so it exposes that Popen boundary. A checkout mutator must fail before execution if custody cannot be established. Regress the hosted-runner outer-job case, the .bat → cmd.exe → Python/Node descendant chain, owner death, and a successful fresh installer.
2. [P1] The shared marker protocol still has active writers that bypass its mutex and exclusive claim
The new Rust/Python protocol says every marker decision and mutation is serialized, but the preferred live Windows handoff still uses unconditional WriteAllText and PID-only Remove-MarkerIfOwned. The POSIX handoff has the same legacy printf > marker / first-line removal pattern. Electron's unchanged reader/writer remains the destructive v1 consumer from F10.
Current-head native probe:
live Python owner: 33912, v2 marker present
real windows.ps1 -SelfTestMarker: exit 0, rewrote marker to dead v1 owner 39848
original owner after handoff: still alive
fresh Python reader: no live update; marker removed
second UpdateLock acquire while first owner was alive: acquired=true
The checkout lock can still refuse a second checkout mutation, but the marker itself is no longer a trustworthy exclusion or Desktop launch-gate fact. Migrate every active handoff producer/cleaner and the Electron reader/writer in the same rollout, using the same sidecar mutex, incarnation rules, exclusive/adopt semantics, and identity-based release. The cross-language suite needs the real PowerShell and POSIX handoffs as producers, not only Python/Rust readers of prepared fixture bytes.
3. [P1] Recovery upgrades a marker the gateway rejects into permanent stop evidence
_request_on_disk() checks only target_pid and stopper_pid. It does not apply the gateway's 60-second expiry or start-time predicate from _read_live_pid_marker(). drop_never_stopped() then permanently copies that disagreement into stop_sent.
Executed current-head sequence with a real live child and the actual gateway marker parser:
pause record accepts 120-second marker: true
gateway consumer accepts it: false; marker unlinked
first recovery persists stop_sent=[pid] and keeps profile=default as draining
process later exits; next recovery invokes resume(default)
That later exit can be the user's deliberate stop, so this reintroduces exactly the behavior N1 says it prevents: a gateway the update never successfully asked is restarted later. Derive pause evidence from the same expiry / target-identity / home predicate the consumer uses, or persist an acknowledgement the consumer owns. Regress expired and start-time-mismatched markers, plus a fresh accepted-marker control, through later user stop.
4. [P1] Any argument value equal to update suppresses startup recovery
The new recovery call is nested under raw membership: if "update" not in sys.argv[1:]. The recovery function already parses the command correctly and skips only actual update and gateway run commands.
A real CLI current-head probe seeded an orphaned no-work record, then ran two valid version invocations:
hermes --resume update --version -> exit 0, record still present
hermes --resume ordinary-session --version -> exit 0, record retired
A session id, option value, branch-like value, or prompt argument equal to update can therefore leave the gateway outage in place for an entire long-lived launch. Call recovery unconditionally and let recover(command_argv(...)) own the command exclusion, or gate on the parsed command. Regress option values separately from the actual hermes update command.
Prior-finding reconciliation at this head
The earlier findings were treated as hypotheses and rechecked against current bytes and behavior.
- Still reproducible — F2, adoption replaces the orphan's evidence. The original token rejects a real partial checkout (
gate=false);record_pause()merges it, re-stamps the same unrepaired tree, changesdirty_at_pausefrom[]to['a.py'], dropscold_start_profiles, and the adopted token returnsgate=true. Prior thread: F2. - Still reproducible — F3, pathname equality is not content restoration. A token stamped with dirty
a.pyaccepts different later partial-update bytes ata.pywith unchanged HEAD (gate=true). Prior thread: F3. - Still reproducible — F5, tree admission is not held through start. Recovery's real tree gate returned true; before the restart effect, a competing process acquired the checkout lock and published the interrupted-pull marker. The effect then observed
update_in_progress=trueandinterrupted_pull_present=truewhile startingdefault. Prior thread: F5. - Still present — F4 service-only settlement. Manual profiles now retain debt per target, but
_resume_windows_services()still removes a service after SCM wrapper state alone, and the outcome treats its profile as relaunched without gateway readiness. This is the previously reproduced service-only branch of F4.
The original killed-recovery claim, acknowledged-drain case, controlled SCM rollback, runtime-host argv forms, claim-transfer duplication (F7), foreign-checkout relabeling (F8), and independent-runtime starvation (F9) are repaired in the inspected paths. Completion now acquires checkout custody, but finding 1 shows that the new Windows Node propagation mechanism does not yet carry that repair through the actual build process tree.
Verification boundary
- Native Windows, Python 3.14.7, Git 2.53.0.windows.3.
- Exact current-head custom probes exercised real process liveness, kernel checkout locks, real Git state, the real PowerShell handoff, real CLI dispatch, gateway marker expiry, and recovery record transitions.
- Focused repository runs: marker corpus 61 passed; update-lock protocol 5 passed / 6 skipped on Windows; source-completion 9 passed; venv-sync checkout test 1 passed; update custody 1 passed / 2 POSIX-only skipped; Windows live custody 2 passed / 1 failed. The broader batch also exposes a native CRLF byte-identity failure in
test_update_lock.py. - Current GitHub state includes the x64 and arm64 Windows-only failures and the HEAD → NEXT installer failure above. Native SCM gateway readiness, a full Electron launch, macOS handoff behavior, and power-loss durability were not exercised locally.
4fd7836 to
5d299f1
Compare
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Current-head re-review — 5d299f1983a5ba46a09b5c0abbeeeb7d3bbd4a5d
The ordinary Windows Node-build custody failure is repaired, but the custody contract now explicitly fails open. Eight P1 correctness / hardening blockers therefore remain.
This review is supplemental to review 5405535944. Relative to that reviewed head, the product delta is confined to hermes_cli/update_custody.py; the other edits repair its Windows tests. Every file carrying the other seven findings is byte-identical between 286b70a0135cbb0250c13c79d409f5100b706348 and this head.
[P1] A refused job join now runs the checkout mutator without custody
The base-interpreter change in _join_launcher_python() closes the demonstrated venv-redirector failure: the direct, Git-child, and Node-build owner-death cells now pass. The stable-ZIP platform rows also pass 4/4 after moving fault injection to the actual custody seam.
But the new refusal branch at _JOIN_JOB prints a warning and then unconditionally executes the checkout writer outside the job. The sibling failure path in contained_command() also yields the raw command when the inheritable job handle cannot be obtained. Both paths release the checkout lock with the owner while allowing a child that may still write to survive.
Current-head executed failure path:
job join: refused (error 6); child runs outside it
owner exit: 71
wrapper alive after owner exit: true
checkout-writing child alive after owner exit: true
second UpdateLock while child is alive: acquired=true
This violates the invariant the custody work introduced: “checkout lock free implies no update-tree mutator alive.” The normal-path fix does not make refusal safe. A checkout mutator whose custody cannot be established must not start; availability degradation is valid only for a read-only child whose lifetime cannot affect the tree. Regression needs the refusal schedule while a real checkout lock is held: either no child executes, or a contender remains refused until every child is gone.
The new test_a_refused_job_join_still_runs_the_child now executes under the canonical runner, but it pins the unsafe branch as desired behavior. Its oracle should be inverted for checkout writers: refusal means the child body did not execute.
Reconciliation of the previous review
The preceding review's Node normal-path failure and Windows test-routing failures are repaired in the tested paths. The other seven P1 findings remain mechanically and behaviorally applicable because all of their load-bearing source blobs are unchanged:
- Marker protocol rollout is still incomplete: Rust/Python protocol, PowerShell/POSIX handoffs, and Electron reader/writer files are byte-identical. The demonstrated live-owner overwrite and second marker acquisition remain applicable.
- Expired planned-stop marker authority remains:
update_pause_record.pyandgateway/status.pyare byte-identical; recovery can still persist a marker the gateway rejects asstop_sentand later undo a user stop. - Raw
updateargument suppression remains:main.pyis byte-identical; option values equal toupdatestill bypass startup recovery. - F2 remains: adoption still re-stamps the unrepaired checkout and drops cold-start evidence.
- F3 remains: a previously dirty pathname still authorizes different later bytes.
- F5 remains: tree admission still is not held through the actual gateway start.
- F4 service-only settlement remains: SCM wrapper state still retires service debt without matched gateway readiness.
No new credential-exfiltration, external-authorization, or OS-isolation crossing was established in the current product delta. The retained P1 is an updater integrity/availability failure inside the local trust envelope.
Verification boundary
- Exact head:
5d299f1983a5ba46a09b5c0abbeeeb7d3bbd4a5d. - Native Windows, Python 3.14.7.
test_update_lock_windows_live.py: 4 passed.test_update_target_identity.py -m platforms: 4 passed on the immediately preceding product-identical head; this head changes only the live-system-guard fixture shape in the other test file.- The refused-join custody probe used a real checkout lock, real subprocesses, the actual
_JOIN_JOBbody, owner process exit, and a real competingUpdateLock; the child was allowed to exit through its normal completion flag after the observation. - PR CI was still running during this review; no green-state claim is made.
JoaoMarcos44
left a comment
JoaoMarcos44
left a comment
There was a problem hiding this comment.
Teknium, I re-read the existing review series, including the linked review and the subsequent correction concerning the ordinary Node launcher versus its refused-join fallback.
Reviewed head: 5d299f1983a5ba46a09b5c0abbeeeb7d3bbd4a5d. Base captured: 8b66a51036c1e20920a17cdd049fdf55c968d683. This is still a stack on #132365.
Verdict: I would not merge this head yet. There is one newly reproduced cleanup-error variant of the previous F7/R7 duplicate-obligation problem, plus three previous findings independently reproduced again below. I am adding one new inline rather than duplicating the existing findings' threads.
N1 — [P1] Failed source retirement resurrects an already-completed recovery
Sources: claim publication and suppressed source unlink, successful claim discharge and next-launch recovery.
The stable ID and mutex repair the previously reported crash/steal paths, but do not conserve completion when cleanup itself fails:
- An orphaned source record exists. A Windows reader opens it with
FILE_SHARE_READ | FILE_SHARE_WRITE, withoutFILE_SHARE_DELETE. claim()successfully publishes a new claim with the samepause_id. Itssrc.unlink()fails with a real Windows sharing error, which is suppressed. The function nevertheless returns the new claim as acquired.- Recovery completes.
_hand_back()deletes the new claim because nothing remains owed. The original source still contains the old, unfinished obligation. - The reader closes. A later launch sees the old source with no surviving completion carrier against which to deduplicate it, claims it, and invokes recovery for the same target again.
retire_redundant() at the beginning of recovery does not close this hole: the duplicate is created later, during claim(). Also, sync() explicitly does nothing for token['recovery'], so the real resume wrapper does not independently retire the canonical source.
Observed effect: the resume boundary received alpha twice after its first successful completion. This is not merely an extra file. The actual profile restart builder emits gateway run --replace; replay can therefore replace an already healthy gateway. I did not disrupt a live gateway to measure that consequence: the repeated resume invocation was executed, and the replacement consequence follows from the downstream builder.
Executed reproduction and negative controls
Native Windows 11 (10.0.22621), Python 3.14.1, psutil 7.1.3; private temporary homes and immutable source snapshots fetched at the SHA above. The filesystem operations, JSON persistence, process-incarnation checks and msvcrt mutex were real. No mocked unlink() or synthetic filesystem exception was used. Only the gateway resume effect was replaced by a recorder that clears its work and sets resume_needed=False.
For each source shape, the probe wrote a valid orphan token with a stable pause_id, opened the native read handle, called recover(['status']), closed the handle, and called recovery twice more. It then repeated the case with FILE_SHARE_DELETE added to the handle's sharing flags.
| Case | Executions | Observed | Required |
|---|---|---|---|
Canonical .json, reader denies deletion |
5 | Two resume calls; old source survives first completion | One completed recovery overall, or deferred recovery until transfer can finish |
Abandoned .claim, reader denies deletion |
5 | Same duplicate resume | Same invariant |
| Both source shapes, reader permits deletion | 10 | Exactly one resume; subsequent calls do nothing | Control passes |
| Canonical source, three separate recovery processes, deletion denied during first | 5 | Process 1 resumes; process 2 resumes again; process 3 does nothing | No second completed recovery |
| Same fresh-process sequence, deletion permitted | 5 | Only process 1 resumes | Control passes |
The last two rows used 30 fresh Python process invocations, not repeated calls sharing interpreter state. All five failing sequences and all five matched controls agreed.
Required repair: do not discard the only durable completion/deduplication evidence while any source or absorbed copy can still advertise unfinished work. Either make failed source retirement prevent execution while leaving a safely recoverable obligation, or retain an explicit completed carrier/tombstone until all redundant sources are confirmed retired. Merely adding another best-effort unlink and deleting the carrier anyway recreates the same failure.
Regression contract: exercise the real Windows sharing mode above for both canonical and abandoned-claim sources. Hold the handle through successful recovery, then close it and perform two fresh-process recoveries. Completion must not be repeated. Also retain the existing kill-between-publication-and-retirement coverage: solving cleanup failure must not lose obligations after process death. Check the sibling release_claims()/absorbed-source retirement path against the same invariant; I am not counting that sibling as a separately reproduced finding.
Previous findings rechecked, not presented as new
These each reproduced in five fresh fixture runs on the current head:
| Existing finding | Current-head observation | What still needs correction |
|---|---|---|
| F2: adoption loses the token contract | In a real temporary Git repository, a modified tracked file makes the original gate return False. Actual adopt_orphans() → record_pause() changes the gate to True without repairing the bytes. An adopted cold_start_profiles entry also disappears. |
Preserve the original safety evidence and all pending cold-start work across adoption; do not certify the unrepaired current tree merely by restamping it. |
| F3: pathname-only dirty baseline | With unchanged HEAD and a path already dirty at pause time, replacing its bytes still produces tree_is_whole=True. The unchanged-original-bytes control also returns True, so the gate cannot distinguish them. |
Retain content/index or incomplete-mutation evidence that distinguishes the allowed original edit from a later partial write. |
| F4: SCM-only readiness | The actual _resume_paused_set() with a successful SCM-start boundary makes zero calls to the gateway-readiness probe and clears resume_needed. SCM and readiness effects were recording stubs, not real service operations. |
Require the matching gateway runtime to become ready before retiring its service obligation; SCM wrapper state alone is insufficient. |
The seven-case primary matrix ran five times: 35 case executions, 25 invariant-failure observations and 10 passing controls. Those are repeated observations of four finding groups, not 25 different bugs. The fresh-process experiment above is additional evidence for N1.
Reconciliation with the rest of the review history
I am not repeating the original ordinary-path Node redirector failure as still present: the later review distinguishes its repair from the remaining refused-job-join fallback. Similarly, N1 does not assert that the old claim-publication race was never repaired; it identifies an additional real I/O-failure path after that repair.
This pass does not clear the other previously reported obligations: marker producer/consumer parity, expired planned-stop evidence, raw update argument membership, and checkout exclusion through the actual recovery start. Their prior receipts remain in their respective threads; I am not claiming new native reproductions for them in this pass.
Validation limits: these were focused executable probes, not a run of scripts/run_tests.sh or the full Windows installer suite. The new failure uses real native Windows file-sharing semantics and real recovery/persistence code, but gateway/SCM effects and ambient home/config plumbing were isolated test boundaries. No live service or gateway was stopped, no production source was changed, and no CI-green claim is made. Source blob hashes were rechecked against the fetched-head manifest after the primary probes.
Please fix N1 together with the still-failing prior contracts above before treating durable recovery as completed exactly once.
5d299f1 to
6b44b5e
Compare
a2d6e51 to
e6a1dd4
Compare
e6a1dd4 to
35d63bf
Compare
_pause_windows_gateways_for_update decided whether to abandon the durable pause record by searching the error text for "rollback failures"; rewording that message would delete the record while gateways are still stopped. The service pause now raises ServicePauseFailed carrying rollback_failures, and the record is abandoned only when that list is empty. Any other exception keeps the record.
…tdout update_pause_record.recover() resumes paused gateways ahead of every hermes command and prints its own notices to stderr, but the Windows resume path it calls printed deferral/restart notices to stdout, corrupting the output of unrelated commands (--json ones included). A recovering resume now routes its stdout to stderr.
Retiring each paused runtime on its own readiness made the wait serial: every profile and unmapped entry ran its own 2 s confirmation window (plus a 6 s per-target floor past the deadline), so 3 profiles took 6 s and 5 took 10 s where the fleet-wide wait had taken one window. All targets now share one poll: one fleet process scan per tick, each target credited only on its own hits for a whole confirmation window, and a PID one target vouched for is never credited to another. Measured with readiness stubbed to succeed: N=1/3/5 went from 2.0/6.0/10.0 s to 2.0 s flat (N=10 also 2.0 s). The reconciliation tests' liveness stub moves to the probe the verifier now polls (_live_gateway_pids); their assertions are unchanged.
…ee gate first Only the principal pull recorded its target (mark_move) before git wrote the tree. The first branch switch in _prepare_checkout_for_update (and its `checkout -B` fallback), a fork's early and late upstream sync, and the post-pull syntax rollback wrote the tree unrecorded: a torn switch or sync at an unmoved HEAD was judged against whatever refs a later fetch left, and a failed second move (upstream sync after a successful pull) had no baseline at its starting HEAD at all, so the gate admitted the torn tree. All of them now run inside _checkout_move(token, *targets). Before git runs it persists the resolved target(s): on the pause's own baseline when the step starts from the pause HEAD (it knows the edits already there), otherwise on a new baseline taken at the step's starting HEAD with its dirty digests. A record that cannot be written raises, so the step does not run. Once git returns, a per-step baseline is retired if HEAD moved or the tree is exactly as the step found it, so a clean refusal never holds the set. The upstream sync takes the context as `checkout_move`; the pull's move is extracted to _move_checkout_to so _pull_updates does not nest deeper.
…ommit it reached
`git checkout <existing branch>` can exit 0 and move HEAD while leaving a file
unwritten ("unable to unlink old ..."): HEAD and the index name the new commit,
the file keeps the old bytes. `tree_is_whole` only judged baselines at an
unmoved HEAD, so that torn tree was admitted and the paused gateways started
on it.
`_checkout_move` now judges every move the moment git returns: the paths
between the commit it started from and the one HEAD reached that are changed
and were not already there byte for byte (`update_pause_record.torn_by_move`)
are recorded on the baseline as `landed[<head>]`. The gate holds while any of
those paths is still changed. Judging at return time, not at resume, keeps a
later dependency sync or stash restore that rewrites an upstream-changed file
(package-lock.json) from holding the set. A recorded move the updater died
before judging is judged at resume time against what the pause saw.
A step's own baseline is retired only when its move landed whole (it was
retired on any HEAD move before). The moved-HEAD gate reuses tracked_changes
and the dirty digests the record already carries.
…fore writing files Round 6 bound the branch switch, the upstream syncs, the pull and the rollback. Other steps that run while the Windows pause holds the gateways still rewrote tracked files unbound: - `git stash push` and its cleanup `reset --hard`; - `git stash apply` and the reset after a conflict; - the rejected-restore `reset --hard` + `clean`; - the lockfile-churn `checkout -- <locks>` and the line-ending `checkout`. If one of them was interrupted mid-write, the gate judged the torn file only against whatever refs a fetch had left. A restore after the pull had no baseline at the moved HEAD at all. Either way the paused set was admitted. Each step now runs inside `_checkout_move`. `paths=` binds the files it rewrites in place, and `revert=True` binds every tracked change it finds, because it puts them back to HEAD. A restore binds its stash commit. The gate counts those paths as writable. A restore that applied its tracked changes in full reports `whole`, so the tree it changed by design retires its baseline instead of holding the set. Removed the "already up to date" switch-back `checkout <current_branch>`. It only ran when no checkout had left current_branch: in-place and release updates never switch, and a parked switch is excluded. It was a no-op that nothing gated. Helpers take a `checkout_move` binder (`_moves_for(token)`); test doubles of `_prepare_git_command` accept its new keyword.
When the gateway's home refuses a new .accepted receipt, mark_stop_accepted stamps the kept request itself with accepted=true; that stamp is then the only durable trace that this draining incarnation accepted the update's stop. _read_live_pid_marker unlinked every expired marker, so a second signal past the request TTL (another Ctrl+C / SIGTERM, which calls the consumer again) or a planned-stop watcher probe in the same home deleted it, and recovery then retired the restart debt of a gateway that was still draining. An expired stamped request still matches nobody (no PID-reuse grief), but the TTL cleanup no longer unlinks it.
…on disk and drops settled receipts consume_planned_stop_marker_for_self took .hermes-update-paused-gateways.lock on every platform for every stop (up to 2 s when busy), creating the lock file in the root home, and on a busy lock fell back to keeping the request and writing a .accepted receipt that nothing ever removed. The lock only protects the checkpoint into a Windows update pause record; with no record or claim of this checkout on disk (every non-Windows host, or a pause recovery/the updater already retired) there is nothing to checkpoint. The consumer now holds the lock only when a pause is on disk. The checkpoint hook re-checks after the request was read, so a pause that appeared after the first look (an update writes its record before its request) is still checkpointed under the lock. With no pause on disk, the consumer removes the .accepted receipt and an expired stamped request an earlier drain left: their debt is settled. While any pause is on disk both stay (never earlier). test_a_busy_pause_mutex_never_turns_a_planned_stop_into_an_unplanned_one now puts a pause record on disk first: without one, keeping the request on a busy lock was the leftover this fixes; with one, its assertions (planned, request retained, no block) are unchanged.
… verifier polls The post-update relaunch verifier now polls every target in one loop through `gateway_windows._live_gateway_pids` (d243ecf); these Windows-only tests still stubbed only `_wait_for_gateway_ready`, so the relaunched `beta` was never seen and resume raised "relaunch ... was not verified alive: beta". The same probe also answers "is this profile already running?" before a cold start, so the stub follows the lifecycle: a home answers (with a fresh PID, through the verifier's own PID filter) only once its profile was relaunched via `launch_detached_profile_gateway_restart`. A profile that has not started yet still reads as down and is cold-started, and one profile never vouches for another. Assertions are unchanged.
…out_move #132386 added three ZIP tests that replace update_cmd._prepare_git_command with a no-argument lambda. This branch calls it as _prepare_git_command(checkout_move=...), so after the rebase onto main each fake died with "unexpected keyword argument 'checkout_move'". They now take **_ like every other fake of that function in the tree; the assertions are unchanged.
…n_install_repair On main, #132365 moved container-exec routing out of main.py, which lowered main.py's code-health line cap to its new size (3643). This branch's paused-gateway recovery added four lines to main() and pushed the file to 3647, so the code-health check refused it. The owed-fleet-restart warning and the paused-gateway recovery move together into main_install_repair._recover_update_debts_on_startup, next to the Windows quarantine sweep that already lives there; main() calls it at the same point, so startup order and behaviour are unchanged. Both steps stay late-imported, so tests patching update_cmd_fleet._warn_pending_fleet_restart_on_startup or update_pause_record.recover still intercept them. The fleet warning's silent `except Exception: pass` now logs at debug with the traceback, which is what the health check asks of a catch-all boundary in new code; a failure there still never blocks the launch.
…pts checkout_move The same #132386 interaction as the ZIP completion tests: this test (from main) replaces update_cmd._prepare_git_command with a no-argument lambda inside its child process, and this branch calls it with checkout_move=..., so the child died with TypeError and the test read it as a failed update. The fake takes **_ like every other fake of it.
08aeda0 to
6bb7daa
Compare
There was a problem hiding this comment.
Re-review: two P1 debt-loss paths and one P2 readiness defect remain
Reviewed 08aeda0b4134, against the current main snapshot. Reconciled with my prior review at 4be55b50e776 and the latest author response. This is not an approval.
F1 — P1: failed acceptance persistence can permanently retire debt while the gateway still drains
This is the still-present accepted-stop finding, not a new count of the same original defect. The additional downstream result matters: the debt does not necessarily return when the draining process finally exits.
The consumer fallback still returns true when the record checkpoint fails and both acceptance writes fail. Consider an updater killed before its stop_sent checkpoint lands, with the stop intent and original request already durable. Restore storage access after the gateway accepts, age the un-stamped request past its 60-second TTL, and run recovery before that same gateway incarnation finishes draining.
The request classifier and drop_never_stopped see an expired request and absent receipt as false, not unknown. They remove the live profile. With no remaining work, _resume_fenced clears resume_needed; _hand_back then retires the obligation and deletes its carrier. A later gateway exit cannot reconstruct that retired record.
Source-excerpt probe result: accepted=true, gateway live at recovery, retired_ids=[old-pause], zero remaining record/claim files, and zero resume calls after the simulated later exit. Controls allowing either the sidecar or the in-place stamp retained the live debt and reached the resume seam after exit.
Repair: preserve a durable unresolved-delivery state established before the irreversible stop, rather than treating missing acceptance evidence plus expiry as proof that delivery never happened. This does not require turning an already-planned stop into a failure exit. Regression: deny the producer/consumer checkpoint and both fallback writes, restore storage before recovery, recover while still draining, then finish the drain and recover again. Assert that the obligation is never retired before a verified restart.
F2 — P1: orphan adoption loses the evidence needed to preserve an unknown stop
drop_never_stopped correctly retains a live entry when its request/receipt cannot currently be read. It does not mark that entry sent. But adopt_orphans and record_pause fold the entry through merge_into, which carries stop_requested, stop_sent, identities, profiles and absorbed IDs—not stop_markers, stopper_pid, or an explicit unresolved verdict.
The reachable interruption boundary is particularly small: the new updater successfully rediscovers the live gateway and publishes record_pause, which deletes the old claim, then dies before the next mark_stop_requested call. No discovery failure is required. On recovery, the replacement record has no marker location. _request_on_disk returns false without consulting the original acceptance receipt—even when that receipt has become readable again. The live obligation is dropped and both the new and absorbed pause IDs can be retired.
Probe result: the original token retained the live profile under an injected read refusal; after adoption/publication the proof fields were absent, the old claim was gone, and recovery retired old-pause and new-pause without a resume. Controls: unknown evidence without merging stayed owed; readable evidence survived adoption because it had already been resolved into stop_sent.
Repair: carry unresolved evidence and its original stopper/incarnation per obligation through every merge. A single replacement stopper_pid cannot describe several adopted delivery histories. Apply the same representation through the write collision merge, finish_pause and abandon_pause, not only the first adoption. Regression: transiently deny reads of accepted evidence, adopt, publish, interrupt before the new request checkpoint, restore reads, and verify that the original live debt remains owed.
F3 — P2: target-level elapsed time credits a succession of short-lived PIDs as one stable gateway
The new _poll_until_ready stores since by target key and resets it only on an empty probe. It never verifies that the same process incarnation survived the confirmation window. With a different PID on every nonempty tick, _verify_relaunched_gateways_alive credits the last replacement and removes the profile's debt.
Deterministic probe: observations (0.0,50000), (0.4,50001), (0.8,50002), (1.2,50003), (1.6,50004), (2.0,50005) produced relaunched_profiles=['p'], profiles={}. No observed PID completed a two-second confirmation interval. Stable-PID and empty-gap controls behaved as expected; a disappearing gateway retained its debt.
Sibling attribution: the unchanged gateway_windows._confirm_gateway_stable has the same identity-blind behavior and reproduced it in the companion probe. This weakness is not solely introduced by parallelization. The new poll carries it into per-target durable retirement; the SCM readiness path still consumes the older helper.
Repair: bind provisional readiness to the observed PID and creation time, reset confirmation when that identity changes, and keep the deadline bounded. Preserve the shared fleet scan and one-PID/one-debt exclusion. Cover PID churn in both helpers, alongside the existing parallel, disappearing, and duplicate-credit tests.
SSA — authority and consumer inventory
| Boundary | Authority/consumer | Review result |
|---|---|---|
| Discover → persist → stop | Windows pause producer | The publish-before-next-request interruption is reachable; F2. |
| Accept → preserve evidence | Planned-stop consumer | Accepts despite failure of all persistence paths; F1. |
| Adopt → serialize → release old carriers | Record lifecycle | Unknown delivery provenance must survive all merger consumers; F2. |
| Classify → drain/launch → retire | Recovery and hand-back | False “never asked” classifications become durable retirement; F1/F2. |
| Observe → confirm → retire runtime | Parallel verifier and older confirmation helper | Nonempty samples are not continuity of one incarnation; F3. |
%%{init: {"theme":"dark","themeVariables":{"darkMode":true,"background":"#090d14","primaryTextColor":"#f8fafc","lineColor":"#94a3b8","clusterBkg":"#0f172a","clusterBorder":"#475569","edgeLabelBackground":"#111827"},"flowchart":{"htmlLabels":true,"curve":"linear"}}}%%
flowchart TB
subgraph CANVAS[" "]
direction TB
subgraph MAIN[" "]
direction TB
A["Discover runtime identity"]:::entry --> B["Persist pause authority"]:::durable
B --> C["Request and accept stop"]:::pending
C --> D["Delivery evidence"]:::durable
B --> E["Adopt and merge orphan"]:::pending
E --> D
D --> F["Classify asked / unknown / never asked"]:::owner
F --> G["Live drain remains owed"]:::pending
F --> H["Relaunch and confirm identity"]:::pending
H --> I["Verified debt settlement"]:::success
F -. "F1 / F2" .-> J["Unproven retirement"]:::failure
H -. "F3" .-> J
end
subgraph LEGEND["Legend"]
direction LR
LE["Entry / incoming"]:::entry ~~~ LO["Ownership / policy"]:::owner ~~~ LP["Transition / pending"]:::pending
LD["Durable state"]:::durable ~~~ LS["Successful outcome"]:::success ~~~ LF["Failure"]:::failure ~~~ LC["Context / transient"]:::context
end
MAIN ~~~ LEGEND
end
style CANVAS fill:#090d14,stroke:#090d14,color:#f8fafc
style MAIN fill:#090d14,stroke:#090d14
style LEGEND fill:#0f172a,stroke:#475569,color:#f8fafc
classDef entry fill:#172554,stroke:#60a5fa,color:#eff6ff
classDef owner fill:#3b0764,stroke:#c084fc,color:#faf5ff
classDef pending fill:#451a03,stroke:#fbbf24,color:#fffbeb
classDef durable fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa
classDef success fill:#052e16,stroke:#4ade80,color:#f0fdf4
classDef failure fill:#450a0a,stroke:#f87171,color:#fef2f2
classDef context fill:#1e293b,stroke:#94a3b8,color:#f8fafc
click A "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_cmd_windows.py#L943-L988"
click B "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L743-L756"
click C "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/gateway/status.py#L1990-L2009"
click D "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L496-L516"
click E "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L724-L756"
click F "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L792-L852"
click G "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L854-L862"
click H "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_cmd_windows.py#L1451-L1530"
click I "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L993-L1006"
click J "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L993-L1006"
RCA — two distinct ways to destroy the same delivery uncertainty
%%{init: {"theme":"dark","themeVariables":{"darkMode":true,"background":"#090d14","primaryTextColor":"#f8fafc","lineColor":"#94a3b8","clusterBkg":"#0f172a","clusterBorder":"#475569","edgeLabelBackground":"#111827"},"flowchart":{"htmlLabels":true,"curve":"linear"}}}%%
flowchart TB
subgraph CANVAS[" "]
direction TB
subgraph MAIN[" "]
direction TB
A["F1: all acceptance writes refused"]:::failure --> B["Expired request; no receipt"]:::context
C["F2: evidence temporarily unreadable"]:::context --> D["Unknown retained in original token"]:::context
D --> E["Merge omits original evidence provenance"]:::failure
B --> F["Classifier returns never asked"]:::failure
E --> F
F --> G["Live profile removed before drain completes"]:::failure
G --> H["No work: retire pause IDs and carrier"]:::failure
H --> I["Later exit has no debt to recover"]:::failure
A -. "either fallback succeeds" .-> J["Accepted evidence survives expiry"]:::durable
D -. "without lossy merge" .-> K["Live obligation remains owed"]:::pending
J --> K
end
subgraph LEGEND["Legend"]
direction LR
LE["Entry / incoming"]:::entry ~~~ LO["Ownership / policy"]:::owner ~~~ LP["Transition / pending"]:::pending
LD["Durable state"]:::durable ~~~ LS["Successful outcome"]:::success ~~~ LF["Failure"]:::failure ~~~ LC["Context / transient"]:::context
end
MAIN ~~~ LEGEND
end
style CANVAS fill:#090d14,stroke:#090d14,color:#f8fafc
style MAIN fill:#090d14,stroke:#090d14
style LEGEND fill:#0f172a,stroke:#475569,color:#f8fafc
classDef entry fill:#172554,stroke:#60a5fa,color:#eff6ff
classDef owner fill:#3b0764,stroke:#c084fc,color:#faf5ff
classDef pending fill:#451a03,stroke:#fbbf24,color:#fffbeb
classDef durable fill:#134e4a,stroke:#2dd4bf,color:#f0fdfa
classDef success fill:#052e16,stroke:#4ade80,color:#f0fdf4
classDef failure fill:#450a0a,stroke:#f87171,color:#fef2f2
classDef context fill:#1e293b,stroke:#94a3b8,color:#f8fafc
click A "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L496-L516"
click B "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L792-L852"
click C "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L810-L832"
click D "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L833-L852"
click E "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L864-L904"
click F "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L792-L852"
click G "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L833-L852"
click H "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L993-L1006"
click I "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L1009-L1022"
click J "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L496-L516"
click K "https://github.com/NousResearch/hermes-agent/blob/08aeda0b4134fcc1058dbdb2e559708d354f668b/hermes_cli/update_pause_record.py#L993-L1006"
Reconciliation with the prior review
| Prior item | Current disposition and evidence boundary |
|---|---|
| First checkout lacked its own move binding | Repair present in source: initial checkout and fallback now enter _checkout_move before Git writes. The stash, upstream merge, cleanup and rollback context plumbing was inspected. Evidence gap: the earlier native interrupted-checkout schedule was not rerun here; it is not republished as an open reproduced finding. |
| Accepted-stop evidence loss | Still present: F1 follows the failure through permanent retirement, contradicting the bounded-to-drain conclusion in the author response. |
| Newly unknown home received legacy matching | Repaired at the tested predicate boundary: missing field versus null field are distinguished. Four controls accepted legacy missing-home matching, rejected new null, accepted the exact home and rejected a different home. The producer now refuses unproved homes before stopping; native discovery was not rerun. |
| Unreadable retired-list and detached-writer/custody controls in the previous review | No new finding asserted. Those earlier native results remain historical evidence, not fresh current-head passes. This run does not claim a new sharing-violation or process-group-kill matrix. |
Execution receipts and remaining validation boundary
Executed 10 focused probe methods using selected function bodies transcribed from this head. These execute the consumer fallback, JSON/atomic-file operations, adoption/serialization, recovery classification, _hand_back retirement, the new verifier and the sibling confirmation helper. Filesystem failure points, process identities, locks, discovery, launch effects and the polling clock were injected; recovery fixtures begin at _resume_fenced after a successful claim. Passing probe assertions include assertions of the defective outcomes—they are not a green product verdict.
No full-repository test run, native Windows/macOS execution, or new real-process crash experiment is claimed. Current source and inline anchors were reread from the pinned GitHub files. At the CI read, CI was still running; Install & Update E2E had succeeded. Neither result establishes coverage of the fault schedules above.
The acceptance tests currently exercise a working sidecar or in-place fallback, and the parallel-poll tests exercise disappearance, not continuous PID replacement. Please add the three regression schedules at the actual producer/consumer/recovery boundaries, preserving the positive controls and covering the shared merger/readiness consumers in the same repair.
| except OSError as exc: | ||
| print(f" ⚠ Could not record the accepted stop request {path}: {refused}; {exc}", file=sys.stderr) |
There was a problem hiding this comment.
F1 — P1: accepting without any durable acceptance evidence can permanently retire a live drain's restart debt. This is the prior accepted-stop finding, now followed through retirement. Start with a durable intent/request but no stop_sent checkpoint; deny the record checkpoint, sidecar creation and in-place write. The consumer still returns true. Restore writes, expire the unstamped request, and recover while the same incarnation is still draining. drop_never_stopped drops the profile; _hand_back retires its ID and deletes its carrier. In the source-excerpt probe, later exit found zero outstanding files and made zero resume calls. Thus the gap is not necessarily bounded to the drain, as the response claims. Persist delivery uncertainty before the irreversible stop and do not resolve it to 'never asked' from missing receipt + expiry. Control: allowing either fallback retained debt and resumed after exit. Add the full accept → expire → recover while live → exit → recover regression.
| token["stop_requested"] = sorted({*map(str, token.get("stop_requested") or []), *map(str, requested)}) | ||
| token["stop_sent"] = sorted({*map(str, token.get("stop_sent") or []), *map(str, sent or [])}) |
There was a problem hiding this comment.
F2 — P1: this merge carries unresolved obligations without the evidence needed to keep them unresolved. When an old request/accepted receipt is temporarily unreadable, drop_never_stopped retains the live gateway but leaves it out of stop_sent. This merge drops stop_markers and stopper_pid. record_pause publishes that lossy token and removes the old claim before the next stop-request checkpoint. Kill the new updater in that gap—even with successful rediscovery of the same gateway. The next recovery returns false for the missing marker location and retires the still-live obligation, even after the original accepted receipt becomes readable. The source-excerpt probe retired both new and absorbed pause IDs without resuming. Carry per-obligation evidence provenance or an explicit durable unresolved-delivery state through adoption and every other merge_into consumer; a replacement global stopper ID is insufficient. Controls: unknown evidence without merging stayed owed; readable evidence survived the merge via stop_sent.
| if now - since.setdefault(key, now) >= _READY_CONFIRM_S: | ||
| ready[key] = vouched |
There was a problem hiding this comment.
F3 — P2: reset confirmation when the observed process incarnation changes. since is keyed only by the target and is cleared only for an empty probe. Six nonempty observations of six different PIDs at 0.0/0.4/0.8/1.2/1.6/2.0 seconds therefore credit the last PID as stable. The source-excerpt verifier probe returned relaunched_profiles=['p'] and removed profiles['p'], although no observed PID completed the two-second window. This is not merely a post-poll death: churn occurs during confirmation. Track PID plus creation time per provisional target, reset on identity change, and keep the deadline bounded while preserving the shared scan and one-PID/one-debt rule. The unchanged sibling helper reproduced the same weakness, so this is not solely introduced by parallelization; repair both consumers. The existing flap test goes to None and cannot catch continuous identity replacement.
…tion-time probe identity() took its creation time from process_identity._process_create_time (psutil only), while identity_is_live judges with update_lock.incarnation_live, whose producer update_lock.process_create_time falls back to the stdlib probe. Without psutil every record was written with ct=None, which the reader can only call unprovable (live), so a killed updater's pause was never orphaned on such a host. identity() now calls update_lock.process_create_time, the same producer the marker uses. Review 5411136378 (#132338) fix 2.
…in the update's custody update_pause_record._git ran a bare ["git", "-C", root] through subprocess.run. On a Windows install whose only git is the copy install.ps1 staged in PM's store (expose_pm_git puts it on PATH only inside the updater), every gate read died with WinError 2: head_sha returned None and tree_is_whole refused on every launch, so paused gateways stayed stopped. Reads now resolve git like the updater does without installing anything (PATH, then PM's recorded entry, then the lockfile's staged entry) and run through update_custody.run_git, so inside an update they are job-bound on Windows and carry the custody config. The resolver mirrors _early_recovery._git_executable from #132361 minus the marker-recorded path; this lane does not carry that module's version. Review 5411136378 (#132338) fix 3.
#132338 (on main) binds every checkout-writing update step to the paused gateways' tree gate before git writes a file; this branch rewrote those steps around its commit point, so the replay kept the commit-point code and this commit carries the gate into it: - the principal move (target_sha) runs under _checkout_move via _move_checkout_to, main's extraction, on the commit-point form - the branch switch at its commit point binds both resolved commits - the broken-update rollback binds pre_pull_sha around every write, its in-process restore included, from all three callers - stash, restore (all four), fork sync (both) and churn cleanup pass checkout_move; a failed switch's stash restore too - "already up to date" no longer switches back to current_branch (main's behaviour) The gate helpers live in update_cmd_pause_gate.py (re-exported by update_cmd) so update_cmd.py stays under its line cap.
…ings' allowance Two ratchets this suite arms for its own siblings fired once they landed: - the Windows pause (#132338) now imports gateway/status_inline_source.py to recognise inline-bootstrapped gateways, so a change there starts the e2e_upgrade suite like gateway/status.py (only two importers: not a hub) - the marker-corpus consumers #132345 and #132354 ship are on main, so _LANDS_IN_SIBLING_PR empties as its comment asks
…ings' allowance Two ratchets this suite arms for its own siblings fired once they landed: - the Windows pause (#132338) now imports gateway/status_inline_source.py to recognise inline-bootstrapped gateways, so a change there starts the e2e_upgrade suite like gateway/status.py (only two importers: not a hub) - the marker-corpus consumers #132345 and #132354 ship are on main, so _LANDS_IN_SIBLING_PR empties as its comment asks
…ings' allowance Two ratchets this suite arms for its own siblings fired once they landed: - the Windows pause (#132338) now imports gateway/status_inline_source.py to recognise inline-bootstrapped gateways, so a change there starts the e2e_upgrade suite like gateway/status.py (only two importers: not a hub) - the marker-corpus consumers #132345 and #132354 ship are on main, so _LANDS_IN_SIBLING_PR empties as its comment asks
* test(e2e): the orphaned-update cell allows the marker custodian's release poll
Round 5's Start-MarkerCustodian takes a killed hand-off script's claim over and releases it on its
next 1 s poll after the update ends. Strict run 37393368977 read the marker ~2 s before that
release (custodian removed it at 01:01:32) and called it a surviving marker. The cell now waits up
to 30 s for the release; a genuinely stuck marker lives until the heartbeat ceiling, minutes.
* fix: the native Windows lane runs the CI replay and its dispatch controls
tests-os[windows] imports only the files list_os_marked_tests.py emits and
keeps `-m "platforms and not integration"` tests its host admits. The replay
regression for the Windows interpreter layout (venv holding python.exe alone,
Git Bash) carried no platforms marker, so the Windows selector omitted the
whole file and green Windows jobs never ran it.
The replay test, the R15 dispatch chain and the disabled/advisory/no-command/
empty-selection controls now carry platforms("linux", "windows"): Linux keeps
its coverage, the Windows selector lists the file (194 -> 195 files) and the
Windows -m filter collects 35 of its nodeids. macOS is left out on purpose:
the replay has not been run on bash 3.2. A routing test pins both gates via
the real selector and its spec semantics.
* fix: the crash-cell oracle judges the marker with update_lock itself
test_crash_cells carried its own marker parser and identity rule, and it
decides the orphaned_update verdict. It disagreed with the product on three
inputs: a float started_at (product: malformed, copy: live), a create time
1.5 s off (product: live within its 2.0 s tolerance, copy: dead at 1.0 s) and
a run: line before delegate: (product: delegate live, copy: dead, it read
line 4 only). The self-check claimed parity but never called update_lock and
used the test's own pid, which the product judges by exact incarnation.
_marker_live now asks update_lock._parse_marker(...).owner_live() /
delegate_live(); the crash cells run in a pytest process on the same Windows
host as the marker's pids, so the product's process table reads them as the
updater does. The self-check marks a separate live process with an integer
started_at, compares the oracle to judge_marker, and adds the three rows.
The ORPHAN_RELEASE_GRACE poll is unchanged.
* fix: plugin-install modules no longer start the update journeys they never reach
Replaying the last 131 non-merge main commits (4787e4d56fc) through the
classifier: main selects e2e_upgrade 5 / e2e_desktop_update 3, the CI branch
12 / 10. Four of the seven new hits per lane touched only plugins_cmd.py or
plugins_cmd_install.py. migrate_all_homes reaches them only when a home's
configured memory provider left core (the catalog install); no upgrade or
Desktop update journey configures one, so those suites cannot observe the
change, and the unit lane already runs on every .py edit. Measured on a
fresh home: memory_provider_migration, pm/plugins_state, agent/memory_provider
and plugins/memory run; plugins_cmd_install executes only its module body,
plugins_cmd is never imported.
Kept, because a fresh-home update runs them: memory_provider_migration.py
(migrate_all_homes on every source build), plugins/memory/__init__.py (the
post-update Honcho profile sync), pm/install.py (sealed() /
lazy_installs_allowed() gate every update's default-tool install), and
scripts/build/ (frontend-common.mjs is imported by freshness/web/tui/desktop).
After: 8 / 6. A guard test fails if the journey-home migration starts running
plugin-install code, the signal to route those modules again.
* ci: route the ZIP recovery module and the SSH gate's corpus test like their siblings
`hermes_cli/_early_recovery_zip.py` (split out of `_early_recovery.py` on the
commit-point PR) is imported by the update path, so a change to it must start
the update pipeline and install E2E suites. Both lists and the install-e2e path
filter now match the `_early_recovery` prefix, which also routes any later
sibling. `remote-lifecycle-v2-marker.test.ts` reads the update-marker corpus
and is now a listed consumer; `remote-update-marker-programs.test.ts` exists in
this tree, so its sibling-PR allowance is gone.
* test: the CI replay puts git on the step's PATH on native Windows too
The replayed steps call git. On Linux os.defpath reaches it; on Windows the
replay's PATH only reached it through the located bash's directory, and Git
for Windows ships a bash.exe in usr\bin with no git.exe beside it. The
Windows arm64 lane resolved that bash and 13 replay cases failed with
"git: command not found" (1e547a814c3 had just put them on the native
Windows lanes). git's own directory now rides along like bash's.
* ci(update): route the inline-source parser and retire the landed siblings' allowance
Two ratchets this suite arms for its own siblings fired once they landed:
- the Windows pause (#132338) now imports gateway/status_inline_source.py
to recognise inline-bootstrapped gateways, so a change there starts the
e2e_upgrade suite like gateway/status.py (only two importers: not a hub)
- the marker-corpus consumers #132345 and #132354 ship are on main, so
_LANDS_IN_SIBLING_PR empties as its comment asks
* test(ci): an import-time class body is not the update running plugin-install code
The journey-home guard counted every non-<module> frame in plugins_cmd*.py as
the migration running plugin-install code. d6813a5dd781 gave
plugins_cmd_install.py its first class statement (_ConsentRefusal), whose body
runs at import as a frame named after the class, so a plain import (via
plugins_cmd.py's module-level import) read as a call and the guard failed with
no change in what the update does: migrate_all_homes() still returns [] and
calls nothing in the module.
Frames called by their own file's <module> frame (class statements,
module-level comprehensions) are import-time execution and are skipped; a real
call into an install function is still recorded.
* fix(local-runtime): price a split GGUF as the sum of its shards
A split model was priced by its first shard alone, so the physics check
and the residency cap admitted giants the card cannot hold: on a 256 GB
box a 175 GB model priced as 6.7 GiB and the router booted with room for
three residents, two of which then co-resided and OOM'd every session.
Sum the tensor table, the host-side embedding duplicate and the per-block
FFN map across every part on disk; architecture metadata still comes from
the first part. The truncated FFN map also disabled spill placement,
which is the lever that would have relieved the pressure.
A part that cannot be parsed is priced out rather than fatal — refusing an
incomplete split to serve remains staged_in's job.
* fix(local-runtime): skip a truncated GGUF header and size a split row by all its parts
A header cut short raised struct.error, which no caller of
read_gguf_header catches (they skip a model on ValueError or OSError),
so one truncated file escaped the whole preset pass. A short read now
raises ValueError.
The status row sized a side-loaded split by its first file, which can
be a metadata stub of a few MB; it now sums every part on disk. The
split fixture writes model metadata into part 1 only, as gguf-split
does: a published Flash Next split carries 67 keys in part 1 and only
its 3 split.* keys in parts 2-3.
* feat(local-runtime): price tensors the engine reads from disk as not loaded
llama.cpp leaves a tensor its architecture marks TENSOR_READ_LAZY in the
file and reads its rows on demand; under the default --lazy-mode auto it
does so for a marked tensor over 4 GiB (b11370: per_layer_token_embd in
qwen4exp and gemma4). Hermes priced those tables as loaded weights, so
Qwen3.8 Flash Next's 26.8 GiB per-layer embedding table counted against
memory it never occupies.
The reader records the candidate tensors per part and applies the
architecture from part 1, so a table in a later split part is found.
profile_from_gguf prices weights without them, and a catalog build
carries lazy_bytes so the same holds before download.
* feat(local-models): run Qwen3.8 Flash Next as UD-IQ4_XS with its MTP head
Flash Next's catalog build moves from UD-Q4_K_XL (103.6 GiB, too big for
a 128 GB unified-memory machine) to UD-IQ4_XS: 91.9 GiB on disk, ~60 GiB
loaded once the per-layer embedding table stays on disk. The model has
no MTP layers; unsloth ships the head as its own file, so the catalog
gains an mtp_head companion. It downloads with the model, is priced
before and after download, and only once it is on disk does the preset
add --spec-type draft-mtp with the head as --model-draft (depth 3, from
the vendor recipe). min_engine is b11370, the first pinned build with
Qwen4Exp MTP.
With the projector and head, Flash Next needs ~74 GiB at its native
256K window, so the derived recommendation now picks it on 96 GB
discrete and on 96 and 128 GB unified memory. The catalog test admits
4-bit IQ4 builds as Q4-class, the pane's MTP flag covers a shipped head,
and the Hugging Face browser stops listing mtp-*.gguf heads as models.
* fix(local-runtime): read lazy tensors on demand on integrated GPUs too
llama.cpp's --lazy-mode auto resolves to off on any device without mmap
support, which covers every integrated GPU (b11370, #28160). On a DGX
Spark, Flash Next's 26.8 GiB per-layer embedding table therefore loaded
in full (27.2 GiB resident) while Hermes priced it as left on disk.
--lazy-mode on keeps it out (0.4 GiB resident) at the same decode speed
and within 2% of the prefill speed on GB10.
The preset now passes lazy-mode = on wherever Hermes prices the table as
on disk, so the engine's per-device default never decides residency.
AMD and Intel integrated GPUs, where reading on demand halved prefill,
keep the engine default and are priced with the table loaded.
* fix(local-runtime): price QSA compute buffers at the launch posture
Qwen4Exp's QSA attention scores the whole window each microbatch. Under
the router's unified KV every slot sees the full window, and b11370
reserves ~46 bytes per microbatch token per window token for each
context, device plus host; an MTP head is a second context. At 256K that
is 5.9 GiB per context at -ub 512 and ~23 GiB at -ub 2048, so the
prefill posture Hermes chose on a 128 GB Spark loaded the target and ran
out of memory creating the MTP context.
plan_launch now prices each posture with its own microbatch and context
count, LaunchPlan carries the profile it priced, and catalog entries
name their architecture so the price holds before download. On the
Spark, Flash Next now runs 256K at -ub 512, priced at 83.1 GiB against
81.5 GiB of measured engine buffers.
* fix(local-models): draft Flash Next from ggml-org's MTP head
unsloth's MTP heads mark the MTP block as plain attention (compress ratio
0) beside a QSA trunk. Mainline llama.cpp then builds the k-pool inputs
with nothing reading k_idxs, and the server aborts at load with
GGML_ASSERT(buffer), on b11370 and b11430 alike.
ggml-org publishes a head that loads. On a DGX Spark at 256K and
temperature 1.0 it drafts at 39 tok/s on prose and 32 on code, against
~27.5 without MTP. AssetFile gains a repo so a companion can download
from a repo other than the model's.
* refactor(desktop): local-models copy moves to per-locale siblings
The six locale files that carry local-models copy sit past the file-size cap and may only shrink.
Each settings.localModels block moves into <locale>_local_models.ts, as the model-menu copy did,
so the next string lands without growing them. No string changes.
* feat(local-models): the RTX Spark defaults to the 27B
The RTX Spark ships with the 27B as its default model. Flash Next fits its 128 GB pool and stays
available, but Windows backs GPU memory with commit outside the carve-out, so Flash Next loads
only once the carve-out leaves Windows enough memory to back it: with no page file, an 8 GB
carve-out loads it and a 64 or 96 GB one does not.
recommended_entry consults a product-default table before the derived rule, keyed like the
measured-rate table on platform, effective backend and recognized GPU. The derived rule still
decides when the default is ineligible or cannot run resident, under a Vulkan or CPU engine, and
on the DGX Spark. The Recommended badge reads "The default model for this machine, chosen by its
manufacturer."
* fix(local-models): keep older apps on their packaged catalog
Older apps refresh catalog.json from main and ignore fields they do not know. Given this
catalog, main's code recommends Flash Next on an RTX Spark and launches it without its MTP head,
with the 26.8 GiB per-layer table loaded and its QSA compute buffer unpriced. Catalog schema 2
makes those apps skip the refresh and keep the catalog they shipped with; they read schema 2
once they update.
* feat(local-models): pin Flash Next's downloads to the revisions the catalog describes
Downloads resolved files from main, so a re-upload on the publisher's side would change the bytes
behind the catalog's exact sizes and the pricing measured from them. AssetFile carries a revision
and the download plan resolves each file at it. Flash Next pins unsloth's weights and projector to
38bb39ee and ggml-org's head to 052beea; entries without a revision still follow main.
Co-authored-by: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com>
* fix(local-models): store ggml-org's MTP head under its own name
ggml-org's head and unsloth's share a filename, and a download is skipped when its destination
already exists. A machine holding unsloth's head, which aborts mainline llama.cpp at load, would
launch with it instead of fetching ggml-org's. The head now lands as
mtp-Qwen3.8-Flash-Next-ggml-org-Q8_0.gguf.
Co-authored-by: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com>
* fix(deepseek): honor effort=none as thinking disabled
Desktop Thinking Off can arrive as {effort: "none"} without enabled=False,
so V4 kept sending thinking:enabled and billed reasoning tokens.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(desktop): resolve a cold focused tile's cwd beyond the recents page (#76535)
$focusedWorkspaceCwd's tile branch searched only the paginated $sessions
recents page, so a restored cold tile older than that window (and outside
the project preview caps) resolved an empty cwd and Files showed no
project, even while the row sat in a loaded sibling store. Resolve the
tile's session row through the same loaded-row space the owner lookup and
tileStoredRow already use — recents + cron + messaging + unlisted owner
rows (where a hidden bot chat's resolved row is parked on purpose) + the
project tree's preview/lane rows — keyed by the tile's durable stored
session id, never by persisting the cwd on the tile identity (cwd is
backend-authoritative state another surface can change; the renderer only
caches it). Unresolved secondary focus still fails closed: no rung falls
through to the primary's cwd.
* refactor(desktop): owner holds + $projectTree leaf out of session-states, breaking the projects cycle
The #76535 diff grew session-states.ts past its main-pinned FILE_LINES cap
(3264 > 3226) and its ./projects import pulled store/gateway into every
suite that partially mocks the gateway, killing check:test:ui. Offset per
AGENTS.md: the create→foreground owner-hold block moves to topical sibling
session-owner-holds.ts (moved code keeps its cap), and $projectTree moves
to leaf project-tree.ts so session-states reads it without the gateway edge;
projects.ts re-exports it, so every existing import path is unchanged.
* fix(gateway): replay cap-dropped transcript spool files in drop order on restart
recover_pending_to_db walks sorted(flush_dir.glob("*.json")) to replay
spool files left behind by a restart. Spool files are named
pending-<uuid4>.json by _write_payload, so that sort is effectively
random: a transcript recovered after a crash comes back scrambled.
The damage is permanent rather than cosmetic. SessionDB restores a
conversation with "ORDER BY id" — AUTOINCREMENT insertion order, never
timestamp — precisely so a non-monotonic clock cannot sort an assistant
tool_calls row after its tool response, "breaking tool-call/response
adjacency and triggering an HTTP 400 on replay". Replaying the spool in
filename order writes exactly that inversion into the row ids, so the
session errors out on the next turn.
The payloads already carry the ordering fields: spool_dropped_transcript_
message stamps every one with "ts" and a monotonic "seq". The sibling
consumer of the same spool in this file, drain_transcript_spool, already
sorts on them via sorted(entries, key=lambda e: e[:3]); only the restart
path ignored them. Order on (ts, seq, filename) so the live drain and the
cross-restart drain agree.
Ordering by the payload rather than by a new file-naming scheme also
recovers spool files that were written before this change, which are the
ones an affected user already has on disk.
Payloads that cannot be parsed keep their previous treatment: they sort
last, are handed back unparsed, and are re-read inside the loop so the
existing handler reports and preserves them unchanged.
(cherry picked from commit 0e46604abc001a798f7f8567aedcaf19923f72df)
(cherry picked from commit c043f0116e55b87b6e631592b12abbf60496aec8)
* fix(gateway): preserve structured transcript fields when recovering spooled messages
The cross-restart replay of a cap-dropped transcript message forwarded
only four fields to append_message — session_id, role, content, timestamp
— and then unlinked the spool file, making the loss permanent.
Everything else on the message was discarded: tool_calls, tool_call_id,
tool_name, the reasoning columns, codex items, platform_message_id,
observed, and the api_content sidecar. append_message already accepts all
of them, and the payload already carries them, because
spool_dropped_transcript_message writes the full transcript message dict.
Two of the dropped fields are load-bearing rather than decorative. Losing
tool_calls/tool_call_id orphans a tool result from the call it answers,
which is the adjacency SessionDB's "ORDER BY id" exists to protect.
Losing api_content contradicts the requirement stated at the live writer,
that the sidecar "must survive any gateway-side persistence path or the
next turn's replay diverges at this row" — and this is such a path.
The same spool is drained two ways: by drain_transcript_spool during live
operation, which replays through SessionStore._append_transcript_message
and keeps every field, and by this function after a restart. Same files,
same payloads, different fidelity. Mirror _append_transcript_message field
for field so the outcome no longer depends on whether the gateway happened
to restart, including its role gate on the assistant-only reasoning
columns and its message_id fallback for platform_message_id.
Fields are whitelisted explicitly rather than splatted from the message
dict: the payload is arbitrary JSON from disk, and an unexpected key would
raise TypeError and abort the whole recovery pass. content is now passed
through as-is instead of being coerced to "", since an assistant tool-call
row legitimately has no content.
(cherry picked from commit 99ef8fbb41fca1b47873188ec93d64476fa59902)
(cherry picked from commit 27ee00d618c973304c9e64bffccaeeaf1b54fbaf)
* fix(gateway): stop a session's spool replay after the first failed append
Ordering the spool files fixes the happy path only. If append_message
fails partway through — the DB is still unhealthy, which is the situation
that produced the spool in the first place — the loop logs the failure,
keeps that file for a later retry, and then carries on and writes the
messages that come after it.
Those later messages land now; the failed one lands on some future start.
Because SessionDB orders a conversation by AUTOINCREMENT id, the retried
message then gets a HIGHER row id than the messages it originally
preceded, and the inversion this pass just prevented is written to disk
anyway — this time permanently, since both files are gone.
drain_transcript_spool, the live drain of the same spool, already states
the rule: "On the first replay failure the drain stops and remaining
files are kept for the next attempt (the DB is likely still unhealthy)."
Apply it here too.
The block is per-session rather than global. Replay order is only defined
within a session, and this function drains every session's spool in one
pass, so a single unhealthy session must not strand the others' messages
on disk. Non-transcript pending payloads are unaffected.
(cherry picked from commit 5ea5104b6f4f896884508a36c11eb81010ccaefa)
(cherry picked from commit a9591541728fa3bd76ef5a349dd55f066a039243)
* test(gateway): cover spool replay ordering across a restart
Names the spool files so that filename order is the exact reverse of drop
order, which is what uuid4 names produce on average, and asserts the
replay comes back in drop order. A second case gives three payloads the
same one-second ts so only the monotonic seq can separate them.
Both fail on the previous sorted(glob()) implementation. A third case
pins the pre-existing treatment of a corrupt payload — reported by the
loop's own handler and left on disk — so the new ordering pass cannot
silently swallow one.
Lives in its own file rather than tests/gateway/test_shutdown_flush.py to
keep the restart-recovery cases together with the spool fixtures they
need.
(cherry picked from commit 555d8719d0854b038a7663e96e4ac477842f345e)
(cherry picked from commit c372b25d103436a373c9da14c26e71426afae908)
* test(gateway): cover field fidelity and failure handling in spool recovery
Round-trips an assistant tool-call row carrying every field the live
writer persists and asserts each one reaches append_message: tool_calls,
tool_call_id, tool_name, the reasoning and codex columns,
platform_message_id, observed, timestamp, and the api_content sidecar.
Also pins content=None passing through uncoerced, since an assistant
tool-call row has no content, and the role gate that keeps the
assistant-only reasoning columns off a user row.
A separate case covers a failed replay: with the first of two messages
for one session rejected, neither is written and both spool files stay on
disk, while a third message for an unrelated session still recovers.
All but the two behaviour-preservation cases fail on the previous
implementation.
(cherry picked from commit 869acc61272b835ae35c7e4f65efe25df8699ae6)
(cherry picked from commit e02d9fcd6414e549f2eb7791f4c7388d6e07bacb)
* fix(gateway): fall back to the payload clock only when a timestamp is absent
_transcript_append_kwargs chose the spooled message's timestamp with
`message.get("timestamp") or payload.get("ts")`, carried over from the
code this PR replaces. Truthiness is the wrong test: epoch 0 is a valid
timestamp and would be silently rewritten to the payload's ts, which is
the fidelity loss this PR exists to remove.
Fall back only when the field is genuinely absent.
(cherry picked from commit 62206b0bcd71ea22c3a383f05fe12dd9242ab4a1)
(cherry picked from commit a7b6828ee97f0e876fc033c74f3026aea779e9fd)
* fix(gateway): the live and restart transcript writers share one row builder
Review follow-ups (teknium1):
- One kwargs builder, two callers. transcript_append_kwargs() in
gateway/session_transcript.py builds the append_message kwargs, and both
_append_transcript_message and the spool's restart replay call it, so the
restart writer cannot lag the live writer again (display_kind and
display_metadata were added after #84785 was written). fallback_ts applies
only to a message with no timestamp; the live writer passes none, so its
rows are byte-identical.
- A failed replay says what it held back: one INFO line per pass with the
file count per blocked session, so a spool dir that never empties has a
reason in the log.
- seq is per process and ts has one-second resolution; the sort key now
says so where it reads seq.
- Tests fold to three invariants: drop order beats name order (seq breaks a
same-second tie), a replayed row is the row the live writer writes, and a
failed replay holds back that session's later messages only. The two that
already passed on main (a corrupt file is kept; the payload-clock
fallback) are dropped; the fallback is asserted inside the fidelity test.
(cherry picked from commit e2bacb06b3311b00aa354ffce7d53fc15ba44d04)
* fix(gateway): an out-of-range spool ts or seq no longer aborts recovery
_order_flush_files runs before recover_pending_to_db's per-file error handling, so float(10**400) raising OverflowError stopped every file from being recovered. Unusable ordering values (too large for a float, NaN, Infinity) now sort first like any other non-numeric value.
(cherry picked from commit 50c9b97954ae85f3acee96352a9f1a883ba75688)
* fix(gateway): catch only read and decode errors when ordering spool files
The code-health gate (BLE001) rejects the blind except added to _order_flush_files. A file that cannot be read or parsed still sorts last with a None payload, so malformed-file isolation is unchanged.
* fix(gateway): drain boot-held-back spool files before the session's next live write
recover_pending_to_db held back a session's later spool files after a failed replay, but never told the running gateway. The live writer drains the spool only for sessions in SessionStore._spooled_drop_sessions, which only this process's own spooling filled, so the session's next live row landed ahead of the held-back files until the next restart.
recover_pending_to_db now adds the sessions it held back to a caller-supplied set, and _recover_pending_flushes passes the store's own drain set (exposed as spooled_drop_sessions()), so the next live write replays them first.
Found in review by @BearHuddleston.
* fix(gateway): bound decoded payload memory during restart recovery
Retain only sort keys and paths; re-read payloads one at a time for replay. Regression with 6 MiB of spool data fails on the previous head at 8.1 MiB peak allocations and passes with the metadata-only index. Review finding by @keeltrace.
* fix(gateway): order shutdown slot heads before same-second overflow
Treat an absent sequence as -1 during restart sorting, before the overflow tail starting at zero. This also repairs ordering for existing spool files, without requiring a new writer format. Frozen-clock adversarial-filename regression reproduces B,A,C before the fix and A,B,C afterwards. Review finding by @keeltrace.
* fix(gateway): keep live transcript rows queued while older spool files fail to drain
The live writer replayed a session's spool before each write but ignored a failed replay and wrote the new row anyway, so a second failure (boot hold-back, then the pre-write drain) gave the live row a lower row id than the older spooled rows. _drain_spooled_drops now reports files left on disk, and the write takes the existing retry path instead, leaving the row queued in memory behind them.
Review finding by @ehz0ah.
* fix(gateway): replay the pending spool before boot resume turns and queued inbound
Boot recovery ran in main() only after runner.start() returned, by which point start() had already run the auto-resume turns and drained the inbound queued during startup. Those turns write live transcript rows, and a fresh store does not know which sessions have spooled rows, so the previous run's spool landed after them. _start_finish_wiring now runs _recover_pending_flushes just before _schedule_resume_pending_sessions(), and failures log at WARNING instead of being swallowed silently.
Review finding by @dokterdok.
* fix(gateway): keep the spool replay error classifiable when it blocks a live write
Gating the live write on a pending spool raised a generic RuntimeError, hiding the replay's real failure from the FTS-rebuild, replaced-database and compression handlers, so a repairable FTS fault became a permanent stall. _drain_spooled_drops now returns the replay's own exception, the caller raises it, and after a successful FTS rebuild the loop drains the older spool again before writing the live row.
Found in independent review.
* fix(gateway): stop one malformed spooled row from stalling its session
transcript_append_kwargs passed tool_call_id, tool_name, role, reasoning and platform_message_id to sqlite as they were, so a spooled dict or list in one of them failed to bind on every replay. With live writes now waiting behind a failing spool, that one row stopped the whole session persisting, across restarts too. Those fields are now made bindable, and a row the database still rejects on its own values is moved to *.json.bad and logged once instead of holding the session back, both at boot and in the live drain.
Also from the same review: the live drain and the restart pass share one sort key (_spool_sort_key), so a string or NaN ts no longer raises in the live drain; boot recovery returns the sessions it held back and the store marks them under its lock (mark_spooled_drop_sessions) instead of handing out its set; _recover_pending_flushes moves out of gateway/run.py as shutdown_flush.recover_gateway_pending; the transcript_append_kwargs import sits outside the try; blocked_sessions is required; the agent-history reason is a constant; the recovery tests share one store fixture and compare replayed rows with the live writer's.
Review findings by @kshitijk4poor.
* fix(gateway): quarantine only rows sqlite cannot bind
is_row_rejection treated every sqlite3.InterfaceError as a bad row, but SessionDB retries InterfaceError('no more rows available') as WAL contention and re-raises it once its patience runs out, so a valid row could be moved out of replay for good. Among sqlite errors only a 'binding parameter' message now counts; OverflowError does too. _bindable_text also passed ints outside sqlite's 64-bit range, which raise OverflowError even for a TEXT column and stalled the session the same way a dict did; they are now stored as text.
* fix(gateway): never replay a written spool row whose file cannot be deleted
When a spooled row was written but its file could not be deleted, the OSError aborted the drain: the files after it were left for later, so the live row landed ahead of them, and because the file stayed, the next live write replayed and wrote the same row again, once per write. The boot pass had the same repeat. _remove_replayed now logs the failure, remembers the path so neither replay pass in this process touches it again, and lets replay go on with the later files. A restart still writes that row once more, since nothing on disk marks it; the log says to delete the file. Live rows are not held back for this: an unwritable spool directory stays fail-open.
Review finding by Greptile.
* fix(desktop): multi-select folders when adding to an existing project (#68741)
The add-folder dialog picks through pickProjectFolder, which hardcodes
multiple: false, so an existing project takes one folder per interaction.
Add a plural pickProjectFolders that enables the native dialog's
multiSelections, and addProjectFolders, which writes each picked folder
(excluding ones the project already has and repeats within the pick) in a
single submit beat. The create flow reuses the same multi-pick to extend
its accumulated folder list; single-folder pickers are unchanged.
* test(desktop): webhooks profile ownership invariants (red on base)
Regression coverage for #71352: the create draft must be dropped when the
backend profile changes under All profiles, a create completion owned by the
previous profile must not surface its one-time secret or toast, and a normal
create completion must still land after React StrictMode's effect rehearsal
(the lifecycle bug flagged on #71538's cleanup-only ownerActive effect).
* fix(desktop): key Webhooks state and REST to the active backend profile
The Webhooks view keyed React Query by the profile scope but kept every
profile-owned local state (create draft, create dialog, one-time created
secret, pending delete, restart/error banners) in plain component useState,
and its REST calls resolved the mutable global profile at execution time.
Switching the active backend profile with the page open leaked the old
profile's draft dialog into the new profile, and a delayed create
completion could surface the old profile's one-time webhook secret under
the new profile view (#71352).
Bind the whole operation to one captured backend owner:
- Split WebhooksView into a thin wrapper that keys a ProfileOwnedWebhooksView
by the normalized active gateway profile: a profile switch unmounts the old
operation scope synchronously, so drafts, dialogs, secrets and pending
state disappear at the same boundary as their owner (AGENTS.md: a profile
swap is a re-home; the previous context must not leak into the next).
- Route all Webhooks REST calls (get/enable/create/delete/toggle), the
gateway restart request and its status polls through an explicit profile
argument captured at mount, instead of ambient mutable profile
resolution. Optional arguments keep the ambient scope for callers
without an owner.
- Guard every async completion with a StrictMode-safe ownerActive ref
(set in effect setup, cleared in cleanup) so a stale profile's create/
toggle/delete/restart completion cannot notify or repaint the new
profile view.
* fix(gateway): preserve session lineage in the session.list RPC
The session.list projection dropped the durable lineage root returned by
SessionDB, so RPC consumers could not group a compressed conversation
with its continuation — REST (/api/sessions) already projected it, and
the desktop store keys pinning and lineage dedup on the field
(apps/desktop/src/store/session.ts::sessionPinId).
Forward `_lineage_root_id` through `_session_row_summary`, declare it on
the `SessionListRow` wire contract, and regenerate the TS/OpenRPC
artifacts so every consumer sees it.
Fixes #66663
Salvaged from #66675 by @Sanjays2402 (authorship preserved); adapted to
methods_session.py and extended with the wire contract + regenerated
artifacts.
* feat(approvals): flag credential uploads and invisible Unicode in terminal commands
Two content-level shapes the dangerous-command detector missed and that only the
bundled tirith scanner caught:
- a curl/wget request body carrying a secret-named variable or a credential file
(-d "k=$OPENAI_API_KEY", -F file=@.env, -T ~/.ssh/id_rsa, --post-file=/etc/passwd),
or a credential file piped into an uploading curl/wget. Authorization headers are
ordinary API use and stay unflagged; flag letters are case-sensitive (-D/-f do not
upload).
- zero-width / bidirectional Unicode controls that make the approved command differ
from the executed one. Emoji ZWJ sequences are not flagged.
Measured on a 40-command corpus: core now catches 13/16 attacks (was 9/16; tirith
0.4.2 caught 14/16) while flagging 5/24 benign commands (tirith flagged 12/24).
* refactor(security): remove the bundled tirith scanner from core
tirith ran `tirith check` on every terminal command, default-on with fail-open, as a
pinned PM binary auto-downloaded at startup. Measured against the shipped 0.4.2 binary
it caught 14/16 attack commands but flagged 12/24 everyday agent commands
(`echo $((6*7))`, `"$VAR/x.sh"`, `gh api | python3`, LAN curls, `.dev` hosts,
`pip install` via analysis_incomplete), and in cron/-q deny mode each of those is a
hard block. It only ever saw terminal strings (execute_code, writes and the browser
bypass it), fail-open meant no protection wherever the binary could not install
(Windows, Termux, x64 musl), and it produced a steady issue/PR stream (fail_open
semantics four times, install loops filling disks, a SIGSEGV hang, session-approval
scoping, third-party promo text on approval cards).
Removed: tools/tirith_security.py, the tirith branches of the command guard (the
is_tirith tuple, session-only "always" downgrade, import-error/fail-open paths,
unattended tirith pass), the CLI and gateway startup installers, the PM package and
lock pins, the nix runtime dep, security.tirith_* defaults, the CLI warning string
and three tirith tips (gaps refilled from the tail so translations survive), plus
the gateway's "no automated assessor" warning that only existed to point at tirith.
Config v50 drops security.tirith_* and enables nothing in its place. Hermes ships
no tirith plugin; if tirith upstream publishes one, it goes through normal catalog
admission under their name.
* fix(dashboard): ChatSessionList test type-checks under tsc -b
#134100 added a test that assigns globalThis.IS_REACT_ACT_ENVIRONMENT directly
(TS7017) and destructures ghost/outlined/size from a plain ButtonHTMLAttributes
prop type (TS2339). Vitest strips types, so the test passes, but `tsc -b` in the
web build fails, which breaks every Docker image build and the Desktop update
E2E seed on main. Cast globalThis like the sibling tests do and widen the mock
prop type with the three props it consumes.
* fix(plugins): refuse checkout mutations while the gateway is live
Git plugin update/remove/force-reinstall ran git pull or tree swaps
directly in the installed checkout while the messaging gateway could
still be running with that plugin's callbacks loaded. A callback that
defers a relative import until first invocation then fails with
ModuleNotFoundError even though the command reported success (#70473).
One shared guard (hermes_cli.plugins_cmd._refuse_live_gateway_mutation)
now fails every mutating path BEFORE any git operation when the active
profile's gateway is live:
- plugins update (CLI) and dashboard_update_user_plugin
- plugins remove / rm / uninstall, dashboard_remove_user_plugin and the
plugins.manage RPC (shared _remove_user_plugin tail)
- plugins install --force (a forced reinstall replaces a loaded
checkout; a brand-new install is not gated)
The liveness read is the canonical profile-scoped probe
(profiles._check_gateway_running -> resolve_gateway_liveness), never
mutating the profile's identity files; a probe failure fails open so an
uninstallable plugin cannot strand. The message gives the
stop/apply/start recovery sequence; --allow-live-gateway on
update/install/remove is the explicit CLI-idiom override.
Integration test (tests/hermes_cli/test_plugin_live_gateway_guard.py)
spawns a real child holding gateway.lock with a gateway-shaped command
line against a temp HERMES_HOME, so the guard is exercised through the
platform's actual liveness probe — not a mock. Stopped-gateway behaviour
is asserted unchanged.
* fix(dashboard): authenticate the gateway health probe without leaking the key to /health
The dashboard's _probe_gateway_health() sent no Authorization header, so an
auth-protected gateway (API_SERVER_KEY set) answered 401 to every
/health/detailed poll and logged a warning per probe. Read API_SERVER_KEY at
probe time and send it as a Bearer header — to the authenticated
/health/detailed endpoint only; the public /health fallback never receives
the credential.
Fixes #76051
* fix(pricing): wire kilocode into the pricing dispatcher
Kilo Gateway models showed no prices and no Free badge because
_PRICING_FETCHERS had no kilocode branch, so get_pricing_for_provider
returned {} and _apply_pricing skipped the provider row entirely.
Add _fetch_kilocode_pricing, which reads the public OpenRouter-shaped
catalog at {kilocode profile base_url}/models (KILOCODE_BASE_URL
overrides it — the same endpoint the doctor probe uses, no second
hardcode). The Kilo catalog's explicit isFree flag is authoritative
over the rates (a free router can carry non-zero underlying pricing),
and its -1 rate sentinel (auto-router models, 'rate varies') is
skipped rather than shown as a fabricated price.
Fixes #68838
* test(model-catalog): pin inclusionai/ling-3.0-flash:free in both picker sources
RED on base: the curated fallback list and the published manifest both omit
the model, so the Desktop picker never offers it even though the CLI accepts
it as model.default (#73686).
* fix(models): add Ling 3.0 Flash to OpenRouter picker
Cherry-picked from #73788 (commit b952f0b0fa17) and re-applied to
hermes_cli/models_catalog_static.py, where OPENROUTER_MODELS moved in the
catalog refactor.
Fixes #73686
* chore(catalog): regenerate model-catalog.json for Ling 3.0 Flash
Output of scripts/build_model_catalog.py after adding
inclusionai/ling-3.0-flash:free to OPENROUTER_MODELS.
* fix(dashboard): enable touch scrollback in chat
* fix(desktop): Simple mode keeps the statusbar up for a signed-out free-tier user
Simple's policy rested the whole statusbar hidden, and the bar's Sign in chip
is the only standing way a free-tier user can sign in. Picking the Basic
layout (Simple) left them with no visible Sign in.
- $freeTierSignInOpen (free tier available, i.e. signed out, and no guided
setup running) is the one condition: the chip shows on it, and the shell
feeds it into the mode context.
- Simple's statusbarVisible becomes context => context.freeTierSignInOpen,
the same shape as profileRailVisible. Signed-in Simple users keep a hidden bar.
- Replace the stale comment that pointed at the old onboarding ready screen.
- Test: in every interface mode a signed-out free-tier user sees the chip.
* fix(desktop): a profile switch no longer keeps the previous profile's Sign in (review)
Drop the cached free-tier status when the gateway scope (connection + profile) changes; a flap on the same scope keeps it.
Feed freeTierSignInOpen into the mode context from store/free-tier, so the statusbar test runs the production link instead of a copy.
* fix(dashboard): web typecheck checks the app project; ChatSessionList test types its mocks
web/tsconfig.json is a solution file (files: [], references), so 'tsc -p . --noEmit' checked nothing and CI's web check passed while the build's 'tsc -b' failed on four errors in ChatSessionList.test.tsx (#134100). The typecheck script now runs 'tsc -b . --noEmit' like hermes-ink, and the test types the act-environment global and the Button mock's extra props.
* feat(free-tier): send preview_full_connectors on account creation
HERMES_PREVIEW_FULL_CONNECTORS=true|false makes the free-tier mint send
{"preview_full_connectors": <bool>} in the /api/anonymous/create body, so
the account service can record the cohort on the account and gate
toolkits from it. Any other value (unset included) keeps the body {}.
The variable joins the desktop bundle-env allowlist (release script,
banner writer, channel request decoder, BUILDING.md) so a bundle can
bake it in. It is self-reported and visible in the packaged JS: the
service must treat it as a preference, not proof.
* fix(desktop): persist project cwd for new sessions
* fix(desktop): persist project cwd for new sessions (WIP continuation)
* test(update): a failed pause-record test no longer orphans its draining gateway
The _DRAINING child traps SIGTERM and then polls for a flag file every 10 ms,
and each test released it only in a finally that begins after its first
assert. Any earlier failure (the "up"/"asked"/"stopping" reads, the updater
spawn) skipped that block: the gateway was reparented to init and spun
forever. Two such orphans from red-on-base runs were still alive on a dev
host three days later; a CI red leaks the same way onto its runner.
_child now records every process it spawns and an autouse fixture SIGKILLs
any survivor once the test is over, so the whole file is covered, including
the updater children (time.sleep(120)) and any test added later.
Sabotaging the "stopping" assert: origin/main leaves 1 draining gateway
alive after the failed test, this change leaves 0; the file passes 42/42.
* fix(local-runtime): hermes update from v2026.9.24 no longer crashes importing local_runtime
After the pull, the v2026.9.24 updater lazily imports
hermes_cli.local_runtime.processes (in _subprocess_compat.bounded_probe_run)
from the new checkout. Its process still holds the old
hermes_platform.host.facts. The package __init__ eagerly imported every
submodule, so hardware.py ran `from hermes_platform.host.facts import
gpu_class` (added in 827f4ff1d1) against the old facts module, and the
update died with ImportError. This broke e2e-upgrade (core) on main and
on every PR.
The package-level re-exports now resolve on first access through a module
__getattr__, so importing one submodule runs only that submodule. Callers
of `hermes_cli.local_runtime.<name>` are unchanged.
* ci: package __init__ files on the update path start the update E2Es
Importing a/b/c.py runs a/__init__.py and a/b/__init__.py first. The
update-lane routing listed hermes_cli/local_runtime/processes.py (the
v2026.9.24 updater imports it after the pull) but not the package init
that import runs. #134116 changed what that init loads, its PR never ran
e2e-upgrade, and main went red until #134242.
classify_changes now routes the package inits above every module the
e2e_upgrade and e2e_desktop_update lanes own, and above each shared hub,
because a hub's inits run whenever the hub is imported. An init that is
itself a hub stays out. The routing guard's import walker counts the
package inits a module's imports run, so the next unrouted one fails it.
This routes eight inits that were missing: local_runtime, gateway, cron,
tools, tools/environments, plugins, hermes_platform,
hermes_cli/observability.
* fix(pm): give Store a long-path root on Windows
Windows cuts plain file calls at 260 characters when LongPathsEnabled
is 0, which is the default. PM's copy, walk, digest, rename and delete
calls under the store then fail or silently skip files (#130242).
Store now spells its root in the extended-length form (\\?\C:\...).
Every path built from it with entry() or / inherits that spelling, so
the file calls beneath the root no longer hit the limit. Relative
paths, and so digests, are unchanged. On POSIX the root is untouched.
native() returns the ordinary spelling for text that leaves PM's own
file calls (argv, environment values, hashed identities, records and
messages). The call sites adopt it in follow-up commits.
* test(pm): live Windows proof for PM with LongPathsEnabled=0 (#130242)
Drives a real Store through scratch, the bundled-copy copytree, tree_digest,
publish and removal over a tree with files and a directory beyond MAX_PATH,
then checks the facts record and composed child env carry no \\?\ prefix.
The test first asserts RtlAreLongPathsEnabled() is false so it cannot pass
as a false proof. The wine2e lane sets LongPathsEnabled=0 in a separate,
final step and selects the test with -m 'platforms and integration'.
* fix(pm): give text that leaves PM the ordinary spelling of store paths
Store now spells its root \\?\C:\... on Windows so PM's own file calls
under it pass MAX_PATH. That spelling must not escape: the same folder
spelled two ways hashes and compares differently, and children such as
cmd.exe, gpg or uv refuse or record the prefix (uv would write it into
pyvenv.cfg).
native() now converts store-derived paths where their text leaves PM:
- process argv and cwd: uv via PythonEnvironment._run (its one exit),
binary version probes, npm self-install, the PortableGit extractor,
gpg provenance checks, the plugin-eviction interpreter probe;
- environment values: compose_env (PATH and package env diffs), the
activation PATH dirs, HERMES_RUNTIME_DIR;
- hashed identities: the PM runtime inputs and isolated-environment
identity, so existing records still match and nothing rebuilds;
- API results callers execute or compare: installed_package,
uv_launcher, verified_tools entries;
- user-facing verification messages.
Facts records already store {STORE}-templated env, so no absolute path
reaches them. Tests pin that identities are spelling-independent and that
records, composed env and uv invocations from a verbatim root carry no
prefix (both fail on e8c97320ac).
* fix(pm): spell store roots PM opens without Store like Store does
Some PM code reaches a store root from paths.store_root() or
writable_store_root() directly instead of through Store, so its file
calls kept the ordinary spelling and stay cut at MAX_PATH on Windows
with LongPathsEnabled=0. Worse, its comparisons against a Store root
(now \\?\-spelled) never matched.
Each such root now gets long_root() once, where the variable is born:
- Facts.refresh_digests digests whole payload tool trees;
- verified_tools walks and digests the source store's tool trees;
- the uv cache seed copies the payload's deep uv-cache tree out;
- _toolchain's writable store, so `location.root != writable` compares
like with like and the copy store is built from the same spelling;
- _installed_location passes store.root to Facts.installed;
- ensure() picks the shipped facts by comparing two Store roots;
- the corrupt-facts guard compares both sides spelled long.
Returned and exported paths stay ordinary (previous commit).
* ci(wine2e): run the long-paths-off PM proof even when an earlier lane fails
The PM long-path steps do not use anything the earlier live lanes set up.
Gating them on those lanes hid the proof whenever an unrelated lane went
red. The test step still requires the long-paths-off step to succeed, so
it never runs with the runner image's setting.
* fix(pm): _toolchain hands callers the ordinary spelling of uv and Python
The long-path commits now on onboarding/next spell PM's store root extended-length (\\?\) on Windows, and _toolchain returned uv and Python under that root. Its callers run and compare those paths outside PM's own file calls, and on main they got the ordinary path. tests/pm/test_uv_python.py::test_bundled_uv_uses_a_verified_writable_python_without_changing_runtime fails on both Windows lanes for that reason. Return native() spellings; the existence checks inside still use the store's.
(cherry picked from commit acad756e3378ffbb3b23616bb0b5647e251fc895)
* fix(pm): the long-paths live test runs the real bundled-copy install (review)
test_long_paths_windows_live calls _install(copy_from=...) with facts.json inside each store instead of copying and publishing by hand.
* fix(agent): a credit-limited 402 retries once with the affordable output cap instead of failing as billing
OpenRouter answers a request whose max_tokens exceeds the remaining balance with
"402 ... You requested up to 65536 tokens, but can only afford 56272". The account has
credit; only the cap was too large. The main loop classified it as terminal billing and
failed the turn (or rotated off a credential that still had credit), so a request that
needed a few hundred tokens was dropped (#49769).
recover_after_classification now retries once with the output cap lowered to the affordable
budget before pool rotation, reusing the auxiliary client's _affordable_max_tokens_from_error
parse (same 64-token margin and 512-token floor) and the existing one-shot
_ephemeral_max_output_tokens seam. A second credit-limited 402 on the same call, or a body
with no parseable budget, keeps today's billing handling. Classification is unchanged.
Approach from #49785 by @tcconnally (closed unmerged), applied at the recovery layer.
Co-authored-by: Thomas Connally <51974392+tcconnally@users.noreply.github.com>
* test(desktop): pin one-backend-per-host across reconnect storms and the PPID-1 orphan reap (#81275)
The causal fixes for the duplicate-serve/orphan class are on main (dial-claim
coalescing #90812, host-backend singleton 58eb248d080, ownership-ledger
orphan reaping #89298/#87295) but no test pinned the whole contract at the
seams the report exercised. These tests hold a reconnect storm to one spawn,
hold the spawn guard against a routing bypass, and drive the reaper with REAL
child processes and the real platform start-marker probe: a crashed parent
leaves an orphan the next launch stops, and a live parent's backend is never
touched.
* fix(ci): drop unused imports, sort imports in orphan-reap test
* fix(desktop): Windows SSH host locks let a superseded backend retire
The owner watchdog (#132219) retires a detached serve --isolated backend once
backend.lock.json names a newer spawn, but it only trusts a lock whose logPath
ends in /<ownershipId>/<nonce>.log. Locks on native Windows SSH hosts are
written by windows_ssh_runtime.write_lock from the Desktop's record, which
carries no logPath, so read_valid_backend_lock returned None there and a
reconnect storm kept stacking backends.
write_lock now stamps the runtime's own log path for the lock's nonce, in
forward-slash form so the suffix check matches. All three Windows lock writes
(the in-mutex spawn record and the two Desktop updates) pass through it, and
the fix ships with the host install, so no Desktop rebuild is needed.
Refs #132034
* test(desktop): a valid Windows SSH lock makes its backend lock-owned
The update sweep (_kill_stale_dashboard_processes via _lock_owned_serve_pids)
now leaves a Windows SSH backend to the code-skew watchdog, as on POSIX.
Pin that flip so it isn't undone as an accident.
* fix(desktop): honor default remote SSH profile
Signed-off-by: Shawn Wang <32839114+enwaiax@users.noreply.github.com>
* fix(plugins/telegram): clamp negative inline-query offsets to page zero
Telegram echoes back the client-controlled inline_query.offset string on
every inline query. build_inline_results passed it through int() and
used it directly as a slice start, so a negative offset indexed the
catalog from the tail: small catalogs served their last few entries with
negative-prefixed result ids, and larger catalogs could return an empty
page with a misleading next_offset that skipped most of the catalog.
Clamp the parsed offset to >= 0. Malformed offsets still fall back to
page zero via the existing except path, and valid pagination is
unchanged.
* fix(desktop): wrap SSH exec payloads in sh -c for fish login shells
sshd runs remote commands through the users login shell. Desktop payloads are POSIX sh, so fish remotes reject POSIX assignment syntax. Probe uname -s once per connection; wrap Linux/Darwin in sh -c; leave Windows PowerShell raw.
* fix(desktop): the fish-shell uname probe spawns the injected sshBinary
The sh -c wrap's one-time uname -s probe called runSsh without
command: this.sshBinary, so a Git-for-Windows ssh.exe connection spawned
bare ssh for the probe and failed the #103288 every-spawn contract. Also
restores the withRemoteTimeout test's closing brace and the blank lines
padding-line-between-statements wants in the new tests.
* feat(desktop): theme typography baseSize/lineHeight/letterSpacing
The dashboard theme system carries baseSize/lineHeight/letterSpacing but
the desktop's DesktopThemeTypography stopped at font stacks, so a user
who wanted larger text had to patch the ASAR-packed styles.css by hand —
reverted on every update. Add the three optional knobs to the theme
model and paint them as --dt-base-size/--dt-line-height/--dt-letter-spacing
in applyTheme(), mirroring the dashboard's typographyVars.
applyTheme otherwise writes every var unconditionally, but these three
have :root fallbacks in styles.css, so a theme that omits one must REMOVE
the inline value — otherwise the previous skin's size sticks after a
switch. Converted VS Code themes and CLI skins carry no typography
opinion, so built-ins and imports are unchanged.
Fixes https://github.com/NousResearch/hermes-agent/issues/41766
* test(pty): pin the 1:1 marker discard when the disconnect cancels mid-close
2c76125128f moved the legacy PTY path's marker discard into a finally so a
handler cancelled while `bridge.close` is still running in a worker thread
drops it. It shipped without a regression test; the existing disconnect
test only hit that timing intermittently on CI.
The new test makes `bridge.close` slow, waits until it has started, then
leaves the client context (which cancels the handler there) and checks the
marker goes. It fails on the pre-2c76125128f handler and passes on main.
* fix(desktop): preserve intentional turn interruptions
* test(desktop): an interrupted turn settles without a no-reply card
Regression test for #131205: when the silence watchdog confirms the
backend ended a turn the user already interrupted (Stop, superseding
prompt, session delete), markTurnWithoutReply must keep the
interrupted state instead of painting a retryable no-reply card. The
uninterrupted control keeps the card, guarding the genuine no-reply
path.
Co-authored-by: Yuan Li <dskwelmcy@163.com>
* fix(desktop): keep interruption guard within file health cap
* fix(tools): recommend set_value on a zero-delivery type verdict
When a type action delivers 0 of the requested characters, the field is
dropping synthetic keystrokes at the source (trusted-event checks on web
inputs). No delivery rung can fix that; the AX set_value path writes the
value directly and bypasses event filtering, so the escalation verdict
now recommends it instead of the generic px/foreground ladder.
Trimmed from #103157 / #103174 to the unique zero-delivery delta; the
snapshot_id element addressing landed via #111313 (element_token) and
#116367 (snapshot_id carrier).
Co-authored-by: Konstantin Khlopkov <konstantin.khlopkov93@gmail.com>
* fix(tools): gate the set_value verdict on type_text_incomplete
kokhlo's measurement on the trimmed PR: the zero-delivery gate also catches
type_text_synthesis_budget_exceeded, whose delivered_chars is pinned to 0
because the bounded synthesis budget declined to emit at all. That target is
not dropping synthetic keystrokes, and re-routing it to set_value replaces the
driver's own chunk recommendation with a different write path and a wrong
explanation. The gate now keys on the driver's own partial-delivery verdict;
the hint also names set_value as an action rather than a delivery rung.
* chore: map PRELVAS94 for salvage of #132933
* fix(cron): keep dispatching when the due scan cannot persist its repairs
_get_due_jobs_locked repairs store-side problems mid-scan (half-paused records
self-disable, completed one-shots are swept, records are normalized) and then
persists them with a bare save_jobs(...) before returning the due list. Any
store-write failure there — ENOSPC on a full disk, a read-only mount, a
permissions problem — raised out of get_due_jobs(), so cron_tick() aborted and
NO job on that profile ran until a write succeeded. Live on a full /opt/data:
"Cron tick error for profile at /opt/data: [Errno 28] No space left on device"
from that exact call site, repeating every tick, with every job then reporting
"missed its scheduled time" and re-anchoring.
The repairs are already applied in memory — that is what the scan keys off — so
the persist is a side effect the next tick retries. Contain the store write and
keep dispatching; a non-store error still propagates.
(cherry picked from commit 76676fb0084f06890f76280f50387325ad2d59c7)
* chore(evals): drop the PR-local cron due-scan eval script
The salvaged fix ships with a real tick() regression test; a standalone eval script under evals/
duplicates it and is not run by CI.
* fix(cron): skip dispatch, not crash, when the store cannot persist the advance
The salvaged due-scan guard only moved the crash: on an unwritable store (ENOSPC/EROFS/EACCES)
advance_next_runs raised out of tick() for recurring jobs, and claim_job_for_fire raised in the
worker for one-shots. Running without either write would break at-most-once (a crash mid-run
re-fires), so both sites now skip that dispatch instead: recurring jobs are dropped from this tick,
an unclaimable job's execution is closed as failed. The jobs stay due and fire once writes work.
A rate-limited WARNING (once per errno per 15 minutes) lives in scheduler_tick.py, the tick's
topical sibling, rather than in the 3.5k-line jobs.py.
* test(cron): trim due-scan tests to the scan guard + one real tick() regression
The contributor tests called get_due_jobs() directly and stayed green while tick() still crashed
in advance_next_runs. Keep one scan-level invariant and add a full tick() run on an ENOSPC store:
returns cleanly, runs nothing, warns, and leaves the store unchanged.
* refactor(cron): move tick-yield logging and catch-up counter into topical siblings
Code-health ratchet: the store-unwritable fix grew cron/jobs.py (3505 > 3495) and
cron/scheduler.py (4464 > 4457), both already over the 2000-line target. Pure moves, bodies
unchanged: _log_tick_yield_once (+ its rate-limit state; sole caller is the tick) into
scheduler_tick.py, and the catch-up occurrence counter pair into occurrences.py. Importers
(cron_health metrics, the stale-edit test) updated; no re-exports left behind.
* test(cron): pin the claim-path store-unwritable guard and the one-shot pass-through
Gate r1 (2ab + 2c, Medium): the tick regression stubbed _process_due_job and
held only recurring jobs, so neither the `except OSError` around
claim_job_for_fire nor the "one-shots still dispatch past a failed advance"
filter was exercised; reverting either kept every test green.
Add a due one-shot to the same test, stub run_one_job instead, and assert the
one-shot's execution row closes as failed ("Cron store unwritable") rather
than being left claimed. Red with the claim guard reverted (row stays
"claimed") and red with the filter dropping every due job (no row at all).
* refactor(cron): move _claim_owner_is_dead into scheduler_ownership to keep jobs.py under its size cap
Code-health ratchet: cron/jobs.py is capped at 3495 lines (its value on
main). The next commit moves the store-unwritable warning helper down into
jobs.py (gate r1 quality finding), which would push it over. Offset that by
a pure move of the self-contained claim-owner liveness probe into the
existing ownership sibling; body is byte-identical, its only caller
(_claim_is_live) late-imports it, and nothing in tests/ patches it.
* fix(cron): rate-limit the due-scan save warning through the store-unwritable helper
Gate r1 quality (Medium): the due-scan save guard in _get_due_jobs_locked
called logger.warning directly, so while the store stayed unwritable it
logged every 60s tick (the repairs are never persisted, so needs_save stays
true) -- the spam _STORE_WARN_INTERVAL_SECONDS exists to stop. Its wording
("due jobs still dispatched") also contradicted the tick-level warning that
recurring dispatch was skipped in the same tick.
Move warn_store_unwritable and its state into cron/jobs.py next to
_write_marker (the lower layer; jobs.py never imports scheduler_tick), route
the due-scan guard through it with neutral wording ("due-scan repairs not
persisted"), and have scheduler_tick.py and scheduler.py import it from
cron.jobs (scheduler.py via its existing top-level block, not inside the
except). The helper now takes the whole consequence phrase so each caller
states what it did.
The scan test now runs two scans and asserts exactly one neutral WARNING:
red on the previous code (2 warnings), green here.
* fix(cron): key the store-unwritable rate limit by store, and name the store
Gate r1 2c (Low): _last_store_warning was keyed by errno alone and process
global. In a multiplexed gateway every profile ticks in one process, so
profile A's ENOSPC warning silenced profile B's for 15 minutes, and the text
gave no store path, so an operator could not tell which profile was stuck.
Key the limiter by (cron_dir, errno) via _current_cron_store() (same module,
no lazy import) and put the store path in the warning. The scan test now
also warns from a second profile's store: red on the errno-only key (1
warning), green here (2, each naming its store).
* fix(cron): reap MCP orphans on the all-skipped unwritable-store tick
Gate r1 efficiency + quality (Low): when advance_next_runs fails and every
due job is recurring, the tick returned 0 without _sweep_mcp_orphans(),
unlike the idle and dispatch paths, so stdio children of earlier crashed
jobs were never reaped for the whole store outage. The partition also ran
the same predicate twice and filtered by O(n*m) dict-equality membership.
Partition in one pass (skipped count = n - len(filtered)) and sweep before
the early return, like the idle branch. The tick test is parametrized over
a recurring-only store (the skip-all branch) and asserts one sweep: red
without the sweep, green here.
* style(cron): blank-line, test docstring and ENOSPC stub cleanups
Gate r1 quality + reuse (Low): drop the third blank line before tick()
(E303); rewrite the test module docstring, which still claimed a writable
store is pinned and that the dispatch is never lost, to describe what the
two kept tests assert; lift one module-level _enospc stub shared by the
full_disk fixture and the tick test instead of two copies. No behaviour
change.
* docs(cron): say occurrences.py also holds the catch-up counter marker
Gate r1 quality (Low): the size-gate move put get/record_catch_up_occurrence
into occurrences.py, whose docstring only described scheduled identities.
Widen it to name the counter (the larger cron/markers.py consolidation is
out of scope for this fix).
* fix(cron): rate-limit store-unwritable warnings per call site so skipped jobs are always reported
The due scan's save runs first on every tick and fails whenever a recurring job is due, so a shared (store, errno) …
alt-glitch
added
os/windows
…tests Two hand-off tests in test_update_lock.py failed whenever pytest ran from a terminal that a gateway agent opened: test_delegate_keeps_the_claim_live_after_the_partner_dies and TestHandoffFromOrchestratingUpdater::test_child_runs_under_the_parents_live_claim. Root cause: the V9 rule (a hand-off is refused when a Hermes gateway/serve/dashboard sits above us) walks the real process ancestry, and the stand-in partner is not our ancestor, so the walk reaches the gateway that ran pytest and refuses. The code is right; the tests read the host they run on. An autouse fixture in that file now treats no ancestor as a runtime host, and a new test sets every ancestor to one to pin the V9 refusal, which had no test before. The tests broke when eebfab2 (#132338, @teknium1, 2026-10-03) added the V9 walk.
Stacked on #132365, merge after it. (This branch is rebased onto
fix/update-lock-owner-liveness; the diff againstmainincludes #132365's commits until it lands.)A
hermes updatekilled after pausing Windows gateways no longer leaves them stopped forever: the paused set is recorded on disk before anything stops and the nexthermeslaunch brings it back. No resume path starts a gateway onto a half-pulled tree or onto pre-sync dependencies. A launch killed while it is recovering, or an update driven by Desktop/Tauri, no longer loses that record.Changes
hermes_cli/update_pause_record.py(new) — the paused set is recorded at<root hermes home>/.hermes-update-paused-gateways.json:os.replace) before the first stop;pid+ct:<create_time>, the same format as line 3 of the update marker);pause_id, only after the resume finishes.renameto<record>.<pid>.<nonce>.claimand writes its own identity into the claim. A claim whose claimer has died (taskkill, console close mid-resume) is an orphan and is re-adopted, exactly like a record whose owner died. A launch that still owes part of the set hands the claim back unowned, so the next launch retries at once instead of waiting for a long-lived chat to exit.update_lock.holds_checkout_lock). Every other process usesupdate_lock.update_in_progress(install_root).write()never overwrites another pause: a dead owner's set is merged in, and a live owner's makes it refuse (RecordConflict, so the update aborts before stopping anything).mainstill unlinks the marker on a failed fast-forward.pm.venv_is_current) count. A tracked file rewritten by a build step no longer keeps the gateways stopped on every later launch.hermes_cli/main.pycallsupdate_pause_record.recover()beside the fleet-restart hint (2 lines). It resumes every orphan through the same gated path. It is skipped forhermes update(which adopts the orphans into its own pause) and forgateway run.update_cmd_windows.py_pause_windows_gateways_for_updateis split into record / cold-start / stop phases (review m11; McCabe 14 → 6,mainis 8).update_lock.py(fix(update): update marker v2 (owner liveness, no age ceiling) + checkout lock held by the whole update tree #132365's file, smallest hunk; §7 V9 + review m4)_adopt_or_refusetreats a partner as a partner only when no Hermesgateway run/serve/dashboardhost sits between us and it:(p == _handoff_pid() or _is_ancestor_pid(p)) and not _runtime_host_below(p).gateway.status.looks_like_gateway_command_line,update_cmd_windows._hermes_holder_subcommand), never by argv substrings.--profile X gateway run,-p X …and the…/hermes_cli/main.pypath form are now recognised.holds_checkout_lock();update_in_progress()reuses it.Validation
taskkill /F /Tthe updater right after "Paused gateway profile(s)", thenhermes gateway statusmain: ✗ gateway left stopped, no record (run 37139296887)M AGENTS.md)main: ✗ restarted onto the torn tree (pid 11196)the paused gateway was not restarted by a successful update, although the update printed "✓ Restarting 1 unmapped Windows gateway process(es)". ✓ on the main-based round-1 code (37149751065). ✓ on this head +JOB_OBJECT_LIMIT_BREAKAWAY_OKin #132365'sbind_child_to_update_tree(37149753432). This is review B1 in #132365, not this PR. Round 3: restacked on #132365 @ 16691ad (breakaway fix) → ✓ on this PR's Windows install+update E2E (run 37155187411: test_pause_durable.py 3/3, test_gateway_across_update.py 6/6).assert ['done'] == ["resume ['default']"](the stranded.claimis never read)default; launch 3 finds nothing (resumed exactly once)the update treated its own orchestrator as another update and skipped the orphan['mine', 'orphan']hermes updateunder a host with argv--profile work gateway run,/opt/hermes/hermes_cli/main.py -p work gateway run, or-m hermes_cli.main --profile work dashboard'True False' == 'False True'(adopted) ×3statushost still adoptsthe checkout has 1 file(s) git left half-written (e.g. b.py)Touched suites run locally (
test_update_pause_record,test_update_lock, and 10 update/Windows files): the only failures are home-I/O-guard lines for a worktree under the hermes home (.worktrees/manifest.json,installs/…/test-environment). That is the known worktree environment issue; PR CI is the gate.Root cause
The pause token lived only in the updater's memory (plus an
atexitresume), and nothing checked whether the tree was whole before resuming. In round 2: recovery claims were never re-read, and the update counted its own orchestrator as a foreign update.Tests
tests/e2e/core/windows_update/test_pause_durable.py: cells (a), (c) and (b), each with a premise assertion.tests/e2e/core/upgrade/hosts/test_hostile_pause_record.py(new): the m3 cell on a real clone of the commit under test.tests/hermes_cli/test_update_pause_record.py:test_resume_waits_for_a_whole_treenow rewrites a tracked file after the moved HEAD and asserts that the verdict is about dependencies, not "half-written". That is the m5 invariant; round 1's version pinned the too-strict gate.-p+ path form, and dashboard.NOT_COVERED
UpdateLock(install_root=…).Round 3 (CI reds)
update_lock.py:188raw env copy)_FakeLock() takes no arguments)install_root)'done' == "resume ['default']", green on retry)['done']; free → resumes. No production change.node-ptynative build failed under node 26 (gyp); no JS in this PR; rerunRound 4 (composite red: torn-pull cell vs #132361's restore)
Red: on the composite (origin/main + all 7 updater PRs)
test_pause_durable.py::test_failed_pull_does_not_start_gateway_on_torn_treefailed its premise:rc=1, HEAD=pre, status=''(run 37159782760, job 111310649104). The updater output shows the held file did fail git (error: unable to unlink old 'website/package.json': Invalid argument), then⚠ git stopped partway through writing the new code — restoring the checkout to 0566bbfbfd... ✓ Checkout restored, then✓ Restarting 1 unmapped Windows gateway process(es).Cause (class b — the contract changed): #132361 (commit point) calls
restore_interrupted_pull(after_failure=True)viaupdate_cmd_commit.settle_failed_tree_movewhen git exits mid-move, so HEAD, index and files go back to the pre-update commit and the pull marker is removed. The torn tree the cell relied on no longer exists on that base. This PR's whole-tree gate (update_pause_record.tree_is_whole) then correctly sees HEAD == pre, no new tracked change, no marker, and resumes the paused gateway.Changed assertions (
test_failed_pull_does_not_start_gateway_on_torn_tree):unable to unlink olderror naming the held file is in the output. That still proves the pull really failed on the held file, and it does so on both bases.AGENTS.mddirty; this PR alone): gateway NOT restarted, pause record kept. These are the old assertions, unchanged.hermes-update-pullmarker, the updater reportedCheckout restored; with fix(update): make the git/ZIP swap a single crash-safe commit point #132361): the paused gateway runs again with a new pid on the old code, is still alive 15 s afterhermes updateexited, and the pause record is discharged. Why this is the right contract: the failed update is a no-op on a whole tree. Leaving the user's gateway down would turn a failed update into an outage, and holding the record would only defer the same restart to the next launch.Proof on real Windows (full
windows_updatesuite, which includes all oftest_pause_durable.py):wine2e-install/upd-txn-r4y-winwine2e-install/upd-txn-r4y-compositeRound 5 (R7, R12, R13, per-target readiness)
Head
10c00044a78, stacked on #132365c6f803b0b48.Fixes
.hermes-update-paused-gateways.lock, A7). A claim is published with the claimer identity already inside it, by atomic replace, and only then is the source retired. A rival never sees a file that names a dead claimer.pause_id, and an update that folds orphans in lists their IDs underabsorbed. A crash between publish and retire leaves a copy that readers skip and the next mutator retires (retire_redundant).stop_requestedbefore its first stop request (mark_stop_requested). A live entry that was never asked to stop is dropped (it is still serving). A live entry that was asked is draining: it is not restarted, and its debt goes back unowned until it has exited._resume_paused_setretires each profile, unmapped argv and SCM service only on its own readiness (profile home / argv filter). A failed service or profile is raised at the end and never keeps the independent targets stopped.identity_is_livecallsupdate_lock.incarnation_live, the one incarnation rule. Our pid is ours only at its exact creation time, and our pid with no ct is dead. There is no private copy..hermes-update-paused-gateways.<sha256(install_root)[:12]>.json. Another checkout sharing the home never imports, relabels or certifies this checkout's set, andwrite()refuses a foreigninstall_root.Proof (real processes)
Linux probe
pause_probe_r5.py, extending the reviewer'slocking/pause_probe.py:0f998f61ef2)8 invariant tests were added to
tests/hermes_cli/test_update_pause_record.py. They use real child processes andsettraceboundaries. All 8 fail on pre-R5 and pass on head.Native Windows
tests/e2e/core/windows_update/test_pause_custody.pyhas 4 cells and no xfail. Each schedule is planted with the installed module, and each recovery is a realhermes.exelaunch restarting the real gateway.26f934a30ec+ cells, run 3718247109610c00044a78, run 37184232243 (fullwindows_updatesuite 30/30; custody 4/4; 0 xfail)Harness notes:
hermes_bootstraplike a launch.gateway_state.jsonis cleared between cells.gateway.respawn_storm.max_startsoff, because the cells restart the gateway about 10 times on purpose.NOT_COVERED (R5)
.hermes-update-paused-gateways.json, which has no checkout key) is not read after upgrade. This affects only an unmerged-PR window, and reading it would re-open R13.Round 6 (re-review N1 / D12: never-asked gateway held as draining)
Head
2915b5866dc.Defect.
mark_stop_requestedrecorded intent for every running gateway before the first stop request. An updater killed between that write and the request left a gateway that was never asked and is still serving. Every launch then held it as "draining" with no expiry, and restarted it once it exited, including after a deliberatehermes gateway stop.Contract.
Fix (
update_pause_record.py,update_cmd_windows.py):mark_stop_requestedrecords intent, plusstopper_pidand each mapped gateway's planned-stop marker path.mark_stop_sentrecords each request from_request_socket_pausesviaon_request:drop_never_stoppedkeeps a live entry only when its pid isstop_sent, or when this update's own marker is still on disk naming it (target_pid,stopper_pid). A user's stop marker does not count. That evidence is resolved intostop_sent, so merges and hand-backs carry it._planned_stop_markers): a profile process without a home has none.Proof.
after-markandafter-mark + user stop markerare RED on10c00044a78. Every stage is GREEN here, including "request on disk, killed before recorded sent → owed, restarted once".tests/hermes_cli/test_update_pause_record.py:test_a_stop_intended_but_never_issued_leaves_a_serving_gateway_alone_and_a_later_user_stop_finalis RED pre-fix ("held as draining").test_a_request_on_disk_is_owed_even_before_it_is_recorded_sentcovers the marker-evidence path.test_pause_custody.py). Both cells drive the REAL_pause_windows_gateways_for_updateagainst a real gateway:_never_asked: killed atprofiles = _stop_windows_gateways(. The launch neither holds nor restarts the gateway, and after a realhermes gateway stopthe next launch does not restart it._asked: killed at the socket call after the marker is on disk. The gateway exits by itself and is restarted exactly once._drainingcell now recordsmark_stop_sent.wine2e-install/upd-r6-win@728e7f4f706: run 37190511880, custody 6/6 and 32/32 tests.wine2e-install/upd-r6-win-red(pre-fix code plus the new cells): run 37190509746. It fails exactlytest_a_gateway_the_update_never_asked_is_left_serving_and_a_user_stop_is_final("a gateway never asked to stop is held as draining", recordstop_requested: ['14540'],resume_needed: True); the other 5 pass.Infographic
Round 6 (continuation) — restack
Restacked on LOCK
c729ffc5af7, which adds the Windows custody fix: the Node build's job launcher runs on the base interpreter, and a refused join degrades instead of failing the build. The WIN commits are unchanged.Round 7 — restack
Restacked on the new #132365 head (reset + cherry-pick of this PR's own commits, no conflicts). No R6 finding targeted this PR's files.
Verification: head
6b44b5ee733, CI green (wait_for_pr_green exit 0).Round 8: reviewer port + D4 (Windows restart-debt paths)
d8669631836(keeps pause debt intact across marker consumption and adoption). Cherry-picked cleanly as2f4fd37ebd6.7f5b98a0e10: a completed obligation can no longer run again from a copy Windows kept. When a recovering launch finishes its claim, or an update discharges its record, the obligation's ids (plus every id it absorbed) are first written to a durable retired list (<record stem>.<key>.retired), and only then are the files unlinked. If a reader withoutFILE_SHARE_DELETEblocks the unlink, the leftover copy is now redundant by id instead of being an orphan the next launch would resume again. When copies share an id, the one with the highestrevwins, so a partly resumed claim is not shadowed by the stale source. The list is pruned once every file is readable and no file carries a listed id.d3d8349c821: an SCM service reportingrunningonly proves the wrapper started. A restarted service now leaves the token only after a stable gateway in its profile home passes the same readiness poll that relaunched profiles use. That gateway must also descend from the service's current process. Otherwise the service stays owed and the durable record survives for the next launch.9e01b731511: the force-kill after the drain is now authorized by the birth time captured at discovery (a mapped gateway's strict PID-file identity first, launchers before the first request). It no longer usesget_process_start_time(pid)re-read at kill time, which would let a PID reused during the drain validate against its replacement's own birth. A PID with no discovered birth is still refused.Native tests:
tests/hermes_cli/test_update_pause_debt_windows_live.py(platforms("windows"), nothing mocked). It uses real reader processes holding the file, a compiled .NET service wrapper under the real SCM running a realhermes gateway run, and real survivors killed with taskkill.2f4fd37ebd6+tests (run)9e01b731511(run)The PR's own Windows-only lanes (x64 and arm64) at
9e01b731511also ran all 7 cells green. On Linux, 13 related update/pause test files produce the same pass/fail set at base and head.Still not covered: a storage failure while writing the retired list makes the resume run again (fail-safe). Old code completing an obligation does not write the list. The race between the identity check and
taskkillis unchanged. A real PID reuse was not induced; the cell supplies the discovered birth as data. The SCM cell runs a test wrapper as LocalSystem, not a Hermes-installed service.Round 9: re-review M2, M3, m5
M2
08ce58ae672: an unreadable retired list no longer means "nothing retired". Only an absent list is empty. A list that exists but cannot be read (a Windows sharing violation, an AV scanner) or is malformed raisesRetiredUnknown, and then:claim()refuses;_retire()raises before it rewrites the list or deletes a carrier;hermes updateaborts before its first stop and says it cannot read what an earlier update paused.orphans()now reads the list only under the pause mutex.M3
823c214534e: a launch that recovers an orphaned pause (recover(), which runs before every command) now probes an SCM service's gateway for 5 s instead of waiting the update's full 60 s. When it finds a running service whose gateway is not ready, it recordsservice_retry(60 s, doubling, capped at 1 h). Later launches skip that still-running service at once until the backoff elapses. The debt stays owed throughout. A stopped service is still started.hermes updatekeeps the full wait.m5 (first bullet)
7145a469d59:consume_planned_stop_marker_for_selfwaits at most 2 s for the pause mutex. If the mutex stays busy or the checkpoint cannot be written, the stop is still classified as planned. The marker is not consumed: it stays on disk as the request evidence that recovery reads in place of the checkpoint, and it expires by TTL.13017670871+ testsRecordBusyafter 10.03 sAll 8 native cells in
test_update_pause_debt_windows_live.pypass on GREEN. On Linux, the 11 related files give 161 passed and 0 failed.Still not covered:
.retired; the Linux test stands in for it.