fix: flag restart when redeploying an active jsResource component (#1817)

[kriszyp]

Summary

Redeploying an existing, active jsResource-backed component with restart: false neither flagged that a restart was needed (get_status.restartRequired stayed false) nor invalidated its stale cached module — it silently kept serving the old code (harper#1817).

Mechanism: a redeploy pauses/resumes the component's file watcher (deploy lifecycle in Scope/EntryHandler). On resume the fresh chokidar scan re-emits every existing file as 'add', including one whose contents just changed. jsResource's handler only requested a restart on non-'add' events, so the changed file was re-imported against the never-invalidated module cache and no restart was ever flagged.

The fix (direction 1 from the issue)

resources/jsResource.ts now tracks the set of files each scope has already loaded (a closure Set that survives the pause/resume, since the Scope + handler are preserved across a redeploy). A re-'add' of an already-loaded file is a redeploy of code we can't hot-swap, so it's treated exactly like a change: request a restart and return. A first-time 'add' — initial load, or a genuinely new file dropped in at runtime — still loads without a restart.

This closes both halves of the bug:

• Restart flag: restartRequired now becomes true after such a redeploy (and in auto-reload/dev mode drives an actual reload).
• Stale code: because the re-'add' returns without re-importing, the stale module cache is never consulted for the changed file; a real worker restart rebuilds the per-scope caches from scratch.

static.ts-backed components are unaffected (they apply file changes to an in-memory map, no restart by design).

Why not also invalidate ApplicationScope.moduleCache (direction 2)

I deliberately did not proactively clear the shared per-scope moduleCache. With this fix nothing re-imports a changed resource file before a restart (we return on the re-add), so the stale cache is never read for it, and a genuine restart recreates the caches anyway. Proactively clearing a live, shared cache mid-life risks module-identity duplication (two instances of a dependency held by old-vs-new code) and re-running module side effects — real correctness hazards for no observable benefit given direction 1. This tradeoff is the main thing worth a second opinion.

Where to look

• resources/jsResource.ts — the whole change is the loadedResourceFiles set + the extended 'add' gate.

Tests

• unitTests/resources/jsResource.test.js — a re-add of a known path requests a restart and does not re-import; a distinct new file still hot-loads. The first test fails on main, passes here.
• integrationTests/deploy/redeploy-restart-flag.test.ts (+ fixture) — end-to-end: deploy a jsResource component, redeploy a modified copy with restart: false, assert get_status.restartRequired flips to true (and the old code stays live until a restart). Fails on main (30s poll never sees the flag), passes here.

Notes

• Generated by an LLM (Claude Opus 4.8). Standard cross-model review was not run in this headless worker — flagging for the human reviewer.

Refs #1817

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request fixes an issue where redeploying a jsResource component with restart: false failed to flag restartRequired because the file watcher's post-redeploy scan re-emitted existing files as 'add' events. To resolve this, the changes track loaded files in a Set and request a restart if an already-loaded file is re-added. The review feedback highlights that this approach does not handle file deletions during a redeploy, which would leave stale resources active in memory without requesting a restart. Additionally, the reviewer recommends adding nullish guards in the test helper functions when parsing API and HTTP responses to prevent potential runtime errors.

[claude]

Reviewed; no blockers found. Commit 85cfe7a is a mechanical extraction of the previously-duplicated test helpers (authHeader/operation/getRestartRequired/readVersion) into redeploy-restart-flag-helpers.ts, addressing the prior inline suggestion - verified the extracted code is byte-for-byte identical to what it replaced, and resources/jsResource.ts (the core fix) is untouched by this push. All prior review threads remain resolved.

[heskew]

Traced the fix: the restart-flag gate relies on EntryHandler emitting ready once per redeploy rescan batch, which holds (chokidar native ready + in-flight reads draining), and recurs correctly on later unrelated events. Prior bot findings all resolved at head. LGTM.

_{🤖 Reviewed via cross-model pipeline; approved by @heskew.}