Skip to content

Test/pr wire#21

Closed
div0rce wants to merge 21 commits into
mainfrom
test/pr-wire
Closed

Test/pr wire#21
div0rce wants to merge 21 commits into
mainfrom
test/pr-wire

Conversation

@div0rce

@div0rce div0rce commented Apr 28, 2026

Copy link
Copy Markdown
Owner

Status: Active
Last updated: 2026-01-02

Pull Request Checklist

Current behavior

  • Use this template for all PRs to ensure CI and guardrails are respected.

Summary

Testing

  • Not run (explain why)
  • npm run check
  • npm test
  • npm run build
  • npm run ci:verify

Engine Impact

  • This PR does not modify engine behavior (no changes under lib/engine, lib/vine, engine APIs, or engine-adjacent files).
  • This PR does modify engine behavior and is allowed because:
    • Reason:
    • Linked justification in docs/engine-roadmap.md:

During an active engine freeze, engine-changing PRs are prohibited unless explicitly whitelisted in docs/engine-roadmap.md. CI enforces this via check:engine-freeze.

Future/Target behavior

  • If CI entrypoints change, update this checklist to match.

Related docs

  • CONTRIBUTING.md
  • docs/ci-and-guardrails.md
  • docs/engine-roadmap.md

@vercel

vercel Bot commented Apr 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cherry Error Error Apr 28, 2026 6:21pm

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a8eef6001

ℹ️ 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".

on:
workflow_run:
workflows:
- CI

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Match workflow_run trigger to actual CI workflow name

workflow_run.workflows is configured as CI, but this repo’s CI workflow is named ci in .github/workflows/ci.yml (name: ci). Because this filter matches workflow names, the notifier job will never run after CI completions, so n8n will miss all intended events.

Useful? React with 👍 / 👎.

Comment thread lib/automation/events.ts
Comment on lines +172 to +173
const existing = await findAutomationEventByIdempotencyKey(input.idempotencyKey);
if (existing !== null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make automation event idempotency write atomic

This idempotency flow does a read-before-write (find...ByIdempotencyKey then create later), so two concurrent requests with the same key can both observe “missing” and race into create. The second insert will hit the DB unique constraint and currently bubbles as an unhandled error (500) instead of returning the existing event or a structured idempotency response, which breaks webhook retry safety under concurrent delivery.

Useful? React with 👍 / 👎.

Comment on lines +95 to +96
const existing = await findStatusCheckByIdempotencyKey(statusIdempotencyKey);
if (existing !== null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make GitHub status idempotency race-safe

This has the same non-atomic read-then-create pattern for statusIdempotencyKey: concurrent posts can both pass the existence check and one will fail on the unique index during create. Since that DB uniqueness error is not converted into an idempotent response, legitimate parallel retries can fail with 500/502 instead of returning the already-created status record.

Useful? React with 👍 / 👎.

@div0rce div0rce closed this Apr 28, 2026
@div0rce div0rce deleted the test/pr-wire branch April 28, 2026 18:38
@div0rce

div0rce commented Apr 28, 2026

Copy link
Copy Markdown
Owner Author

this shit was genuinely a disaster im never listening to instagram again

@div0rce div0rce added the wontfix This will not be worked on label Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant