feat: support drawer registry packages - #230
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR extends the CLI and registry model to support drawer packages alongside buttons. It adds drawer-aware creation checks, version/schema fields, source and install/publish handling by package kind, kind-aware update/status reporting, documentation updates, and tests covering drawer install, publish, and update flows. ChangesDrawer package support
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as buttons add/install/publish
participant Store as store.InstallManifest/PublishToRegistry
participant Source as LocalSource/HTTPSource
participant FS as local package files
CLI->>Store: request package operation
Store->>Source: Resolve/Fetch bundle
Source-->>Store: Bundle with Kind
alt drawer bundle
Store->>Store: installDrawerPackage / loadLocalDrawerBundle
Store->>FS: write drawer.json and lock data
Store->>Store: walk drawer steps for dependencies
else button bundle
Store->>Store: installButtonPackage / loadLocalButtonBundle
Store->>FS: write button.json and lock data
end
Store-->>CLI: status / result output
sequenceDiagram
participant Updater as CheckContent/Apply
participant Lock as buttons-lock.json
participant Registry as store.Resolve/Fetch
participant FS as installed package files
Updater->>Lock: merge manifest and lock dependencies
loop each dependency
Updater->>Registry: resolve latest package
Registry-->>Updater: ref.Kind and version
Updater->>FS: inspect local files for modifications
alt local edits present
Updater-->>Updater: mark skipped
else newer version available
Updater->>Registry: fetch updated bundle
Updater->>FS: write updated content
Updater->>Lock: update kind/version/hash
end
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
internal/store/http_source.go (1)
174-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError messages still say "button" for package-kind-agnostic failures.
Lines 174, 177, 188, and 193 hardcode
"button %q..."in error messages, but at this point inFetchthe kind (button vs. drawer) isn't yet known — these errors can now fire for drawer downloads too, producing misleading messages likebutton "my-drawer": download: .... Line 159 already switched to the generic "package" wording; these should follow suit.💬 Proposed fix
- return nil, fmt.Errorf("button %q: download: %w", name, err) + return nil, fmt.Errorf("package %q: download: %w", name, err) } if int64(len(tarball)) > maxArtifactBytes { - return nil, fmt.Errorf("button %q: artifact exceeds %d bytes", name, int64(maxArtifactBytes)) + return nil, fmt.Errorf("package %q: artifact exceeds %d bytes", name, int64(maxArtifactBytes)) } ... if want != "" && got != want { - return nil, fmt.Errorf("button %q@%s: content hash mismatch (registry %s, got %s)", name, version, want, got) + return nil, fmt.Errorf("package %q@%s: content hash mismatch (registry %s, got %s)", name, version, want, got) } files, err := untarGz(tarball) if err != nil { - return nil, fmt.Errorf("button %q: %w", name, err) + return nil, fmt.Errorf("package %q: %w", name, err) }🤖 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 `@internal/store/http_source.go` around lines 174 - 193, Package-kind-agnostic failures in Fetch still hardcode “button” in error text, which can mislabel drawer downloads. Update the error formatting in the tarball download, size check, hash mismatch, and untar paths inside Fetch to use the generic “package” wording already used earlier, keeping the same contextual fields like name and version while removing button-specific terminology from these shared failure messages.internal/store/source.go (2)
107-127: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSilent precedence when both
button.jsonanddrawer.jsonexist.If a package directory contains both files,
button.jsonsilently wins anddrawer.jsonis never even opened. Given this PR elsewhere adds explicit button/drawer name-collision guards, consider making this same ambiguity fail loudly here too (at least for defensive parity), though this is a dev-only local source and the scenario requires a malformed on-disk package.🤖 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 `@internal/store/source.go` around lines 107 - 127, The Source refs scan in internal/store/source.go currently gives silent precedence to button.json over drawer.json in the source listing logic. Update the reference collection path in the Source/refs enumeration so it detects when both files exist for the same package directory and fails loudly or otherwise rejects the ambiguous package instead of silently picking button.json; use the existing os.ReadFile checks around button.json and drawer.json and the refs append logic as the place to enforce this defensive parity.
156-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: directory-not-found error still says "button" for drawer package lookups.
The new branches correctly generalize the "no button.json or drawer.json" message (Line 178), but the earlier
os.ReadDir(dir)failure message just above ("button %q not found in source") wasn't updated and will misleadingly say "button" even when resolving a drawer package.✏️ Suggested wording fix (outside the shown diff range)
- return nil, fmt.Errorf("button %q not found in source: %w", name, err) + return nil, fmt.Errorf("package %q not found in source: %w", name, err)🤖 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 `@internal/store/source.go` around lines 156 - 178, The source lookup error message in the package resolver still hardcodes “button” when the directory read fails, which is misleading for drawer lookups. Update the `LocalSource`/bundle resolution path around the `os.ReadDir(dir)` failure to use the actual package kind or a neutral term derived from the lookup context, matching the existing `button.json` and `drawer.json` branches in this function. Keep the rest of the validation logic unchanged, including the version checks and the final “no button.json or drawer.json” fallback.internal/updater/content.go (1)
145-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated Kind/Name fallback logic.
reportsWithSourceError(Lines 150-157) andcheckOneDependency(Lines 183-188) both independently implement "default Kind tobutton" and "default Name tolocalNameFromPackage". Consider extracting a small shared helper to avoid drift if a third kind is ever added.♻️ Proposed refactor
+func defaultKindAndName(entry manifest.LockEntry, pkgName string) (kind, name string) { + kind = entry.Kind + if kind == "" { + kind = "button" + } + name = entry.InstalledName + if name == "" { + name = localNameFromPackage(pkgName) + } + return kind, name +}Then use it in both
reportsWithSourceErrorandcheckOneDependency.Also applies to: 172-188
🤖 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 `@internal/updater/content.go` around lines 145 - 170, The fallback logic for Kind and Name is duplicated between reportsWithSourceError and checkOneDependency, which can drift if defaults change. Extract a small shared helper that resolves the effective Kind (defaulting to "button") and Name (defaulting to localNameFromPackage) from the dependency/lock entry, then call that helper from both reportsWithSourceError and checkOneDependency so both paths stay consistent.
🤖 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 `@cmd/status.go`:
- Around line 79-82: The summary message in status reporting is still hardcoded
as “Buttons: up to date” even though the item labels now use
packageKindLabel(b.Kind) and can represent drawers as well. Update the final
availability check in status output to use the same generalized wording as
cmd/update.go’s summary so it stays accurate regardless of package kind, and
keep the change aligned with the status logic around packageKindLabel and the
available counter.
In `@internal/store/install.go`:
- Around line 296-303: The writeDrawerBundle loop currently writes every bundle
file with the same mode, so code/helper artifacts lose executable permissions.
Update the file-install logic in writeDrawerBundle to choose permissions based
on the bundle file type: keep spec/history JSON files at 0600, but write
non-JSON code/helper files at 0700. Use the existing rel/data handling in the
bundle.Files iteration to distinguish JSON from executable artifacts before
calling os.WriteFile.
In `@internal/store/publish_test.go`:
- Around line 58-68: The drawer fixture in the publish test is using overly
permissive permissions for private data. Update the helper in publish_test.go
that creates the test drawer contents so directories created with os.MkdirAll
and the pressed subdirectory use 0700, and the JSON fixtures written with
os.WriteFile for drawer.json and the pressed/run1.json history/spec files use
0600; keep the AGENTS.md fixture as-is if it is not part of the private
JSON/data set. Use the existing setup block around the drawer fixture to make
these permission changes consistently.
In `@internal/store/publish.go`:
- Around line 156-164: The artifact publishing loop in publishDrawer currently
reads every non-directory entry via os.ReadFile, which will follow symlinks and
can escape the drawer root. Update the entries handling in publishDrawer to
mirror LocalSource behavior by checking each os.DirEntry for symlinks before
reading, and skip any symlinked entry while still keeping the existing directory
skip and file collection logic intact.
- Line 105: Remove the trailing period from the fmt.Errorf message in the
ambiguous package error path so it matches ST1005 style; update the error string
in the publish flow where slug ambiguity is reported to say the package is
ambiguous and to rename one, but without ending punctuation.
---
Nitpick comments:
In `@internal/store/http_source.go`:
- Around line 174-193: Package-kind-agnostic failures in Fetch still hardcode
“button” in error text, which can mislabel drawer downloads. Update the error
formatting in the tarball download, size check, hash mismatch, and untar paths
inside Fetch to use the generic “package” wording already used earlier, keeping
the same contextual fields like name and version while removing button-specific
terminology from these shared failure messages.
In `@internal/store/source.go`:
- Around line 107-127: The Source refs scan in internal/store/source.go
currently gives silent precedence to button.json over drawer.json in the source
listing logic. Update the reference collection path in the Source/refs
enumeration so it detects when both files exist for the same package directory
and fails loudly or otherwise rejects the ambiguous package instead of silently
picking button.json; use the existing os.ReadFile checks around button.json and
drawer.json and the refs append logic as the place to enforce this defensive
parity.
- Around line 156-178: The source lookup error message in the package resolver
still hardcodes “button” when the directory read fails, which is misleading for
drawer lookups. Update the `LocalSource`/bundle resolution path around the
`os.ReadDir(dir)` failure to use the actual package kind or a neutral term
derived from the lookup context, matching the existing `button.json` and
`drawer.json` branches in this function. Keep the rest of the validation logic
unchanged, including the version checks and the final “no button.json or
drawer.json” fallback.
In `@internal/updater/content.go`:
- Around line 145-170: The fallback logic for Kind and Name is duplicated
between reportsWithSourceError and checkOneDependency, which can drift if
defaults change. Extract a small shared helper that resolves the effective Kind
(defaulting to "button") and Name (defaulting to localNameFromPackage) from the
dependency/lock entry, then call that helper from both reportsWithSourceError
and checkOneDependency so both paths stay consistent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52fe94c1-96ed-4d61-bd37-91802b9ae6f7
📒 Files selected for processing (34)
cmd/add.gocmd/install.gocmd/passive_update.gocmd/passive_update_test.gocmd/publish.gocmd/status.gocmd/update.godocs/cli/buttons.mddocs/cli/buttons_add.mddocs/cli/buttons_install.mddocs/cli/buttons_publish.mddocs/cli/buttons_status.mddocs/cli/buttons_update.mddocs/concepts/drawer-json.mdxdocs/concepts/folder-structure.mdxdocs/concepts/registry.mdxdocs/schemas/drawer.schema.jsoninternal/button/service.gointernal/button/service_test.gointernal/drawer/entity.gointernal/drawer/schema_embedded.jsoninternal/drawer/service.gointernal/drawer/service_test.gointernal/store/http_publish.gointernal/store/http_source.gointernal/store/install.gointernal/store/publish.gointernal/store/publish_test.gointernal/store/source.gointernal/store/store_test.gointernal/updater/content.gointernal/updater/content_test.gointernal/updater/types.gotest/integration/ota_update_test.go
1 participant
Summary
Validation
Notes
Summary by CodeRabbit