chore: bump version to 1.9.0#11
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48c2c6738a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: wcpos/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughHidden review stack artifactWalkthroughAdds native and network printer adapters (Epson, Star), a network router, storage migration runner/primitives and verification, an OPFS-backed worker, regenerated Tailwind CSS and updated build assets, and bumps package version to 1.9.0. ChangesMulti-Vendor Printer Adapter Support
Storage Migration System and Verification
OPFS Worker, Web Assets, and Release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
Fix round 2 triage
Skipped threads
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
build/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.js (1)
1-1: ⚡ Quick winShould Star URL scheme be configurable or protocol-aware?
On Line 1,
StarWebPrntAdapteris always initialized withhttps://.... Could this block printers/environments that expose only HTTP WebPRNT, especially since Epson logic already adapts protocol/port? Would you consider deriving scheme similarly or allowing an explicit scheme in settings?
As per coding guidelines, "When author intent is unclear, ask a question rather than request a change."🤖 Prompt for 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. In `@build/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.js` at line 1, The Star branch in NetworkAdapter.constructor hardcodes https for StarWebPrntAdapter which can break printers only exposing http; update NetworkAdapter (constructor) to derive the URL scheme similarly to the Epson branch (use window.location.protocol when available) or accept an explicit scheme parameter in the constructor/options and use that to build the StarWebPRNT URL passed to StarWebPrntAdapter, ensuring the host (s) and port (o) logic remains unchanged and preserving existing behavior when port is 9100.
🤖 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
`@build/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.js`:
- Line 3: In StarWebPrntAdapter.printRaw the response handling only throws when
a <Status> tag exists and is not "Normal", so a 200 non-WebPRNT/malformed
response is treated as success; update the logic after reading response text
(variable c / const u) to treat a missing <Status> as an error (throw) and
include the response body (or status/text) in the error message for debugging,
i.e., if u is null OR u[1] !== 'Normal' then throw a descriptive Error
mentioning the parsed status or the raw response.
In
`@build/_expo/static/js/web/run-storage-migration-f05d8684c4a6f54aa51497bbc8719e4b.js`:
- Line 3: The migration currently tombstones (calls f.bulkWrite on the old
storage) regardless of whether the target write succeeded; in the
migrateCollection function (the async function o and its inner loop N) check the
result of o.storageInstance.bulkWrite(...) (the variable h assigned from that
call) for errors and abort/throw before constructing tombstone rows and calling
f.bulkWrite(...); specifically, validate that h.error is empty (or that
h.success indicates full success), log/propagate the error and return/throw so
the old-storage delete path (f.bulkWrite and subsequent cleanup) is not executed
on partial/failed target writes.
In
`@build/_expo/static/js/web/verify-migration-8b158f6ee6fa844a09031efae0961c1d.js`:
- Line 1: The deferred-cleanup guard currently compares S(M) (migratedAt)
against the module-scoped constant c (moduleLoadedAt), which is wrong for
lazy-loaded chunks; change verifyStorageMigration to accept a launch-scoped
marker (e.g., launchStartedAt or launchId) from the migration runner instead of
using c, replace references to c in the comparison (where S(M) is checked
against c) with that injected launch marker, and update the caller (the
migration runner / runStorageMigration) to pass the shared launch timestamp/ID
so deferred cleanup is consistently deferred to the next app launch.
In `@build/opfs.worker.js`:
- Line 1: The index maintenance currently finds rows by the encoded index value
only (see kn.appendWriteOperations using wn(this.rows,[u],Dn) and
kn.changeDocumentPosition/runChangelogOperation operating on this.rows), which
causes ambiguity for non-unique indexes; change the row representation to
include the primary key (or maintain a per-index metaIdMap from primaryKey->row
index) and use that to disambiguate lookups/updates: update
kn.getIndexableString/appendWriteOperations to store [indexableString,
primaryKey, meta1, meta2], update
kn.initRead/persistInMemoryRows/runChangelogOperation/changeDocumentPosition to
handle the new tuple shape and update metaIdMap usage, and ensure
serialization/deserialization (file read/write) and all binary-search helpers
(wn/mn/vn/etc. usage sites) compare on indexableString+primaryKey or consult
metaIdMap to find the exact row before replacing/deleting so non-unique values
cannot overwrite other documents.
---
Nitpick comments:
In
`@build/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.js`:
- Line 1: The Star branch in NetworkAdapter.constructor hardcodes https for
StarWebPrntAdapter which can break printers only exposing http; update
NetworkAdapter (constructor) to derive the URL scheme similarly to the Epson
branch (use window.location.protocol when available) or accept an explicit
scheme parameter in the constructor/options and use that to build the
StarWebPRNT URL passed to StarWebPrntAdapter, ensuring the host (s) and port (o)
logic remains unchanged and preserving existing behavior when port is 9100.
🪄 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: Repository: wcpos/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be629341-459b-4bc8-839b-5f39887d5c4d
⛔ Files ignored due to path filters (7)
build/assets/node_modules/expo-router/assets/arrow_down.017bc6ba3fc25503e5eb5e53826d48a8.pngis excluded by!**/node_modules/**,!**/*.pngbuild/assets/node_modules/expo-router/assets/error.d1ea1496f9057eb392d5bbf3732a61b7.pngis excluded by!**/node_modules/**,!**/*.pngbuild/assets/node_modules/expo-router/assets/file.19eeb73b9593a38f8e9f418337fc7d10.pngis excluded by!**/node_modules/**,!**/*.pngbuild/assets/node_modules/expo-router/assets/forward.d8b800c443b8972542883e0b9de2bdc6.pngis excluded by!**/node_modules/**,!**/*.pngbuild/assets/node_modules/expo-router/assets/pkg.ab19f4cbc543357183a20571f68380a3.pngis excluded by!**/node_modules/**,!**/*.pngbuild/assets/node_modules/expo-router/assets/sitemap.412dd9275b6b48ad28f5e3d81bb1f626.pngis excluded by!**/node_modules/**,!**/*.pngbuild/assets/node_modules/expo-router/assets/unmatched.20e71bdf79e3a97bf55fd9e164041578.pngis excluded by!**/node_modules/**,!**/*.png
📒 Files selected for processing (15)
build/_expo/static/css/global-68a8a3225c7216959703b116b3266f09.cssbuild/_expo/static/css/global-6e58b490378cf79363a0e49dc257ebf2.cssbuild/_expo/static/js/web/chart-9f67c2091c00e341c0ce22998297f418.jsbuild/_expo/static/js/web/entry-9de6c33a3b5446ee6241ab8139103522.jsbuild/_expo/static/js/web/epson-native-adapter-b4fa0da6d7d6a8538b0834408fc46bbd.jsbuild/_expo/static/js/web/index-345a124fb6802d68234ca030105e6ddd.jsbuild/_expo/static/js/web/index-6dc268e2df9d94a175c10c0bb4f93d53.jsbuild/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.jsbuild/_expo/static/js/web/run-storage-migration-f05d8684c4a6f54aa51497bbc8719e4b.jsbuild/_expo/static/js/web/star-native-adapter-447f65051c90e466214dcaa379cb64cf.jsbuild/_expo/static/js/web/verify-migration-8b158f6ee6fa844a09031efae0961c1d.jsbuild/index.htmlbuild/metadata.jsonbuild/opfs.worker.jspackage.json
💤 Files with no reviewable changes (1)
- build/_expo/static/css/global-6e58b490378cf79363a0e49dc257ebf2.css
d56fa97 to
e28e7af
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@build/opfs.worker.js`:
- Line 1: The remove() implementation in Yn.remove only truncates documents,
changelog and index files leaving changes.json, attachment files
(z-attachment-*) and the collection directory behind; update Yn.remove to (1)
await this.internals.statePromise and include internals.changesFileHandle and
each indexStates fileHandlePromise when collecting file handles, (2) for
attachments iterate the collection dirHandle entries and remove files whose name
startsWith "z-attachment-" (using dirHandle.removeEntry or the equivalent)
before truncating, and (3) once all file entries are removed, remove the
collection directory itself via dirHandle.removeEntry to fully delete the OPFS
store; use the existing sn(...) helper to open access handles and ensure the
broadcastChannel shutdown remains intact.
🪄 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: Repository: wcpos/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a189cbbb-d489-4246-bf74-0efcde5f2665
📒 Files selected for processing (6)
build/_expo/static/js/web/entry-9de6c33a3b5446ee6241ab8139103522.jsbuild/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.jsbuild/_expo/static/js/web/run-storage-migration-f05d8684c4a6f54aa51497bbc8719e4b.jsbuild/_expo/static/js/web/verify-migration-8b158f6ee6fa844a09031efae0961c1d.jsbuild/index.htmlbuild/opfs.worker.js
🚧 Files skipped from review as they are similar to previous changes (3)
- build/index.html
- build/_expo/static/js/web/verify-migration-8b158f6ee6fa844a09031efae0961c1d.js
- build/_expo/static/js/web/run-storage-migration-f05d8684c4a6f54aa51497bbc8719e4b.js
There was a problem hiding this comment.
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
`@build/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.js`:
- Line 3: The recovery hint in StarWebPrntAdapter (class t) loses custom ports
by using new URL(this.url).hostname; update the catch block that throws the
connection Error to reference the full origin instead (e.g. new
URL(this.url).origin) so the hint preserves protocol and port (for example
https://printer:9443) and directs users to the exact endpoint to accept the
certificate.
🪄 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: Repository: wcpos/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f57b20c-6b3b-41a9-97f0-18ab38f48cf9
📒 Files selected for processing (6)
build/_expo/static/js/web/entry-9de6c33a3b5446ee6241ab8139103522.jsbuild/_expo/static/js/web/network-adapter-300e404dc0027469f95ab174a2f92430.jsbuild/_expo/static/js/web/run-storage-migration-f05d8684c4a6f54aa51497bbc8719e4b.jsbuild/_expo/static/js/web/verify-migration-8b158f6ee6fa844a09031efae0961c1d.jsbuild/index.htmlbuild/opfs.worker.js
🚧 Files skipped from review as they are similar to previous changes (1)
- build/index.html
Fix round 3 triage
Skipped threads
|
Summary
1.9.0.1.9.0.Test plan
pnpm --filter @wcpos/web-bundle run buildpnpm --filter @wcpos/web-bundle run lintcurrently fails because ESLint reports thescriptsglob is ignored; this appears to be a pre-existing tooling/config issue.Companion PRs
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores