Skip to content

fix(http): stop SSR fault rules once their devtools server closes - #219

Merged
erkamyaman merged 1 commit into
pangular-inspector:mainfrom
erkamyaman:fix/http-rules-after-dispose
Oct 7, 2026
Merged

erkamyaman merged 1 commit into
pangular-inspector:mainfrom
erkamyaman:fix/http-rules-after-dispose

Conversation

@erkamyaman

@erkamyaman erkamyaman commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

What and why

Most of #69 was fixed by #184: turning the http inspector off clears stored client rules, setup clears server rules when http or its actions are off, and the panel is seeded with the registry's rules after a restart.

One server gap was left. Rules live on globalThis so they survive a Vite restart, but if a config edit removes the plugin, the old hub is disposed, no new setup runs, and the rules kept failing SSR requests through withPangular(). Clearing them in dispose isn't safe, because Vite creates the new server before closing the old one.

  • http.ts: SSR rules apply only while a devtools server owns the registry (registry.record), so a normal restart keeps them and a closed server stops them.
  • inspectors/ssr-http.md: says SSR rules stop when the devtools server closes.

Refs #69

How it was verified

  • pnpm commit:check, pnpm format:check, pnpm typecheck, pnpm skills:check
  • pnpm test:devtools (1168) and pnpm test:panel
  • pnpm docs:build, pnpm test:axe
  • New tests in vite-restart.test.ts: rules keep applying when a new server takes over in the same process, and stop once the owning server closes (fails without the fix)
  • Checked in a running app (Analog example on Vite 8.3 with SSR on /dashboard and withPangular()), with a server rule returning 503 for /api/v1/orders:
Step This PR main
Rule set, SSR load fails as expected fails as expected
Vite restart, plugin kept rule still applies rule still applies
Plugin removed, Vite restarted rule stops rule keeps failing (the bug)

Removing the plugin pauses server rules rather than deleting them: put the plugin back and the rule applies again, and the panel lists it.

Notes for reviewers

Still open in #69: after turning the http inspector off and reloading the same tab, requests made before the overlay connects still use the rules saved in sessionStorage. Options are to hold matching requests until the overlay connects, inject the http flag into the page before bootstrap, or stop saving client rules. That needs a decision, so it's left out here.

Summary by CodeRabbit

  • Bug Fixes
    • Server-side HTTP fault rules now stop applying when their owning devtools server closes, while remaining active when a replacement server takes over.
  • Documentation
    • Clarified when server-side fault rules apply and when they stop.

Server rules live on globalThis so they survive a Vite restart, but if a config edit removed the plugin the disposed hub left them applying to every SSR request. The interceptor now applies server rules only while a devtools server owns the registry, so a normal restart keeps them and a closed server stops them.

Refs pangular-inspector#69
@erkamyaman erkamyaman self-assigned this Oct 6, 2026
@github-actions github-actions Bot added area: package The ng-devtools package (packages/ng-devtools) area: docs The documentation site labels Oct 6, 2026
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 48b756f2-212a-46ef-8d12-8750d1ce794a
📥 Commits

Reviewing files that changed from the base of the PR and between 3255b06 and 313a8ab.

📒 Files selected for processing (3)
  • apps/docs/src/content/inspectors/ssr-http.md
  • packages/devtools/src/__tests__/vite-restart.test.ts
  • packages/devtools/src/http.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The HTTP interceptor now reads server-side rules only when the HTTP registry has a record callback. New tests cover server-rule behavior during server restarts and closure. The SSR HTTP documentation now states when those rules apply.

Changes

SSR HTTP rule lifecycle

Layer / File(s) Summary
Server-rule selection and lifecycle coverage
packages/devtools/src/http.ts, packages/devtools/src/__tests__/vite-restart.test.ts, apps/docs/src/content/inspectors/ssr-http.md
The interceptor reads server-side rules only when the registry has a record callback. Tests check rule behavior after a newer server takes over and after the owning server closes. The documentation states that SSR rules stop applying when the devtools server closes.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 313a8

SSR fault rules should remain active through a server restart and stop after the owning server closes. No actionable merge-blocking risk is identified.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 313a8

The change prevents stale SSR fault rules from affecting new requests after the owning development server closes, while preserving normal restart behavior. No material security risk was found in this scoped change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Existing server rules can affect matching intercepted development-mode SSR requests across the shared process; the registry has no per-request tenant partition. This PR does not broaden that scope and instead makes retained rules ineffective for newly intercepted SSR requests after callback removal.

Trust Boundaries and Controls

  • observed — The existing rule-writing action rejects writes when HTTP actions are disabled and sanitizes accepted input. Setup clears retained rules when HTTP inspection or actions are disabled. Sanitization caps rule count, delay, and string sizes. The new recording-callback predicate adds a lifecycle condition without replacing these controls.

Resilience and Maintainability Implications

  • inferred — A recording callback is a lifecycle proxy, not proof that setup completed successfully: callback installation precedes owner assignment and later asynchronous setup work. The pre-existing lack of transactional setup rollback bounds the guarantee provided by this fix, but does not establish an introduced or worsened security condition.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: SSR fault rules stop applying when their owning devtools server closes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@erkamyaman
erkamyaman merged commit 16e35f6 into pangular-inspector:main Oct 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs The documentation site area: package The ng-devtools package (packages/ng-devtools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant