fix(web): surface Browse Playbook Exchange in AutoRun empty state - #973
Closed
chr1syy wants to merge 9 commits into
Closed
fix(web): surface Browse Playbook Exchange in AutoRun empty state#973chr1syy wants to merge 9 commits into
chr1syy wants to merge 9 commits into
Conversation
Mobile users can now browse, preview, and import community playbooks from inside the AutoRun setup sheet — closing the AR-PARITY-01 gap between desktop and the mobile interface. - Extracted marketplace cache/fetch/import logic into src/main/services/marketplace-service.ts so the IPC handler and the web-server share one code path (no renderer round-trip). - Added four WebSocket message types: marketplace_get_manifest (with refresh:true), marketplace_get_document, marketplace_get_readme, marketplace_import_playbook. Filename traversal + missing-field validation at the boundary. - Wired callbacks in web-server-factory; the import callback resolves autoRunFolderPath and SSH config from the session itself, so mobile clients can't override them — SSH remote sessions import to the correct host the same way as desktop. - Created MarketplaceSheet (mobile bottom sheet with list → detail → import flow, category chips, search, README/document preview) and added a "Browse Playbook Exchange" entry point at the top of AutoRunSetupSheet. After import, AutoRun docs auto-refresh so freshly imported docs appear in the selector. Tests: new marketplace block in messageHandlers.test.ts covers all four message types + traversal rejection + unconfigured-callback paths. AutoRun App.test mocks the new sheet. All scoped suites pass (marketplace IPC: 45, message handlers: 113, mobile App: 95). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves merge conflict in src/main/ipc/handlers/marketplace.ts by keeping the new thin-wrapper refactor; ports the upstream watcher error-handler addition (Windows UNKNOWN / EPERM / ENOENT classification) into createLocalManifestWatcher in the new src/main/services/marketplace-service.ts. Also addresses PR RunMaestro#947 review feedback from greptile + coderabbit: Security - WS handlers reject `playbookPath` values that point at the local filesystem (absolute or `~`-prefixed). Local paths remain reachable via desktop IPC where the renderer is trusted, but mobile/web clients can no longer coerce the server into reading arbitrary files via the marketplace fetch helpers. - WS `marketplace_import_playbook` rejects `targetFolderName` values containing path separators or traversal tokens. Service layer adds `assertSafeTargetFolderName` as the source-of-truth guard. - IPC `marketplace:importPlaybook` fails loudly when `sshRemoteId` is provided but cannot be resolved, instead of silently downgrading to a local import (matches CLAUDE.md SSH-spawn pattern: never silently downgrade). Correctness - Persisted `newPlaybook.documents` only references docs that actually wrote to disk, so a partial import never produces a playbook pointing at missing files. - Reading the per-session playbooks file no longer silently resets to `[]` on non-ENOENT errors. Corrupt JSON / EACCES now throws rather than overwriting existing user data. - `fetchReadme` re-throws unexpected I/O / network failures (only ENOENT and 404 still map to "no README"), so production faults surface in Sentry. - WS document/readme handlers return a structured failure when the underlying callback resolves to `null`, instead of treating unconfigured wiring as a successful empty response. Code quality - Replaced dynamic `require('os')` in marketplace-service with a static `import os from 'os'` (consistent with the rest of the module). UI polish - AutoRunSetupSheet entry button gets a focus ring on `:focus` so keyboard users still see focus state after `outline: none`. - MarketplaceSheet swaps `||` to `??` for README/document fallback text so empty files render instead of showing the placeholder. - MarketplaceSheet adds a monotonic preview-request id; stale README/document responses are discarded if the user has already picked another playbook or document. Error responses now surface in the import-error banner instead of being swallowed. Tests - Updated marketplace IPC tests for the new SSH fail-loud behavior (replaces "fall back to local fs" cases with "fail loudly"). - Added missing `mockRejectedValueOnce({ code: 'ENOENT' })` for the per-session playbooks file read in 7 tests that previously relied on the bare-catch swallowing JSON.parse errors. - Fixed `os` mock to expose `default` for the new static import. - Asserts that partially-imported playbooks only persist the successful docs. All scoped suites pass (marketplace IPC: 45, message handlers: 113, mobile App: 95). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves 6 conflicts that came in from upstream/rc's PR2 of the CLI surface refactor (list_desktop_sessions / get_session_history) plus the AutoRunSetupSheet playbook-CRUD redesign and FolderPickerSheet: - WebServer.ts, types.ts, CallbackRegistry.ts: combined the marketplace callback registrations with upstream's listDesktopSessions and getSessionHistory callbacks (both sets coexist; ordering preserves marketplace-first since that was already on the branch). - handlers/messageHandlers.ts: combined the four marketplace WS handlers (get_manifest, get_document, get_readme, import_playbook) with the new list_desktop_sessions and get_session_history handlers. - web/mobile/App.tsx: kept both the MarketplaceSheet wiring and the new FolderPickerSheet + handleAutoRunFolderConfirm flow; passed sendRequest, send, currentDocument and onOpenMarketplace through to the redesigned AutoRunSetupSheet. - web/mobile/AutoRunSetupSheet.tsx: kept the upstream playbook-CRUD refactor (sendRequest/send/currentDocument props, playbooks panel, inline name-prompt / delete-confirm modals) and re-anchored the Playbook Exchange entry button above the new Playbooks section so marketplace import remains discoverable. - __tests__/main/web-server/handlers/messageHandlers.test.ts: kept the marketplace mocks/tests alongside upstream's listDesktopSessions / getSessionHistory mocks and tests. Also addresses CI failures and a new coderabbit comment from the last push: CI fix - web-server-factory.test.ts MockWebServer was missing the four setMarketplaceManifestCallback / setMarketplaceDocumentCallback / setMarketplaceReadmeCallback / setImportMarketplacePlaybookCallback stubs, causing 36 test failures with `TypeError: ... is not a function`. Added the stubs (combined with upstream's new setListDesktopSessionsCallback / setGetSessionHistoryCallback). Coderabbit P3 (Major) - Marketplace WS validation branches were emitting generic `type: 'error'` frames rather than the request-scoped `marketplace_*_result` types. Clients waiting on the typed result would miss the failure or hang. Introduced `sendMarketplaceFailure(client, type, error, message, extra?)` helper and routed all validation-failure paths through it (missing fields, untrusted local paths, traversal in filename, separators in targetFolderName, callback unconfigured). Updated the relevant tests (assert typed-result failures and added two new cases for absolute playbookPath rejection and targetFolderName separator rejection). Verification - `npm run lint` clean (TypeScript, all configs). - `npm run lint:eslint` clean. - `prettier --check` clean across all touched files. - Scoped tests pass: marketplace IPC (45), message handlers (123 — 7 new), web-server-factory (95), mobile App (95). 319 total. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The mobile Playbook Exchange prefilled the import folder with a `category/title` slug, which the server's `assertSafeTargetFolderName` guard rejects (no separators allowed). Every user tapping Import on a category-namespaced playbook hit this and had to manually edit the input down to a single segment. Switch the prefill to a title-only slug. Category is already shown via the tile chip and detail-view eyebrow, so re-encoding it into the folder name added no information. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The marketplace sheet's `importError` was overloaded — `handleSelectPlaybook` and `handleSelectDocument` set it on README/document fetch failures, but it rendered in the import footer next to the Import Playbook button. A user who tapped a playbook, saw the preview fail, and read the error in the footer would reasonably believe an import had been attempted and failed. Split into a dedicated `previewError` state that renders inline in the preview area, leaving `importError` exclusively for actual import failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds a Playbook Exchange: a marketplace service for manifest caching and safe local/remote fetching, refactors IPC and WebSocket handlers to delegate to the service, registers main-process callbacks, introduces a mobile MarketplaceSheet, and updates tests for import and handler edge cases. ChangesMarketplace Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
Greptile SummaryThis PR closes Gap 3 from the post-soak parity work: it adds a "Browse Playbook Exchange" CTA to the
Confidence Score: 5/5Safe to merge once #947 lands; the Gap 3 commit is a small, opt-in prop addition on top of well-validated marketplace infrastructure. The net-new commit is minimal — an optional prop threaded through four components with no breaking changes to existing callers or tests. The stacked #947 infrastructure is the larger surface: path-traversal guards appear at both the WS handler layer (isUntrustedLocalPath) and the service layer (assertSafeTargetFolderName / validateSafePath), the import flow is SSH-aware, and the manifest cache uses a conservative 6h TTL. The MarketplaceSheet's monotonic request-ID pattern correctly prevents stale preview responses from clobbering newer selections. No data-loss paths, no missing guards, and the four AutoRunInline tests cover the exact backward-compatibility contract the PR relies on. No files require special attention. The largest new file, MarketplaceSheet.tsx, follows established patterns from the codebase and its async state management is sound. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AutoRunInline
participant App as App.tsx
participant MarketplaceSheet
participant WS as WebSocket Handler
participant MarketplaceService
User->>AutoRunInline: taps "Browse Playbook Exchange"
AutoRunInline->>App: onOpenMarketplace()
App->>App: setShowMarketplaceSheet(true)
App->>MarketplaceSheet: render (sessionId, sendRequest, onImported, onClose)
MarketplaceSheet->>WS: marketplace_get_manifest
WS->>MarketplaceService: getMarketplaceManifest()
MarketplaceService-->>WS: manifest (cached or fetched)
WS-->>MarketplaceSheet: manifest_result
User->>MarketplaceSheet: selects playbook
MarketplaceSheet->>WS: marketplace_get_readme
WS->>MarketplaceService: getMarketplaceReadme(playbookPath)
MarketplaceService-->>WS: readme content
WS-->>MarketplaceSheet: readme_result
User->>MarketplaceSheet: taps "Import Playbook"
MarketplaceSheet->>WS: marketplace_import_playbook (sessionId, playbookId, targetFolderName)
Note over WS: validates sessionId, playbookId,<br/>targetFolderName (no path traversal)
WS->>MarketplaceService: importMarketplacePlaybook()
MarketplaceService-->>WS: importedDocs, importedAssets
WS-->>MarketplaceSheet: import_result (success)
MarketplaceSheet->>App: onImported(folderName)
App->>App: loadAutoRunDocuments(activeSessionId)
MarketplaceSheet->>App: onClose() [after 300ms animation]
App->>App: setShowMarketplaceSheet(false)
Reviews (1): Last reviewed commit: "fix(web): surface Browse Playbook Exchan..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/main/services/marketplace-service.ts`:
- Around line 451-469: The import path currently ignores a stale cache when
fetchManifest() fails, causing official playbooks to disappear; update the logic
in the block using readCache/isCacheValid/fetchManifest/writeCache so that on
fetchManifest() failure you fall back to the existing cache.manifest (even if
isCacheValid(cache) is false) and log that you are using a stale cache;
specifically, in the catch for fetchManifest() check if cache is present and set
officialManifest = cache.manifest before logging, otherwise keep the current
behavior of capturing the exception and continuing, ensuring mergeManifests
still receives the stale official manifest when available.
- Around line 491-625: The code currently proceeds even when importedDocs is
empty; add a guard after the document import loop (before building newPlaybook /
persisting) that checks if importedDocs.length === 0 and, if so, throws a
MarketplaceImportError with a clear message including marketplacePlaybook.title
so the import fails fast; use the existing MarketplaceImportError constructor
(same style as the read error) to abort before creating newPlaybook or writing
playbooksFilePath.
In `@src/main/web-server/handlers/messageHandlers.ts`:
- Around line 3957-3964: The catch block in messageHandlers.ts that currently
sends a typed failure for marketplace (the anonymous .catch handler that calls
this.send with type 'marketplace_get_manifest_result' and message.requestId)
must report unexpected exceptions to Sentry before replying; update those catch
blocks (also the similar ones around lines handling
marketplace_get_manifest_result, marketplace_list, marketplace_get_item, etc.)
to call reportHandlerError(err, { handler: 'marketplace_*', requestId:
message.requestId }) or captureException/captureMessage first, and only then
send the typed failure for known/recoverable errors (or re-throw for truly
unexpected ones); reuse the existing reportHandlerError utility or add a
typed-marketplace wrapper so every catch path records the error with context
before responding via this.send.
- Around line 3973-3980: Update isUntrustedLocalPath to reject dot-segments and
any backslash characters: ensure the function not only checks
absolute/tilde/Windows-drive prefixes but also returns true if playbookPath
contains backslashes or any segment equals '.' or '..' (e.g., split on '/' and
validate segments). Then, in each marketplace catch block that currently calls
sendMarketplaceFailure (the catches around lines handling marketplace import),
call captureException(err) or reportHandlerError(err) before invoking
sendMarketplaceFailure so exceptions are reported to Sentry; keep the existing
sendMarketplaceFailure behavior after reporting. Use the symbols
isUntrustedLocalPath, sendMarketplaceFailure, captureException, and
reportHandlerError to locate and change the code.
In `@src/main/web-server/web-server-factory.ts`:
- Around line 2595-2599: The catch block that handles marketplace import
failures (where err is normalized to errorMsg and logger.error is called for
playbookId) currently returns { success: false } without reporting to Sentry;
update that catch to call captureException(err, { tags: { area:
'marketplaceImport', playbookId } , level: 'error' }) from the Sentry utilities
(import captureException from src/utils/sentry.ts) before returning, so the
original Error object and context (playbookId) are sent to Sentry; keep the
existing logger.error and return value after the capture.
In `@src/web/mobile/MarketplaceSheet.tsx`:
- Around line 147-150: The catch blocks around the async marketplace loading
logic (the one that sets setManifestError when cancelled isn't true — and the
other similar catches at the noted ranges) are swallowing unexpected exceptions;
update those catch handlers to (1) distinguish expected/recoverable errors
(e.g., specific NETWORK_ERROR or known error types) and continue to set
setManifestError for those, (2) for any other unexpected error call the Sentry
helper (imported from src/utils/sentry.ts, e.g., captureException or
captureMessage) with contextual info (include which operation and any relevant
variables like cancelled or manifest id), and then re-throw the error so it
bubbles to global handlers; keep the existing local UI error behavior only for
handled/recoverable errors and do not suppress unexpected exceptions.
- Around line 219-227: The cancel-branch that handles filename === null
currently increments previewRequestIdRef and clears selection/content/loading
state but does not clear previewError or re-trigger the README fetch, leaving
the sheet stuck on an error/placeholder; update this branch to also clear
previewError (via its setter, e.g. setPreviewError(null)) and then invoke the
same README-loading routine used on mount/initial load (call the existing README
fetch function) so the README is re-fetched when returning from a doc preview
while keeping previewRequestIdRef, setSelectedDocFilename, setDocumentContent,
and setIsLoadingDocument behavior 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df857283-0cc4-48fb-8e97-b7130b3af47d
📒 Files selected for processing (19)
src/__tests__/main/ipc/handlers/marketplace.test.tssrc/__tests__/main/web-server/handlers/messageHandlers.test.tssrc/__tests__/main/web-server/web-server-factory.test.tssrc/__tests__/web/mobile/App.test.tsxsrc/__tests__/web/mobile/AutoRunInline.test.tsxsrc/main/ipc/handlers/marketplace.tssrc/main/services/marketplace-service.tssrc/main/web-server/WebServer.tssrc/main/web-server/handlers/messageHandlers.tssrc/main/web-server/managers/CallbackRegistry.tssrc/main/web-server/types.tssrc/main/web-server/web-server-factory.tssrc/web/mobile/App.tsxsrc/web/mobile/AutoRunInline.tsxsrc/web/mobile/AutoRunPanel.tsxsrc/web/mobile/AutoRunSetupSheet.tsxsrc/web/mobile/MarketplaceSheet.tsxsrc/web/mobile/RightDrawer.tsxsrc/web/mobile/RightPanel.tsx
Comment on lines
+147
to
+150
| } catch (err) { | ||
| if (cancelled) return; | ||
| setManifestError(err instanceof Error ? err.message : 'Failed to load marketplace data'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Report unexpected async failures instead of only showing inline UI errors.
These catches turn every thrown exception into local component state and return. That preserves the recoverable UX path, but unexpected failures here never reach Sentry, so production breakages in the new marketplace flow become invisible.
As per coding guidelines, "Do not silently swallow errors. Let unhandled exceptions bubble up to Sentry for error tracking in production. Handle expected/recoverable errors explicitly (e.g., NETWORK_ERROR). For unexpected errors, re-throw them to allow Sentry to capture them. Use Sentry utilities (captureException, captureMessage) from src/utils/sentry.ts for explicit error reporting with context."
Also applies to: 206-210, 245-249, 278-281
🤖 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 `@src/web/mobile/MarketplaceSheet.tsx` around lines 147 - 150, The catch blocks
around the async marketplace loading logic (the one that sets setManifestError
when cancelled isn't true — and the other similar catches at the noted ranges)
are swallowing unexpected exceptions; update those catch handlers to (1)
distinguish expected/recoverable errors (e.g., specific NETWORK_ERROR or known
error types) and continue to set setManifestError for those, (2) for any other
unexpected error call the Sentry helper (imported from src/utils/sentry.ts,
e.g., captureException or captureMessage) with contextual info (include which
operation and any relevant variables like cancelled or manifest id), and then
re-throw the error so it bubbles to global handlers; keep the existing local UI
error behavior only for handled/recoverable errors and do not suppress
unexpected exceptions.
Comment on lines
+219
to
+227
| if (filename === null) { | ||
| // Cancel any in-flight document fetch and immediately surface | ||
| // the README — bumping the request id ensures a slow document | ||
| // response can't overwrite this state later. | ||
| ++previewRequestIdRef.current; | ||
| setSelectedDocFilename(null); | ||
| setDocumentContent(null); | ||
| setIsLoadingDocument(false); | ||
| return; |
There was a problem hiding this comment.
Re-load README when switching back from a document preview.
This branch cancels the in-flight document request, but it never clears previewError or re-fetches the README. If the user taps a doc before the initial README load finishes, then goes back to README.md, the sheet can stay stuck on the old error / “No README available” placeholder even when a README exists.
🤖 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 `@src/web/mobile/MarketplaceSheet.tsx` around lines 219 - 227, The
cancel-branch that handles filename === null currently increments
previewRequestIdRef and clears selection/content/loading state but does not
clear previewError or re-trigger the README fetch, leaving the sheet stuck on an
error/placeholder; update this branch to also clear previewError (via its
setter, e.g. setPreviewError(null)) and then invoke the same README-loading
routine used on mount/initial load (call the existing README fetch function) so
the README is re-fetched when returning from a doc preview while keeping
previewRequestIdRef, setSelectedDocFilename, setDocumentContent, and
setIsLoadingDocument behavior intact.
Conflict resolution - src/main/ipc/handlers/marketplace.ts: kept this PR's thin-wrapper refactor (logic in src/main/services/marketplace-service.ts) over upstream's inline import body. Ported upstream's marketplace compatibility check (isCompatible against minMaestroVersion) into the service layer instead, so both the IPC and WebSocket import paths get the defense-in-depth gate without duplicating the inline body. Verification - npm run lint clean (TypeScript, all configs). - prettier and eslint clean on touched files. - Scoped suites pass: marketplace IPC, message handlers, web-server-factory, mobile App, marketplace-compatibility (344 total). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
|
Hey @chr1syy — thanks so much for the follow-up to close Gap 3! 🙏 The "Browse Playbook Exchange" CTA in the AutoRun empty state is a clean parity fix and the opt-in prop pattern keeps existing callers unaffected. Heads up though: GitHub is reporting merge conflicts against Thanks again! |
Resolves seven CodeRabbit findings on PR RunMaestro#947's Playbook Exchange code. All edits keep the existing typed-result protocol and recoverable-UX contracts; the changes are about making real production failures visible and refusing to persist a half-imported playbook. Service layer (`src/main/services/marketplace-service.ts`): - Import path now falls back to a stale official cache when `fetchManifest()` fails, mirroring the browse path. Without this, a visible playbook served from the same stale cache could disappear at import time and the user would see "Playbook not found". - Refuse to persist a playbook when every document failed to write. The per-doc loop is intentionally tolerant so one bad file doesn't block the rest, but the previous code still returned success with `documents: []` — closing the marketplace sheet and leaving the user with an unusable imported entry. Now throws a `MarketplaceImportError` before the playbook file is touched, and only when the playbook actually had documents (asset-only playbooks still import). WebSocket handlers (`src/main/web-server/handlers/messageHandlers.ts`): - `isUntrustedLocalPath` now rejects `.` / `..` segments and any backslash anywhere in the path, not just absolute / `~` / Windows drive prefixes. Defense-in-depth at the entry point so future downstream changes can't re-open a relative-traversal bypass. - All four marketplace catch blocks (`get_manifest`, `get_document`, `get_readme`, `import_playbook`) now route through a new `reportMarketplaceHandlerError` helper that calls `captureException` before sending the typed failure. Mirrors the existing `reportHandlerError` for non-marketplace handlers; preserves the client-visible typed result so mobile clients waiting on the `marketplace_*_result` type still get their response. Web server factory (`src/main/web-server/web-server-factory.ts`): - Marketplace import catch now reports the original exception to Sentry with `{ operation, sessionId, playbookId, targetFolderName }` context before returning `{ success: false }`. Keeps the existing recoverable-UX path while making the real cause visible in production error tracking. Mobile MarketplaceSheet (`src/web/mobile/MarketplaceSheet.tsx`): - Manifest / README / document / import catch blocks now `console.error` the underlying exception so the original cause is visible in browser devtools. Web bundle has no Sentry, so this matches the existing pattern in `AutoRunDocumentViewer` rather than introducing a new Sentry surface. - Returning to the README view from a document preview now clears `previewError` and re-fetches the README via a new `loadReadmeFor` helper. Previously the sheet could stay stuck on a stale "No README available" / fetch error after the user tapped a doc and went back. Tests: - `marketplace.test.ts`: new "should fail the import when all documents fail to fetch" verifies the empty-import guard fires and no playbook is persisted. New "should fall back to expired cache when network fetch fails during import" exercises the import-path stale-cache fallback. - `messageHandlers.test.ts`: new parameterized suite validates that `isUntrustedLocalPath` rejects `..`, `.`, embedded dot segments, and embedded backslashes via the typed `marketplace_get_document_result`. All scoped marketplace + web-server-handler tests pass (198/198). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chr1syy
force-pushed
the
fix/web-followup-add-docs
branch
from
May 9, 2026 08:34
5b39a3c to
1f66432
Compare
Contributor
Author
|
Addressed all seven CodeRabbit findings in Service layer (
WebSocket handlers (
Web server factory (
Mobile sheet (
Tests (
All 205 tests in the scoped suites pass; lint + prettier clean. |
chr1syy
added a commit
to chr1syy/Maestro
that referenced
this pull request
May 10, 2026
…n AutoRun empty state
Collaborator
|
This is stacked on #947 and will rebase down cleanly once that lands. Holding here pending #947 — see my comment there for the two fixups needed (rebase onto current Once #947 is rebased and merged, this should auto-resolve to its actual ~186-line delta and I'll merge it. |
Collaborator
8 tasks
Closes Gap 3 from the PR RunMaestro#946/RunMaestro#947 mobile/web AutoRun parity follow-up. Desktop has an "Add Docs" affordance that surfaces both Create-doc and Marketplace as co-equal entry points. On mobile/web, the empty state of AutoRunInline only offered "+ Create document" — users with a fresh AutoRun folder had no in-UI path to discover existing playbooks and had to know to open the launch sheet just to find the marketplace entry. - Add onOpenMarketplace?: () => void prop to AutoRunInline. Render a "Browse Playbook Exchange" CTA next to "+ Create document" in the empty state when the prop is provided. Both buttons share the same vertical stack so the discoverability matches desktop's overview. - Plumb the prop through AutoRunPanel (full-screen overlay), AutoRunTabContent / RightDrawer (inline tab), and RightPanel. - Wire handleOpenMarketplaceSheet through from App.tsx for both AutoRunPanel and RightPanel render sites; reuses the same MarketplaceSheet and WS infrastructure introduced by PR RunMaestro#947, so no new IPC/server surface. The CTA is opt-in (prop is optional) so existing callers and tests keep working without change. Existing scoped tests pass; new AutoRunInline.test.tsx covers the empty-state CTA visibility, hidden-when-unset, and onClick behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chr1syy
force-pushed
the
fix/web-followup-add-docs
branch
from
May 10, 2026 17:25
1f66432 to
e67ca7a
Compare
Collaborator
|
Cherry-picked Now on rc as @chr1syy thanks for the rebase + SSH fix on #947 — everything in the queue is now in. |
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
2 participants
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
Follow-up to #947 (
feat(web): expose Playbook Exchange to mobile/web) closing Gap 3 from the post-soak parity gist.Desktop has an "Add Docs" affordance that surfaces both Create document and Browse Marketplace as co-equal entry points. On mobile/web, the empty state of
AutoRunInlineonly offered "+ Create document" — users with a fresh AutoRun folder had no in-UI path to discover existing playbooks and had to know to open the launch sheet just to find the marketplace entry.This PR adds a "Browse Playbook Exchange" CTA next to "+ Create document" in the empty state, matching desktop's Docs Overview behaviour. It reuses the
MarketplaceSheetand WS infrastructure introduced by #947 — no new IPC/server surface.Stacked on #947
This branch is stacked on top of
feat/web-playbook-exchange, so the diff currently includes #947's commits. Once #947 lands inrc, this PR's diff collapses to just the Gap 3 commit (5b39a3c). The single new commit on top of #947 is what's actually being added here.Changes (this PR's commit only)
src/web/mobile/AutoRunInline.tsx: optionalonOpenMarketplaceprop; second CTA rendered in the empty state when provided.src/web/mobile/AutoRunPanel.tsx,RightDrawer.tsx(AutoRunTabContent),RightPanel.tsx: prop plumbed through.src/web/mobile/App.tsx:handleOpenMarketplaceSheetwired into bothAutoRunPanel(full-screen overlay) andRightPanel(inline tab).src/__tests__/web/mobile/AutoRunInline.test.tsx: 4 new scoped tests covering CTA visibility, hidden-when-prop-omitted, render-when-provided, and onClick.The CTA is opt-in (prop is optional), so existing callers and tests keep working.
Test plan
MarketplaceSheetopens.onOpenMarketplace(regression guard).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests