ReviewOS

home-lang/home

fix(ts-watch): invalidate deleted sources in the CLI program

#923
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Follow-up to #922. The polling watcher now reports one removal and a later re-addition, but the actual home-tsc --watch loop in packages/ts_cli/src/tsc_main.zig explicitly skips every .removed change before Program.updateSource/recompileChanged. Program currently exposes updateSource but no corresponding source-removal operation. This means deleting an imported or root file can leave stale source, diagnostics and emit state until another event; #922 does not solve this CLI-level gap.

Acceptance: model source removal in Program and reverse dependency invalidation without substituting empty source or suppressing diagnostics; make the watch loop process removal and publish the complete current diagnostic/output set; cover delete, repeated absence, same-metadata recreate, imported-file deletion/recreation, and error/OOM paths with actual CLI or Program controls. Preserve existing file identities and unchanged-source incremental behavior. No filename-specific workaround or passing credit from watcher-only tests.

9 days ago

Prerequisite #924 is fixed by 40cfdba: the watch loop now observes initially loaded transitive sources, not only root inputs, with a guarded real-Program regression. This does not close #923: the current watch loop still skips .removed events, and Program has no source-removal API. A correct removal must preserve stable FileIds, invalidate the old importer closure before clearing edges, drop source/owner storage, exclude absent files from all compile/reference/output paths, and handle canonical package-dedup redirects without treating missing files as empty modules. Recreated paths and imports added after the initial closure also need explicit coverage. No deletion pass is claimed.

9 days ago

Guarded live deletion control on current main 5d54d1a (Darwin arm64), using a temporary two-file project that was removed afterward:

  • Both Home home-tsc and local TypeScript 5.7.3 started with 0 errors for main.ts importing dep.ts (number export).
  • After deleting dep.ts, Home printed no incremental-compilation event over the next 10 seconds. It did notice a later re-addition containing a string export and reported importer TS2322, so the loop remained alive.
  • In the separate TypeScript reference run, deletion immediately produced an incremental event with TS2307 at main.ts(1,23); repeated absence produced no second event, and recreating the original number export produced another event and 0 errors.
  • Each persistent watch was run alone under the unchanged 3,840 MB run_bounded guard and intentionally ended at its 35-second wall-clock deadline (exit 124); that is not a suite pass.

Code trace: tsc_main.zig still skips .removed before Program.updateSource/recompileChanged. Program File.source is a borrowed nonoptional slice from Program.sources, and compilation/owner/importer invalidation assumes physical presence; Resolver also memoizes resolution and file-existence results. A correct implementation must represent absence explicitly, invalidate the old reverse closure before clearing edges, clear filesystem-resolution caches, keep stable FileIds for recreation, exclude absent files from compilation/emit, and produce importer TS2307 rather than treating missing bytes as an empty module. No production source or test assertion was changed in this control.

9 days ago

Watch-mode prerequisite landed on origin/main in bbe07c5 (3 files, Glenn author/committer, no coauthors): resolver filesystem caches and their arena are invalidated between successful watch revisions after the checker adapter is disposed. A guarded real home-tsc --watch control moved from missing-import TS2307 to imported-value TS2322 after dep.ts was created and the importer changed. The persistent watch was intentionally stopped at 35 seconds (exit 124), not counted as a suite pass. This does not implement Program source removal, absent FileId state, package-redirect reconciliation, or deletion/recreation diagnostics; #923 remains open. See #925 for exact tests and a separate positive-cache limitation.

9 days ago

Pushed 8510bbc directly to origin/main (four files; Glenn author and committer). Program now retires a removed source while preserving its FileId, invalidates the old reverse importer closure before clearing edges, excludes absent files from compilation/reference/emission, and reconciles package-identity redirects. The CLI watch loop processes removed events and reports TS6053 for absent explicit roots instead of silently retaining old source.

Verification under the 3,840 MB guard: watch-program 7/7 (including removal, TS2307, repeated absence, same-mtime-and-size recreation, stable FileId), tsc_main 52/52, ts_cli 81/81; ts_program, ts_resolver, and ts_driver reported cached success for current sources. Earlier guarded live home-tsc controls on these source changes showed imported-file deletion -> TS2307 -> clean recreation, and explicit-root deletion -> TS6053 -> clean recreation; the intentionally bounded persistent watchers exited 124, not as passing suites. Program tests also cover root recreation, package redirect promotion, and allocation-failure atomicity.

Leaving the issue open pending broader output-file lifecycle and mixed-batch failure verification; this is not a claim that all watch-mode acceptance or the full TypeScript suite is green.

9 days ago

Follow-up output-lifecycle control for 8510bbc: ran one reference TypeScript 5.7.3 watch and one freshly rebuilt Home home-tsc watch, sequentially under the 3,840 MB guard, on the same temporary two-file project with JS emission enabled. Both initially emitted main.js and dep.js with 0 errors. Deleting dep.ts produced importer TS2307 in both; both retained the previously emitted dep.js on disk and re-emitted main.js. Thus removing stale dep.js would diverge from the reference behavior. Each persistent watch intentionally hit its 45-second wall bound (exit 124), which is not a suite pass. The fixture was removed afterward.

Together with the guarded Program allocation-failure test and watch retry-after-update/compile-OOM test in the pushed commit, this covers the deletion/recreation and publication behavior requested here. The broader TypeScript and Bun suite goals remain separately open.

Sign in to comment on this issue.