Allow a component-registered operation to be granted in a role's operations allowlist

[dawsontoth]

An operation a component registers with requiresSuperUser is meant to be grantable in a role's operations allowlist, but naming it in add_role, alter_role, or an impersonation payload was rejected as "not a valid operation name or group" — the grantable mark was made in the worker that registered the operation, while the validation that reads it runs on the main thread. The registration announcement now carries that fact across the thread boundary, so a scoped role can actually be granted a component operation.

Enforcement is unchanged: the main thread only decides whether a name is admissible in an allowlist, and the operation still runs through the registering worker's own chooseOperation.

One finding is disclosed rather than fixed, and is inherited from the registerOperation bridge (#1736) rather than introduced here — the announcement is fire-and-forget, so an add_role naming a component operation can still lose a race against component load at startup. It fails closed (a rejected role), and the existing execution path has the same window.

For the human reviewer

The step-6 framing gate did not clear. The planning review returned Framing-Verdict: better-alternative-exists, and it was right — I adopted its recommendation rather than defending mine, which is why the history carries superseded approaches (squash on merge). The entries below lead with the parts I got wrong, because those are the ones worth your scepticism:

1. The approach changed after review, and the earlier one was genuinely broken. I first mirrored grantability as a set of names. That cannot enforce the invariant the design claimed ("admissible iff a live worker declares it grantable"): on a rolling deploy whose new generation keeps an operation but drops requiresSuperUser, the routing set stays non-empty via the new workers, so the mark is never revoked — add_role accepts a grant whose execution then fails closed with operation-not-found. Grantability is now tracked per declaring thread (name -> Set<threadId>) and the mirrored mark is re-derived from live claims. What to check: that setWorkerGrantable re-derives on every path, including retraction, and that handleThreadExit retracts one thread's claim while preserving routing for survivors.
2. My "main-mediated registration" rejection was overstated, and I did not adopt the correction. I disqualified it as requiring server.registerOperation() to become async. The reviewer pointed out that the loader could instead collect synchronous registrations and await one batched acknowledgement before declaring the worker ready — no API change, and it would also close the fire-and-forget startup window disclosed above. I did not do it: it introduces a worker-readiness protocol in the component loader, which is a materially larger change than the one this bug needs. This is the entry most worth overruling. If you want the startup window closed rather than documented, that is the design to ask for, and this PR is the wrong shape for it.
3. A mixed worker generation can make a granted call succeed intermittently, and I declined to fix it. If the old generation declares an operation grantable and the replacement omits requiresSuperUser, a role grant is accepted while both generations are live, and calls then alternate between succeeding on the declaring worker and returning a permission error on the other. Routing ignores grantableByWorker deliberately: selecting a worker by authorization metadata would require the main thread to know why a caller is authorized (super_user vs. an operations grant), which is the authorization determination this module documents that main must not make — server.registerOperation() calls in a component's resources.js are unreachable via the ops API #1736 split it as main routes, worker enforces. It fails closed (a permission error, never a wrongful success) and it is transient: once the old generation is gone this change retracts grantability and the grant is correctly rejected at write time. If you would rather routing became authorization-aware, that is a design call, and this is where to make it.
4. grantable is derived as requiresSuperUser !== undefined rather than being a new field on OperationDefinition. It reuses the existing tri-state that already decides whether registerOperationPermission is called at all, so the two cannot drift; the cost is that the wire flag's meaning is implicit in that expression. Note this deliberately treats requiresSuperUser: false as grantable, which matches the tri-state's documented meaning ("grantable AND open to any authenticated user") and keeps worker-registered operations behaving like main-thread ones.
5. Arming the thread-exit cleanup at module load widens this PR past the stated bug. attachMainListeners() previously ran only on the first forwarded call. Thread-exit notification fires once per thread and is dropped when no listener is attached, so a worker that registered and died before that first call leaked its entries permanently. That is a pre-existing bug in the routing map — I fixed it here because the new mark would otherwise inherit it, but it is separable if you would rather see it on its own.

Also flagged by the planning review and not addressed: it recommended lifecycle tests driven through a real hot restart/deploy, and integration coverage of a mixed old/new generation. The new unit tests drive the exit seam directly instead, so they prove the state machine but not that a real restart_service produces those transitions.

Adjacent bug found while checking symmetry, filed rather than fixed here: #2203 (P2, under [Epic] Component authoring & packaging DX) — a component's own roles.yaml still cannot grant an operation its own resources.js registers, because DEFAULT_CONFIG orders the roles plugin before jsResource and componentLoader.ts:542 iterates config keys in order. Same-thread ordering, different mechanism, survives this fix.

This invalidates documentation of the limitation in two places that are still in flight, neither on a main branch, so there is no companion docs PR to open yet: the Known limitation comment on assertOperationsAreKnown in security/authn/oidc/trustPolicyOperations.ts on #2173's branch, and the "that registry is process-local, so a policy naming one is rejected" paragraph in reference/operations-api/operations.md on an unpushed HarperFast/documentation branch. Whichever of the two PRs lands second should drop both. Nothing currently on documentation main is made false by this change.

Windows CI is red for reasons outside this PR (#2273, #2313), and the repo owner has accepted that for this merge. Integration Tests 2/6 fails Component: risk-query and describe_all metadata upgrade, both npm work → restart_service → route readiness, which is #2273 (deploy_component (restart: true) hangs after npm pack) — open and reproducing on main. Integration Tests 6/6 fails set_configuration, which is #2313 (Windows config-write rename retry blocking the thread), with fixes in flight in #2339 and #2191. Neither is reachable from this diff.

For the record, since an earlier version of this description said otherwise: I bisected the 2/6 failure to a commit here and reverted it on that basis. That was wrong. The same test fails at 350s with the commit reverted and ranges 259–465s across builds with identical code, so ~200s of variance was being read as a 110s signal. The revert has been undone in 9ac125c45 and the variance data is on #2273 so the next person doesn't repeat it.

Verification

Route (a), extended existing integration test — integrationTests/components/registered-operation.test.ts gains six cases in a nested suite covering add_role, alter_role, impersonation, an end-to-end grant (a non-super_user with the grant executes the operation on a worker), and two negative controls: an unregistered name is still rejected, and a non-super_user without the grant still gets 403. No new CI entry.

HARPER_INTEGRATION_TEST_LOOPBACK_POOL_START=1 HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT=1 \
  npm run test:integration -- --isolation=none integrationTests/components/registered-operation.test.ts
→ 11/11 pass (5 pre-existing + 6 new)

Unit — seven new cases in unitTests/server/serverHelpers/serverUtilities.test.js, covering the rolling-deploy retraction, withdrawal by re-announcement, the exited-thread announcement guard, main/worker mark independence, and the failed-send retraction through its real trigger (sendToThread reporting a dead port). npx mocha "unitTests/server/serverHelpers/*.test.js" → 295 passing, 1 failing. That failure is pre-existing (uwsServer.test.js, "rejects a body over maxBodyBytes with 413"); confirmed by stashing this branch's changes and re-running. Run as the whole suite rather than the changed file alone, because these tests mutate the process-global grantable registries. Also npx mocha unitTests/utility/operationPermissions.test.js unitTests/security/impersonation.test.js unitTests/validation/role_validation.test.js → 133 passing, 0 failing.

fails-on-base — through a detached worktree at the merge-base with origin/main, with only the test changes applied: the new unit cases fail there and reach their intended assertions rather than a setup or compile error; the 4 positive integration cases fail there while both negative controls pass. Everything passes on the branch.

Rebased onto current origin/main (374408e) before review — the branch had fallen 67 commits behind. No conflicts, and origin/main had touched none of these 7 files.

Not run locally: test:unit:main and test:unit:resources abort at load on this machine — another process holds ~/harper/database/data/LOCK, and HDB_ROOT/ROOTPATH overrides do not reroute the path that opens it. Worth knowing separately: mocha exits 0 when it dies this way, so an exit code is not evidence those suites ran. test:integration:all (160 files) was not run locally either: without the loopback address pool configured it can only run sequentially against 127.0.0.1. Both are left to this PR's Actions run.

The coverage field below pins the final round, which was a comment-only delta. The substantive round on this code (b1b267ecd) was classified high-risk / security-auth and ran three outside lenses — codex, gemini and cursor-grok — at high effort; it is the round that found the re-registration authorization divergence fixed in 5daaa6d96. Harper domain adjudication failed or was pruned on every round, so no round was filtered by the leg that drops false positives, and I triaged findings myself.

npm run lint:required clean; npm run format:write produced no changes; npm run build clean; TypeStrip/tabs/node:-prefix/assert-style greps clean over origin/main...HEAD.

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex; blocked=gemini(quota); declined=cursor-grok,cursor-composer,domain; rounds=11 @ 9ac125c}

_{Human-Review-Need: 4 @ 9ac125c}

[gemini-code-assist]

Code Review

This pull request implements cross-thread mirroring of grantable operations from worker threads to the main thread, allowing the main thread to validate worker-registered operations in role allowlists. The review feedback suggests two performance optimizations in server/serverHelpers/registeredOperations.ts: first, to only perform cleanup logic during thread exit if the exiting thread actually registered the operation, and second, to avoid redundant Map/Set deletions in setWorkerGrantable when removing a thread's grantable claim.

[kriszyp]

Nice catch, good work!
_{🤖 Reviewed with Codex}

[dawsontoth]

ah, this needs 1 re-approval

[kriszyp]

Great!
_{🤖 Reviewed with Codex}

[kriszyp]

_{🤖 Reviewed with Codex}