Hide packages from search by default - #746
Merged
Merged
Conversation
Add a user-scoped visibility control so saved packages can be removed from discovery without disabling their runtime or management surfaces. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughChangesSaved package hiding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant setPackageHiddenCapability
participant updateSavedPackage
participant APP_DB
Caller->>setPackageHiddenCapability: package_id, hidden
setPackageHiddenCapability->>updateSavedPackage: update for authenticated user
updateSavedPackage->>APP_DB: write hidden state
APP_DB-->>updateSavedPackage: changed row count
updateSavedPackage-->>setPackageHiddenCapability: update result
setPackageHiddenCapability-->>Caller: ok, package_id, hidden
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
🔎 Preview deployed: https://kody-pr-746.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/worker/src/package-registry/repo.ts (1)
145-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winColumn list duplicated across five queries.
Each of
getSavedPackageById,getSavedPackageByKodyId,getSavedPackageByName,listSavedPackagesByUserId, andlistSavedPackagesPagerepeats the identicalSELECT id, user_id, name, ..., hidden, created_at, updated_atcolumn list. Extracting a shared constant reduces the risk of a future column addition/rename being missed in one of the five call sites.♻️ Suggested refactor
+const SAVED_PACKAGE_COLUMNS = `id, user_id, name, kody_id, description, tags_json, search_text, + source_id, has_app, hidden, created_at, updated_at` + export async function getSavedPackageById(...) { const row = await db .prepare( - `SELECT id, user_id, name, kody_id, description, tags_json, search_text, - source_id, has_app, hidden, created_at, updated_at + `SELECT ${SAVED_PACKAGE_COLUMNS} FROM saved_packages WHERE id = ? AND user_id = ?`, )Apply similarly to the other four queries.
Also applies to: 164-165, 183-184, 201-202, 224-225
🤖 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 `@packages/worker/src/package-registry/repo.ts` around lines 145 - 146, Extract the duplicated package column projection into one shared constant in repo.ts, then reuse it in getSavedPackageById, getSavedPackageByKodyId, getSavedPackageByName, listSavedPackagesByUserId, and listSavedPackagesPage. Preserve the existing column order and query behavior while replacing each repeated SELECT list.packages/worker/src/mcp/capabilities/meta/search-include-hidden.node.test.ts (1)
89-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSplit into two separate
test()cases.Both the default (
includeHiddenPackages: false) and opt-in (include_hidden: true) scenarios are asserted inside onetest()with manualresetMocks()calls in between. Splitting into two named tests would improve isolation and make failures easier to diagnose.♻️ Suggested split
-test('meta search remaps include_hidden through to package rows and search-scope retrievers', async () => { - resetMocks() - const ctx = createCtx() - - await searchCapability.handler({ query: 'notes' }, ctx) - - expect(mockModule.loadSearchRowsAndRegistry).toHaveBeenCalledWith( - expect.objectContaining({ - includeHiddenPackages: false, - }), - ) - expect(mockModule.runPackageRetrievers).toHaveBeenCalledWith( - expect.objectContaining({ - scope: 'search', - includeHiddenPackages: false, - }), - ) - - resetMocks() - await searchCapability.handler({ query: 'notes', include_hidden: true }, ctx) - - expect(mockModule.loadSearchRowsAndRegistry).toHaveBeenCalledWith( - expect.objectContaining({ - includeHiddenPackages: true, - }), - ) - expect(mockModule.runPackageRetrievers).toHaveBeenCalledWith( - expect.objectContaining({ - scope: 'search', - includeHiddenPackages: true, - }), - ) -}) +test('meta search defaults include_hidden to false', async () => { + resetMocks() + const ctx = createCtx() + await searchCapability.handler({ query: 'notes' }, ctx) + expect(mockModule.loadSearchRowsAndRegistry).toHaveBeenCalledWith( + expect.objectContaining({ includeHiddenPackages: false }), + ) + expect(mockModule.runPackageRetrievers).toHaveBeenCalledWith( + expect.objectContaining({ scope: 'search', includeHiddenPackages: false }), + ) +}) + +test('meta search remaps include_hidden: true through to package rows and search-scope retrievers', async () => { + resetMocks() + const ctx = createCtx() + await searchCapability.handler({ query: 'notes', include_hidden: true }, ctx) + expect(mockModule.loadSearchRowsAndRegistry).toHaveBeenCalledWith( + expect.objectContaining({ includeHiddenPackages: true }), + ) + expect(mockModule.runPackageRetrievers).toHaveBeenCalledWith( + expect.objectContaining({ scope: 'search', includeHiddenPackages: true }), + ) +})🤖 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 `@packages/worker/src/mcp/capabilities/meta/search-include-hidden.node.test.ts` around lines 89 - 121, Split the combined test for searchCapability.handler into two independently named test cases: one covering the default include_hidden behavior and one covering the explicit true opt-in. Move each scenario’s setup and assertions into its respective test, removing the manual resetMocks() between scenarios while preserving the existing expectations.packages/worker/src/mcp/tools/search-format.ts (1)
948-962: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMarkdown summary omits hidden status.
hasAppgets a- Has app: yes/noline in the package markdown summary (line 960), but there's no equivalent line forhidden, even though the structured output for this same detail (line 1054) now includes it. Since a hidden package is still directly reachable via entity lookup, surfacing this in the human-readable text too would avoid the markdown and structured outputs diverging.♻️ Suggested addition
`- Has app: ${detail.record.hasApp ? 'yes' : 'no'}`, + `- Hidden from search: ${detail.record.hidden ? 'yes' : 'no'}`, ...(detail.hostedUrl ? [`- Hosted URL: \`${detail.hostedUrl}\``] : []),🤖 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 `@packages/worker/src/mcp/tools/search-format.ts` around lines 948 - 962, Update the package markdown summary construction in the lines array to include a Hidden status line derived from detail.record.hidden, matching the yes/no formatting used by the existing Has app line and the structured detail output.packages/worker/src/mcp/capabilities/meta/search.ts (1)
110-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
include_hiddenbreaks casing convention used by sibling fields and the public tool.
conversationIdandmemoryContextin this sameinputSchemaare camelCase, but the new field isinclude_hidden(snake_case). The publicsearchtool (tools/search.ts) exposes the equivalent flag asincludeHiddenPackages— and this capability's description explicitly states it mirrors "the same discovery surface as the public MCP search tool." Mixed casing on the identical concept increases the chance of integration mistakes for callers switching between the two surfaces.✏️ Suggested rename for consistency
- include_hidden: z + includeHiddenPackages: z .boolean() .optional() .describe( 'Include hidden packages in search results (hidden packages are excluded by default).', ),and update the handler's arg type/usage accordingly (
args.includeHiddenPackages).Also applies to: 138-138
🤖 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 `@packages/worker/src/mcp/capabilities/meta/search.ts` around lines 110 - 129, Rename the inputSchema field include_hidden to includeHiddenPackages in the capability search definition, matching the public search tool and sibling camelCase fields. Update the handler’s argument type and all usages to read args.includeHiddenPackages while preserving the existing optional behavior and description.
🤖 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.
Nitpick comments:
In
`@packages/worker/src/mcp/capabilities/meta/search-include-hidden.node.test.ts`:
- Around line 89-121: Split the combined test for searchCapability.handler into
two independently named test cases: one covering the default include_hidden
behavior and one covering the explicit true opt-in. Move each scenario’s setup
and assertions into its respective test, removing the manual resetMocks()
between scenarios while preserving the existing expectations.
In `@packages/worker/src/mcp/capabilities/meta/search.ts`:
- Around line 110-129: Rename the inputSchema field include_hidden to
includeHiddenPackages in the capability search definition, matching the public
search tool and sibling camelCase fields. Update the handler’s argument type and
all usages to read args.includeHiddenPackages while preserving the existing
optional behavior and description.
In `@packages/worker/src/mcp/tools/search-format.ts`:
- Around line 948-962: Update the package markdown summary construction in the
lines array to include a Hidden status line derived from detail.record.hidden,
matching the yes/no formatting used by the existing Has app line and the
structured detail output.
In `@packages/worker/src/package-registry/repo.ts`:
- Around line 145-146: Extract the duplicated package column projection into one
shared constant in repo.ts, then reuse it in getSavedPackageById,
getSavedPackageByKodyId, getSavedPackageByName, listSavedPackagesByUserId, and
listSavedPackagesPage. Preserve the existing column order and query behavior
while replacing each repeated SELECT list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cd726bbb-c181-434d-9e54-7a53c3ad1551
📒 Files selected for processing (53)
docs/contributing/architecture/data-storage.mddocs/contributing/packages-and-manifests.mddocs/use/packages.mddocs/use/search.mdpackages/worker/migrations/0058-saved-packages-hidden.sqlpackages/worker/src/app/handlers/account-package-invocation-tokens.node.test.tspackages/worker/src/app/handlers/account-secrets.node.test.tspackages/worker/src/app/handlers/package-app.node.test.tspackages/worker/src/community/community-flow-test-schema.tspackages/worker/src/community/community-service.node.test.tspackages/worker/src/email/inbound.workers.test.tspackages/worker/src/email/system-email-subscriptions.workers.test.tspackages/worker/src/mcp/capabilities/meta/search-include-hidden.node.test.tspackages/worker/src/mcp/capabilities/meta/search.tspackages/worker/src/mcp/capabilities/openapi-provider/operation-request.node.test.tspackages/worker/src/mcp/capabilities/packages/create-stub-package.node.test.tspackages/worker/src/mcp/capabilities/packages/create-stub-package.tspackages/worker/src/mcp/capabilities/packages/domain.tspackages/worker/src/mcp/capabilities/packages/get-package.node.test.tspackages/worker/src/mcp/capabilities/packages/get-package.tspackages/worker/src/mcp/capabilities/packages/list-package-subscriptions.node.test.tspackages/worker/src/mcp/capabilities/packages/list-packages.tspackages/worker/src/mcp/capabilities/packages/save-package-entitlements.node.test.tspackages/worker/src/mcp/capabilities/packages/save-package-private-visibility.node.test.tspackages/worker/src/mcp/capabilities/packages/save-package.tspackages/worker/src/mcp/capabilities/packages/set-package-hidden.node.test.tspackages/worker/src/mcp/capabilities/packages/set-package-hidden.tspackages/worker/src/mcp/capabilities/packages/shared.tspackages/worker/src/mcp/capabilities/repo/repo-list-sessions.node.test.tspackages/worker/src/mcp/capabilities/repo/repo-open-session.node.test.tspackages/worker/src/mcp/capabilities/repo/repo-workflow.node.test.tspackages/worker/src/mcp/capabilities/services/service-start.node.test.tspackages/worker/src/mcp/capabilities/services/services-domain.node.test.tspackages/worker/src/mcp/fetch-gateway.node.test.tspackages/worker/src/mcp/tools/search-format.node.test.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/mcp/tools/search-handler.node.test.tspackages/worker/src/mcp/tools/search-hidden-packages.node.test.tspackages/worker/src/mcp/tools/search.node.test.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/package-invocations/service.node.test.tspackages/worker/src/package-registry/package-reindex.node.test.tspackages/worker/src/package-registry/repo.tspackages/worker/src/package-registry/saved-packages-hidden-migration.node.test.tspackages/worker/src/package-registry/service.node.test.tspackages/worker/src/package-registry/service.tspackages/worker/src/package-registry/types.tspackages/worker/src/package-retrievers/manifest-cache.node.test.tspackages/worker/src/package-retrievers/service.tspackages/worker/src/package-runtime/module-graph.node.test.tspackages/worker/src/package-runtime/module-graph.workers.test.tspackages/worker/src/package-runtime/published-bundle-artifacts.node.test.tspackages/worker/src/repo/published-bundle-artifacts-repo.workers.test.ts
Use one include-hidden contract across search surfaces and expose visibility in human-readable package details. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hiddenproperty andpackage_set_hiddenmanagement capabilityTest plan
npm run test(843 tests passed)npm run typecheckoxfmt --checknpm run validate(local all-files format scan also sees unrelated untracked orchestration config)System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@18f67ce3· Head:c026c366Classification: extends — this PR changes saved-package storage and ranked-search contracts without adding a new system primitive.
Primitives touched
mcp-servercapability-registrypackage_set_hiddenand returns visibility statesaved-packageshiddenproperty preserved across savesd1-app-dbsaved_packages.hiddencolumnvectorize-searchSystem map
Package visibility flows from MCP search and management capabilities through saved-package metadata into D1-backed, user-scoped ranked search.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
Invariants
per-user-isolation: visibility reads and writes remain scoped byuserId; search continues filtering package state by the authenticated user.compact-mcp-surface: management is a domain capability behind search/execute, not a new top-level MCP tool.Made with Cursor
Summary by CodeRabbit
includeHiddenPackagessupport to search (including meta search), and exposed hidden status in returned package details.hiddenflag to saved packages and a capability to set it.