Skip to content

fix(desktop): tool edits and failed rebuilds keep the session profile - #104982

Merged
teknium1 merged 3 commits into
mainfrom
fix/desktop-profile-config-atomic-e461
Sep 7, 2026
Merged

teknium1 merged 3 commits into
mainfrom
fix/desktop-profile-config-atomic-e461

Conversation

@teknium1

@teknium1 teknium1 commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Tool changes and rebuild failures now stay attached to the live Desktop session's profile and reachable database owner.

Stacked follow-up to #104842, based on its exact head 194074d75528326843f07bceb0bc88f7cffc9c34; base branch is fix/desktop-profile-rebuild-store, not main. The original branch and its existing scoped validation receipt are unchanged.

  • Scope tools.configure config load/save, reset, and response readback from the live session's profile_home; the real client sends session_id without profile.
  • Prepare model configuration before constructing a replacement. Install the replacement and transfer dedicated DB ownership together under the session registry lock; both reset and capability refresh use that path.
  • Add rebuild invariants and extend target-isolation regression coverage, extend the reusable serve/WebSocket probe with session-only ingress plus failure/ownership controls, and document the contract.

Root cause: the previous fix scoped reconstruction but not the configuration write, and transferred ownership before fallible model-target preparation and session installation.

Live repro

Actual isolated hermes serve, WebSocket dispatch, real AIAgent construction and SQLite with a credential-free loopback model. Failure arms inject one model-config read error through a probe-only bootstrap; rebuild predicates, construction and teardown remain production code.

Scenario Base #104842 Follow-up cba2b4dfe66172af253fd92e293aebccc0f6c011
Session-ID-only tool enable Launch config changed; worker unchanged Launch byte-identical; worker tool enabled
Failed tools reset Same old agent, ownership false; profile ref leaked after close Same old agent remains owner; close releases profile ref
Failed capability refresh Same old agent, ownership false; profile ref leaked after close Same old agent remains owner; close releases profile ref
Successful reset and refresh controls Existing persistence fix retained New agent owns DB; profile ref released on close

Each final live arm persisted four rows in the worker store and zero in the launch store. Four final controls/failure arms passed on the committed head. All probe listener ports were closed afterward. Probe commands use evals/desktop_bug_campaign/persistence_live.py --repo <checkout> --out <receipt-dir>, with --observe / --failure and optional --reset.

Validation

  • RED on base: six new parameterized cases failed for wrong-profile writes or ownership loss/allocation before preparation.
  • GREEN: four focused files, 41 passed / 0 failed; two sibling files, 672 passed / 0 failed. Both runs used the campaign flock and scripts/run_tests.sh -j 1.
  • Initial independent review was superseded by parent review identifying two additional fail-open gaps; the correction received independent read-only review with no P1/P2 findings.
  • Ruff, Windows-footgun scan, compatibility-pointer check, subprocess-stdin check and git diff --check passed.
  • Parent owns the broad combined integration harness and CI monitoring. Native Electron, Windows, multi-hour automatic compression and concurrent close-during-build stress are not claimed. Intentional archive generations remain untouched.

Addresses the independent-review findings for #104079. Prior substantive work remains in #104842 with @HexLab98 (#104143) and @liuhao1024 (#101740) credited there; this PR is only the corrective delta. No issue closure or merge is requested automatically.

Campaign: #104904.

Infographic

Profile-safe rebuilds

Final-review corrections

  • Missing, disappeared, non-directory, or failed explicit profile resolution no longer falls back to launch history/configuration. Existing omitted/default/launch resolution is preserved.
  • A stale nonempty tools.configure session ID returns existing error 4001 before config read/write; genuinely sessionless global settings remain supported.
  • Actual CI run 34108999632 had one root failure: test_config_read_guard rejected raw config YAML readback in the live eval. It now uses read_user_config_raw(explicit_path); no allowlist relaxation. Aggregate failure was downstream only.
  • Live WebSocket A/B on prior head: stale session wrote launch config and all six missing-profile list/get/set calls succeeded. Fixed: stale session rejected, all six calls rejected, launch config byte-identical; real worker reset/history and global configuration controls succeed. No changed predicate is patched.
  • Four old full-file fixtures needed real temporary profile-root binding after fail-closed resolution; assertions remain intact.
  • Final serial recheck: 6 files, 665 passed / 0 failed. The committed-head live replay passed at 50aa1ba618c50ee14f664aff17c9ddf5de398555 (4 worker message rows, 0 launch rows; all 6 unavailable-profile requests rejected). Raw wire receipts and SHA-256 evidence are recorded in the campaign final results. Parent accepted the exact-head source, independent review and live evidence. CI on the new push and broad combined coverage are separate; no CI-green or merge claim.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 7, 2026
@teknium1
teknium1 marked this pull request as draft September 7, 2026 10:46
@teknium1
teknium1 marked this pull request as ready for review September 7, 2026 11:15
Base automatically changed from fix/desktop-profile-rebuild-store to main September 7, 2026 11:54
@teknium1
teknium1 force-pushed the fix/desktop-profile-config-atomic-e461 branch from 96d7c44 to dd0c9be Compare September 7, 2026 12:53
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on dd0c9be — test: enforce fail-closed project profile targeting

⚠️ Warnings

OSV vulnerability scan · View job

28 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.


debug info

CI timings

CI timings · View report · View job

Wall time 4m44s vs 4m41s (+1.1%). 9 job(s) slower, 5 faster, 1 unchanged.

  • Python tests / Run tests: +31.0s
  • Python lints / Windows footguns (blocking): +13.0s
  • OSV scan / Scan lockfiles / osv-scan: -7.0s
  • OS-specific tests / macOS-only tests: -7.0s
  • Check no committed infographics / check-no-committed-infographics: +3.0s

@teknium1
teknium1 merged commit 6927ff8 into main Sep 7, 2026
37 checks passed
@teknium1
teknium1 deleted the fix/desktop-profile-config-atomic-e461 branch September 7, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants