Conversation
Production deploys were triggered by every push to main through Vercel's Git integration. Disable that for main and deploy from a workflow that runs when a GitHub release is published, so shipping is an explicit act. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds a design specification for a versioned, shareable Docs API and introduces release-triggered Vercel production deployment configuration, including disabling Git deployments from the ChangesDocs API versioning design
Production deployment configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.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 @.github/workflows/production-deploy.yml:
- Around line 21-24: Move VERCEL_ORG_ID, VERCEL_PROJECT_ID, and VERCEL_TOKEN
from the job-level env block to step-level env blocks on Pull Vercel
environment, Build, and Deploy only; remove their availability from checkout,
Bun setup, and bun install while preserving the existing deployment behavior.
- Around line 34-36: Update the oven-sh/setup-bun step in the production deploy
workflow to set its no-cache option to true, while preserving the existing
bun-version configuration.
In `@docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md`:
- Around line 241-247: Make both multi-write document workflows atomic in
docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md: lines 241-247
must transactionally insert the document and its initial document_version, while
lines 316-318 must transactionally insert the new version and update
documents.updatedAt. Ensure each related write shares one transaction and
preserves the existing response behavior.
- Around line 271-278: Update the documented API behavior so unauthorized access
to an existing document returns 404, matching the response for missing documents
and preventing document-existence disclosure. Revise the relevant behavior steps
to remove the separate 403 outcome while preserving validation, authorization,
and version-loading semantics.
- Around line 198-211: Update the API schemas createDocumentRequestSchema,
updateDocumentRequestSchema, and shareDocumentRequestSchema with explicit
maximum HTML/string lengths and recipient-array counts, and add a request-body
size limit before request parsing in the documented API flow. Preserve the
existing non-empty, optional, and enum validation behavior while ensuring
oversized payloads are rejected before parsing or persistence.
- Around line 38-45: Normalize the primary email in the Clerk synchronization
flow, including src/server/clerk/user-sync.ts and the code exposing
ctx.userEmail, by reusing the same shared normalization function used for share
inputs. Ensure trimming and lowercasing occur at the synchronization boundary so
all downstream comparisons use the normalized value, and add mixed-case coverage
for synchronization, ctx.userEmail, and share operations.
- Around line 466-510: Expand the Testing section to require coverage for core
invariants: create rollback on failure, concurrent update retry and atomicity,
normalized-email matching for sharing/access, and unauthorized-document
responses that preserve the intended security behavior. Place these alongside
the existing document service and HTTP handler tests, without removing current
coverage.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d028c66f-efb4-4b3c-8021-1fd8c00f5328
📒 Files selected for processing (3)
.github/workflows/production-deploy.ymldocs/superpowers/specs/2026-06-12-docs-api-versioning-design.mdvercel.json
📜 Review details
🧰 Additional context used
🪛 LanguageTool
docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md
[style] ~18-~18: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... - Authorize updates only for owners. - Authorize sharing for any user who currently has ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ing. - No document deletion endpoint. - No metadata-only update endpoint for `name...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~46-~46: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...zed primary email as ctx.userEmail. - Expose the Drizzle database handle as ctx.db...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~482-~482: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... but no active local user row exists. - Returns ApiContext with local user and normal...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~508-~508: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test by default. - PUT /api/docs/{id} returns 403 for non-owner. - `GET /api/docs?a...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~518-~518: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ndation tests before implementation. 5. Add the shared handler wrapper, response he...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~519-~519: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...parsing under src/server/handlers. 6. Add Zod contracts and document types in `sr...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~520-~520: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...cument types in src/types/docs.ts. 7. Add repository and service tests before imp...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~521-~521: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...service tests before implementation. 8. Add repository and service implementation u...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~522-~522: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...itory/docsandsrc/services/docs`. 9. Add handler tests before each handler imple...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~523-~523: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...before each handler implementation. 10. Add thin Next.js Route Handlers in `src/app...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 zizmor (1.26.1)
.github/workflows/production-deploy.yml
[error] 34-34: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🔇 Additional comments (4)
docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md (1)
23-27: 🔒 Security & PrivacyDefine a safe rendering boundary for raw HTML.
The API accepts arbitrary HTML and returns it to shared users without sanitization. If any client renders this as markup, a document owner can inject script that executes for viewers. Require sanitization or escaping at every render sink, and test that contract before shipping.
.github/workflows/production-deploy.yml (2)
1-19: LGTM!
30-30: 🔒 Security & PrivacyCheckout should use the release commit, not a mutable tag.
github.event.release.tag_namecan move; pin this checkout to the release commit SHA if that’s what this workflow is meant to deploy.vercel.json (1)
1-8: LGTM!
| env: | ||
| VERCEL_ORG_ID: ${{ secrets.VERCEL_ORG_ID }} | ||
| VERCEL_PROJECT_ID: ${{ secrets.VERCEL_PROJECT_ID }} | ||
| VERCEL_TOKEN: ${{ secrets.VERCEL_TOKEN }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the workflow and inspect the relevant section with line numbers.
git ls-files .github/workflows/production-deploy.yml
wc -l .github/workflows/production-deploy.yml
cat -n .github/workflows/production-deploy.yml | sed -n '1,220p'Repository: AnaBerg/shareable-docs
Length of output: 1819
Scope the Vercel secrets to the deploy steps only.
Keeping them at the job level exposes VERCEL_TOKEN to checkout, Bun setup, and bun install; bun install can run lifecycle scripts from the release tag before deployment. Move the secrets onto Pull Vercel environment, Build, and Deploy only.
🤖 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 @.github/workflows/production-deploy.yml around lines 21 - 24, Move
VERCEL_ORG_ID, VERCEL_PROJECT_ID, and VERCEL_TOKEN from the job-level env block
to step-level env blocks on Pull Vercel environment, Build, and Deploy only;
remove their availability from checkout, Bun setup, and bun install while
preserving the existing deployment behavior.
Source: MCP tools
| uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 | ||
| with: | ||
| bun-version: 1.3.13 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' .github/workflows/production-deploy.ymlRepository: AnaBerg/shareable-docs
Length of output: 1400
🏁 Script executed:
python3 - <<'PY'
import urllib.request, sys
url = "https://raw.githubusercontent.com/oven-sh/setup-bun/0c5077e51419868618aeaa5fe8019c62421857d6/action.yml"
print(urllib.request.urlopen(url, timeout=20).read().decode())
PYRepository: AnaBerg/shareable-docs
Length of output: 2446
🏁 Script executed:
curl -Lks https://raw.githubusercontent.com/oven-sh/setup-bun/0c5077e51419868618aeaa5fe8019c62421857d6/action.ymlRepository: AnaBerg/shareable-docs
Length of output: 2694
Disable setup-bun caching here. .github/workflows/production-deploy.yml:34-36 This production deploy job runs with Vercel secrets, and oven-sh/setup-bun defaults no-cache to false with a post-job cache save step. Set no-cache: true to avoid persisting runtime artifacts from a secrets-bearing workflow.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 34-34: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 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 @.github/workflows/production-deploy.yml around lines 34 - 36, Update the
oven-sh/setup-bun step in the production deploy workflow to set its no-cache
option to true, while preserving the existing bun-version configuration.
Sources: MCP tools, Linters/SAST tools
| - Read the current Clerk auth state. | ||
| - Require an authenticated Clerk user. | ||
| - Load the active local `users` row by `clerkUserId`. | ||
| - Normalize and expose the local user's primary email when present. | ||
| - Return a clear `401` response when no Clerk session exists. | ||
| - Return a clear `409` response when a Clerk session exists but the local user row has not been synchronized yet. | ||
| - Expose the local user as `ctx.user`. | ||
| - Expose the local user's normalized primary email as `ctx.userEmail`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize primary email at the synchronization boundary.
src/server/clerk/user-sync.ts:58-77 currently preserves email casing and whitespace, while shares are lowercased and trimmed. Consequently, Reviewer@Example.com will not match reviewer@example.com, breaking shared reads and listings. Use one normalization function for Clerk sync, ctx.userEmail, and share inputs, with mixed-case tests.
🤖 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 `@docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md` around lines
38 - 45, Normalize the primary email in the Clerk synchronization flow,
including src/server/clerk/user-sync.ts and the code exposing ctx.userEmail, by
reusing the same shared normalization function used for share inputs. Ensure
trimming and lowercasing occur at the synchronization boundary so all downstream
comparisons use the normalized value, and add mixed-case coverage for
synchronization, ctx.userEmail, and share operations.
| API schemas: | ||
|
|
||
| - `createDocumentRequestSchema` | ||
| - `name`: non-empty string. | ||
| - `description`: optional string; blank strings may be normalized to `null`. | ||
| - `html`: non-empty string. | ||
| - `updateDocumentRequestSchema` | ||
| - `html`: non-empty string. | ||
| - `getDocumentQuerySchema` | ||
| - `version`: optional positive integer parsed from the query string. | ||
| - `listDocumentsQuerySchema` | ||
| - `access`: optional enum `"all" | "owned" | "shared"`, default `"all"`. | ||
| - `shareDocumentRequestSchema` | ||
| - `emails`: non-empty array of valid email strings. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Bound document and sharing payload sizes.
These schemas only require non-empty values, so authenticated callers can submit arbitrarily large HTML or recipient arrays. That creates unbounded parsing, database storage, and insert work; immutable versions make oversized content persistent. Add explicit maximum lengths/counts and enforce a request-body limit before parsing.
🤖 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 `@docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md` around lines
198 - 211, Update the API schemas createDocumentRequestSchema,
updateDocumentRequestSchema, and shareDocumentRequestSchema with explicit
maximum HTML/string lengths and recipient-array counts, and add a request-body
size limit before request parsing in the documented API flow. Preserve the
existing non-empty, optional, and enum validation behavior while ensuring
oversized payloads are rejected before parsing or persistence.
| Behavior: | ||
|
|
||
| 1. Build `ApiContext`. | ||
| 2. Validate request body with Zod. | ||
| 3. Insert `documents` with the current local user as owner. | ||
| 4. Insert `document_versions` with `versionNumber = 1`. | ||
| 5. Return `201`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make every multi-write document workflow atomic.
Both creation and update can leave partially committed state unless all related writes share one transaction.
docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md#L241-L247: transactionally insert the document and initial version.docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md#L316-L318: transactionally insert the new version and updatedocuments.updatedAt.
📍 Affects 1 file
docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md#L241-L247(this comment)docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md#L316-L318
🤖 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 `@docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md` around lines
241 - 247, Make both multi-write document workflows atomic in
docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md: lines 241-247
must transactionally insert the document and its initial document_version, while
lines 316-318 must transactionally insert the new version and update
documents.updatedAt. Ensure each related write shares one transaction and
preserves the existing response behavior.
| Behavior: | ||
|
|
||
| 1. Build `ApiContext`. | ||
| 2. Validate route params and query string with Zod. | ||
| 3. Verify the current user can access the document. | ||
| 4. Load the requested version or latest version. | ||
| 5. Return `404` if the document or requested version does not exist. | ||
| 6. Return `403` if the document exists but the current user cannot access it. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid disclosing document existence to unauthorized users.
Returning 403 for existing documents and 404 for missing ones lets authenticated callers probe document IDs; those IDs are returned by the API. Prefer 404 for both inaccessible and missing documents, or explicitly document and mitigate this existence disclosure.
🤖 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 `@docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md` around lines
271 - 278, Update the documented API behavior so unauthorized access to an
existing document returns 404, matching the response for missing documents and
preventing document-existence disclosure. Revise the relevant behavior steps to
remove the separate 403 outcome while preserving validation, authorization, and
version-loading semantics.
| ## Testing | ||
|
|
||
| Use Vitest and follow the existing unit-test style. | ||
|
|
||
| Test the contract layer: | ||
|
|
||
| - Create request accepts valid input. | ||
| - Create request rejects missing name and empty HTML. | ||
| - Query schema parses positive integer versions. | ||
| - Share schema normalizes and deduplicates emails. | ||
| - DB insert schemas reject invalid persisted shapes. | ||
|
|
||
| Test the API context helper: | ||
|
|
||
| - Returns `401` when Clerk has no authenticated user. | ||
| - Returns `409` when a Clerk user exists but no active local user row exists. | ||
| - Returns `ApiContext` with local user and normalized email when present. | ||
|
|
||
| Test the API foundation: | ||
|
|
||
| - Handler wrapper catches typed `ApiError` instances and returns the expected status and JSON error envelope. | ||
| - Handler wrapper catches unexpected errors, logs them through `src/server/foundation/logs.ts`, and returns a generic `500`. | ||
| - Request parser in the handler layer returns `400` for malformed JSON. | ||
| - Zod request helpers in the handler layer format validation failures without echoing submitted HTML. | ||
| - Completion logs include request id, method, pathname, status, duration, and user id when available. | ||
|
|
||
| Test the document service: | ||
|
|
||
| - Create inserts metadata and initial version. | ||
| - Get latest version defaults correctly. | ||
| - Get specific version returns requested HTML. | ||
| - Owner can read and update. | ||
| - Shared email can read. | ||
| - Shared email cannot update. | ||
| - Owner and shared user can share. | ||
| - List returns owned, shared, or all based on the filter. | ||
| - User without primary email gets no shared documents. | ||
|
|
||
| Test HTTP handlers with mocked service/context dependencies where useful: | ||
|
|
||
| - `POST /api/docs` returns `201`. | ||
| - `GET /api/docs/{id}` returns latest by default. | ||
| - `PUT /api/docs/{id}` returns `403` for non-owner. | ||
| - `GET /api/docs?access=shared` validates the filter. | ||
| - `POST /api/docs/share/{id}` validates emails. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add tests for the core invariants.
The test plan omits create rollback, concurrent update retry/atomicity, normalized email matching, and unauthorized-document response behavior. These should be required coverage because they protect the design’s main data-integrity and authorization contracts.
🧰 Tools
🪛 LanguageTool
[style] ~482-~482: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... but no active local user row exists. - Returns ApiContext with local user and normal...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~508-~508: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test by default. - PUT /api/docs/{id} returns 403 for non-owner. - `GET /api/docs?a...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 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 `@docs/superpowers/specs/2026-06-12-docs-api-versioning-design.md` around lines
466 - 510, Expand the Testing section to require coverage for core invariants:
create rollback on failure, concurrent update retry and atomicity,
normalized-email matching for sharing/access, and unauthorized-document
responses that preserve the intended security behavior. Place these alongside
the existing document service and HTTP handler tests, without removing current
coverage.
1 participant
What
Production deploys stop happening on every merge to
mainand now run when a GitHub release is published.vercel.json: disables Vercel's Git-triggered deploys formain. PR previews are unaffected — only production auto-deploy is turned off..github/workflows/production-deploy.yml: onrelease: published, checks out the release tag and deploys via the Vercel CLI (pull→build --prod→deploy --prebuilt --prod). Pre-releases are skipped.Required before merge
Add these repository secrets, otherwise the workflow fails and nothing deploys:
VERCEL_TOKEN— Vercel → Account Settings → TokensVERCEL_ORG_IDandVERCEL_PROJECT_ID— from.vercel/project.jsonafter runningvercel linkNotes
🤖 Generated with Claude Code