fix: optimize agent list/search UX and smoke cleanup reliability - #147
Merged
Conversation
Align read-command ergonomics by supporting --tags alias handling and MCP top-level filter ingestion while trimming compact list/search metadata to reduce agent token overhead. Harden pack-smoke temp cleanup retries and capture pm/docs/contracts/test evidence, including full 100% coverage and local telemetry health verification.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Reviewer's GuideExtends list/search agent UX by treating --tags as a first-class alias for --tag across parsing, contracts, and shell completion; trims compact list/search JSON trailers to only active filters while omitting projection/sorting/now; enhances MCP pm_list/pm_search to hoist selected top-level filter keys into options with correct precedence; and hardens smoke pack temp-directory cleanup retries for more reliable CI. Sequence diagram for MCP pm_list/pm_search top-level filter hoistingsequenceDiagram
participant Client
participant McpServer
participant optionsWithAuthor
participant normalizeMcpOptionsArrays
Client->>McpServer: call pm_list args{status, type, tag, options}
McpServer->>optionsWithAuthor: optionsWithAuthor(args, action=list)
optionsWithAuthor->>optionsWithAuthor: baseOptions = asRecordClone(args.options)
optionsWithAuthor->>optionsWithAuthor: hoistKey(status|type|tag|priority|limit|offset)
optionsWithAuthor->>optionsWithAuthor: merged = { ...hoistedTopLevel, ...baseOptions }
optionsWithAuthor->>normalizeMcpOptionsArrays: normalizeMcpOptionsArrays(merged, list)
normalizeMcpOptionsArrays-->>optionsWithAuthor: options
optionsWithAuthor-->>McpServer: optionsWithAuthor result
McpServer-->>Client: pm_list result
Client->>McpServer: call pm_search args{mode, status, tag, options}
McpServer->>optionsWithAuthor: optionsWithAuthor(args, action=search)
optionsWithAuthor->>optionsWithAuthor: hoistKey(mode|status|type|tag|priority|limit)
optionsWithAuthor->>normalizeMcpOptionsArrays: normalizeMcpOptionsArrays(merged, search)
normalizeMcpOptionsArrays-->>optionsWithAuthor: options
optionsWithAuthor-->>McpServer: optionsWithAuthor result
McpServer-->>Client: pm_search result
Flow diagram for smoke pack cleanupTempRoot retry logicflowchart TD
A["cleanupTempRoot(tempRoot)"] --> B["attempt = 1"]
B --> C{"attempt <= 8?"}
C -->|No| N{"existsSync(tempRoot)?"}
N -->|No| O["Return"]
N -->|Yes| P["Throw lastError or<br>new Error"]
C -->|Yes| D["rmSync(tempRoot,<br>recursive, force,<br>maxRetries=8, retryDelay=120)"]
D --> E{"rmSync threw?"}
E -->|No| F{"existsSync(tempRoot)?"}
F -->|No| G["Return"]
F -->|Yes| H["Increment attempt"]
E -->|Yes| I["readErrorCode(error)"]
I --> J{"code in<br>CLEANUP_RETRYABLE_CODES?"}
J -->|No| Q["Set lastError,<br>break loop"]
J -->|Yes| K["Set lastError"]
K --> L{"existsSync(tempRoot)?"}
L -->|No| M["Return"]
L -->|Yes| R["Try readdirSync(tempRoot)<br>and rmSync children"]
R --> S["sleepSync(attempt * 120)"]
S --> H
Q --> N
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR implements four interrelated features across the pm-cli codebase: adds ChangesCLI Enhancements: --tags Alias and Compact Output
Infrastructure: MCP Filter Hoisting and Cleanup Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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
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 |
Owner
Author
|
@codex review\n/gemini review\n@gemini-code-assist please review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
To use Codex here, create a Codex account and connect to github. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
isNonEmptyRecordhelper is now duplicated in bothlist.tsandsearch.ts; consider moving it into a shared utility module to avoid divergence and keep behavior consistent. - The MCP top-level filter hoisting logic in
optionsWithAuthorand theLIST_TOP_LEVEL_OPTION_PROPERTIES/SEARCH_TOP_LEVEL_OPTION_PROPERTIESschemas need to stay in lockstep; it might be worth centralizing the allowed hoisted keys to a single source of truth to reduce the chance of future drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isNonEmptyRecord` helper is now duplicated in both `list.ts` and `search.ts`; consider moving it into a shared utility module to avoid divergence and keep behavior consistent.
- The MCP top-level filter hoisting logic in `optionsWithAuthor` and the `LIST_TOP_LEVEL_OPTION_PROPERTIES`/`SEARCH_TOP_LEVEL_OPTION_PROPERTIES` schemas need to stay in lockstep; it might be worth centralizing the allowed hoisted keys to a single source of truth to reduce the chance of future drift.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/cli/commands/list.ts`:
- Around line 695-711: The branch that returns a compact payload when
compactSummaryMode is true builds a different shape (uses projected, count,
filters, warnings) but force-casts it to ListResult, hiding a real type
mismatch; replace the unsafe cast by introducing an explicit CompactListResult
type (e.g., { items: typeof projected; count: number; filters?: ...; warnings?:
... }), change the function/handler return signature to return ListResult |
CompactListResult (or add an overload) and return the compact object as
CompactListResult (no cast), and update any callers to narrow on
compactSummaryMode (or the presence of filters/warnings) before accessing
ListResult-only fields like projection/sorting/now; reference symbols:
compactSummaryMode, buildCompactListFilterSummary, projected, warnings, and
ListResult.
In `@src/cli/commands/search.ts`:
- Around line 978-998: The compact-mode branch (controlled by compactSummaryMode
/ projection.mode === "compact") constructs an abbreviated object and
force-casts it to SearchResult, violating the SearchResult contract (missing
projection, now and sometimes filters) and risking runtime undefined access; fix
by removing the unsafe cast and either (A) return an object that fully satisfies
SearchResult (include projection and now and any required fields such as
filters/warnings) in the compact branch or (B) introduce an explicit union type
(e.g., CompactSearchResult | FullSearchResult) and change the function's return
type so the compact branch returns a proper CompactSearchResult (constructed via
buildCompactSearchFilterSummary and including required minimal fields) and
update callers to handle the union instead of relying on a cast; ensure you
update any usages that assume projection/now to handle the compact union form.
🪄 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 UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fc813747-9b5d-458a-8ec3-5315c3c1df8e
📒 Files selected for processing (30)
.agents/pm/extensions/.managed-extensions.json.agents/pm/features/pm-vhx6.toon.agents/pm/history/pm-6l17.jsonl.agents/pm/history/pm-i2xg.jsonl.agents/pm/history/pm-jozc.jsonl.agents/pm/history/pm-vhx6.jsonl.agents/pm/issues/pm-6l17.toon.agents/pm/issues/pm-i2xg.toon.agents/pm/issues/pm-jozc.toondocs/AGENT_GUIDE.mddocs/COMMANDS.mdscripts/smoke-npx-from-pack.mjssrc/cli/commands/completion.tssrc/cli/commands/list.tssrc/cli/commands/search.tssrc/cli/register-list-query.tssrc/mcp/server.tssrc/sdk/cli-contracts.tssrc/sdk/cli-contracts/commander-types.tstests/fixtures/contracts/full.jsontests/integration/cli.integration.spec.tstests/integration/help-runtime.spec.tstests/integration/mcp-dynamic-package-actions.spec.tstests/integration/mcp-handshake.spec.tstests/integration/release-readiness-runtime.spec.tstests/integration/smoke-pack-cleanup.integration.spec.tstests/unit/bootstrap-args.spec.tstests/unit/completion-command.spec.tstests/unit/list-command.spec.tstests/unit/search-command.spec.ts
Replace compact-mode list/search unsafe casts with explicit compact/verbose result unions while preserving token-light compact payloads. Add deterministic timestamp fallbacks in downstream list consumers and record review-loop verification evidence with full 100% coverage and static/contracts/security reruns.
Owner
Author
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
To use Codex here, create a Codex account and connect to github. |
Owner
Author
|
@coderabbitai review latest changes |
|
✅ Action performedReview finished.
|
unbraind
added a commit
that referenced
this pull request
Jun 8, 2026
Record merged PR #147 evidence and finalize lifecycle transitions for pm-vhx6, pm-6l17, pm-jozc, and pm-i2xg by closing each item and releasing active claims on main.
This was referenced Jul 5, 2026
This was referenced Jul 24, 2026
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
--tagsas a declared read-command alias, extending parser/contracts/completion parity, and preventing never-block retry false positivespm_list/pm_searchnarrow actions and preserve nestedoptionsprecedence when both are suppliedprojection/sorting/now) while retaining active filter summaries, and harden smoke pack temp cleanup retry behavior for ENOTEMPTY/EBUSY/EPERM pathsTest plan
node scripts/run-tests.mjs coveragepnpm contracts:checkpnpm quality:staticpnpm security:scanpnpm audit --audit-level moderate --jsonpnpm outdated(only outdated root dependency:commander14.0.3 -> 15.0.0)node dist/cli.js health --check-only --check-telemetry --summary --jsonnode dist/cli.js telemetry status --jsonpm_listtop-level filtersgh(dependabot,code-scanning,secret-scanningall open=0)403scope limitation), local telemetry+health evidence used per approved execution modeSummary by Sourcery
Optimize agent-facing list/search UX and MCP filter handling while hardening smoke-pack temp cleanup and updating docs/contracts.
New Features:
Bug Fixes:
Enhancements:
Tests: