test: trigger webhook#1
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a complete GitHub App webhook server infrastructure for DocSync. It adds environment configuration for GitHub App credentials, establishes GitHub API client utilities, implements webhook server handling with Express, creates PR processing logic for drift detection, and sets up a Node.js entrypoint to orchestrate the webhook service. Changes
Sequence Diagram(s)sequenceDiagram
actor GitHub as GitHub Platform
participant Webhook as Webhook Server<br/>(Express)
participant Client as GitHub Client<br/>(`@octokit/app`)
participant Processor as PR Processor
participant API as GitHub REST API
participant Storage as File System<br/>(temp/snapshot)
GitHub->>Webhook: POST webhook event<br/>(pull_request.opened)
Webhook->>Webhook: Verify signature
Webhook->>Webhook: Extract PR/repo details
Webhook->>Client: getInstallationOctokit(installationId)
Client->>Client: Rotate/fetch installation token
Client-->>Webhook: Return authenticated Octokit
Webhook->>Processor: processPR(octokit, PR context)
Processor->>API: getPRFiles(pullNumber)
API-->>Processor: List of changed files
Processor->>Storage: Create temp directory
Processor->>API: getFileContent (for each file)
API-->>Processor: File contents
Processor->>Storage: Write files to temp
Processor->>Storage: Load snapshot.json from base branch
Processor->>Processor: analyzeDrift(files, snapshot)
alt High drift detected
Processor->>Storage: Write drift-report.md
Processor->>API: createBranch (companion)
API-->>Processor: New branch created
Processor->>API: commitFile (drift report)
API-->>Processor: File committed
Processor->>API: createPullRequest (companion)
API-->>Processor: Companion PR created
Processor->>API: postPRComment (drift info + links)
else No drift or zero score
Processor->>API: postPRComment (docs in sync)
end
Processor->>Storage: Cleanup temp directory
Processor-->>Webhook: Return result
Webhook-->>GitHub: HTTP 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/github/pr-processor.js`:
- Around line 263-266: The catch currently turns any snapshot read failure into
buildEmptySnapshot(), causing false drift; change the handler so only a genuine
"file not found" (e.g., error.code === 'ENOENT' or equivalent) falls back to
buildEmptySnapshot(), while other errors are logged with full error details and
rethrown (or allowed to propagate) so the PR processor fails loudly instead of
creating spurious baselines; update the catch to check error.code, include error
object in logger.warn/error, and rethrow non-ENOENT errors (referencing the
existing buildEmptySnapshot() call and the surrounding snapshot-load try/catch).
- Around line 315-372: The catch is conflating failures posting comments with
companion PR creation; split responsibilities so
createBranch/commitFile/createPullRequest are wrapped to detect PR creation
failure separately from postDriftComment. Specifically: keep the existing try
around getting baseSha, createBranch, commitFile and createPullRequest
(functions: getBranchSha, createBranch, commitFile, createPullRequest), but
after successfully creating the companionPR call postDriftComment in its own
try/catch so a comment failure only logs an error (and optionally returns a
companionCommentError or companionCommentPosted=false) without changing the
action to 'drift_detected_pr_failed'; only use the outer catch to handle true
PR-creation failures and call postDriftWarningComment there
(postDriftWarningComment) as a best-effort fallback. Ensure logger messages and
the returned object reflect whether the companion PR was created even if the
follow-up comment failed.
In `@src/github/webhook-server.js`:
- Around line 197-201: The current loop-prevention (if
pr.head.ref.startsWith('docsync/')) is too broad and can be bypassed; change it
to require both the branch prefix AND a trusted bot/app author check before
skipping. Update the webhook handler that references pr.head.ref and pullNumber
to verify pr.head.ref.startsWith('docsync/') AND that the PR author
(pr.user.login or the webhook actor id) matches the configured DocSync bot/app
identity (e.g., 'docsync-bot' or the app installation ID); only then log and
return. Ensure the log message mentions both the branch and author for clarity
and keep the shortcut check centralized so other handlers reuse the same
isDocSyncBot+branch predicate.
In `@src/index.js`:
- Around line 8-12: The code parses WEBHOOK_PORT into the variable port but
never validates it before calling startWebhookServer(port); ensure the parsed
value from process.env.WEBHOOK_PORT is a finite integer and within a valid port
range (e.g., 1–65535) and not NaN. If validation fails, log a clear error via
logger.error (including the raw env value) and exit (process.exit(1)) or fall
back to a safe default before calling startWebhookServer; update the parseInt
usage and the start sequence around the port variable and startWebhookServer to
perform this check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c43cf0a4-a63c-4380-a460-01107a5aa564
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.env.example.gitignorepackage.jsonsrc/github/client.jssrc/github/pr-processor.jssrc/github/webhook-server.jssrc/index.jssrc/utils/logger.js
| } catch (error) { | ||
| logger.warn(`Could not load snapshot: ${error.message}. Using empty baseline.`); | ||
| return buildEmptySnapshot(); | ||
| } |
There was a problem hiding this comment.
Don’t default to empty snapshot on every load error.
Line 263-Line 266 currently converts any snapshot read failure into an empty baseline, which can generate false drift and unnecessary PR churn.
Suggested fix
} catch (error) {
- logger.warn(`Could not load snapshot: ${error.message}. Using empty baseline.`);
- return buildEmptySnapshot();
+ logger.error(`Could not load snapshot: ${error.message}`);
+ throw error;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| logger.warn(`Could not load snapshot: ${error.message}. Using empty baseline.`); | |
| return buildEmptySnapshot(); | |
| } | |
| } catch (error) { | |
| logger.error(`Could not load snapshot: ${error.message}`); | |
| throw error; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/github/pr-processor.js` around lines 263 - 266, The catch currently turns
any snapshot read failure into buildEmptySnapshot(), causing false drift; change
the handler so only a genuine "file not found" (e.g., error.code === 'ENOENT' or
equivalent) falls back to buildEmptySnapshot(), while other errors are logged
with full error details and rethrown (or allowed to propagate) so the PR
processor fails loudly instead of creating spurious baselines; update the catch
to check error.code, include error object in logger.warn/error, and rethrow
non-ENOENT errors (referencing the existing buildEmptySnapshot() call and the
surrounding snapshot-load try/catch).
| try { | ||
| // Get the SHA of the base branch to branch from | ||
| const baseSha = await getBranchSha(octokit, owner, repo, baseBranch); | ||
|
|
||
| // Create the companion branch | ||
| await createBranch(octokit, owner, repo, companionBranch, baseSha); | ||
| logger.success(`Created branch: ${companionBranch}`); | ||
|
|
||
| // Commit the documentation file to the companion branch | ||
| const docFilePath = 'docs/drift-report.md'; | ||
| await commitFile( | ||
| octokit, | ||
| owner, | ||
| repo, | ||
| docFilePath, | ||
| docContent, | ||
| `docs: auto-update documentation for PR #${pullNumber} [DocSync]`, | ||
| companionBranch | ||
| ); | ||
| logger.success(`Committed documentation to ${companionBranch}`); | ||
|
|
||
| // Open the companion PR | ||
| const companionPR = await createPullRequest(octokit, owner, repo, { | ||
| title: `📄 DocSync: Update docs for PR #${pullNumber} — "${prTitle}"`, | ||
| body: buildCompanionPRBody(driftReport, pullNumber), | ||
| head: companionBranch, | ||
| base: baseBranch, | ||
| }); | ||
| logger.success(`Opened companion PR #${companionPR.number}: ${companionPR.html_url}`); | ||
|
|
||
| // Post a comment on the original PR linking to the companion | ||
| await postDriftComment( | ||
| octokit, owner, repo, pullNumber, driftReport, companionPR | ||
| ); | ||
|
|
||
| return { | ||
| action: 'companion_pr_created', | ||
| driftScore: driftReport.driftScore, | ||
| companionPRNumber: companionPR.number, | ||
| companionPRUrl: companionPR.html_url, | ||
| }; | ||
|
|
||
| } catch (error) { | ||
| logger.error(`Failed to create companion PR: ${error.message}`); | ||
|
|
||
| // Even if PR creation fails, post a comment warning about drift | ||
| try { | ||
| await postDriftWarningComment(octokit, owner, repo, pullNumber, driftReport); | ||
| } catch (commentError) { | ||
| logger.error(`Also failed to post comment: ${commentError.message}`); | ||
| } | ||
|
|
||
| return { | ||
| action: 'drift_detected_pr_failed', | ||
| driftScore: driftReport.driftScore, | ||
| error: error.message, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Separate companion PR creation from follow-up comment posting.
The single catch block treats comment-post failures as full PR-creation failures, even when the companion PR was already created successfully.
Suggested fix
- try {
+ try {
// ... create branch, commit docs, open companion PR
const companionPR = await createPullRequest(octokit, owner, repo, {
title: `📄 DocSync: Update docs for PR #${pullNumber} — "${prTitle}"`,
body: buildCompanionPRBody(driftReport, pullNumber),
head: companionBranch,
base: baseBranch,
});
logger.success(`Opened companion PR #${companionPR.number}: ${companionPR.html_url}`);
- // Post a comment on the original PR linking to the companion
- await postDriftComment(
- octokit, owner, repo, pullNumber, driftReport, companionPR
- );
+ try {
+ await postDriftComment(octokit, owner, repo, pullNumber, driftReport, companionPR);
+ } catch (commentError) {
+ logger.warn(`Companion PR created, but failed to post PR comment: ${commentError.message}`);
+ }
return {
action: 'companion_pr_created',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/github/pr-processor.js` around lines 315 - 372, The catch is conflating
failures posting comments with companion PR creation; split responsibilities so
createBranch/commitFile/createPullRequest are wrapped to detect PR creation
failure separately from postDriftComment. Specifically: keep the existing try
around getting baseSha, createBranch, commitFile and createPullRequest
(functions: getBranchSha, createBranch, commitFile, createPullRequest), but
after successfully creating the companionPR call postDriftComment in its own
try/catch so a comment failure only logs an error (and optionally returns a
companionCommentError or companionCommentPosted=false) without changing the
action to 'drift_detected_pr_failed'; only use the outer catch to handle true
PR-creation failures and call postDriftWarningComment there
(postDriftWarningComment) as a best-effort fallback. Ensure logger messages and
the returned object reflect whether the companion PR was created even if the
follow-up comment failed.
| // Skip PRs from DocSync itself — prevents infinite loops | ||
| // (DocSync opens a companion PR → that triggers another webhook → infinite) | ||
| if (pr.head.ref.startsWith('docsync/')) { | ||
| logger.info(`PR #${pullNumber} is from DocSync — skipping to prevent loop`); | ||
| return; |
There was a problem hiding this comment.
Loop-prevention check is too broad and allows easy bypass.
Line 199 skips all PRs from docsync/* branches, so non-DocSync contributors can bypass processing by choosing that prefix.
Use a stricter predicate (e.g., branch prefix and trusted bot/app author check) before skipping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/github/webhook-server.js` around lines 197 - 201, The current
loop-prevention (if pr.head.ref.startsWith('docsync/')) is too broad and can be
bypassed; change it to require both the branch prefix AND a trusted bot/app
author check before skipping. Update the webhook handler that references
pr.head.ref and pullNumber to verify pr.head.ref.startsWith('docsync/') AND that
the PR author (pr.user.login or the webhook actor id) matches the configured
DocSync bot/app identity (e.g., 'docsync-bot' or the app installation ID); only
then log and return. Ensure the log message mentions both the branch and author
for clarity and keep the shortcut check centralized so other handlers reuse the
same isDocSyncBot+branch predicate.
| const port = parseInt(process.env.WEBHOOK_PORT || '3000', 10); | ||
|
|
||
| logger.info(`Starting DocSync webhook server (Node.js ${process.version})`); | ||
|
|
||
| startWebhookServer(port); No newline at end of file |
There was a problem hiding this comment.
Validate WEBHOOK_PORT before starting the server.
Line 8 parses the port but never validates it. Invalid values can crash startup at runtime.
Suggested fix
-const port = parseInt(process.env.WEBHOOK_PORT || '3000', 10);
+const rawPort = process.env.WEBHOOK_PORT || '3000';
+const port = Number(rawPort);
+if (!Number.isInteger(port) || port < 1 || port > 65535) {
+ logger.error(`Invalid WEBHOOK_PORT: "${rawPort}". Expected an integer between 1 and 65535.`);
+ process.exit(1);
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.js` around lines 8 - 12, The code parses WEBHOOK_PORT into the
variable port but never validates it before calling startWebhookServer(port);
ensure the parsed value from process.env.WEBHOOK_PORT is a finite integer and
within a valid port range (e.g., 1–65535) and not NaN. If validation fails, log
a clear error via logger.error (including the raw env value) and exit
(process.exit(1)) or fall back to a safe default before calling
startWebhookServer; update the parseInt usage and the start sequence around the
port variable and startWebhookServer to perform this check.
🔍 DocSync — Documentation Drift DetectedThis PR changes code that has documentation implications. Drift Score: What DocSync Found
Action Taken✅ DocSync has automatically opened PR #2 with updated documentation. Please review and merge the documentation PR alongside this one. 🤖 DocSync — Docs that stay true to your code |
📄 DocSync: Update docs for PR #1 — "test: trigger webhook"
Summary by CodeRabbit
New Features
Chores
.gitignoreto exclude credential files