fix(policy): dedup co-located cascade policies + extractions#156
Merged
Conversation
Bug A from the coverage-gaps investigation: an unscoped cascade _policy.yaml shared by N routes produced N identical copies in the engine. The old materialiser keyed each engine id on (source, index, route) and called engine.addPolicies() once per route, so the same logical policy was added N times, each with scope.routes = [singleRoute]. Because addPolicies replaces by id in place, only the last route's scope survived for any shared id. Fix: introduce a per-bootstrap CoLocatedAccumulator. Discovery feeds (policy, routeName) pairs into it during a load; applyDiscoveryResult flushes once after every route, channel, and GraphQL resource is registered. The flush materialises exactly one policy per (source, index) with scope.routes set to the union of routes that referenced it — so 5 routes under one cascade yield 1 engine entry whose scope.routes lists all 5. Per-route nearest-wins (closer _policy.yaml overriding a broader one with the same id) is resolved in addCoLocatedPoliciesToEngine before feeding the accumulator, so two unrelated sibling files that happen to share an id (users.policy.yaml / projects.policy.yaml both 'id: read') stay separate — they never share a route's cascade chain. Also: - GraphQL resources now register before the flush (they accumulate their own co-located policies); previously the flush ran first and dropped them. - Protocol-bound surfaces (GraphQL, channels) feed the accumulator with applyRouteScope=false so no route filter is added — their engine action is independent of any route name. - Hot reload resets the accumulator before re-applying so a removed route's policies don't linger. - Audit _meta (owner/ticket/description/deprecation) survives materialisation and still surfaces via server.policy.list(). - Removed the now-dead materializeCoLocatedPolicies/applyAutomaticScope helpers and the unused createHash import. coverage-gaps Bug A test now asserts exactly 1 engine copy with scope.routes covering all 5 routes. Full suite: 3383 passing.
…lator leak Two related cleanups on top of the Bug A accumulator fix: 1. Channel enforcer correctness. After the Bug A change, addCoLocatedPoliciesToEngine feeds the per-load accumulator instead of calling engine.addPolicies() directly. The WebSocket channel enforcer ran it at SUBSCRIBE time (request time), where no flush happens — so the call became dead (no engine effect) and worse, mutated accumulator state on every subscribe. The channel's co-located policies are already loaded into the engine at discovery time (registerChannel -> buildAuthzInterceptorsForOperation), so the enforcer now only evaluates; it no longer re-registers. 2. Extract createGraphQLPolicyBridge and the channel enforcer into src/server/builder/policy-bridges.ts. Keeps the policy-evaluation logic next to the rest of the policy code and trims builder.ts from 1788 to 1723 lines (45 below its main baseline of 1768). No behaviour change for GraphQL or channel authorization — covered by the existing co-located + websocket suites (81 tests). Full suite: 3383 passing.
…ader Split two cohesive, self-contained concerns out of the 1700-line loader.ts to bring it under the 1500-line cap: - route-naming.ts: pure path/middleware/auth helpers (parseRoutePath, applyHttpVerbConvention, collectMiddlewareChain, findAuthConfig, findDirectoryMeta, HTTP_VERB_SEGMENTS, LoadedMiddleware). No I/O — operates on already-loaded maps and strings. - co-located-attach.ts: attachCoLocatedPolicies + attachCoLocatedPoliciesToFileItems (sibling/folder policy resolution). loader.ts: 1700 -> 1424 lines. No behaviour change; fs-routes, discovery, and co-located suites (118 tests) pass.
…actory Move mount + addProcedure/addStream/addEvent/addChannel/addRest/ addResource/addTcpHandler/addUdpHandler/registerHandler out of the 1700-line createServer closure into builder/programmatic-registration.ts. The factory takes the registries, policy factories, and register* helpers as deps plus a getServer thunk (the methods return server for chaining), and the result is spread into the server object literal. Behaviour is identical — verified by the full suite (3383 tests). builder.ts: 1723 -> 1472 lines, now under the 1500-line cap.
…avior The Bug A fix changed the internal mechanism: per-route nearest-wins resolution happens before the engine, and a per-load accumulator commits one engine entry per source policy with scope.routes = union of routes. Update the folder-cascade and engine-driver sections that still described the old 'engine dedupes by id on repeated addPolicies' / 'bridge replays per route' behaviour. User-facing semantics (nearest-wins, deny precedence, sibling isolation) are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
_policy.yamlshared by N routes now produces one engine entry (withscope.routes= union) instead of N copies. Per-route nearest-wins is resolved before the engine; unrelated files sharing anidstay separate.route-naming.ts+co-located-attach.tsfromloader.ts(1700→1424),programmatic-registration.ts+policy-bridges.tsfrombuilder.ts(1723→1472).co-located.mdaligned with the new dedup mechanism.Tests
Full suite 3383/3383, tsc clean, hexagonal boundaries pass. New regression coverage in
coverage-gaps.int.test.ts.🤖 Generated with Claude Code