Skip to content

fix(auto-update-checker): skip the check when auto_update is false - #870

Open
Oscar-Williams wants to merge 3 commits into
cortexkit:masterfrom
Oscar-Williams:fix/update-checker-registry-and-disable
Open

Oscar-Williams wants to merge 3 commits into
cortexkit:masterfrom
Oscar-Williams:fix/update-checker-registry-and-disable

Conversation

@Oscar-Williams

@Oscar-Williams Oscar-Williams commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Approved issue: #854

What does this PR do?

Narrow this PR to the disable path requested in review: OpenCode 1 now passes enabled: false to its background checker when auto_update: false, so no npm update request is scheduled. Unset/true settings keep the existing enabled behavior. The hook's separate notification-only mode remains available to direct callers.

All registry-resolution changes and their tests have been removed from this PR. Registry resolution is left to #855; this PR does not read project or user .npmrc files.

How to test

  • bun test packages/plugin/src/hooks/auto-update-checker/ — 44 passed, including a regression asserting no latest-version request with auto_update: false, plus unset/true defaults. The regression failed semantically with the old option mapping (one request instead of zero), then passed with the fix.
  • bun test packages/plugin/src/v2/hooks/update-check.test.ts — 4 passed.
  • bun run --cwd packages/plugin typecheck — passed.
  • bun run --cwd packages/plugin lint — passed (existing warnings only).
  • Changed-file Biome formatting and git diff --check — passed.

Tests and checks ran locally on Linux with Bun 1.4.2. Full monorepo tests were not run.

Harness coverage (required)

  • OpenCode 1: covered — the real plugin config entry maps auto_update into the checker options; regression checks that disabled updates make no npm request.
  • OpenCode 2: covered — existing src/v2/server.ts already skips startUpdateChecks when the same config key is false; reviewed and its update-check tests pass, no code change needed.
  • Pi: not applicable — the Pi plugin has no npm auto-update-checker surface.
  • OMP: not applicable — the shared Pi/OMP implementation has no npm auto-update-checker surface.
  • Rust module: not applicable — no npm auto-update-checker surface exists there.

RetriggerView in GreptileConfidence Score: 5/5

This PR appears safe to merge.

Summary

OpenCode 1 now skips its background update checker when auto_update is false.

  • OpenCode 1 skips its update check when auto-update is off.

Reviews (1) · Last reviewed commit: "refactor(auto-update-checker): keep the ..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Harness coverage advisory: these conventional twins were not touched:

  • packages/plugin/src/index.ts → packages/pi-plugin/src/index.ts

Cover every shipped harness or explain why the surface does not exist there. See CONTRIBUTING.md. This is advisory only; no draft state is changed.

@magic-alfonso

magic-alfonso Bot commented Nov 6, 2026

Copy link
Copy Markdown

Thanks for this. This PR and #855 overlap on the registry part, so please narrow this one to the disable path.

Once it's down to the disable path, mark it ready for review.

@magic-alfonso

magic-alfonso Bot commented Nov 8, 2026

Copy link
Copy Markdown

Hi @Oscar-Williams, checking in on this one. Do you still plan to narrow it to the auto_update: false path, so update checks are skipped when updates are turned off? Registry resolution is covered by #855, so that part isn't needed here. If you'd rather not continue, that's fine too. Just let us know.

…te is false

Drop the registry resolution (covered by cortexkit#855) and keep only the disable
path: auto_update: false now disables the checker so no npm request is
made. Adds a regression test asserting no registry lookup when disabled.
@Oscar-Williams Oscar-Williams changed the title fix(auto-update-checker): resolve the configured npm registry and skip the check when disabled fix(auto-update-checker): skip the check when auto_update is false Nov 9, 2026
@Oscar-Williams
Oscar-Williams marked this pull request as ready for review November 9, 2026 14:39
@Oscar-Williams

Copy link
Copy Markdown
Contributor Author

Follow-up on the narrowing question: this PR is now limited to the auto_update: false disable path (OpenCode 1 maps enabled: false so no npm update request is scheduled). Registry-resolution work was removed and left to #855. A regression asserting zero requests when disabled is included and was verified locally.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 3 files

View guided diff | Re-trigger cubic

@magic-alfonso

magic-alfonso Bot commented Nov 9, 2026

Copy link
Copy Markdown

Thanks for narrowing this. The change looks right, and the bot review is clean.

One CI check fails on the trial merge: packages/e2e-tests/tests/opencode2/pins.test.ts pins the sha256 of packages/plugin/src/index.ts, so any intended edit to that file needs the pin updated. With your change the new value is:

68ef7e0fb67c5d90270646b513105a1fc116e9646871c9bd9ca92f1974ae5555

Please replace bbe50e7bcd9bd7d8ea12dfbbcc1ef6d08e7be045d45e19c4cb2cd2cfcc031211 with it in that test, and push. You can confirm it locally with bun test packages/e2e-tests/tests/opencode2/pins.test.ts. We'll merge once CI passes.

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.

1 participant