Skip to content

feat/playlist-ext-note#40

Draft
jollyjoker992 wants to merge 2 commits into
mainfrom
feat/playlist-ext-note
Draft

feat/playlist-ext-note#40
jollyjoker992 wants to merge 2 commits into
mainfrom
feat/playlist-ext-note

Conversation

@jollyjoker992

Copy link
Copy Markdown
Contributor

Send: cast hosted playlists by URL (live updates)

Summary

When send is used with an http(s) playlist URL, the CLI still loads and verifies the JSON locally, but the cast request sends playlistUrl so the FF1 device fetches the playlist instead of receiving inline dp1_call JSON. Local file sends are unchanged and still embed dp1_call.

Changes

  • Add optional playlistUrl through sendPlaylistToDevice / sendToDevice and build the cast body with buildDisplayPlaylistCastRequestBody.
  • Wire URL detection via isPlaylistSourceUrl from the CLI send command and from main.ts interactive flows.
  • Update docs (README, EXAMPLES, FUNCTION_CALLING) and note rebuilding dist/ when using the ff1 shim.
  • Add unit tests for buildDisplayPlaylistCastRequestBody.

Testing

  • tests/ff1-device-cast-request.test.ts covers inline vs URL cast body shape and validation.

@feralfile-bot feralfile-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewers: 2 (approve: 0, comment: 0, request-changes: 2)

  • src/utilities/ff1-device.ts:193-196 logs requestBody unconditionally before every cast. I re-checked this path: it changes CLI output for every send and can print the full inline DP-1 payload for local playlists, which is a user-visible regression and a data-leak risk. Both reviewers independently flagged this, so it should be removed or moved behind an explicit debug/verbose path.
  • The new coverage in tests/ff1-device-cast-request.test.ts only exercises buildDisplayPlaylistCastRequestBody; it does not exercise the actual send wiring in index.ts:1019-1035 or src/main.ts:279-287 / src/main.ts:398-409. That leaves the URL-detection and argument-threading behavior unprotected, so a regression in how hosted sources become playlistUrl would slip through. Both reviewers called out this gap.

Overall verdict: request changes.

@feralfile-bot

Copy link
Copy Markdown
Contributor

Automated human review request for PR #40.

Risk zone: yellow
Responsibility area: Publish + validation
Why this needs human review: This changes the FF1 cast request contract for hosted playlists from inline dp1_call to playlistUrl, which is narrow but crosses the CLI-to-device boundary and benefits from steward review.

This PR needs targeted human review on the scoped areas below, not a full second pass of the whole PR.

  • @jollyjoker992: please review hosted playlist URL cast body and send-command URL detection. Context: publish-to-play contract (Hieu Pham).

@feralfile-bot feralfile-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewers: 2 (approve: 0, comment: 2, request-changes: 0)

  • Missing end-to-end coverage for the new playlistUrl wiring in both entry points. The helper itself is covered in tests/ff1-device-cast-request.test.ts, but the actual decision points that propagate playlistUrl live in index.ts and the two src/main.ts send paths (lines 279-287, lines 398-409). Both reviewer reports independently flagged this same gap, and a regression in URL detection or argument propagation would still slip through without CLI-level coverage.

@jollyjoker992 jollyjoker992 marked this pull request as draft May 19, 2026 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants