Skip to content

fix(core): reconcile compatibility watches instead of rebuilding them - #54450

Closed
argszero wants to merge 1 commit into
anomalyco:devfrom
argszero:compat-watch-reconcile
Closed

argszero wants to merge 1 commit into
anomalyco:devfrom
argszero:compat-watch-reconcile

Conversation

@argszero

@argszero argszero commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #54114

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

opencode.config.compatibility rebuilt its watches every time it saw a change. refresh() opened with FiberMap.clear(watches) and then re-subscribed each compatibility directory, which defeats the onlyIfMissing: true on the FiberMap.run inside watch(): an event reaching the 100 ms debounce tore down every watch and attached them again, and attaching a subscription publishes again, so the debounce was handed a fresh event to consume. On a project with ./.agents/skills the service settled into that cycle — watcher subscribe for that path grew without bound while no file changed, pinning the service at ~37% CPU and filling the log.

The change reconciles instead of clearing: build the plan of directories that should be watched, FiberMap.remove the keys that dropped out of it, and subscribe only the keys that are not already present. A directory whose resolved path has not changed keeps its watch, so there is no re-subscription to publish an event and the cycle has nothing left to sustain it.

I took this shape from the two places in the codebase that already work this way — Config.reconcileWatches in packages/core/src/config.ts, and ConfigSkillPlugin.watchDirectory. refresh() keeps its lock.withPermit, so the plan is not raced.

One honest limit: I could not reproduce the reporter's trigger here — it needs Linux inotify and, per the triage thread, the data directory coinciding with the config directory. What I verified is the mechanism the report named, watches being cleared and rebuilt per event, and that it is gone.

How did you verify your code works?

Added keeps a compatibility watch instead of rebuilding it on every event to packages/core/test/config/skill.test.ts. It starts the plugin under the existing Watcher.Test layer, which records every subscribe call, asserts the compatibility directory is watched once, emits a single update beneath that directory, and asserts the directory is still subscribed exactly once. That last assertion fails on the unmodified source — the re-subscription this report is about is precisely what it catches — and passes with the change.

  • bun test test/config/skill.test.ts → 11 pass / 0 fail
  • bun test test/config → 209 pass / 19 fail, and the same 19 fail on the unmodified tree at e31136bdbd (they are config-discovery tests that pick up a real document in this environment), so this adds no failure
  • tsgo -b tsconfig.json tsconfig.tests.json in packages/core → exit 0
  • oxlint on both changed files → 0 warnings / 0 errors

Screenshots / recordings

If this is a UI change, please include a screenshot or recording.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Related: #53393

@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@opencode-agent opencode-agent Bot left a comment

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.

This fixes the idle re-subscribe loop. I ran the #54114 reproduction from source with the data directory inside the config directory. On the base, <project>/.agents/skills was torn down and re-subscribed 25 times in 30 s of idling. With this change it subscribes once. Idle CPU drops to the same level as a control run with separate data and config directories. The new test fails on the base source and passes here. test/config (228 pass) and the packages/core typecheck are clean.

The public API is unchanged.

The new key-diffing mirrors Config.reconcileWatches, and the keys match the ones watch() uses, so I have no blocking concerns. Note that config.changes() still runs a full rescan and skill reload on every event under the config directory, with no debounce. This change removes the watcher teardown that kept the loop going, not the extra reloads, so #53393 is only partly addressed. The open #53764 (debounce, and skip unchanged compatibility roots) covers that part and complements this change.

Related: #53393

@argszero
argszero force-pushed the compat-watch-reconcile branch from f8186b4 to a0d19f7 Compare October 18, 2026 05:39
@argszero

Copy link
Copy Markdown
Contributor Author

Closing this to stay within GitHub's 3-open-pull-request limit for contributors without write access, which currently blocks every new pull request here. The branch remains on my fork, so I can reopen it on request.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: service skill/compatibility watcher self-loops on .agents/skills, ~37% idle CPU

1 participant