feat: forward PR and release metadata to CI run endpoint#5
Merged
Conversation
📝 WalkthroughWalkthroughAdds CI metadata extraction and propagates PR number and PR title into the trigger run request payload sent to the CI API. Changes
Sequence DiagramsequenceDiagram
participant Main as main.ts
participant CiMeta as getCiMetadata()
participant GHEnv as GitHub Actions (env)
participant API as triggerRun API
Main->>CiMeta: getCiMetadata()
CiMeta->>GHEnv: read GITHUB_EVENT_PATH
GHEnv-->>CiMeta: event JSON
CiMeta->>CiMeta: parse & extract prNumber, prTitle
CiMeta-->>Main: return CiMetadata
Main->>API: triggerRun(request + prNumber, prTitle)
API-->>Main: response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ci-metadata.ts (1)
24-28: Include parse/read error details in the warning.Current warning loses the root cause, making troubleshooting harder in CI logs.
Proposed patch
- } catch { - core.warning('Failed to read GitHub event payload for CI metadata') + } catch (error) { + core.warning( + `Failed to read GitHub event payload for CI metadata: ${ + error instanceof Error ? error.message : String(error) + }`, + ) return {} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ci-metadata.ts` around lines 24 - 28, The warning in the try/catch around JSON.parse(fs.readFileSync(eventPath, 'utf-8')) discards the thrown error; update the catch to capture the exception (e.g., catch (err)) and include its message/stack in the core.warning call so the log shows the read/parse failure details for eventPath and the variable/event name (event). Keep the behavior of returning {} but ensure core.warning contains both a descriptive message and the error details to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/ci-metadata.ts`:
- Around line 24-28: The warning in the try/catch around
JSON.parse(fs.readFileSync(eventPath, 'utf-8')) discards the thrown error;
update the catch to capture the exception (e.g., catch (err)) and include its
message/stack in the core.warning call so the log shows the read/parse failure
details for eventPath and the variable/event name (event). Keep the behavior of
returning {} but ensure core.warning contains both a descriptive message and the
error details to aid debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2945056d-1bd2-489e-ac1b-b53e82993e1b
📒 Files selected for processing (3)
src/api.tssrc/ci-metadata.tssrc/main.ts
Auto-detect prNumber and prTitle from pull_request / pull_request_target events via GITHUB_EVENT_PATH and forward them to the updated /api/v1/ci/run contract. Best-effort — no-op on events that don't carry PR metadata, no new action inputs required.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/ci-metadata.ts (1)
22-26: Include the parse/read error detail in the warning for debuggability.Current warning is generic; adding the caught error message makes failed event parsing much easier to triage in Actions logs.
💡 Proposed patch
- } catch { - core.warning('Failed to read GitHub event payload for CI metadata') + } catch (error) { + core.warning( + `Failed to read GitHub event payload for CI metadata: ${ + error instanceof Error ? error.message : String(error) + }`, + ) return {} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ci-metadata.ts` around lines 22 - 26, The catch block in src/ci-metadata.ts swallows the JSON read/parse error making debugging hard; update the try/catch around JSON.parse(fs.readFileSync(eventPath, 'utf-8')) to capture the thrown error (e.g., catch (err)) and include the error message (and optionally eventPath) in the core.warning call so the log reads something like "Failed to read GitHub event payload for CI metadata: <error>". Reference the event/eventPath usage and the core.warning call when applying the change.
🤖 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/ci-metadata.ts`:
- Around line 4-7: Update the CiMetadata type and the getCiMetadata() extractor
to include an optional releaseName string and ensure release-triggered runs
populate it: add releaseName?: string to the CiMetadata interface and in
getCiMetadata() detect release payloads (or the release object) and set
releaseName = release.name || release.tag_name (falling back to tag_name when
name is empty); keep existing prNumber/prTitle handling for PR runs and ensure
callers expecting releaseName will receive undefined for non-release runs.
---
Nitpick comments:
In `@src/ci-metadata.ts`:
- Around line 22-26: The catch block in src/ci-metadata.ts swallows the JSON
read/parse error making debugging hard; update the try/catch around
JSON.parse(fs.readFileSync(eventPath, 'utf-8')) to capture the thrown error
(e.g., catch (err)) and include the error message (and optionally eventPath) in
the core.warning call so the log reads something like "Failed to read GitHub
event payload for CI metadata: <error>". Reference the event/eventPath usage and
the core.warning call when applying the change.
🪄 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: ef71d417-f294-43d2-9019-1b78c24d45b7
📒 Files selected for processing (3)
src/api.tssrc/ci-metadata.tssrc/main.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main.ts
- src/api.ts
Comment on lines
+4
to
+7
| export interface CiMetadata { | ||
| prNumber?: number | ||
| prTitle?: string | ||
| } |
There was a problem hiding this comment.
Missing releaseName extraction required by the new CI contract.
CiMetadata and getCiMetadata() currently only support PR fields. The PR objective includes releaseName (with fallback to tag_name when name is empty), so release-triggered runs will silently drop required metadata.
💡 Proposed patch
export interface CiMetadata {
prNumber?: number
prTitle?: string
+ releaseName?: string
}
@@
- * - `pull_request` / `pull_request_target` events → `prNumber`, `prTitle`
+ * - `pull_request` / `pull_request_target` events → `prNumber`, `prTitle`
+ * - `release` events → `releaseName` (fallback to `tag_name` when `name` is empty)
@@
const pr = event?.pull_request as
| { number?: number; title?: string }
| undefined
@@
}
+
+ const release = event?.release as
+ | { name?: string; tag_name?: string }
+ | undefined
+ if (release) {
+ const name =
+ typeof release.name === 'string' ? release.name.trim() : ''
+ const tagName =
+ typeof release.tag_name === 'string' ? release.tag_name.trim() : ''
+ if (name) metadata.releaseName = name
+ else if (tagName) metadata.releaseName = tagName
+ }Also applies to: 9-16, 29-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ci-metadata.ts` around lines 4 - 7, Update the CiMetadata type and the
getCiMetadata() extractor to include an optional releaseName string and ensure
release-triggered runs populate it: add releaseName?: string to the CiMetadata
interface and in getCiMetadata() detect release payloads (or the release object)
and set releaseName = release.name || release.tag_name (falling back to tag_name
when name is empty); keep existing prNumber/prTitle handling for PR runs and
ensure callers expecting releaseName will receive undefined for non-release
runs.
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
1 participant
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
testing-service/api/v1/ci/runcontract gained three optional fields:prNumber,prTitle,releaseName. This PR auto-detects them from the GitHub event payload and forwards them.Changes
src/ci-metadata.ts(new):getCiMetadata()readsGITHUB_EVENT_PATHand extracts:prNumber+prTitlefrompull_request/pull_request_targeteventsreleaseNamefromreleaseevents (falls back totag_namewhennameis empty){}for events that don't carry this metadata (push, workflow_dispatch, etc.), never throwssrc/api.ts: extendedTriggerRunRequestwith the three new optional fields (camelCase, matches the Pydantic alias generator on the server)src/main.ts: wiresgetCiMetadata()into thetriggerRuncallDesign notes
No new action inputs — metadata is fully auto-detected, keeping the action zero-config. Consistent with how
commit-titledefaults to event payload detection. If explicit override inputs are wanted later, they're an additive change.Summary by CodeRabbit