feat: accept workspace publish requests - #9339
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 52ef600. Configure here.
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
I was hoping that we could replace all regexp based parsing with peggy at this point.
There was a problem hiding this comment.
| uses: actions/checkout@v5 | |
| uses: actions/checkout@v7 |
Let's go with latest version
| requires_workspace_discovery="$(node -e ' | ||
| const { needsWorkspaceDiscovery } = require("./.__publish__/src/modules/publish-location"); | ||
| const input = JSON.parse(process.env.PUBLISH_ARGS || ""); | ||
| process.stdout.write(String(needsWorkspaceDiscovery(input))); | ||
| ')" |
There was a problem hiding this comment.
Why not put this into a file too? Even better, make it a runnable node script to avoid passing JS through YAML and bash?
| @@ -0,0 +1,45 @@ | |||
| { | |||
| function join(characters) { | |||
There was a problem hiding this comment.
I wish this file was generated from the code block in docs/publish-issue-format.md (or vice versa, the code block was generated from this) so there's only one source of truth.
| @@ -0,0 +1,29 @@ | |||
| const CHECK_RUNS_LINK = | |||
There was a problem hiding this comment.
I wonder if this can be part of the peggy parser/language too?
| core.setOutput( | ||
| "result", | ||
| resolvePublishLocation({ | ||
| path: input.path, | ||
| workspaceNames, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Bug: A plain JavaScript object is passed to core.setOutput() without being serialized, which will cause the consuming workflow's fromJSON() call to fail.
Severity: HIGH
Suggested Fix
Wrap the object being passed to core.setOutput() with JSON.stringify() to ensure it is correctly serialized into a JSON string before being set as an output. For example: core.setOutput('result', JSON.stringify(result));.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/publish/resolve-location.js#L14-L20
Potential issue: In `src/publish/inputs.js` and `src/publish/resolve-location.js`, plain
JavaScript objects are passed directly to `core.setOutput('result', ...)`. The
`@actions/core` library does not automatically serialize objects to JSON; instead, it
coerces them to the string `"[object Object]"`. The consuming GitHub workflow expects a
valid JSON string for its `fromJSON()` function. When the workflow attempts to parse
`"[object Object]"`, it will fail with a JSON parsing error, causing the 'publish'
workflow to fail.
Also affects:
src/publish/inputs.js:9
There was a problem hiding this comment.
Bug: The code uses a fallback to an empty string for JSON.parse(), which will throw an unhandled SyntaxError if the environment variable is missing.
Severity: MEDIUM
Suggested Fix
Replace the unsafe fallback || "" with a fallback to a valid empty JSON object, such as || '{}'. This ensures that JSON.parse() always receives a valid string to parse, preventing the script from crashing when the environment variable is not set.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/publish/resolve-location.js#L5
Potential issue: The code uses the pattern `JSON.parse(process.env.PUBLISH_ARGS || "")`.
If the `PUBLISH_ARGS` environment variable is not set or is an empty string, the
expression defaults to `""`. Calling `JSON.parse("")` throws a `SyntaxError` because an
empty string is not valid JSON. This unhandled exception will cause the Node.js script
to crash. While a preceding step is expected to set this variable, this incorrect
fallback creates a potential failure point if that step fails to produce an output for
any reason.
Also affects:
src/publish/resolve-release-revision.js:5
1 participant
Summary
Validation
Deployment
Deploy this controller before Craft PR #872. It adds the title parser and secure state contract required for workspace-qualified requests.