fix: externalize update notifier worker by caezium · Pull Request #81 · photon-hq/cli

Henry Zhang (caezium) · 2026-07-19T09:37:56Z

Summary

keep update-notifier external when building the published JavaScript CLI
preserve the package worker at node_modules/update-notifier/check.js
retain the existing TTY, opt-out, 24-hour interval, and deferred notification behavior
add focused coverage for package delegation and notifier opt-outs

Why

update-notifier launches its check.js file as a detached process. Bundling the dependency into dist/photon.js removed that physical file, so the background check silently failed. The published CLI now resolves the existing dependency at runtime, where the worker remains available.

Upstream version

No change (@photon-ai/dashboard-api@1.6.12)

Routes added/removed/changed

(none)

Snapshot changes

(none)

New runtime dependencies

(none) — update-notifier was already a runtime dependency

Testing

bun run check — 48 tests passed
packed and installed the generated tarball with Bun
verified the installed package contains node_modules/update-notifier/check.js
tested against a fake npm registry in a real TTY
- first invocation populated the upstream ConfigStore cache
- second invocation displayed the standard update box
confirmed dist/photon.js retains an external runtime import for update-notifier

Scope

This fixes package-manager installations of the JavaScript CLI. Bun single-file standalone binaries do not ship an external node_modules worker; standalone updating is handled separately by the forthcoming photon update command.

Checklist

API type changes come from a @photon-ai/dashboard-api version bump, not hand edits (no API type changes)
No new runtime deps (or justified above)
All snapshot diffs explained above
bun run check passes locally

Linear: ENG-2014

Summary by CodeRabbit

Bug Fixes
- Build output now excludes the update-notifier package, improving update-notification behavior in bundled distributions.
- Update checks better respect interactive vs. non-interactive runs and the PHOTON_NO_UPDATE_NOTIFIER opt-out, while still issuing deferred global notifications.
Tests
- Added unit tests covering notifier creation in interactive mode, suppression in non-interactive mode, and skipping when PHOTON_NO_UPDATE_NOTIFIER is set.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-07-19T09:38:12Z

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a7e41dda-dc15-47c1-a77d-007857c0e27a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The update notifier now supports injected dependencies for testing, remains external in the Bun build, and has unit coverage for creation, interactive-mode gating, environment-variable disabling, and deferred global notifications.

Changes

Update notifier integration

Layer / File(s)	Summary
Notifier dependency injection and build wiring `src/lib/update-check.ts`, `package.json`	`startUpdateNotifier` accepts optional interactive and notifier-factory overrides, while the build excludes `update-notifier` from the generated bundle.
Notifier behavior tests `tests/unit/update-check.test.ts`	Tests verify notifier options, deferred global notification, non-interactive gating, environment-variable gating, and environment restoration.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

photon-hq/cli#80: Modifies the overlapping startUpdateNotifier logic and dependency-injection behavior.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: externalizing update-notifier during the build.
Description check	✅ Passed	The description includes all required template sections and a completed checklist with testing and scope details.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch henry/eng-2014-cli-auto-update-notifier-never-works

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/update-check.test.ts`:
- Around line 30-34: Update the factoryOptions assertion in the update-check
test to replace the hardcoded pkg.version value with expect.any(String), while
keeping the package name and all other expected options unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eb7b85a9-1734-4645-86ac-548f29e87783

📥 Commits

Reviewing files that changed from the base of the PR and between 8910fd8 and c3a7353.

📒 Files selected for processing (3)

package.json
src/lib/update-check.ts
tests/unit/update-check.test.ts

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Henry Zhang (caezium) · 2026-07-19T09:50:35Z

CodeRabbit (@coderabbitai) review

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/update-check.test.ts`:
- Around line 6-14: Update the test setup around previousOptOut and afterEach to
add per-test initialization that clears PHOTON_NO_UPDATE_NOTIFIER before each
test runs, while retaining restoration of the captured original baseline in
afterEach. Ensure startUpdateNotifier tests execute without inheriting the
developer or CI environment value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d324ecbf-a4b0-4577-a4d1-422c7a61c952

📥 Commits

Reviewing files that changed from the base of the PR and between c3a7353 and 197ab7d.

📒 Files selected for processing (3)

package.json
src/lib/update-check.ts
tests/unit/update-check.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

package.json
src/lib/update-check.ts

coderabbitai · 2026-07-19T09:50:51Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Henry Zhang (caezium) · 2026-07-19T09:52:00Z

CodeRabbit (@coderabbitai) review

coderabbitai · 2026-07-19T09:52:09Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…kflow (#93) buildspace commit 2535977 (Jul 17) added a github-packages-publish job with 'permissions: packages: write' to typescript-service-release.yaml. GitHub validates called-workflow job permissions against the caller's grant when the workflow graph is compiled — even for jobs skipped via 'if:' — so every push to main since then died with startup_failure (runs for #79, #81, #82) and no release was published. Co-authored-by: Cursor Agent <cursoragent@cursor.com>

Copilot AI review requested due to automatic review settings July 19, 2026 09:37

Copilot AI reviewed Jul 19, 2026

coderabbitai Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread tests/unit/update-check.test.ts

Copilot AI review requested due to automatic review settings July 19, 2026 09:48

Henry Zhang (caezium) force-pushed the henry/eng-2014-cli-auto-update-notifier-never-works branch from c3a7353 to 197ab7d Compare July 19, 2026 09:48

Copilot AI reviewed Jul 19, 2026

Henry Zhang (caezium) changed the title ~~fix: fix update notifier by externalizing update notifier worker~~ fix: externalize update notifier worker Jul 19, 2026

coderabbitai Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread tests/unit/update-check.test.ts Outdated

fix: externalize update notifier worker

5c35c15

Copilot AI review requested due to automatic review settings July 19, 2026 09:51

Henry Zhang (caezium) force-pushed the henry/eng-2014-cli-auto-update-notifier-never-works branch from 197ab7d to 5c35c15 Compare July 19, 2026 09:51

Copilot AI reviewed Jul 19, 2026

coderabbitai Bot mentioned this pull request Jul 19, 2026

feat: add photon update command to update the cli #82

Merged

4 tasks

Ryan Zhu (underthestars-zhy) approved these changes Jul 19, 2026

View reviewed changes

Ryan Zhu (underthestars-zhy) merged commit b1dd848 into main Jul 19, 2026
12 checks passed

Henry Zhang (caezium) deleted the henry/eng-2014-cli-auto-update-notifier-never-works branch July 19, 2026 22:44

Ryan Zhu (underthestars-zhy) mentioned this pull request Jul 23, 2026

ci(release): grant packages: write to fix Release startup_failure #93

Merged

4 tasks

Uh oh!

Conversation

Henry Zhang (caezium) commented Jul 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Upstream version

Routes added/removed/changed

Snapshot changes

New runtime dependencies

Testing

Scope

Checklist

Summary by CodeRabbit

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Henry Zhang (caezium) commented Jul 19, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Henry Zhang (caezium) commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Henry Zhang (caezium) commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading