Repository navigation
Conversation
Add redacted capability discovery and Kanban-backed cross-profile delegation behind an explicitly enabled executive_bus toolset. Gate the model-facing tools on profile/platform config and local prerequisites, preserve credential isolation, and add tests for capability lookup, delegation policy, tool exposure, profile-delegation continuation, and existing-board migration.
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 830543e7c3c8696797119497c4fc0cb6c5ea9a9d against exact current main / PR base 63279301bcbdc185c1b07b98a9312eb0c862f26d. I read the 18-file diff, the full delegation/policy/gating paths and their tests, the current approval + profile boundaries on main, existing review/comment state (none on this head), exact-head workflow state, and the adjacent delegation/Bot Mode/Kanban work.
There is good architecture here worth preserving: the Bus is opt-in instead of always-on, capability discovery returns redacted metadata rather than credentials, execution stays inside the selected profile, and the implementation attempts to give the handoff a durable Kanban audit trail plus same-session continuation. Those are the right boundaries for this feature class.
I found two blockers before this can safely become an execution authority.
BLOCKER 1 — CONSEQUENTIAL_WRITE is enforced from intent text, not at the mutation boundary
The PR says consequential writes are blocked pending explicit approval, but the implementation does not currently make that property true for the actual executor.
The chain is:
tools/profile_delegation_tool.pymakesriskoptional/defaultREAD;tool_name,action_name, andoperationare also optional.hermes_cli/delegation_policy.pyclassifies the request with a finite substring list. If no listed write term is found, the fallback is the requested risk — normallyREAD.delegate_to_profile()then launches a normal worker under the executor profile, with that profile's normal tools and credentials. For aREADclassification, the only write restriction after launch is the English instruction inbuild_delegation_worker_body()("inspect only"). There is no server-side read-only execution scope or tool-call mutation gate attached to the delegated run.- Conversely, an
approval_idis treated as sufficient authority merely by being non-empty. I do not see validation binding that value to the requester, originating session, executor, exact action, target, or current generation.
That leaves deterministic write-shaped requests outside the classifier vocabulary able to enter as READ. Examples include push this branch, merge the PR, commit these edits, restart the gateway, or apply the migration. None of those verbs is in _WRITE_TERMS; with the model-facing defaults and no structured operation, policy returns ALLOW, then the executor receives its ordinary tool surface. The existing test_consequential_write_is_blocked_before_task_spawn does not exercise this boundary: it explicitly supplies risk="CONSEQUENTIAL_WRITE" and a task containing Deploy, so it proves declared-risk handling rather than fail-closed classification.
The fix needs to move authority to the effect boundary. READ / PREPARE delegations should execute with a mechanically constrained tool surface (or every mutating tool call should pass through the existing approval/mutation machinery) so an LLM cannot turn a read-authorized delegation into a write by choosing different words later. If an approval receipt is supported, validate it against the exact requester/session + executor + action + target being executed; a truthy opaque string cannot be the authorization decision.
Please add regressions that prove pre-effect refusal, not only policy-label output: at minimum push/commit/merge/restart/apply-migration/upload variants, an indirect/nested write attempt after a read-only initial task, valid READ/PREPARE controls, and one exact approved write showing the approval cannot be replayed against a different action or target.
BLOCKER 2 — queued profile delegations have a consumer, but no completion producer
The synchronous path can finish during delegate_to_profile()'s polling window. The timeout path is different: it returns
Delegation is queued/running; result will be returned via session continuation when available.
and gateway/run.py has been extended to consume profile_delegation events from process_registry.completion_queue.
But the patch does not wire the other half of that protocol. hermes_cli/profile_delegation.py defines _emit_completion_event(...), and that is the only place in this PR that enqueues a type: "profile_delegation" event; there is no call to _emit_completion_event anywhere in the patch. The new complete_profile_delegation() / fail_profile_delegation() DB helpers only update SQLite + append Kanban events. Nothing observes a delegation that outlives the tool timeout, extracts its terminal result, updates the delegation row, and enqueues the continuation event.
So the exact advertised asynchronous case can strand permanently: the caller gets queued, the worker later finishes, the gateway watcher is ready to receive an event, but no producer sends one. The current continuation tests exercise event delivery machinery, not this full queued → worker terminal → DB reconciliation → completion_queue → original-session path.
Please wire one durable reconciler/terminal hook that owns this transition, and add an end-to-end regression with a delegation deliberately completing after the initial wait returns. It should prove exactly one terminal event, exactly one originating-session continuation, restart/recovery behavior, and idempotence under repeated reconciliation. This is also the point to keep routing provenance explicit rather than reconstructing it from ambient profile state.
Interlocks / merge order / adjacent work
These are related but should not be flattened into duplicates:
- #102195 / #102155 is adjacent: it makes Bot Mode's existing
message_agentroster declarative for CLI-only/multiplexed installs. It does not provide capability selection or delegated execution, so this PR should not supersede it. - #102390 is complementary and collides mechanically on
hermes_cli/kanban_db.py: it is explicitly working on mechanically verifying delegated completion instead of trusting worker self-report. This PR currently derives its structured result from worker-supplied Kanban metadata, so the two changes need an explicit composition decision rather than independent additions to the same completion boundary. - #100758 is broader Bot/profile orchestration and overlaps this PR on
gateway/run.py; preserve whichever routing/continuation semantics land first and rebase the other rather than duplicating the hook. - #102117 is an active current-base decomposition by
teknium1and owns both giant surfaces this PR edits (gateway/run.pyandhermes_cli/kanban_db.py). If that lands first, this feature should target the extracted owners instead of restoring feature growth to the facades; if this lands first, #102117 will need an explicit forward-port. That merge order should be stated before either branch is treated as ready.
Historically, the background-completion/multiplex work in #94744 is also useful evidence for the "other side of the shape": runtime profile, session identity, and transport owner are not interchangeable. The current profile_delegation event initially carries only session_key plus requester/executor data and relies on _enrich_async_delegation_routing() later, so the queued/restart regression above should deliberately cover a secondary-profile/multiplex route rather than only a default-profile happy path.
The PR itself currently has no owning issue / Fixes / Closes / Refs interlock. For a new cross-profile execution authority this should have a dedicated design/bug record; please don't point it at #102155 as if these were the same feature.
Exact-head verification state
The single reviewed commit is the exact PR head. Hosted CI, Docker, and Nix workflows for 830543e7c3c8696797119497c4fc0cb6c5ea9a9d are currently action_required and the CI run has zero jobs, so there is no exact-head green hosted receipt yet. I am not treating that as a code failure, but it means this head is not release-clear until those workflows actually run and all commit-level gates are green. The head commit also uses Crusty Crabs <crustycrabs@Michaels-Mac-mini.local> and I found no current repository mapping for that email, so the contributor/attribution check needs to be allowed to run rather than assumed green.
Once the mutation authority is enforced where effects occur, the post-timeout completion protocol is closed end-to-end, and the merge/interlock + exact-head gates are reconciled, the core idea is strong. The opt-in surface and executor-side credential isolation are worth keeping intact through those fixes.
|
Topology correction after rechecking the live successor and existing issue graph. There is already an open repository issue for the cross-profile trust boundary this feature enters: #53309. That issue specifically requires per-profile #102445 is the current attempted successor to this head, but it is not yet a valid replacement carrier. At exact head The integration graph is therefore:
Do not let merge order choose among those contracts. The eventual Executive Bus carrier needs to compose #53309's target-authority rule, the corrected independent-completion evidence from #102390, and the bounded owners from #102117 before this feature can be considered closed. |
|
Closing at owner request. This was opened from a fork while validating a private Phantom deployment path; we are withdrawing the public contribution. |
BLOCKER 1: CONSEQUENTIAL_WRITE now enforced fail-closed at the mutation boundary. - Expanded _WRITE_TERMS to cover push/commit/merge/restart/apply/upload/install/create/delete/destroy and 60+ other write verbs - Reordered classifier: explicit read verbs (read/inspect/list/get/check/query/status/view/analyze) always return READ even when other terms present - Any unrecognized verb with a write term now classifies as CONSEQUENTIAL_WRITE (fail-closed) - approval_id is now HMAC-bound to requester+executor+capability+task to prevent replay attacks - Added generate_approval_id() and validate_approval_id() helpers - Short (<32 char) or mismatched approval_ids are rejected BLOCKER 2: Wired the async completion protocol end-to-end. - Added reconcile_queued_delegation() as the durable reconciler/terminal hook - It observes terminal task state, extracts structured results, updates the delegation row, and emits exactly one completion_event - Idempotent: second reconciliation returns None (already reconciled) - _emit_completion_event() is now called from the reconciler Tests added: - tests/hermes_cli/test_delegation_policy.py: 28 tests for fail-closed classification, approval binding, enforcement - tests/hermes_cli/test_profile_delegation_async.py: 8 tests for the async completion protocol Co-authored-by: andrexibiza <andrexibiza@users.noreply.github.com>
PR: feat: add gated Executive Capability Bus delegation
Summary
Adds a service-gated Executive Capability Bus for Hermes profiles:
find_capabilitydelegate_to_profileexecutive_busis explicitly enabled for the current profile/platformThis is intentionally not an always-on core surface. It is a narrow, opt-in toolset for orchestrator profiles such as Phantom/Director.
Why
Michael's operating model needs Phantom/Director to coordinate Mr Crabs/CEO and department-head Hermes profiles without sharing all credentials into one monolithic runtime.
The Bus gives Director a safe route to ask:
Design
Capability registry
New module:
It inspects profile configs and ranks matching capabilities such as:
Returned data is redacted capability/workload metadata only.
Delegation policy
New module:
Phase 1 policy is conservative:
Profile delegation
New module:
Creates a Kanban task for the executor profile and stores an audit row in
profile_delegations.Tool gating
New module:
The model-facing tools only appear when:
executive_busis explicitly enabled in profile/platform toolset configModel-facing tools
New tools:
Tool names:
Runtime continuation
Existing gateway/process notification paths are extended so profile-delegation completion events can re-enter the requester session with a structured internal result block.
Files changed
Diff stat:
Tests run
Local branch after rebase onto current
origin/main:Phantom deployment branch before restart:
Runtime smoke on Phantom/Director:
Production pilot evidence
Phantom Director pilot is already running this code on a deployment branch:
Post-fix logs show no new
nemo_relay,snowballstemmer, Relay initialization, Traceback, or ERROR matches since dependency drift was fixed.Safety notes
executive_bustoolset gating.Follow-up after merge