Repository navigation
fix(core): parse run metadata returned as a JSON string - #5052
princejain756 wants to merge 2 commits into
Conversation
GET /api/v1/runs/:runId/metadata returns the stored metadata string unparsed, while the response schema expects a record, so metadata.refresh() always failed validation. Accept a string payload and JSON.parse it before validating, keeping object responses and the rejection of non-JSON values unchanged. Fixes triggerdotdev#5039
🦋 Changeset detectedLatest commit: 6ba06cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: parsing run metadata returned as a JSON string. |
| Description check | |
The description explains the failure, the response-shape difference, the schema change, and the reason for client-side parsing. It also references issue #5039. It does not include the template’s check… |
| Linked Issues check | |
Issue #5039 requires metadata.refresh() to accept the serialized JSON metadata returned by the endpoint. The supplied change summary says UpdateMetadataResponseBody.metadata parses string input be… |
| Out of Scope Changes check | |
The supplied change summary lists a schema change, focused tests, and a changeset. Each change supports Issue #5039. The unavailable diff did not provide additional evidence, but the supplied whole-PR… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
Full details: Description check
-
Explanation
The description explains the failure, the response-shape difference, the schema change, and the reason for client-side parsing. It also references issue
#5039. It does not include the template’s checklist, testing steps, or separate changelog and screenshots sections.
Full details: Linked Issues check
-
Explanation
Issue
#5039requiresmetadata.refresh()to accept the serialized JSON metadata returned by the endpoint. The supplied change summary saysUpdateMetadataResponseBody.metadataparses string input before record validation and leaves non-string values unchanged. The tests cover object input, valid JSON-string input, and rejection of invalid JSON strings. This meets the coding requirement.
Full details: Out of Scope Changes check
-
Explanation
The supplied change summary lists a schema change, focused tests, and a changeset. Each change supports Issue
#5039. The unavailable diff did not provide additional evidence, but the supplied whole-PR summary identifies no unrelated changes.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b0283923-8a20-4dd1-9395-6e978c75593a
📒 Files selected for processing: 3
.changeset/run-metadata-json-string.mdpackages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/runMetadata.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⚠️ CI failures not shown inline: 2
GitHub Actions: Vouch - Check PR / 1_check-vouch.txt: fix(core): parse run metadata returned as a JSON string
Conclusion: failure
##[group]Run hustcer/setup-nu@920172d92eb04671776f3ba69d605d3b09351c30
with:
version: *
features: default
check-latest: false
enable-plugins: false
github-***REDACTED_SECRET_ASSIGNMENT***
env:
GH_***REDACTED_SECRET_ASSIGNMENT***
##[endgroup]
versionSpec: *
coerce version: *
Try to get assets for Nu: arch = x64, platform = linux, feature = default
##[error]No release for Nushell matching version specifier * of default feature.
GitHub Actions: Vouch - Check PR / check-vouch: fix(core): parse run metadata returned as a JSON string
Conclusion: failure
##[group]Run hustcer/setup-nu@920172d92eb04671776f3ba69d605d3b09351c30
with:
version: *
features: default
check-latest: false
enable-plugins: false
github-***REDACTED_SECRET_ASSIGNMENT***
env:
GH_***REDACTED_SECRET_ASSIGNMENT***
##[endgroup]
versionSpec: *
coerce version: *
Try to get assets for Nu: arch = x64, platform = linux, feature = default
##[error]No release for Nushell matching version specifier * of default feature.
🧰 Additional context used
📚 Code guidelines: 4
packages/core/AGENTS.md — auto-discovered
.claude/rules/sdk-packages.md — auto-discovered
AGENTS.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
📓 Path-based instructions: 4
**NEVER import the root** (`@trigger.dev/core`).
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Files:
packages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/runMetadata.test.ts
Source excerpt: Changes to `packages/` are **customer-facing**.
📄 CodeRabbit inference engine (.claude/rules/sdk-packages.md)
Files:
packages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/runMetadata.test.ts
Source excerpt: **packages/core** (`@trigger.dev/core`): Shared types.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/runMetadata.test.ts
Source excerpt: This is the repo for Trigger.dev, a background jobs platform written in TypeScript.
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/core/src/v3/schemas/api.tspackages/core/src/v3/schemas/runMetadata.test.ts
🔇 Additional comments: 2
packages/core/src/v3/schemas/api.ts: 1
1772-1785: LGTM!packages/core/src/v3/schemas/runMetadata.test.ts: 1
1-32: LGTM!
| @@ -0,0 +1,4 @@ | |||
| "@trigger.dev/core": patch | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the opening changeset fence.
Line 1 starts with the package bump, but Changesets requires --- before the bump and after it. This file has only the closing fence. Changesets cannot read it as a valid patch changeset, which can block release commands. Add --- above Line 1. (github.com)
As per coding guidelines, “Changes to packages/ are customer-facing. Always add a changeset: pnpm run changeset:add.”
Proposed fix
--- "a/.changeset/run-metadata-json-string.md"
+++ "b/.changeset/run-metadata-json-string.md"
@@ -1,4 +1,5 @@
+---
"@trigger.dev/core": patch
---
`metadata.refresh()` now works against deployments where `GET /api/v1/runs/:runId/metadata` returns the run metadata as a JSON string rather than an object. The response schema parses a string payload before validating, so `refresh()` resolves with the same metadata record on every version.📝 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.
| "@trigger.dev/core": patch | |
| --- | |
| "@trigger.dev/core": patch |
Source: Coding guidelines
metadata.refresh()inside a task always rejects withExpected object, received string at "metadata".The GET route at
/api/v1/runs/:runId/metadatareturns the stored metadata as a serialized JSON string (withmetadataType: "application/json"), whileUpdateMetadataResponseBodyexpects a record, so client-side validation rejects every response.runs.retrieve()and the PUT handler both return a parsed object, which is why only this path breaks.The schema now parses a string payload before validating, so
refresh()works against servers returning either shape. Object responses and rejection of non-JSON values are unchanged.I checked whether the server route should instead deserialize before responding, but the client still has to tolerate deployed servers and self-hosted installs on older versions, so the lenient parse belongs here regardless.
Fixes #5039