Skip to content

docs(pi): add integration docs and example - #5796

Open
eersnington wants to merge 2 commits into
feat/headless-pi-actorfrom
docs/headless-pi-actor
Open

eersnington wants to merge 2 commits into
feat/headless-pi-actorfrom
docs/headless-pi-actor

Conversation

@eersnington

@eersnington eersnington commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Adds the Pi page at /integrations/pi, its quickstart snippets, and examples/pi-credentials.

  • Quickstart: install, define the actor, add a sandbox, send a prompt, deploy.
  • Configuration table and tracing.
  • Bring your subscription: credentials backed by a credentials actor.
  • examples/pi-credentials: a credentials actor that stores and refreshes each user's logins, and pnpm provider-login to try it.

This is part 3 of 3 in a stack:

@claude

claude Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Code Review

Reviewed the current diff. The pi-credentials example is small and reads well. Findings below.

1. Scripts run .ts with bare node
examples/pi-credentials/package.json:7-8 uses node src/server.ts and node scripts/provider-login.ts. Node only strips types by default from 22.18 (22.6+ with a flag), and the repo declares node >=20. examples/CLAUDE.md prescribes tsx (or srvx --import tsx). Suggest tsx src/server.ts and tsx scripts/provider-login.ts, with tsx added to devDependencies.

2. Missing pieces required by examples/CLAUDE.md

  • README.md has no ## Features or ## Resources sections, and no ## Prerequisites even though provider logins or API keys are needed.
  • tsconfig.json lacks rewriteRelativeImportExtensions, which the guide pairs with allowImportingTsExtensions.

3. notify switch has no fallback
scripts/provider-login.ts:35-47 handles auth_url, device_code, info and progress only. Any other event type prints nothing and the user sees a silent terminal. A default that logs the event would help, unless the type is a closed union covered by those four cases.

4. Security and robustness

  • The README notes that the credentials actor exposes tokens to any client that can reach it. Still, read returns access tokens and save accepts any credential from any caller. Consider an auth check (for example onBeforeConnect) or a code comment at the actor so copy-paste use is safe by default.
  • refresh writes c.state.saved[provider] after an await. Concurrent refreshes for one provider can both hit the OAuth endpoint, and with rotating refresh tokens the loser may persist a stale token. An in-flight promise per provider in vars would avoid it.
  • refresh on an unknown provider passes undefined to store.modify and silently returns undefined. An explicit error fits the fail-by-default guidance.
  • process.exit(0) at the end of provider-login.ts hides open handles. terminal.close() is cleaner.

5. Other

  • pnpm-lock.yaml shows about 300 lines removed. Please confirm that is intended fallout and not an unrelated lockfile regeneration.
  • docs/general/content/index.mdx removes 2 lines that the PR description does not mention.
  • No tests. setupTest from rivetkit/test could cover save, list and read, and check that read strips the refresh token.

🤖 Generated with Claude Code

@eersnington
eersnington marked this pull request as ready for review September 25, 2026 15:53

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 2 medium · 🔵 1 low

Reviewed commit e5040f4.

Comment thread examples/pi-credentials/src/actors.ts
Comment thread examples/pi-credentials/package.json Outdated
Comment thread docs/integrations/content/docs/pi.mdx Outdated

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 2 medium · 🔵 1 low

Reviewed commit 89a5bd7.

Comment on lines +29 to +30
withoutRefreshToken(c.state.saved[provider]),
refresh: async (c, provider: string) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Medium · Credential actions expose provider tokens to every caller

read returns the stored API key or OAuth access token, and save also lets an unauthenticated caller replace it. Because this actor has no connection or action authorization, anyone who can reach it and select a user's actor key can steal or overwrite that user's login; the README warning does not enforce the boundary. Make the credential store inaccessible to public actor clients, or add authentication and actor-key authorization to the example before exposing these actions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no shit?

Comment thread examples/pi-credentials/package.json Outdated
Comment thread docs/integrations/content/docs/pi.mdx Outdated
@eersnington
eersnington force-pushed the docs/headless-pi-actor branch 2 times, most recently from dc9d5b1 to 3e2a18d Compare September 25, 2026 20:32
@eersnington
eersnington force-pushed the feat/headless-pi-actor branch from b004815 to 1e7248d Compare September 25, 2026 22:24
@eersnington
eersnington force-pushed the docs/headless-pi-actor branch from 3e2a18d to 4a970f3 Compare September 25, 2026 22:24
@eersnington
eersnington force-pushed the feat/headless-pi-actor branch from 1e7248d to 1448347 Compare September 25, 2026 23:40
@eersnington
eersnington force-pushed the docs/headless-pi-actor branch 2 times, most recently from 3f92a6f to b2d96c4 Compare September 26, 2026 00:04
@eersnington
eersnington force-pushed the feat/headless-pi-actor branch from 1448347 to 5719ca6 Compare September 26, 2026 00:04

This branch has not been deployed

No deployments
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.

1 participant