P2 Zapier: public app attachment uploads#702
Conversation
Add the Zapier manifest, auth test, trigger polling, action wrappers, webhook subscription registration, OAuth bearer validation, and setup docs for #589. Constraint: Full reliable webhook delivery worker is not present in this branch, so this slice stores signed Zapier webhook subscriptions and documents the delivery handoff. Tested: make check; npx vitest run tests/zapier.test.ts tests/zapier-routes.test.ts; npx vitest run tests/account-security-route.test.ts tests/workspace-invite-route.test.ts tests/team-cycles-cycleId-route.test.ts tests/project-templates-route.test.ts; npx vitest run tests/auth.test.ts; npx vitest run tests/security-settings-view.test.tsx; npx vitest run tests/agent-logic.test.tsx Not-tested: make test times out under full-suite concurrency in unrelated suites that pass individually; make test-e2e is blocked by local Postgres password authentication failure for user postgres. Confidence: medium Co-authored-by: OmX <omx@oh-my-codex.dev>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99a49bda5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "/api/workspaces", | ||
| "/api/test", | ||
| "/api/account", | ||
| "/api/zapier", |
There was a problem hiding this comment.
The Zapier app exchanges the authorization code by POSTing to /api/oauth/token from Zapier's servers, which do not have a Better Auth session cookie. This change only makes /api/zapier public, so proxy() still redirects unauthenticated /api/oauth/token requests to /login before the token route runs; the OAuth connection flow will fail immediately after the user authorizes the app.
Useful? React with 👍 / 👎.
| const teamRecord = | ||
| readString(body.teamId) || readString(body.teamKey) | ||
| ? await findTeamForZapier(context.workspaceId, body) | ||
| : null; |
There was a problem hiding this comment.
When create_project includes an invalid teamId or teamKey, the project row is inserted first and only then findTeamForZapier throws. Zapier receives an error, but the workspace is left with a newly created unlinked project, so a typo in the optional team field corrupts data instead of making the action atomic.
Useful? React with 👍 / 👎.
| .orderBy(desc(issueHistory.createdAt)) | ||
| .limit(limit * 3); |
There was a problem hiding this comment.
For the status_change trigger, the query fetches only the latest limit * 3 generic update history rows and filters for changedFields.includes("stateId") afterward. In workspaces with more than three times the requested limit of non-status updates after a status change, Zapier will never see those status changes even though they are newer than the cursor; the state-change predicate needs to be applied before limiting.
Useful? React with 👍 / 👎.
| ...commonIssueFields.filter((field) => | ||
| typeof field === "string" | ||
| ? field !== "teamKey" && field !== "teamId" | ||
| : field.key !== "teamKey" && field.key !== "teamId", | ||
| ), |
There was a problem hiding this comment.
The Update Issue action reuses commonIssueFields, whose title field is marked required, so Zapier will force users to supply a title even when they only want to change status, priority, assignee, or another field. The backend supports partial updates with any one editable field, but this public app definition makes those valid update scenarios unusable from Zapier.
Useful? React with 👍 / 👎.
|
Controller intake: blocked before QA/merge. Current blocker:
Required fix:
|
|
This PR's changed files are on the pre-monorepo-split Conflict resolution can't fix this — it needs to be rebuilt on the current architecture from the spec in #589 against current |
1 participant
Summary
Verification