21 KiB
issue, issue_title
| issue | issue_title |
|---|---|
| 598 | pi-permission-system: Authorizer chain infrastructure (allow/deny/defer verdicts) |
Authorizer chain infrastructure (allow/deny/defer verdicts)
Release Recommendation
Release: mid-batch — defer (batch "authorizer-chain"); confirm at ship time
This is Step 4 of the Phase 12 roadmap (Track B — the Authorizer chain) and the head of the two-step release batch "authorizer-chain" whose tail is Step 5 (#599).
The roadmap's Release batches subsection lists Steps 4 and 5 shipping together with Step 5 as the tail, so Step 4 lands on main and its release-please PR stays open until Step 5 ships.
The work is refactor-only (refactor:/test: commits are hidden: true and do not cut a release), with one docs: step-completion commit that will appear in the pending release PR but must not be merged until the batch tail.
Problem Statement
The Phase 9 spine selects exactly one terminal Authorizer per session activation (LocalUserAuthorizer, ParentAuthorizer, or DenyingAuthorizer), each of which must decide.
That shape is closed against a link that reviews an ask and passes it on — the structural reason a case-by-case model judge (#472) has had no home since the spine was built.
ADR 0007 (docs/decisions/0007-model-judge-authorizer-chain-adr.md, accepted) settles the design: model the live-authority layer as a Chain of Responsibility whose links return allow | deny | defer, ending at a terminal that cannot defer.
This issue is the infrastructure step: introduce the verdict type and the chain composition, refactor selectAuthorizer into the terminal-selection step of the chain, and register zero links — so behavior is identical to today, pinned by the existing authorizer-selection tests.
The chain seam exists for Step 5 (#599) to expose via registerAuthorizer.
Goals
- Introduce
AuthorizerVerdict(allow | deny | defer), withdenycarrying an optional teachingreason, insrc/authority/authorizer.ts. - Reshape the interface vocabulary to match ADR 0007:
Authorizerbecomes the non-terminal chain link (returnsAuthorizerVerdict); a newTerminalAuthorizeris the terminal (always decides, returnsPermissionPromptDecision). - Add
composeAuthorizerChain(links, terminal)in a newsrc/authority/authorizer-chain.ts: registered non-terminal links first, then the context-selected terminal; the terminal-cannot-defer invariant is enforced at the type level. - Route
AuthorizerSelection.activatethroughcomposeAuthorizerChain([], terminal)so the chain seam is live with an empty link list. - Preserve behavior exactly: with zero registered links the composed chain is the selected terminal, pinned by the existing
authorizer.test.tsandauthorizer-selection.test.ts.
This change is not breaking: no observable behavior, output shape, config, or default changes on upgrade.
Non-Goals
- The
registerAuthorizerservice method, theauthorizerChainconfig field, and the bounded-delegation enforcement checkpoint — those are Step 5 (#599). - The
PermissionQueryinjection into a link'sauthorize— ADR 0007 §3 ties it to the registration seam; a Step-4 link signature takes onlyPromptPermissionDetails(no link exists yet to consume a query). Deferred to Step 5. - The first-party dogfood link (
@gotgenes/pi-permission-model-judge) — Step 6 (#600). - Collapsing the terminal's return to ADR 0007's illustrative minimal
TerminalVerdict({ kind: "allow" } | { kind: "deny"; reason? }). The terminal keeps returning the richPermissionPromptDecision(session-scope states,confirmationUnavailable,denialReason), which is what preserves behavior; the ADR sketch is explicitly illustrative ("the essentials follow"). - Moving
selectAuthorizerout ofauthorizer.tsintoauthorizer-selection.ts. It stays inauthorizer.ts; only its return type changes. (The issue's phrasing "authorizer-selection.ts—selectAuthorizerbecomes the terminal-selection step" refers to the selection concern, not a file relocation.)
Background
Relevant modules (all under src/authority/):
authorizer.ts— currently declares theAuthorizerinterface (authorize(details): Promise<PermissionPromptDecision>),AuthorizerSelectionDeps, andselectAuthorizer(ctx, deps): Authorizer(the once-per-activationhasUI/isSubagent/ deny dispatch).local-user-authorizer.ts,denying-authorizer.ts,approval-escalator.ts(ParentAuthorizer) — the three concrete terminals, eachimplements Authorizer, each returningPermissionPromptDecision.authorizer-selection.ts—AuthorizerSelection(theAskEscalatorimplementation):activate(ctx)runsselectAuthorizerand stores the result inselected;escalate(details)delegates toprompter.prompt(this.selected, details).permission-prompter.ts—PermissionPrompterApi.prompt(authorizer: Authorizer, details)brackets the review-log entries aroundauthorizer.authorize(details)and returns itsPermissionPromptDecision.permission-dialog.ts—PermissionPromptDecisiontype pluscreateDeniedPermissionDecision(reason?)(maps a reason todenied_with_reason/denied), reused by the chain's verdict→decision mapping.
AGENTS.md constraints that apply:
- Architecture-doc module-tree entries describe current behavior; cite an issue only for an active constraint.
The reshape updates the
authorizer.tstree entry's signature and adds anauthorizer-chain.tsentry. - The package skill's rule: mark the completed roadmap step
✅(heading + Mermaid node) in the implementation doc-update commit, not a deferred ship commit. refactor:/test:commits arehidden: true; an unhiddendocs:commit is release-visible but, mid-batch, its release-please PR is not merged until the batch tail.
Design Overview
Verdict type and the two interfaces
src/authority/authorizer.ts gains the verdict union and splits the interface into non-terminal and terminal per ADR 0007 §2:
/** A non-terminal chain link's ruling: decide (allow/deny) or pass on (defer). */
export type AuthorizerVerdict =
| { kind: "allow" }
| { kind: "deny"; reason?: string }
| { kind: "defer" };
/** A non-terminal chain link: reviews an ask and may decide or defer. */
export interface Authorizer {
authorize(details: PromptPermissionDetails): Promise<AuthorizerVerdict>;
}
/** The terminal link: structurally cannot defer — always returns a full decision. */
export interface TerminalAuthorizer {
authorize(details: PromptPermissionDetails): Promise<PermissionPromptDecision>;
}
The terminal-cannot-defer invariant is type-level: a TerminalAuthorizer returns PermissionPromptDecision (which always carries approved: boolean — it cannot express "defer"), while a deferring link returns AuthorizerVerdict. composeAuthorizerChain's signature (below) accepts links as Authorizer[] and the terminal as TerminalAuthorizer, so a deferring link cannot occupy the terminal slot — the compiler rejects it.
The three concrete terminals (LocalUserAuthorizer, DenyingAuthorizer, ParentAuthorizer) change implements Authorizer → implements TerminalAuthorizer; their bodies are unchanged (they already return PermissionPromptDecision). selectAuthorizer's return type changes Authorizer → TerminalAuthorizer. PermissionPrompterApi.prompt and AuthorizerSelection.selected retype to TerminalAuthorizer.
The chain composition
src/authority/authorizer-chain.ts (new) folds the links ahead of the terminal:
export function composeAuthorizerChain(
links: readonly Authorizer[],
terminal: TerminalAuthorizer,
): TerminalAuthorizer {
if (links.length === 0) {
return terminal; // identity: zero links ⇒ behavior is the terminal's
}
return {
async authorize(details) {
for (const link of links) {
const verdict = await link.authorize(details);
if (verdict.kind === "allow") {
return { approved: true, state: "approved" };
}
if (verdict.kind === "deny") {
return createDeniedPermissionDecision(verdict.reason);
}
// defer → try the next link
}
return terminal.authorize(details);
},
};
}
The composite is a TerminalAuthorizer — it always decides, because the terminal always decides.
The verdict→decision mapping is the seam Step 5 exercises with real links:
allow→{ approved: true, state: "approved" }— a link grant is non-persistent (stateapproved, neverapproved_for_session), matching ADR 0007's off-by-default, non-persistence envelope.deny→createDeniedPermissionDecision(reason)→denied_with_reasonwhen a reason is present, elsedenied— carrying the teaching signal use case 1 needs.defer→ the next link, then the terminal.
The links.length === 0 short-circuit returning the terminal identity is a behavioral invariant, not an optimization: authorizer-selection.test.ts asserts prompter.prompt is called with expect.any(LocalUserAuthorizer), which only holds if the composed value is the selected terminal instance when links are empty.
The extracted module's upstream interaction is minimal and Tell-Don't-Ask-clean: it imports the Authorizer / TerminalAuthorizer / AuthorizerVerdict types and createDeniedPermissionDecision from permission-dialog.ts; it is a pure function over its two parameters, mutates nothing, and reaches through nothing.
Consumer call site
AuthorizerSelection.activate routes the selected terminal through the (empty) chain:
activate(ctx: ExtensionContext): void {
const terminal = selectAuthorizer(ctx, this.deps);
this.selected = composeAuthorizerChain([], terminal);
}
With the literal [], composeAuthorizerChain returns terminal, so escalate still hands the real LocalUserAuthorizer / ParentAuthorizer / DenyingAuthorizer to prompter.prompt — identical behavior.
Step 5 replaces [] with the registered links resolved from authorizerChain config.
Design-review checklist
Ran the design-review checklist against the reshaped interface and the new wiring:
- Dependency width —
composeAuthorizerChain(links, terminal): two parameters, both used. No wide bag. - Law of Demeter — no reach-through; the function talks only to its two parameters.
- Output arguments — none; returns a value, mutates nothing.
- Parameter relay —
detailsflows link→terminal; each endpoint genuinely consumes it. - Repeated discriminators — the
verdict.kindswitch is a single dispatch point (the composition function), not scattered===across modules. OCP-compliant. - Test mock depth — the prompter test's
makeAuthorizerbecomes a one-methodTerminalAuthorizerstub; noas unknown as, no nesting.
No structural smells; the change is fit for a single PR.
Module-Level Changes
Source (src/authority/):
authorizer.ts— addAuthorizerVerdict; repurposeAuthorizeras the non-terminal link (authorize(details): Promise<AuthorizerVerdict>); addTerminalAuthorizer(authorize(details): Promise<PermissionPromptDecision>); changeselectAuthorizer's return type toTerminalAuthorizer.AuthorizerSelectionDepsunchanged. ImportPermissionPromptDecisionfor the terminal signature (already imported).authorizer-chain.ts— new:composeAuthorizerChain(links, terminal); importsAuthorizer/TerminalAuthorizer/AuthorizerVerdictfrom./authorizer,createDeniedPermissionDecision+PermissionPromptDecisionfrom./permission-dialog.local-user-authorizer.ts,denying-authorizer.ts,approval-escalator.ts—implements Authorizer→implements TerminalAuthorizer(bodies unchanged).permission-prompter.ts—PermissionPrompterApi.prompt(authorizer: TerminalAuthorizer, …)and thePermissionPrompter.promptparameter; doc comment reference{@link Authorizer}→{@link TerminalAuthorizer}.authorizer-selection.ts— retypeprivate selected: TerminalAuthorizer | null;activatecallscomposeAuthorizerChain([], selectAuthorizer(ctx, this.deps)); importcomposeAuthorizerChain.
Tests (test/authority/):
authorizer-chain.test.ts— new (see TDD Order).permission-prompter.test.ts—makeAuthorizer(decision): TerminalAuthorizer; theimport type { Authorizer }becomesTerminalAuthorizer;vi.fn<Authorizer["authorize"]>→vi.fn<TerminalAuthorizer["authorize"]>.denying-authorizer.test.ts—import type { Authorizer }→TerminalAuthorizer; theconst authorizer: Authorizer = new DenyingAuthorizer()annotation →TerminalAuthorizer.authorizer.test.ts— unchanged (assertsinstanceofonselectAuthorizer's result; the concrete classes are unchanged). Stays green as the behavior pin.authorizer-selection.test.ts— unchanged (assertsprompter.promptcalled withexpect.any(LocalUserAuthorizer); the empty-chain identity preserves it). Stays green as the behavior pin.
Docs:
docs/architecture/architecture.md:- Module tree — rewrite the
authorizer.tsentry (line ~760) toAuthorizerVerdict+ non-terminalAuthorizer(authorize(details): Promise<AuthorizerVerdict>) +TerminalAuthorizer(authorize(details): Promise<PermissionPromptDecision>) +AuthorizerSelectionDeps+selectAuthorizer(ctx, deps): TerminalAuthorizer; add a newauthorizer-chain.tsentry (composeAuthorizerChain— non-terminal links then the terminal; terminal-cannot-defer is type-level; empty-links identity). Refine thelocal-user-authorizer.ts/denying-authorizer.ts/approval-escalator.tsandauthorizer-selection.ts/permission-prompter.tsentries where they call the concrete classes "Authorizer" to "TerminalAuthorizer" (current-behavior accuracy). - Step 4 completion — add
✅to the#### Step 4:heading and theS4Mermaid node, and aLanded:note under Step 4's Outcome. - Do not edit the fixed
Baseline (2026-07-15)column or the Step-5authorizerChainschema-sites metric row (that metric is Step 5's).
- Module tree — rewrite the
docs/configuration.md,README.md— no change (no config field or command added in Step 4)..pi/skills/package-pi-permission-system/SKILL.md— no change; its only reference isParentAuthorizer.authorize(src/authority/approval-escalator.ts), whose signature (returnsPermissionPromptDecisionas aTerminalAuthorizer) is unchanged.
No package.json exports, event channel, or Symbol.for() surface changes (the reshaped types are package-internal), so no wider docs/ grep is warranted; the greps above (docs/architecture/, docs/configuration.md, README.md, package skill) found every reference.
Test Impact Analysis
- New tests the extraction enables —
composeAuthorizerChainis a pure function, unit-testable in isolation for the first time: empty-links identity;allow→{approved:true,state:"approved"};denywith reason→denied_with_reason+denialReason;denywithout reason→denied;defer→next link; a mid-list decide short-circuits (first non-defer wins, later links not called); all-defer→terminal. Previously this dispatch did not exist. - Redundant existing tests — none.
No prior test covered chain composition (it did not exist);
authorizer.test.tsandauthorizer-selection.test.tsstill exercise selection and escalation and are not superseded. - Tests that must stay as-is —
authorizer.test.ts(terminal selection by context) andauthorizer-selection.test.ts(escalate/reject contract +expect.any(LocalUserAuthorizer)identity) are the behavior pins that prove the reshape is a no-op with zero links. They must pass unchanged; changing them would defeat the "behavior identical" guarantee.
Invariants at risk
The change touches the Phase 9 spine (selectAuthorizer / AuthorizerSelection), whose documented outcome is "exactly one terminal Authorizer selected per activation; escalate delegates to the prompter with the selected authorizer" and "#556 dissolved canConfirm()".
| Invariant | Pinned by |
|---|---|
One terminal selected per activation by hasUI / isSubagent / deny dispatch |
authorizer.test.ts (instanceof per context) |
escalate hands the selected terminal instance to prompter.prompt (empty-chain identity) |
authorizer-selection.test.ts (expect.any(LocalUserAuthorizer)) |
escalate rejects before activate / after deactivate; returns the prompter decision |
authorizer-selection.test.ts |
| No separate confirmability predicate (#556) | unchanged — DenyingAuthorizer still answers by denying |
All invariants live in existing tests; no new pinning test is needed for them. The reshape must keep the empty-links identity so the second row holds.
TDD Order
- Reshape the interfaces and add
composeAuthorizerChain. Red — addtest/authority/authorizer-chain.test.tscovering the seven cases in Test Impact Analysis §1 (empty-links identity viatoBe(terminal); each verdict mapping; first-non-defer-wins with anot.toHaveBeenCalledon the trailing link; all-defer→terminal). Green — inauthorizer.tsaddAuthorizerVerdict, repurposeAuthorizeras the non-terminal link, addTerminalAuthorizer, retypeselectAuthorizer; addauthorizer-chain.ts; migrate the three concrete terminals andpermission-prompter.tstoTerminalAuthorizer; retypeAuthorizerSelection.selected; migratepermission-prompter.test.tsanddenying-authorizer.test.ts. This is one commit: repurposing the exportedAuthorizertype breaks every implementer and consumer at compile time, so the reshape, all consumer updates, and the two consumer-test updates land together. Commit:refactor(pi-permission-system): reshape live-authority layer as an Authorizer chain (#598) - Route activation through the empty chain.
Green —
AuthorizerSelection.activatecallscomposeAuthorizerChain([], selectAuthorizer(ctx, this.deps)); importcomposeAuthorizerChain. No new test:authorizer-selection.test.tspins the behavior (empty-chain identity preservesexpect.any(LocalUserAuthorizer)); run it to confirm green. Commit:refactor(pi-permission-system): route activation through composeAuthorizerChain (#598) - Mark Step 4 complete and refresh the module tree.
Update
docs/architecture/architecture.md:✅on the Step 4 heading and theS4Mermaid node, aLanded:note, the rewrittenauthorizer.tstree entry, and the newauthorizer-chain.tstree entry (plus the terminal-class prose touch-ups). Commit:docs(pi-permission-system): mark Phase 12 Step 4 complete (#598)
Risks and Mitigations
- A silent behavior change from the reshape.
Mitigation: the empty-links identity (
composeAuthorizerChain([], t) === t) keepsescalatehanding the real terminal instance to the prompter;authorizer.test.tsandauthorizer-selection.test.tspass unchanged as the pins. Any drift breaks theexpect.any(LocalUserAuthorizer)assertion. - A dropped
import typein the atomic reshape edit (AGENTS.md:tscpasses on an unused type import). Mitigation: after Step 1, re-readauthorizer.ts/permission-prompter.ts/authorizer-selection.tsand runpnpm --filter @gotgenes/pi-permission-system run check+run lint(lint flags unused imports), not justtsc. - Vocabulary drift from ADR 0007.
Mitigation: the operator confirmed the ADR-faithful rename (
Authorizer= non-terminal link,TerminalAuthorizer= terminal); Steps 5/6 inherit the ADR vocabulary directly. - Fallow dead-code on the dormant seam.
composeAuthorizerChainis consumed byAuthorizerSelection.activate(Step 2) and covered by its own tests, so it is not dead;AuthorizerVerdictand the non-terminalAuthorizerare referenced bycomposeAuthorizerChain's signature and tests. Runpnpm fallow dead-codebefore pushing.
Open Questions
- Link
authorizesignature gainsPermissionQueryin Step 5. ADR 0007 §3 injects a narrowPermissionQueryinto each link atauthorizetime. Step 4'sAuthorizer.authorize(details)omits it (no link consumes it yet); Step 5 widens the signature when it wires registration and query injection. Deferred to #599 by design, not an oversight. allow/denyverdict → decision mapping richness. Step 4 mapsallow→state:"approved"(non-persistent) anddeny→createDeniedPermissionDecision. Whether a future allow-capable slice needs a session-scoped or audited (origin:"authorizer:model") decision shape is Step 5/6 envelope work per ADR 0007 §6; not in scope here.