fix(server): serialize chokidar watcher batches to prevent heap growth - #1300
Open
Schrotti77 wants to merge 1 commit into
Open
fix(server): serialize chokidar watcher batches to prevent heap growth#1300Schrotti77 wants to merge 1 commit into
Schrotti77 wants to merge 1 commit into
Conversation
chokidar (Windows ReadDirectoryChangesW) emits more granular events than inotify, so a 50ms batch can still be mid-drain (readFile-ing every change) when the next timer fires. Launching batches independently piles up overlapping async work → unbounded heap growth on event storms. Chain batches on an inFlight promise so they drain strictly one at a time. The .catch stays load-bearing: it keeps the chain alive after an error. Regression test: maxActive must stay 1 when a second batch lands mid-drain. Fails without the fix (maxActive=2), passes with it.
|
Thanks for the contribution! What happens next:
|
|
Hermes (Marian's AI Agent) seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Contributor
|
hi @Schrotti77 could you follow the @CLAassistant instructions? and i'll see this through |
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Aug 24, 2026
* feat: relay public OK pull requests into Linear tickets Linear mirrors public GitHub issues into PRD tickets but not pull requests, and it does not follow GitHub's own issue-to-PR link either: PR #623 says "Closes #622" and is still absent from PRD-7349, the ticket that #622 created. So community pull requests were invisible in Linear, including the six of eight open ones that never crossed the contributor bridge into agents-private. The public repo relays a bare PR number here and this repo does the Linear work with its own key. Every workflow on the public repo that reads LINEAR_API_KEY today fires only on schedule, push or *_dispatch, so nothing fork-reachable has ever held that key. Filing a ticket means reading a contributor-controlled PR title and body, so the relay job holds no Linear secret and checks out no code at all. The only thing crossing the trust boundary is an integer, validated on both sides. Creating a ticket is the last branch of a pure resolver, reachable only when three lookups come back empty: an existing attachment on the PR URL, a Linear ticket mirroring an issue the PR closes, and a PRD identifier carried by branch, title or body. A bare cross-reference is deliberately not a dedupe signal, so PR #1300 mentioning "see #1207" does not attach it to an unrelated bug. Dry run against live GitHub and Linear: #1177 resolves to PRD-7971, #623 to PRD-7349, #345 to the hand-filed PRD-7321, and the six genuinely untracked open PRs resolve to create. The shared Linear transport moves to scripts/linear-graphql.mjs rather than becoming a third private copy of the same fetch wrapper; origin-link.mjs now uses it and its 25 tests still pass. PR_LINEAR_SYNC_ARMED is "false", so nothing is written to Linear until someone flips one line. * fix: address Pullfrog review on the public PR Linear relay Three real findings, all confirmed by execution before fixing. Carrier lookup trusted contributor prose. extractIdentifiers was fed headRef + title + body, but origin-link.mjs declares CARRIERS as branch, title and attachment, so the docblock claim that the two agree was false for the body. The consequence was worse than the inaccuracy: a fork PR body containing "PRD-1234" would both suppress the ticket this workflow exists to file and choose which internal ticket receives an attachment, which is the same mention-as-a-claim shape the header rejects for "see #1207". The body is dropped from the carrier text, matching origin-link.mjs exactly, and the docblock now says what is actually true. A missing Linear key swept silently. main() returned 0 when LINEAR_API_KEY was absent, so a rotated secret would report the whole sweep green having evaluated nothing. Unlike origin-link.mjs, which still does its real work without Linear, every lookup here queries Linear, so the key is now required and the job fails loud. The shell integer check was weaker than the layer behind it. grep -qE anchors per line, so a two-line payload whose first line is digits passed it. Verified: printf '1379\nevil' passes the grep and is rejected by the node regex, so nothing was exploitable, but a defence-in-depth layer that is weaker than the one it backs is not one. Switched to the bash [[ =~ ]] form, which anchors against the whole string. The body-exclusion guard was mutation tested: reintroducing pr.body turns the new test red, so it pins the property rather than merely passing alongside it. 52 tests pass, actionlint is clean, and the live dry run is unchanged: 1177 to PRD-7971, 623 to PRD-7349, 345 to PRD-7321, six untracked PRs to create. * fix: close the duplicate and write-primitive paths found in review Two majors from the claude review, plus the minors and considers. Create and attach were not atomic. issueCreate followed by attachmentCreate are two calls with no transaction, so a failure between them left a ticket carrying none of the dedupe signals, and the next sweep would file a genuine duplicate. That is the one invariant this script exists to hold. Three changes: the created ticket is now logged BEFORE the attach so the identifier survives on the failing path, the attach is wrapped so the error names the orphan, and a third lookup searches for a ticket carrying this PR's "(community PR #N)" title suffix before creating. An interrupted run is now self healing: the next run finds the orphan and attaches, so lookup 1 sees it from then on. Multi target attaches also collect failures instead of throwing on the first, because stopping early left later targets unattached while lookup 1 short circuited past them on the retry. The carrier lookup is removed entirely. Scanning contributor text for PRD-#### is right for internal PRs and wrong across this boundary. Identifiers are not secret: AGENTS.md requires one in every PR title and Copybara mirrors those subjects into the public repo's history, so any contributor could both write attacker shaped text onto an internal ticket of their choosing and suppress their own PR's triage ticket by naming any resolvable one. The two remaining signals are GitHub authoritative and covered every case in the dry run. Attachment titles are now "PR #N" alone for the same reason: an attachment renders as first party automation output, so its text must not be contributor shaped. Minors: the relay token is scoped to permission-contents write, since POST /dispatches needs only that and the installation otherwise carries workflows write on the private monorepo; resolveSyncContext now warns when the label or project lookup misses, which previously degraded every ticket in a run while still reporting success; and scripts/linear-graphql gets the unit tests its own docstring justified, covering the swallowed non 2xx body and the 200 carrying a GraphQL errors array. Considers: every fetch now carries an AbortSignal timeout so one hung call cannot consume the sweep's budget, CI.md documents both workflows and LINEAR_API_KEY, and the create test asserts the ticket description rather than only its metadata. 910 root script tests pass, actionlint is clean, and the live dry run is unchanged: 1177 to PRD-7971, 623 to PRD-7349, 345 to PRD-7321, six untracked PRs to create. Those six also exercised the new orphan query against the real Linear API. * fix: identify Copybara sync PRs by head repo, not branch name A fork picks its own branch name, so keying the skip on the string copybara/sync let any contributor name their branch that and drop out of both the relay and the receiver's sweep filter, leaving their PR permanently invisible to Linear. That is evasion of the tracking control this workflow exists to be. "This repo talking to itself" is a property of the head repository, which a stranger cannot forge. The relay now also requires head.repo.full_name to equal the repository before skipping, and the sweep excludes copybara/sync only for same-repo PRs via isCrossRepository. Verified against the four cases: a real same-repo copybara/sync PR is still excluded, a fork branch named copybara/sync is now included, and both ordinary shapes are unaffected. Adds a workflow-shape test file. Every property that keeps the Linear key off a fork-reachable trigger is one line of YAML that a future edit could revert without touching a test: no checkout in the relay, no Linear credential in the relay, the scoped app token, the env-not- interpolation payload, both provenance checks, the disarmed default, the whole-string integer check, and the serialized concurrency group. The no-checkout guard was mutation tested by adding a checkout, which turns it red. The absence assertions strip whole-line comments first, because both workflows explain their own posture in prose and would otherwise match the comment saying why the thing is absent. 919 root script tests pass, actionlint clean, subtree md-audit clean. GitOrigin-RevId: 50dc33f876258793dd23d160914ce5f0267ab89f
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Aug 24, 2026
* feat: relay public OK pull requests into Linear tickets Linear mirrors public GitHub issues into PRD tickets but not pull requests, and it does not follow GitHub's own issue-to-PR link either: PR #623 says "Closes #622" and is still absent from PRD-7349, the ticket that #622 created. So community pull requests were invisible in Linear, including the six of eight open ones that never crossed the contributor bridge into agents-private. The public repo relays a bare PR number here and this repo does the Linear work with its own key. Every workflow on the public repo that reads LINEAR_API_KEY today fires only on schedule, push or *_dispatch, so nothing fork-reachable has ever held that key. Filing a ticket means reading a contributor-controlled PR title and body, so the relay job holds no Linear secret and checks out no code at all. The only thing crossing the trust boundary is an integer, validated on both sides. Creating a ticket is the last branch of a pure resolver, reachable only when three lookups come back empty: an existing attachment on the PR URL, a Linear ticket mirroring an issue the PR closes, and a PRD identifier carried by branch, title or body. A bare cross-reference is deliberately not a dedupe signal, so PR #1300 mentioning "see #1207" does not attach it to an unrelated bug. Dry run against live GitHub and Linear: #1177 resolves to PRD-7971, #623 to PRD-7349, #345 to the hand-filed PRD-7321, and the six genuinely untracked open PRs resolve to create. The shared Linear transport moves to scripts/linear-graphql.mjs rather than becoming a third private copy of the same fetch wrapper; origin-link.mjs now uses it and its 25 tests still pass. PR_LINEAR_SYNC_ARMED is "false", so nothing is written to Linear until someone flips one line. * fix: address Pullfrog review on the public PR Linear relay Three real findings, all confirmed by execution before fixing. Carrier lookup trusted contributor prose. extractIdentifiers was fed headRef + title + body, but origin-link.mjs declares CARRIERS as branch, title and attachment, so the docblock claim that the two agree was false for the body. The consequence was worse than the inaccuracy: a fork PR body containing "PRD-1234" would both suppress the ticket this workflow exists to file and choose which internal ticket receives an attachment, which is the same mention-as-a-claim shape the header rejects for "see #1207". The body is dropped from the carrier text, matching origin-link.mjs exactly, and the docblock now says what is actually true. A missing Linear key swept silently. main() returned 0 when LINEAR_API_KEY was absent, so a rotated secret would report the whole sweep green having evaluated nothing. Unlike origin-link.mjs, which still does its real work without Linear, every lookup here queries Linear, so the key is now required and the job fails loud. The shell integer check was weaker than the layer behind it. grep -qE anchors per line, so a two-line payload whose first line is digits passed it. Verified: printf '1379\nevil' passes the grep and is rejected by the node regex, so nothing was exploitable, but a defence-in-depth layer that is weaker than the one it backs is not one. Switched to the bash [[ =~ ]] form, which anchors against the whole string. The body-exclusion guard was mutation tested: reintroducing pr.body turns the new test red, so it pins the property rather than merely passing alongside it. 52 tests pass, actionlint is clean, and the live dry run is unchanged: 1177 to PRD-7971, 623 to PRD-7349, 345 to PRD-7321, six untracked PRs to create. * fix: close the duplicate and write-primitive paths found in review Two majors from the claude review, plus the minors and considers. Create and attach were not atomic. issueCreate followed by attachmentCreate are two calls with no transaction, so a failure between them left a ticket carrying none of the dedupe signals, and the next sweep would file a genuine duplicate. That is the one invariant this script exists to hold. Three changes: the created ticket is now logged BEFORE the attach so the identifier survives on the failing path, the attach is wrapped so the error names the orphan, and a third lookup searches for a ticket carrying this PR's "(community PR #N)" title suffix before creating. An interrupted run is now self healing: the next run finds the orphan and attaches, so lookup 1 sees it from then on. Multi target attaches also collect failures instead of throwing on the first, because stopping early left later targets unattached while lookup 1 short circuited past them on the retry. The carrier lookup is removed entirely. Scanning contributor text for PRD-#### is right for internal PRs and wrong across this boundary. Identifiers are not secret: AGENTS.md requires one in every PR title and Copybara mirrors those subjects into the public repo's history, so any contributor could both write attacker shaped text onto an internal ticket of their choosing and suppress their own PR's triage ticket by naming any resolvable one. The two remaining signals are GitHub authoritative and covered every case in the dry run. Attachment titles are now "PR #N" alone for the same reason: an attachment renders as first party automation output, so its text must not be contributor shaped. Minors: the relay token is scoped to permission-contents write, since POST /dispatches needs only that and the installation otherwise carries workflows write on the private monorepo; resolveSyncContext now warns when the label or project lookup misses, which previously degraded every ticket in a run while still reporting success; and scripts/linear-graphql gets the unit tests its own docstring justified, covering the swallowed non 2xx body and the 200 carrying a GraphQL errors array. Considers: every fetch now carries an AbortSignal timeout so one hung call cannot consume the sweep's budget, CI.md documents both workflows and LINEAR_API_KEY, and the create test asserts the ticket description rather than only its metadata. 910 root script tests pass, actionlint is clean, and the live dry run is unchanged: 1177 to PRD-7971, 623 to PRD-7349, 345 to PRD-7321, six untracked PRs to create. Those six also exercised the new orphan query against the real Linear API. * fix: identify Copybara sync PRs by head repo, not branch name A fork picks its own branch name, so keying the skip on the string copybara/sync let any contributor name their branch that and drop out of both the relay and the receiver's sweep filter, leaving their PR permanently invisible to Linear. That is evasion of the tracking control this workflow exists to be. "This repo talking to itself" is a property of the head repository, which a stranger cannot forge. The relay now also requires head.repo.full_name to equal the repository before skipping, and the sweep excludes copybara/sync only for same-repo PRs via isCrossRepository. Verified against the four cases: a real same-repo copybara/sync PR is still excluded, a fork branch named copybara/sync is now included, and both ordinary shapes are unaffected. Adds a workflow-shape test file. Every property that keeps the Linear key off a fork-reachable trigger is one line of YAML that a future edit could revert without touching a test: no checkout in the relay, no Linear credential in the relay, the scoped app token, the env-not- interpolation payload, both provenance checks, the disarmed default, the whole-string integer check, and the serialized concurrency group. The no-checkout guard was mutation tested by adding a checkout, which turns it red. The absence assertions strip whole-line comments first, because both workflows explain their own posture in prose and would otherwise match the comment saying why the thing is absent. 919 root script tests pass, actionlint clean, subtree md-audit clean. GitOrigin-RevId: 50dc33f876258793dd23d160914ce5f0267ab89f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On Windows the OK server uses the chokidar watcher fallback (
@parcel/watchercrashes withSTATUS_STACK_BUFFER_OVERRUNon long/non-ASCII paths, see #1207). chokidar (ReadDirectoryChangesW) emits more granular events than inotify, and each 50ms batch callshandleRawEvents, which readFile()s every changed file.When a batch is still mid-drain (slow disk, many files, active sync/backup tool) and the next 50ms timer fires, the new batch launches independently — overlapping async work piles up and the Node heap grows without bound on event storms.
Fix
Serialize batches by chaining them on an
inFlightpromise instartChokidarWatcher. Batches drain strictly one at a time; a batch that lands while another is draining waits its turn. The.catchremains load-bearing: it keeps the chain alive after an error so later batches don't deadlock.Regression test
file-watcher-chokidar-fallback.test.ts— new describe "chokidar backend — batch serialization":active++, hold 200ms,active--), tracksmaxActiveactive === 1(batch 1 draining)expect(maxActive).toBe(1)Verified:
maxActivereaches 2 — batches overlap)Tested with
forceBackend: 'chokidar'on Linux — no Windows needed to exercise the path.