Skip to content

fix: Keep file data and overrides through an in-place rewrite of a file - #2154

Open
kinyoklion wants to merge 15 commits into
mainfrom
rlamb/file-data-empty-files
Open

kinyoklion wants to merge 15 commits into
mainfrom
rlamb/file-data-empty-files

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Summary

Applications and editors rewrite data files in place: they truncate the file and then write the new content. A reload that reads the file in between finds it empty. The file override source applied an emptied YAML file as a document with no entries, so it removed every override from that file until the next reload. With a rewrite that pauses 300 ms after truncating, a client served a -> default -> b: on every run in watching mode, and whenever a poll fell inside the pause in polling mode. An emptied JSON file failed to parse and kept the overrides, but each rewrite that a reload caught was logged as an error (Unable to load flags: Unexpected end of JSON input). The file data source never served an empty file, but it reported each such rewrite as an error through its error handler and the log. It did the same before it moved onto the shared file data engine.

The OVERRIDE spec's retry requirement exists to keep "a partially written or briefly invalid file from emptying the layer" (Requirement 1.3.10). This change makes both consumers of the shared file data engine that reload, FileDataSource and the file override source, keep their data through such a rewrite without reporting it.

An emptied file is held back until it has stayed empty

A file has no entries when its content is white space only (a byte order mark counts as white space), or when its parser reads it as no document: a YAML parser returns null for a file with only a comment or a --- marker, which a read can see of a YAML file that is being written. {} is a document, and is applied right away.

A reload now reads and classifies every configured file even after one fails to read or parse, and fails with the first failure after the loop: failure first, then the hold, then the merge.

FileReloader now remembers, for each configured file, whether it contributed content to the data in effect, that is, to the last reload that was applied or skipped as unchanged; a missing file, or one with no entries, contributed none. For a file whose content is in effect and that reloads have found with no entries, it remembers since when the file has been so without being written, the file's state then (its modification time and size as the platform reports them, which the Node platform reads through any symbolic link, with the poller's file state helper), and when reloads began holding the file back. The clock is read after each file's own read, so that a slow read of another file does not shorten its delay.

A reload that finds such a file with no entries keeps the current data, logs at debug level (File ... is empty, possibly while it is being rewritten; reading it again before applying it), reports no failure, and arms a retry, whatever the format. The file is held back until it has stayed so, without being written, for exactly the retry delay (1 second):

  • A read of the file with content, even content that fails to parse, or a reload that finds it missing, ends its empty state, so its next emptying starts a new delay.
  • A change of the file's state means that it was written since a reload last found it empty, for example rewritten and truncated again before any reload read the new content, so its delay starts over. States are only compared with each other, never with the clock.
  • A file is held back for at most five retry delays. This bounds a file that keeps being written while every reload finds it empty, and files that are rewritten in turn so that every reload finds one of them empty. The bound is the file's own: it starts at the first reload that holds the file back, and starts over only when data is applied, or at a reload that holds nothing back, for every file, whether the file read, parsed, failed or was missing, and whether the reload applies, skips or fails. Since a reload reads every file even after one fails, a failing reload that holds nothing back starts every file's bound over, and one that holds a file back starts none over. So another file that keeps failing does not restart it, files emptied in staggered turns are bounded, a file that fails to load right after a hold gets a new bound, and one file reaching its bound does not cut short another file's ordinary rewrite. The bounds of files that keep being written while empty do not overlap: with n such files whose rewrites start 4.5 retry delays apart, the first application comes about 4.5 n + 0.5 retry delays after the first rewrite started, and a failure of another file that recurs exactly when each bound ends could hold them back without end.
  • The hold's retry is armed for when the first held file's delay or bound ends, plus a tenth of the retry delay, so that a timer that fires a little early still finds the time over. A further change signal while the file stays empty and unwritten does not apply it.
  • A reload that holds back one file still reads and parses the others, so that files emptied together start their delays in the same reload and are applied at the same retry. A file that cannot be read or parsed still fails the reload. Changes to the other files wait while a file is held back, because a reload applies all of the files or none of them.
  • A file whose content is not in effect is never held back: a placeholder that has always been empty, a file whose empty state has already been applied, and, where a missing file contributes nothing (the override source), a file that was deleted and then created empty. Such a file never delays another file's change.
  • The rule applies to every reload: a change signal, a poll, a change to another file while this one is mid-rewrite, and a retry left over from an earlier failure.
  • Holding back is not a failure. It does not make the next success a recovery, so a rewrite with the same content applies nothing. The retry after a hold reports a failure the way the reload that held the file back would have: a failure after a change notification is news, and only a failure after a retry can be a repeat.
  • With retries off (FileDataSource without autoUpdate, and the FDv2 file initializer, which both load once), nothing would read the file again, so an empty file is parsed right away.

A rewrite is covered when the time between truncating and writing is shorter than the retry delay. A writer that pauses longer gets an empty YAML override file applied, or an empty JSON file, or any empty FileDataSource file, reported as a load failure while the data in effect stays. A rewrite within the resolution of the file's modification time that keeps its size is not seen as a write, so its delay does not start over.

The commits show how the rule got here. The first read an emptied file again once, keyed on whether the last read found content; that still applied an empty file when no reload read the content between two truncations. The second and third hold a file back by its state, with a cap per file and a slack of a tenth in the delay check. The fourth applies the review findings: the cap per file let files rewritten in turn be held back without end, the slack let a reload apply a file that had been empty for less than the delay, a YAML read with only a comment was applied as no entries, and a reload that skipped unchanged content did not remember which file contributed it, so after a file moved to another configured path an in-place rewrite of it was applied empty. The fifth adds tests for a filesystem whose modification times are coarse. The sixth applies a second review: the clock read once per reload let a slow read of an earlier file shorten a later file's delay, a failed reload restarted the shared bound so that a file kept written was held without end while another file kept failing, and the shared bound let one file reaching it apply another file a fraction of a delay into an ordinary rewrite; it reads the clock after each file's read and bounds each file on its own. The seventh and eighth add and name tests. The ninth applies a third review: a file that stayed empty for the whole delay cleared its own bound while another file kept every reload holding, so files emptied in staggered turns were held without end, and a file held back while another file failed kept its bound, so a fix of the other file long after, during a rewrite of this one, applied it empty; a bound now starts over only at a reload that holds nothing back. The tenth applies a fourth review: a file whose read or parse failed right after a hold kept its old bound, so an operator who saved a YAML typo in place and then the fix in place seconds later got the fix's truncation applied empty; a reload now starts the bound over for every file it got to, including the one that failed, and content that fails to parse ends a file's empty state. The eleventh applies a fifth review: a file configured after a failing file kept a bound left over from before the failure, because every reload stopped at the failure, so a deploy that fixed the failing file while rewriting the later one in place applied its truncation empty; a reload now reads every file even after a failure. It also keys skip-unchanged by path, below. The twelfth rejects a JSON document that is not an object, below, and the thirteenth and fourteenth reduce the public docs to what an application needs. It is the rule of dotnet-core #394.

Skip-unchanged is keyed by path

A reload skipped apply when the contents of the present files were the same as those of the data in effect, without their paths. Content that moved to another configured path was then not applied: the summary of what each file supplied stayed stale, and since the format is chosen by the path's extension, a YAML parser that reads some content differently from JSON would have kept the old data for the same content moved between x.yaml and x.json. (Content that the other format cannot parse fails to parse before the comparison, so it was reported.) The comparison now covers each configured path, whether it is present, and its content.

An empty JSON file says what is wrong

The parser for a file is chosen by its extension, so whether an empty file is valid is up to the format. An empty or white-space JSON file was already invalid: JSON.parse rejects it and the last good data stays. It now fails with the file has no content, which is not a JSON document (still a SyntaxError) instead of Unexpected end of JSON input. This applies to a file with no extension and to every extension other than .yml and .yaml, since those files are parsed as JSON. A YAML parser's null or undefined for an empty file is still a document with no entries in the override source.

A JSON document must be an object

A JSON override file holding null was read as a document with no entries, so it removed the file's overrides. A JSON data file holding null failed with a TypeError from the merge, and one holding 42, "text" or [] loaded as no flags. Any JSON value that is not an object now fails to parse with file data must be an object, and the last good data stays, for the override source, FileDataSource and the FDv2 file initializer. For a JSON data file holding a value that is not an object, this changes released behavior from loading no flags to a load failure; such a file is not a document in the file data format. A YAML parser's null for a file with no document is still a document with no entries in the override source.

A file named only by its YAML extension, such as .yaml, was already parsed as YAML, as in .NET and Go; a test now pins it.

Documentation

The public docs say what an application needs; how the hold, the retries and the watches work stays in the internal comments and in this description.

  • FileOverrideSourceOptions says that a file whose name ends in .yml or .yaml is YAML and any other file is JSON, read as UTF-8; that if a file that exists cannot be read or parsed, the previously loaded overrides stay in effect, the failure is logged, and the source loads the file once it is valid again; that files can be rewritten in place, and that if the source finds a file empty while it is being rewritten, it keeps the overrides in effect until the new content is written; how to remove a file's overrides (delete the file or write {}); that an empty JSON file is not a valid document; and that replacing a YAML file in one operation avoids briefly applying a partly written file. Its yamlParser says that a null or undefined result, which YAML parsers return for an empty file, means the file has no entries.
  • FileDataSourceOptions.autoUpdate says that the data source watches the files for changes; that a file replaced by a rename, or deleted and created again, is picked up, and so is an update of a mounted Kubernetes ConfigMap or Secret; that a file or directory that cannot be watched yet is still noticed once it can be watched; that if it reads a file while it is being written, or a file cannot be parsed, it keeps the flags it has and reads the file again shortly afterward; and that replacing a YAML file in one operation avoids briefly loading a partly written file. The description of fs.watch, the direct file watches, the retry interval and the retries is gone; the fs.watch note moved to the internal docs of FileDirectoryWatcher, which already describe the rest.

Not changed

  • YAML cut off between two entries. A read that overlaps a write can see the first part of a YAML file. If the cut falls between two entries, the part is valid YAML with fewer entries, and it is applied until the next reload. JSON cut off anywhere fails to parse. This predates this change.
  • An empty YAML file in FileDataSource and the FDv2 file initializer. Their policy hands a YAML parser's null to the merge, which fails with Cannot read properties of null (reading 'flags'). An empty YAML data file therefore fails to load and the last good data stays, as before the shared engine. With this change, an emptied file, or one with only a comment, is held back before that failure is reported. Making it a document with no entries, as it is for override files, changes released behavior, so it is left for a separate change.
  • The case of the extension. The YAML extension check is case-sensitive, so flags.YAML is parsed as JSON, as it always was.

Testing

  • Parser: an empty, white-space or byte-order-mark-only JSON file is rejected with the new message, including a file with no extension or another extension. JSON null, a number, a string, true and [] are rejected as not an object, by the extension-based parser and the override document parser. A file named .yaml or .yml is YAML. A YAML parser's null or undefined is returned as it is, and the override document parser makes it a document with no entries.
  • Reloader (FileReloader.test.ts): a JSON data file holding null is reported as a parse failure and the data stays; a reload reads every file even after one fails and reports the first failure, for content that fails to parse, no content, and a missing file; files that changed places with the same contents are applied, not skipped.
  • Reloader (FileReloader.inPlaceRewrite.test.ts, mock filesystem with a modification time on every write, Jest's fake clock):
    • An emptied file (empty, white space, byte order mark) is held back, and the rewritten content is applied with no empty application in between; the debug line is logged.
    • A YAML file with only a comment, a --- marker, or both is held back; a file with {} is applied right away.
    • A file that stays empty is applied by the hold's retry, a tenth of a retry delay after its delay ends, and not before.
    • A load at 95 % of the retry delay still holds the file back, and a load at the delay applies it.
    • An emptied JSON file is not reported by the load that finds it empty; the retry reports it with the new message, and the data stays.
    • A further change signal does not apply a file that stayed empty, and the retry it arms comes when the delay that started at the first load ends.
    • Two quick rewrites: the write and the second truncation between two loads restart the delay, so a load after the first delay would have ended still holds the file back.
    • Several files: a change to another file while one file is mid-rewrite does not apply the emptied file.
    • Polling-like: the retry that finds the truncation of a later rewrite does not apply it.
    • The bound, with one file kept written so that every load finds it empty, and with two files rewritten in turn: they are applied at the retry when five retry delays have passed since the file was first held back.
    • Another file that fails at every other load does not restart the bound of a file kept written.
    • Two files emptied in staggered turns, 0.9 of a retry delay apart, with a load at each truncation and one standing for the retry, are applied once their bounds are over.
    • A file held back while another file fails, and then read with content by a failed load, gets a new bound: when the other file is fixed long after, during a rewrite of this file, the file is held back.
    • The in-place save that fixes a file saved in place with an error, YAML and JSON, more than the bound later: its truncation is held back and no failure is reported. The same for a file that was missing for longer than the bound and is created empty, and then written.
    • A file kept written while empty is applied after its bound while an earlier file fails at every other load; a load that holds a file back and then fails does not start bounds over.
    • The bound of a file configured after a failing file starts over once a failing load reads it: when the failing file is fixed long after, during a rewrite of the later file, the later file is held back.
    • A missing file created empty again with its first emptying's modification time gets a new delay; with coarse modification times, so does a file emptied again after content that failed to parse.
    • One file reaching its bound does not apply another file that is a quarter of a retry delay into an ordinary rewrite.
    • A slow read of an earlier file (900 ms) does not shorten a later file's delay.
    • A fresh bound or delay: for a file emptied again after its content was applied, for a JSON file that has been empty and failing for long and is then rewritten in place (no extra report), and for a file that is deleted and created empty again. With coarse modification times: a file emptied again after its content was read, applied or in a failed load, and a file deleted and created empty again; and a file emptied again after a read of its content, with its modification time set back to the first emptying's, while another file kept written keeps the load from applying.
    • The delay of the retry that a hold asks for: only the margin when the read of another file takes longer than the rest of the held file's delay; a retry delay and a tenth after the first hold; the rest of the delay after a later hold; the bound when that is sooner; the file whose delay ends first, configured first or second; the same for a hold by a retry; and a retry delay after a failure.
    • Content that moved from one configured file to another, with the contents unchanged, is applied, and is held back when the file it moved to is rewritten in place.
    • An empty placeholder file is never held back and does not delay a change to another file; a file whose empty state is in effect is not held back again; with missing files skipped, a file deleted and then created empty is applied right away.
    • Two files rewritten together are each held back; two files emptied together that stay empty are applied at the same retry; a file that fails to parse fails the load even when another file is held back.
    • A retry left over from a failure that finds the file emptied holds it back instead of applying it empty; with retries off, an emptied file is applied right away.
    • Holding back is not a failure: a rewrite with the same content applies nothing.
    • A JSON override file holding null, [] or 42 is reported, not held back, and the data stays.
    • A file whose state cannot be read is still held back, and its retry still comes.
    • Reporting after a hold: a failure after a hold that followed a change is news; after a hold by a retry, the next retry's failure is a repeat, and a change signal's failure is news.
    • A reload that completes after close does not hold anything back.
  • End-to-end in server-node, with real in-place rewrites (truncate, pause, write; rewriteInPlace in __tests__/inPlaceWrites.ts) while the test samples evaluations:
    • A client with FileDataSource and autoUpdate serves a -> b and reports no error, for JSON and YAML, with a 300 ms pause; and never serves the default and reports no error through two rewrites 20 ms apart.
    • A client with a file override source serves a -> b and logs no load failure, for JSON and YAML, in watching mode (300 ms pause) and in polling mode at the default 1 second interval. In polling mode the rewrite starts 750 ms after a poll and pauses 600 ms, so the next poll finds the file empty.
    • The same client never serves the default and logs no load failure through two rewrites in a row: in watching mode 20 ms apart, and in polling mode with the second truncation 1900 ms after a poll, so that the next poll and the retry find the file empty again before any load read the first rewrite's content.
  • On main, all 12 end-to-end tests fail: the YAML override file serves default in between, and the JSON override file and both data files log an error. 59 of the new unit and parser tests fail too. On the first commit of this branch, the 6 end-to-end tests with two rewrites in a row fail, and 38 unit tests; on the third commit, 25; on the fifth (the run cap), 10; on the eighth (the per-file bound), 10; on the ninth (the bound started over only at a reload that holds nothing back), 8; on the tenth (reached files), exactly the 4 tests of the fifth review: a file after a failing file, reading every file after a failure, and the two tests of moved content.
  • Mutants: 51 mutants, each reverting or changing one part of the change on its own with a clean rebuild. 49 fail at least one of the new tests: no hold; hold whatever the data in effect; the data in effect not remembered after an apply, or for an empty or missing file; the delay kept after a read with content, content that fails to parse, or a missing file; the bound cleared by a read with content, or when a file stayed empty for the whole delay; no restart on a change of state; no bound; the bound restarted by each hold; the bound never started over, started over only without a failure, or started over by a reload that holds something back or that holds a file back and then fails; a file not counted as reached when its read fails, when its content fails to parse, or before its empty content parses; the reload stopping at the first failure, or reporting the last failure, for a read, a parse, or an empty file; skip-unchanged without the paths; a slack in the delay check; the bounds cleared by a failure; the hold's retry armed for a full delay, without the margin, without the clamp at zero, ignoring the bound, or decided by the last or the first held file; YAML with no document not counted as empty; {} counted as no document; the override parser returning {} for no document; stop reading after a held-back file; a hold that hides another file's failure; holding back counted as a failure; no retry armed; no carry-over of the reason, a carry-over to any reload, or always the reason of a change; a file state lookup that throws; no check for close; either debug line removed; the default JSON.parse message for an empty JSON file; white space counted as content. For the twelfth commit, 10 mutants of the parsers (any JSON value accepted, or null, an array or a scalar accepted; a file named .yaml not counted as YAML; and the parser mutants above) all fail at least one test. Two survive and are equivalent: dropping the explicit retryDelayMs <= 0 check from the hold (with no retry delay the delay is zero long), and not clearing the files' entries when data is applied (a reload that applies holds nothing back, so it already starts every file's bound over, and the files it read with content already start their delays over).
  • Loops: the new and changed unit and parser test files (143 tests, with FileReloader.test.ts) passed 15 runs in a row, and the two server-node test files with the end-to-end tests (20 tests) passed 12 runs in a row, on a loaded machine. The polling end-to-end tests take 3 to 4 seconds, so the override end-to-end tests have a 15 second timeout.
  • @launchdarkly/js-server-sdk-common and @launchdarkly/node-server-sdk: full test suites, lint, format:check, build and build:doc pass locally.

Opened from termag

SDK-3419


Note

Overview
File data and file-based overrides now survive in-place rewrites (truncate-then-write). When a reload reads a file that is briefly empty or YAML-with-no-document while its previous content is still in effect, FileReloader holds back the change, keeps the last good flags/overrides, schedules a retry (with per-file delay and a five-retry cap), and no longer treats that as a load failure.

Reload behavior is tightened: all configured paths are read even after one fails (first failure still wins); skip-unchanged compares content per path so moved files are not silently ignored.

Parsing is stricter and clearer: empty/whitespace JSON fails with an explicit message; JSON must be an object (null/[]/scalars rejected); YAML null/undefined maps to a NO_ENTRIES sentinel for overrides. Public options/docs describe rewrite tolerance and YAML-vs-JSON rules.

Coverage adds shared rewriteInPlace / sampleValues helpers, a large FileReloader.inPlaceRewrite suite, and Node SDK e2e tests for FileDataSource and file overrides (watching and polling).

Reviewed by Cursor Bugbot for commit 6058665. Bugbot is set up for automated code reviews on this repo. Configure here.

A file that is rewritten in place is truncated and then written, so a
reload can find it empty. The file override source applied an emptied
YAML file as a document with no entries, which removed its overrides
until the next reload. An emptied JSON file failed to parse and was
reported as an error on every in-place rewrite, by the override source
and by the file data source.

The shared file reloader now remembers, for each file, whether its last
read found content. A reload that finds a file empty after it had
content keeps the current data, logs at debug level, and reads the file
again after the retry delay, without reporting a failure. The next
reload parses whatever the file holds, so the data is held back at most
once per emptying. A file that is empty from the start is never read
again, and with retries off an empty file is parsed right away.

An empty JSON file now fails with a message that says the file has no
content.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 32497 bytes
Compressed size limit: 32500
Uncompressed size: 153345 bytes

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Compressed size: 34573 bytes
Compressed size limit: 35000
Uncompressed size: 123262 bytes

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 26346 bytes
Compressed size limit: 44000
Uncompressed size: 171286 bytes

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…r the retry delay

Reading an emptied file again once, keyed on whether the last read found
content, still applied an empty file in three in-place rewrite
sequences, because a new truncation that no load read in between looks
like a file that stayed empty: two quick rewrites in one settle window,
a change to another file while one file is mid-rewrite, and polling with
two rewrites about a second apart.

The reloader now remembers, per file, whether the file contributed
content to the data in effect, and the empty episode of a file whose
content is in effect: when a load first found it empty, and since when
it has been empty without being written, by its modification time. Such
a file is held back until it has stayed empty, without being written,
for one retry delay, and at the latest five retry delays after a load
first found it empty. The retry is armed for the end of that delay. A
file whose content is not in effect, such as a placeholder, is parsed
right away, as is any empty file when retries are off.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

… a slack

A reload that holds back one emptied file now keeps reading the others,
so that files emptied together start their delays in the same reload
instead of one retry delay after another. A file that cannot be read or
parsed still fails the reload. The retry that the hold arms comes one
retry delay later and a timer can fire slightly early, so the hold is
checked against nine tenths of the retry delay; a file that stays empty
is applied by that retry. The retry is armed for the usual delay again.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…y, and hold YAML with no document

Reviews of the hold found four problems, and this addresses them:

- Files rewritten in turn were held back without end, because the cap of
  five retry delays was per file and reset whenever that file was read
  with content. The cap now bounds the run of reloads that hold files
  back, from the first such reload until a reload that holds none back.
- The slack of a tenth of the retry delay let a reload apply a file that
  had been empty for less than the delay. The hold is now exactly one
  retry delay, and its retry is armed for when the first held file's
  delay or the cap ends, plus a tenth of the retry delay.
- A partial YAML read with only a comment or a document marker parsed to
  no document and was applied as no entries. A file whose parser reads
  it as no document now counts as empty for the hold; `{}` is a document
  and is applied right away.
- A reload that skipped unchanged content did not remember which file
  contributed it, so after a file moved to another configured path an
  in-place rewrite of it was applied empty. The skip now remembers it.

The hold now compares the file's modification time and size, read
through any symbolic link, with the poller's file state helper.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…ach file's read

A review of the round-4 rule proved three regressions:

- The clock was read once per reload, so a slow read of an earlier file
  made a later file's emptiness look older and shortened its delay. The
  clock is now read after each file's own read, and again when the
  hold's retry delay is computed.
- A failed reload ended the run of holding reloads, so a file that kept
  being written while empty was held without end when another file kept
  failing.
- The bound was shared, so one file reaching it applied another file
  that was a fraction of a delay into an ordinary rewrite.

The run cap is replaced by a bound for each file: it starts at the first
reload that holds the file back, is kept across reads with content and
across failed reloads, and starts over when data is applied or when the
file has stayed empty for the whole delay. The hold's retry comes when
the first held file's delay or bound ends, plus a tenth of the retry
delay. The reloader records the delay of the last retry it armed, and
tests pin it.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…nothing back

A review of the per-file bound proved two holes. A file that stayed
empty for the whole delay cleared its own bound while another file kept
every reload holding, so two files emptied in staggered turns were held
back without end. And a file held back while another file failed kept
its bound, so a fix of the other file long after, while this file was
being rewritten, applied this file empty.

A file's bound now starts over only at a reload that holds nothing
back, for every file that the reload read, whatever its outcome; a file
that the reload did not get to keeps its bound. The bound is no longer
cleared when a file stays empty for the whole delay.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…empty state

A review proved that a file whose read or parse failed right after a
hold kept its old bound, because the bound was started over only for
files that a reload read without failing. An operator who saved a YAML
typo in place and then the fix in place seconds later got the fix's
truncation applied empty, and a JSON fix reported a load failure.

A reload now records every file it gets to and does not hold back,
including the file that fails to read or parse and a missing file, and
a reload that holds nothing back starts all of their bounds over. A
file after the one that failed keeps its bound. Content, even content
that fails to parse, ends a file's empty state, as a missing file does.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

A review proved that a file configured after a failing file kept a
bound left over from before the failure: every reload stopped at the
failing file, so the later file was never read and its bound never
started over. When a deploy fixed the failing file while rewriting the
later one in place long after, the later file's truncation was applied
empty.

A reload now reads and classifies every configured file even after one
fails to read or parse, and fails with the first failure after the
loop, so a reload that holds nothing back starts every file's bound
over.

Skip-unchanged now compares each configured path, whether it is present,
and its content. It compared the contents of the present files only, so
the same content moved to a path with another extension, which is
parsed in another format, was skipped instead of loaded.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

A JSON file holding `null` was read as an override document with no
entries, so it removed the file's overrides, and a JSON data file
holding `null` failed with a TypeError from the merge, while one holding
`42` or `[]` loaded as no flags. A JSON document must be an object: any
other JSON value now fails to parse with "file data must be an object",
and the last good data stays, for the override source and the file data
sources alike. A YAML parser's null for a file with no document is still
a document with no entries.

A test also pins that a file named only by its YAML extension, such as
`.yaml`, is parsed as YAML.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…the hold works

The public docs of the file override source and the file data source
described the hold's delay, its bound, other files waiting, and the
retries. They now say what an application needs: files can be
rewritten in place, how to remove a file's overrides, that an empty
JSON file is not a valid document, and that replacing a YAML file in
one operation avoids briefly applying a partly written file. The
override source docs also say which extension means YAML and that files
are read as UTF-8. The details stay in the internal comments.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…cation needs

The autoUpdate docs described how the watches are set up, the retry
interval and the retries. They now say that the data source watches the
files, picks up a replaced file and a Kubernetes ConfigMap or Secret
update, still notices a file or directory that cannot be watched yet,
and keeps its flags through an in-place rewrite. The override source
docs no longer mention retries. The fs.watch note moves to the
watcher's internal docs.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 78bcfd1. Configure here.

@kinyoklion
kinyoklion marked this pull request as ready for review November 6, 2026 21:23
@kinyoklion
kinyoklion requested a review from a team as a code owner November 6, 2026 21:23
// Called when a reload finds a file with content, even content that fails to parse, or finds it
// missing. That ends the file's empty state: the next time a reload finds it empty, its delay
// starts over. Its bound does not.
private _fileChanged(path: string): void {

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.

nit: function name is misleading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This reply was written by an AI agent (Claude Code) working on behalf of @kinyoklion.

Fixed in 6058665: it is now _endEmptyState, which is what it does. It ends the file's empty state, so the delay starts over the next time a reload finds the file empty.

* The clock is read after the file's own read, so that a slow read of another file does not
* shorten this file's delay.
*/
private async _holdsBack(path: string): Promise<boolean> {

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.

nit: the function naming here is also weird... should it be something like _should HoldBack

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This reply was written by an AI agent (Claude Code) working on behalf of @kinyoklion.

Fixed in 6058665: it is now _shouldHoldBack.

The function that ends a file's empty state is now _endEmptyState, and the one that decides whether a reload holds a file back is now _shouldHoldBack.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants