Conversation
Summary: Very large PR (86 files): plugin hooks/commands now carry dispatch-owned session identity (profile-scoped, never ambient), plus session-scoped child-process execution routing and isolated desktop viewers. Focused review on the security-sensitive core below; UI/electron/test files skimmed. Findings (Non-blocking):
Verdict: Non-blocking. Security design looks sound (no ambient identity, no env inheritance for secrets/permissions); remaining items are verification nits. |
Every other section in the composer status stack is self-delimiting: status groups carry a caret + icon + label header, the billing wall and session control bring their own chrome. A plugin's contribution is raw content with none of that, and the slot also inset it by 4px where the rest of the stack uses 8px. Under another section it therefore read as that section's footnote rather than as its own status. Align the row with the stack's gutter and give it one hairline, only when a section actually precedes it -- a lone plugin row keeps no stray line above it, and sections that already have headers are not divided by a line they do not need.
27dc084 to
1aa2034
Compare
Reuse the synchronous test fixture event loop instead of asyncio.run, which clears the policy loop and can orphan its self-pipe sockets. Preserve all consent assertions and keep warnings strict.
|
Superseded by #104567, which now carries these generic plugin session surfaces (execution targets, session hook identity, plugin setup and Desktop session viewers) as one core PR on current |
Consented native setup follow-up
Adds a generic, profile-bound setup gate to the existing enable/install paths (CLI, Desktop RPC, REST, composite selection and packs). Plugins declare a package-root
setup.py; explicit consent binds the canonical key, selected home and setup revision. Cancel/failure preserves enablement, and no setup runs during discovery or tool execution. No Realms-specific core branch or new model tool.Desktop supports review, busy, cancel, failure and retry. Canonical-key collisions and revision changes cannot substitute another setup target; synchronous REST test loops and already-reaped subprocess cleanup are covered.
Latest verification after integrating upstream and the concurrent status-stack change:
Setup is trusted plugin code, not a sandbox. Saved enablement does not hot-reload cached conversations; restart the owning backend to activate. CI for this follow-up is tracked on the current head; older CI results below are historical.
What does this PR do?
Lets trusted plugins own session-specific execution and desktop UI without mutating the process environment, patching Hermes internals, or treating the focused chat as the owner of every session.
The concrete consumer is an external private-desktop plugin: each session needs its own terminal/Cua environment and lifetime, while the desktop provides a status badge and an isolated viewer. The compositor, driver containment, streaming protocol, viewer authentication and plugin implementation stay outside Hermes. This PR adds the generic extension points only; it does not bundle that plugin or add a core model tool.
Related work
No issue is automatically closed. There is partial overlap with #51596 and #56782 (gateway slash-command provenance), #82776 (ambient gateway ContextVars), and #85314 (lifecycle payload consistency). This change needs explicit CLI/desktop/gateway ownership before a lazy session's first turn, plus execution leases and desktop viewers; it does not replace those PRs' other goals. The shared command/lifecycle contract should be reconciled during review rather than landing competing APIs silently.
Type of Change
Changes Made
on_session_identitybefore lazy desktop/TUI create/resume returns; propagate distinct runtime, durable conversation, task and profile/home identities through existing hooks. Preserve resources across turn end and expose actual runtime finalization.handler(raw_args)callbacks and profile-scoped discovery.hermes_cli/session_execution.py; exact environment set/unset and argv prefixes through local foreground, background/PTY and Cua child paths. No process-global environment mutation. Shell startup cannot override registered routing; readonly conflicts abort before user commands.How to Test
Fresh verification on this branch, based directly on upstream
mainat006b1beb00d9:--file-retries 0 -W error -j 2npm run typecheckpassednpm run buildpassed, includingassert-dist-builtThe Python lane uses real imports and temporary Hermes homes, actual shells/PTYs, native plugin discovery, lifecycle dispatch and inert protocol children. The desktop suite covers per-session ownership, first-open consent, viewer lifecycle and permission boundaries. Reproduction commands for the desktop lane are in
apps/desktop/docs/plugin-session-viewers.md; backend commands/contracts are indocs/session-execution-context-api.mdanddocs/session-hook-identity-api.md.The external consumer's real Linux compositor/Cua integration suite was also rerun against this upstream-based host: 112 passed, 0 failed, 5 environment-gated skips. That separate consumer is not bundled in this PR, so this is supplementary local evidence, not an in-tree CI gate. Earlier assembled development-app tests exercised dual sessions, Watch/takeover/return and locked-screen operation; those were on the original companion build, not a fresh full-GUI rerun of this port.
Lanes overlap; do not sum them. A broader 128-file plugin/lifecycle lane returned 1,933 passed, 16 failed, 3 skipped. All 16 failures (Hindsight and FAL provider fixtures) reproduced on the untouched upstream base, with an identical failing-test set. The one additional observer test initially rejected additive identity fields; it now checks the preserved semantic fields and unknown-identity defaults and passes warning-strict. The entire upstream suite and native Windows/macOS acceptance were not rerun. Existing platform/environment skips are not counted as passes.
CI follow-up
Commit
fa6e9ac63155explicitly rejects private runtime/desktop ownership validation when POSIX UID support is unavailable, without disabling generic execution routing or bypassing ownership checks. Added capability-loss regressions and native Windows coverage; independent review passed.GitHub CI run 33984737693 passed, including the All required checks pass gate:
The earlier Relay finalizer timeout also reproduced intermittently on the untouched upstream base, with native Relay installed and retries disabled. No Relay code, test timeout, assertion, or retry policy was changed to obtain this green run; the pre-existing scheduling-sensitive test remains outside this feature fix. Local expanded warning-strict resource-lifecycle tests were not universally green and are not represented by the hosted CI results above.
Boundaries
Checklist