Skip to content

fix(kanban): make nested worker env scrub prefix-wide - #1

Draft
enzo-adami wants to merge 1 commit into
DavidMetcalfe:fix/81508-nested-kanban-env-scrubfrom
enzo-adami:agent/pr81843-prefix-scrub
Draft

fix(kanban): make nested worker env scrub prefix-wide#1
enzo-adami wants to merge 1 commit into
DavidMetcalfe:fix/81508-nested-kanban-env-scrubfrom
enzo-adami:agent/pr81843-prefix-scrub

Conversation

@enzo-adami

Copy link
Copy Markdown

This is a focused fail-closed extension of your NousResearch#81843 branch.

The current normal path removes the seven names in KANBAN_ENV_KEYS, while its import-error fallback already removes the full HERMES_KANBAN_ prefix. That leaves current behavior keys (BRANCH, GOAL_*) and future authority keys such as WORKER_SCOPE available to terminal/execute-code children.

Change

  • make strip_kanban_env() remove the entire HERMES_KANBAN_ namespace
  • retain KANBAN_ENV_KEYS as the public inventory, but do not use a hand-maintained tuple as the security boundary
  • extend all foreground/background/real-subprocess/execute-code regressions with branch, goal, worker-scope, and an unknown future capability

Validation

  • targeted environment suites: 87 passed, 0 failed (3 Windows-only skips on macOS)
  • Ruff: PASS
  • Windows-footgun gate: PASS
  • git diff --check: PASS

This keeps your non-terminal Codex/ACP runtime exception unchanged; only nested terminal and execute-code boundaries become prefix-wide.

@DavidMetcalfe

Copy link
Copy Markdown
Owner

Superseded — the change was cherry-picked into NousResearch#81843 as commit 5f1b678 (authorship preserved). Thank you!

@DavidMetcalfe DavidMetcalfe reopened this Aug 15, 2026
DavidMetcalfe pushed a commit that referenced this pull request Aug 18, 2026
Two independent bugs let a deleted profile reappear / leave orphaned
resources on next launch:

1. hermes_cli/profiles.py's backend-process scanner required argv[0] to
   resolve to an executable literally named "hermes". Electron's
   pool-backend spawn resolves the hermes console-script shim's path and
   execs it via the interpreter directly (python3 /path/to/hermes ...), so
   argv[0] reports as "python3" and the scanner never matched the running
   backend -- delete removed the profile's files but left its live backend
   process running (still bound to a port via uvicorn), which
   accumulates across repeated delete/recreate cycles.
2. The desktop sidebar's ProfileRail only refreshed its cached profile
   list once, on mount, so a delete/create/rename from another surface
   (another window, or the CLI) left a stale ghost entry until something
   unrelated triggered a refetch. Note: a delete via this window's own
   Manage-Profiles view already refreshes the shared $profiles atom
   ProfileRail subscribes to (confirmed by reading refreshProfiles() and
   handleConfirmDelete()) -- this fix only covers the cross-window/cross-
   process staleness gap, not a duplicate of the already-merged
   NousResearch#57329's Manage-Profiles rail-refresh work.

Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named
console-script shim via argv[1]. Fix 2: refresh the profile list on window
focus/visibilitychange, matching the existing pattern used elsewhere in
the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx,
use-gateway-boot.ts all use the same focus+visibilitychange pattern).

## Related work already on main

PR NousResearch#57329 (merged) fixed the *headline* symptom from issue NousResearch#52279
(deleted profile respawns) via a different, non-overlapping mechanism:
routing profile-delete through the primary backend instead of spawning a
fresh pool backend, plus a separate recreation guard in
ensure_hermes_home() (NousResearch#49435, merged) that makes a backend spawned into a
deleted profile's directory raise FileNotFoundError instead of silently
recreating it.

This PR is NOT a duplicate of that fix. Verified: even with both of those
merged, a backend process that survives because of gap #1 above still
holds a bound port via uvicorn -- it just can no longer resurrect the
profile directory. That's real resource-hygiene, not a symptom already
covered. Gap #2 touches a different file/component (ProfileRail /
profile-switcher.tsx) than NousResearch#57329's rail-refresh half (which touched the
Manage-Profiles view's own $profiles.ts / index.tsx) and covers a
distinct staleness path (cross-window/cross-process, not same-window
delete-then-refresh).

Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing +
regression coverage for the argv[0] python-interpreter detection case).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DavidMetcalfe added a commit that referenced this pull request Aug 18, 2026
… read

Addresses teknium1's review (NousResearch#64195) finding #1: the previous PR placed
the migration inside the connection IIFE, AFTER
`resolveRemoteBackend(primaryProfileKey())`. When the preference file
was missing, `primaryProfileKey()` resolved to 'default' and the remote
branch returned immediately without ever reaching the migration. Remote-
mode users got no migration at all.

Move the call site to the top of `startHermes()`, before the connection
IIFE that reads `primaryProfileKey()`. Both remote and local branches now
flow through this path before any profile-dependent resolution, so the
migration runs on first boot regardless of mode.

The inlined implementation is replaced with a thin wrapper that builds a
`MigrationDeps` bag and delegates to `migrateActiveProfileIfMissing` from
`profile-migration.ts`. No production behavior change beyond the call-
site move.

Tests added in a separate commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants