feat: add Devin attribution check workflow with Slack notification#6
Closed
iornstein wants to merge 2 commits into
Closed
feat: add Devin attribution check workflow with Slack notification#6iornstein wants to merge 2 commits into
iornstein wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds an attribution-check workflow and supporting scripts/tests: CI, a workflow template that detects PRs from the bot, a script to parse a "Requested by" username, post a PR comment, and optionally DM the requester via Slack; includes tests, package manifest, and README docs. (44 words) Changes
Sequence DiagramsequenceDiagram
participant GH as GitHub
participant Action as Devin Attribution<br/>Check Action
participant GHApi as GitHub API
participant SlackApi as Slack API
GH->>Action: trigger on PR from bot
Action->>Action: parse "Requested by: `@username`" from PR body
Action->>GHApi: get user by username
GHApi-->>Action: return user (may include email)
alt email present and SLACK_TOKEN set
Action->>SlackApi: users.lookupByEmail
SlackApi-->>Action: return slack_user_id
Action->>SlackApi: conversations.open
SlackApi-->>Action: return conversation_id
Action->>SlackApi: chat.postMessage (DM with PR URL)
SlackApi-->>Action: confirm
else missing email or SLACK_TOKEN
Action->>Action: skip Slack DM
end
Action->>GHApi: issues.createComment (post PR comment with marker)
GHApi-->>Action: confirm comment created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
iornstein
requested review from
a team and
gosha-cb
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/__tests__/devin-attribution-check.test.js (1)
85-98: Minor redundancy in null body test setup.Line 89 creates context with
body: null, then line 90 sets it again. The second assignment is unnecessary.♻️ Suggested simplification
describe("when PR body has no body at all", () => { it("skips without taking any action", async () => { const core = createMockCore(); const github = createMockGithub(); - const context = createMockContext({ body: null }); - context.payload.pull_request.body = null; + const context = createMockContext({ body: null }); await run({ github, context, core });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/__tests__/devin-attribution-check.test.js` around lines 85 - 98, The test duplicates setting the PR body to null: createMockContext({ body: null }) already sets context.payload.pull_request.body, so remove the redundant explicit assignment context.payload.pull_request.body = null in the "when PR body has no body at all" test; keep the createMockContext call and the call to run({ github, context, core }) and the expect on core.info to preserve behavior.scripts/devin-attribution-check.js (1)
82-91: Consider checking for existing comments to prevent duplicates on workflow re-runs.If the workflow is re-triggered (e.g., by closing and reopening a PR, or manual re-run),
postPrCommentwill create another comment. Consider querying for existing comments withPR_COMMENT_MARKERbefore posting.♻️ Proposed fix to check for existing comments
async function postPrComment(github, context, requestedBy) { const commentBody = `${PR_COMMENT_MARKER}\n${PR_COMMENT_MESSAGE(requestedBy)}`; + // Check if we already posted this comment + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.payload.pull_request.number, + }); + + const existingComment = comments.find((c) => c.body.includes(PR_COMMENT_MARKER)); + if (existingComment) { + return; // Already posted + } + await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.payload.pull_request.number, body: commentBody, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/devin-attribution-check.js` around lines 82 - 91, postPrComment currently always creates a new comment which can produce duplicates on workflow re-runs; modify postPrComment to first list existing issue comments via github.rest.issues.listComments for the pull request (use context.repo.owner, context.repo.repo, context.payload.pull_request.number), check for any comment whose body contains PR_COMMENT_MARKER, and only call github.rest.issues.createComment if no matching comment exists; keep function signature and ensure it still composes commentBody using PR_COMMENT_MARKER and PR_COMMENT_MESSAGE(requestedBy).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 18: The README lists SLACK_DEPLNOTIF_APP_SIGNING_SECRET as a required
org-level secret but the workflow only uses SLACK_DEPLNOTIF_APP_TOKEN; remove
SLACK_DEPLNOTIF_APP_SIGNING_SECRET from the "**Required secrets**" list and
update the surrounding text to mention only SLACK_DEPLNOTIF_APP_TOKEN so the
docs match the actual workflow usage.
- Around line 15-16: Update the README bullets to reflect actual behavior: the
script always posts a PR comment first and then attempts to send a Slack DM,
rather than treating the PR comment as a fallback; change the current items 3
and 4 to state (a) a PR comment is posted unconditionally (the call that posts
the PR comment) and (b) a Slack DM is attempted afterward and the DM may fail
for reasons like no public email on the GitHub profile. Reference the code path
that posts the PR comment and the subsequent Slack DM attempt when editing the
text so it matches the real execution order.
In `@scripts/devin-attribution-check.js`:
- Line 96: The current regex in the body.match call (const match =
body.match(/Requested by:\s*@(\S+)/i)) can capture trailing punctuation; update
that expression to only capture valid GitHub username characters by replacing
\S+ with a stricter class and boundary, e.g. use body.match(/Requested
by:\s*@([A-Za-z0-9-]+)\b/i) (refer to the const match and body.match usage) so
usernames like "someuser." do not include the trailing period.
---
Nitpick comments:
In `@scripts/__tests__/devin-attribution-check.test.js`:
- Around line 85-98: The test duplicates setting the PR body to null:
createMockContext({ body: null }) already sets
context.payload.pull_request.body, so remove the redundant explicit assignment
context.payload.pull_request.body = null in the "when PR body has no body at
all" test; keep the createMockContext call and the call to run({ github,
context, core }) and the expect on core.info to preserve behavior.
In `@scripts/devin-attribution-check.js`:
- Around line 82-91: postPrComment currently always creates a new comment which
can produce duplicates on workflow re-runs; modify postPrComment to first list
existing issue comments via github.rest.issues.listComments for the pull request
(use context.repo.owner, context.repo.repo,
context.payload.pull_request.number), check for any comment whose body contains
PR_COMMENT_MARKER, and only call github.rest.issues.createComment if no matching
comment exists; keep function signature and ensure it still composes commentBody
using PR_COMMENT_MARKER and PR_COMMENT_MESSAGE(requestedBy).
🪄 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: CHILL
Plan: Pro
Run ID: 5964a414-ae9b-4c03-8d8c-c5cec82ace7d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.github/workflows/ci.yml.gitignoreREADME.mdpackage.jsonscripts/__tests__/devin-attribution-check.test.jsscripts/devin-attribution-check.jsworkflow-templates/devin-attribution-check.properties.jsonworkflow-templates/devin-attribution-check.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/__tests__/devin-attribution-check.test.js (1)
85-98: Add no-side-effect assertions for the null-body branch.This branch currently checks only the info log. Adding explicit assertions for no GitHub/Slack calls would lock behavior and prevent regressions.
✅ Suggested assertion additions
expect(core.info).toHaveBeenCalledWith( 'No "Requested by: `@username`" found in PR body. Skipping.', ); + expect(github.rest.users.getByUsername).not.toHaveBeenCalled(); + expect(github.rest.issues.createComment).not.toHaveBeenCalled(); + expect(fetch).not.toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/__tests__/devin-attribution-check.test.js` around lines 85 - 98, Add assertions to the "no body" test that verify there were no side effects: after calling run({ github, context, core }) assert that no GitHub API methods were invoked on the github mock (i.e., none of github.rest.* mock methods were called) and that no Slack client methods were invoked (e.g., slack.postMessage or slackClient.send if present); use the existing mocks (createMockGithub, createMockCore, createMockContext, core, github, context) and .not.toHaveBeenCalled() on the relevant mock methods to lock this behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/__tests__/devin-attribution-check.test.js`:
- Around line 7-8: The test suite overrides global.fetch (global.fetch =
jest.fn()) and mutates process.env.SLACK_TOKEN without restoring originals,
causing cross-test leakage; fix by saving the originals (e.g., const _origFetch
= global.fetch; const _origSlack = process.env.SLACK_TOKEN) before mutating and
restore them in an afterAll or afterEach block that runs after the suite
(restore global.fetch = _origFetch and process.env.SLACK_TOKEN = _origSlack) so
the test leaves global state unchanged; update the test file where global.fetch
and process.env.SLACK_TOKEN are set to include these save/restore steps.
---
Nitpick comments:
In `@scripts/__tests__/devin-attribution-check.test.js`:
- Around line 85-98: Add assertions to the "no body" test that verify there were
no side effects: after calling run({ github, context, core }) assert that no
GitHub API methods were invoked on the github mock (i.e., none of github.rest.*
mock methods were called) and that no Slack client methods were invoked (e.g.,
slack.postMessage or slackClient.send if present); use the existing mocks
(createMockGithub, createMockCore, createMockContext, core, github, context) and
.not.toHaveBeenCalled() on the relevant mock methods to lock this behavior.
🪄 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: CHILL
Plan: Pro
Run ID: fbf27d0f-2efb-4e9a-822b-41d4defe9379
📒 Files selected for processing (3)
README.mdscripts/__tests__/devin-attribution-check.test.jsscripts/devin-attribution-check.js
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/devin-attribution-check.js
Comment on lines
+7
to
+8
| // Mock global fetch | ||
| global.fetch = jest.fn(); |
There was a problem hiding this comment.
Restore global state after the suite to prevent cross-test leakage.
Line 8 overrides global.fetch, and Line 65 mutates process.env.SLACK_TOKEN, but prior values are never restored. This can cause flaky behavior in other test files running in the same worker.
🔧 Proposed fix
+const originalFetch = global.fetch;
+const originalSlackToken = process.env.SLACK_TOKEN;
+
// Mock global fetch
global.fetch = jest.fn();
...
beforeEach(() => {
jest.clearAllMocks();
delete process.env.SLACK_TOKEN;
});
+
+afterAll(() => {
+ global.fetch = originalFetch;
+ if (originalSlackToken === undefined) {
+ delete process.env.SLACK_TOKEN;
+ } else {
+ process.env.SLACK_TOKEN = originalSlackToken;
+ }
+});Also applies to: 63-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/__tests__/devin-attribution-check.test.js` around lines 7 - 8, The
test suite overrides global.fetch (global.fetch = jest.fn()) and mutates
process.env.SLACK_TOKEN without restoring originals, causing cross-test leakage;
fix by saving the originals (e.g., const _origFetch = global.fetch; const
_origSlack = process.env.SLACK_TOKEN) before mutating and restore them in an
afterAll or afterEach block that runs after the suite (restore global.fetch =
_origFetch and process.env.SLACK_TOKEN = _origSlack) so the test leaves global
state unchanged; update the test file where global.fetch and
process.env.SLACK_TOKEN are set to include these save/restore steps.
gosha-cb
reviewed
Mar 31, 2026
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 22 |
There was a problem hiding this comment.
we should not use node 22. We use 24 everywhere
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The workflow:
My plan is to create and org ruleset scoped to a particular test repo only to test that it works before rolling it out to be organization-wide (or more conservatively, a few real repositories first). I can alert the platform team when I roll it out.
In order to test it, I need this first merged in.