Skip to content

refactor(gates): own the client-only replay rule as daemon-process reachability (R82) - #3604

Merged
thymikee merged 3 commits into
mainfrom
refactor/daemon-closure-client-only-deny
Oct 14, 2026
Merged

thymikee merged 3 commits into
mainfrom
refactor/daemon-closure-client-only-deny

Conversation

@thymikee

@thymikee thymikee commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Follow-up to the #3517 review (comment). Based on main.

The design question

You asked whether the src/daemon.ts hub closure should own the client-only rule instead of a hand-listed path set. The answer is yes: what the daemon process can load owns it. But the eager closure can't carry it. Measured on main:

  • src/daemon.ts eagerly evaluates 393 modules. Neither src/daemon-composition.ts nor any src/platform-runtime-* module is among them, because they load behind a function-scoped await import. A planted import '@agent-device/replay-port/source-discovery' in src/platform-runtime-daemon-services.ts passes an eager-closure deny set.
  • Over static plus dynamic edges, src/daemon.ts reaches 1,366 modules, including the composition root, the platform-runtime family and every package they reach. Neither collector module is among them.

So the rule is a layering check over the resolved import graph, like R76, instead of a deny set in eager-closure-budgets.

What

  • New R82 daemon-process-reach (scripts/layering/daemon-process-reach.ts): a breadth-first walk from src/daemon.ts over value and dynamic edges. It fails if packages/replay-port/src/script-source-bundle.ts or source-discovery.ts is reachable, naming the importing edge and the shortest chain.
    • The planted import above now fails: src/daemon.ts -> … -> src/platform-runtime-daemon-services.ts -> packages/replay-port/src/source-discovery.ts.
    • Type-only edges load nothing, and client code may import the collector.
  • Deleted: the R11 path gate (CLIENT_ONLY_SUBPATHS, DAEMON_PROCESS_ROOT_FILES, the daemon-path predicate) and client-only-subpaths.test.ts.
  • Non-blocking note taken: the root wiring test imports testRequestScope, now exported next to testInventoryHost, instead of redeclaring it.
  • The seven inline threads you listed were resolved on refactor(daemon): cut daemon test edges into root data fixtures and fakes [11a-3] #3517.

Validation

  • check:layering: 326 tests, guard OK, R82 in the success line.
  • check:quick, check:fallow (no issues in 10 changed files), check:gate-manifest, test-file-size ratchet, check:affected --run: all runnable checks passed.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.21 MB 5.21 MB -2.5 kB
Package (unpacked) 5.21 MB 5.20 MB -2.5 kB
Package (download) 1.57 MB 1.56 MB -841 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 22.8 ms 22.1 ms -0.7 ms
CLI --help 61.4 ms 61.4 ms -0.0 ms

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at 7ed93ce. The rule reads as a correct move from import-specifier matching to reachability from the daemon entry.

Not blocking, and you can take or leave these: the rule passes silently if src/daemon.ts or a denied module such as packages/replay-port/src/source-discovery.ts is renamed or moved, and the pin test only compares the constant to itself, so a stale entry would stay green (a stale-entry check against the source file set, like the one in R78, would close it); the test file has no case for a static-only chain to a denied module or for the second denied module, script-source-bundle, so iterating DAEMON_PROCESS_DENIED_MODULES in the planted-import test would cover both; and the testRequestScope export in inventory-gateways.fixtures.ts is unrelated to R82 and would sit better in its own commit.

CI is still pending: the 7 jobs that have not passed are all queued, and none has failed. Lint & Format, Typecheck & Package and the check:layering step cover this change directly, because the new rule runs against the real graph there. I did not run check:layering at this commit myself, so I have not confirmed that neither collector module is reachable from src/daemon.ts. The test counts and the check:fallow result come from the PR description. Like every graph rule, R82 cannot see dynamic imports with non-literal specifiers.

Before merge, please confirm those three checks go green once the queued jobs run, and land #3603 first, since this PR is stacked on it.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 14, 2026
@thymikee
thymikee added this pull request to stack #3605 October 14, 2026 09:14
…test redeclared

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the refactor/daemon-closure-client-only-deny branch from 7ed93ce to aacd646 Compare October 14, 2026 09:21
@thymikee
thymikee removed this pull request from stack #3605 October 14, 2026 09:21
@thymikee
thymikee changed the base branch from fix/closure-walker-json-imports to main October 14, 2026 09:21
…achability (R82)

The R11 path gate (daemon root plus two named root files) missed root platform-runtime modules
and reached packages. R82 walks every module the daemon process can load from src/daemon.ts,
over static and dynamic edges, and denies replay-port's caller-side collector. The eager
closure would not do: the composition root and platform-runtime modules load behind a
function-scoped await import.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the refactor/daemon-closure-client-only-deny branch from aacd646 to 3389177 Compare October 14, 2026 09:53
A renamed `src/daemon.ts` or collector module turned the reach walk into a
silent pass. R82 now reports an entry or denied module that is not a tracked
source as stale. The tests plant both denied modules over a dynamic chain and a
static-only chain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in 9d95852.

check:layering (330 tests, OK against the real graph, so neither collector module is reachable from src/daemon.ts), check:quick, check:fallow and check:affected --run pass locally at 9d95852.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@thymikee

Copy link
Copy Markdown
Member Author

This is ready for review at 9d95852. The earlier review at 7ed93ce was clean, and the delta adds no new problems: the stale-target check reuses the same entry and denied-module constants, so R82 stays the single owner of the rule. The new tests fail if the reachability walk or the stale check is reverted. There are no conflicts. CI is still running (15 of 18 checks queued or in progress, none failed).

Not blocking: checkDaemonProcessReach builds its own adjacency map and BFS and repeats the executable-edge filter. Could it use importGraphFromResolvedEdges(edges, EXECUTABLE_EDGES) and getShortestPath, like the R76 rule next to it? Then a change to the edge classification in import-graph.ts reaches R82 too.

@thymikee
thymikee merged commit ba7730a into main Oct 14, 2026
19 checks passed
@thymikee
thymikee deleted the refactor/daemon-closure-client-only-deny branch October 14, 2026 10:50
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-14 10:50 UTC

thymikee added a commit that referenced this pull request Oct 14, 2026
* origin/main:
  feat(leasing): proxy CLI names each new lease, daemon keeps a caller-chosen lease id (#3479)
  feat(remote): name the held lease on upload, artifact download and diagnostics requests (#3594)
  fix(android): record the launched intent owner from the device (#3600)
  fix(selectors): type rect-requiring targets as addressable (#3608)
  feat(snapshot): interpret iOS node facts once at producer seams (#3592)
  refactor(gates): own the client-only replay rule as daemon-process reachability (R82) (#3604)
thymikee added a commit that referenced this pull request Oct 14, 2026
* origin/main:
  feat(remote): name the held lease on upload, artifact download and diagnostics requests (#3594)
  fix(android): record the launched intent owner from the device (#3600)
  fix(selectors): type rect-requiring targets as addressable (#3608)
  feat(snapshot): interpret iOS node facts once at producer seams (#3592)
  refactor(gates): own the client-only replay rule as daemon-process reachability (R82) (#3604)
thymikee added a commit that referenced this pull request Oct 14, 2026
* origin/main:
  feat(remote): name the held lease on upload, artifact download and diagnostics requests (#3594)
  fix(android): record the launched intent owner from the device (#3600)
  fix(selectors): type rect-requiring targets as addressable (#3608)
  feat(snapshot): interpret iOS node facts once at producer seams (#3592)
  refactor(gates): own the client-only replay rule as daemon-process reachability (R82) (#3604)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant