-
Notifications
You must be signed in to change notification settings - Fork 0
fix: SCIM compatibility, user deletion FK constraints, and agent update UX #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cac071b
2d22d67
ee11dc8
c5c3dde
cf08c3b
9da431d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| -- DropForeignKey | ||
| ALTER TABLE "TeamMember" DROP CONSTRAINT "TeamMember_userId_fkey"; | ||
| ALTER TABLE "VrlSnippet" DROP CONSTRAINT "VrlSnippet_createdBy_fkey"; | ||
| ALTER TABLE "DeployRequest" DROP CONSTRAINT "DeployRequest_requestedById_fkey"; | ||
| ALTER TABLE "ServiceAccount" DROP CONSTRAINT "ServiceAccount_createdById_fkey"; | ||
|
|
||
| -- AlterTable (make columns nullable where needed) | ||
| ALTER TABLE "VrlSnippet" ALTER COLUMN "createdBy" DROP NOT NULL; | ||
| ALTER TABLE "DeployRequest" ALTER COLUMN "requestedById" DROP NOT NULL; | ||
| ALTER TABLE "ServiceAccount" ALTER COLUMN "createdById" DROP NOT NULL; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "TeamMember" ADD CONSTRAINT "TeamMember_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
| ALTER TABLE "VrlSnippet" ADD CONSTRAINT "VrlSnippet_createdBy_fkey" FOREIGN KEY ("createdBy") REFERENCES "User"("id") ON DELETE SET NULL ON UPDATE CASCADE; | ||
| ALTER TABLE "DeployRequest" ADD CONSTRAINT "DeployRequest_requestedById_fkey" FOREIGN KEY ("requestedById") REFERENCES "User"("id") ON DELETE SET NULL ON UPDATE CASCADE; | ||
| ALTER TABLE "ServiceAccount" ADD CONSTRAINT "ServiceAccount_createdById_fkey" FOREIGN KEY ("createdById") REFERENCES "User"("id") ON DELETE SET NULL ON UPDATE CASCADE; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -367,7 +367,7 @@ export const deployRouter = router({ | |
| try { | ||
| const result = await deployAgent( | ||
| request.pipelineId, | ||
| request.requestedById, | ||
| request.requestedById ?? ctx.session.user.id, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Admin misattributed as requester in PipelineVersion When
So the admin who clicks "Approve" now permanently appears as the person who both requested and deployed the pipeline version — not just as the reviewer. The Consider passing // Option: pass null and let deployAgent handle it
const result = await deployAgent(
request.pipelineId,
request.requestedById, // may be null — deployAgent should handle
request.changelog,
request.configYaml,
);Prompt To Fix With AIThis is a comment left during a code review.
Path: src/server/routers/deploy.ts
Line: 370
Comment:
**Admin misattributed as requester in PipelineVersion**
When `requestedById` is `null` (the original requester was deleted), the fallback `ctx.session.user.id` is passed as the `userId` to `deployAgent`. Inside `deployAgent`, this value is used in two places:
1. `createVersion(pipelineId, ..., userId, ...)` — the immutable `PipelineVersion` snapshot is permanently stamped with the approving admin's ID as the version creator.
2. `prisma.user.findUnique({ where: { id: userId } })` — the git sync commit is attributed to the admin.
So the admin who clicks "Approve" now permanently appears as the person who both requested and deployed the pipeline version — not just as the reviewer. The `DeployRequest` row correctly records `reviewedById = ctx.session.user.id`, but the `PipelineVersion` is a separate record that future audits and git history will show as authored by the admin.
Consider passing `null` (or a sentinel like `"deleted-user"`) and updating `createVersion`/`deployAgent` to accept `string | null` for the userId, then treating null as "original requester deleted":
```typescript
// Option: pass null and let deployAgent handle it
const result = await deployAgent(
request.pipelineId,
request.requestedById, // may be null — deployAgent should handle
request.changelog,
request.configYaml,
);
```
How can I resolve this? If you propose a fix, please make it concise. |
||
| request.changelog, | ||
| request.configYaml, | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Audit action
scim.group_deletedis misleadingThe handler intentionally preserves the team (to avoid cascading deletes of environments, pipelines, etc.) — it only removes memberships. But the audit entry fires
"scim.group_deleted", which would lead an admin reviewing the audit trail to believe the team was removed when it wasn't.Consider using an action name that accurately describes what occurred:
Prompt To Fix With AI