Fix configurable visual recap runner shell#2097
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Visual recap — skippedThe visual recap job did not run for this pull request. This is informational only and does not block the PR. Recap skipped for |
|
thanks @CheckPickerUpper! just need a merge conflict resolution then this is good to go |
c47b454 to
fa22260
Compare
fa22260 to
ce7e529
Compare
|
Resolved the conflict against current |
|
thanks @CheckPickerUpper - apologies i was out this week, we've got one last merge conflict then good to go |
|
No worries I'll fix it tomorrow, thanks 👍🏿 |
ce7e529 to
0f3c091
Compare
|
Resolved the merge conflict against current main and all checks are green on the new head |
|
great, thanks @CheckPickerUpper!! |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
Incremental Code Review Summary
The latest merged PR head is not internally consistent. The canonical workflow files contain the new defaults.run.shell: bash block, but the bundled workflow artifacts do not match the canonical source. The newly added regression tests fail, so the published installer artifacts can still omit the primary fix on configurable Windows/macOS/self-hosted runners.
Key Findings
- 🔴 HIGH — The
@agent-native/recap-clibundle fails its new Bash and byte-identity assertions, leaving the published installer artifact without the primary shell fix. - 🟡 MEDIUM —
@agent-native/core's copied workflow bundle is stale relative to the canonical workflow, causing the core installer artifact and canonical workflow to diverge.
Targeted recap-cli and core regression tests both failed with byte-identity/shell assertion mismatches. This is a standard-risk package/infrastructure change, but the high-severity artifact regression should be corrected before relying on the merged release changesets.
🧪 Browser testing: Skipped — this PR affects workflows and package artifacts only; it has no user-facing browser impact.
|
|
||
| describe("the recap installer workflow", () => { | ||
| it("keeps Bash semantics on configurable runners", () => { | ||
| expect(PR_VISUAL_RECAP_WORKFLOW_YML).toContain( |
There was a problem hiding this comment.
🔴 Recap CLI bundle is missing the Bash shell fix
The new regression assertion fails because the bundled installer string does not contain the canonical defaults.run.shell: bash block. Users installing through @agent-native/recap-cli can still execute Bash-syntax steps under PowerShell on Windows or another default shell on configurable runners; regenerate the bundle from the canonical workflow.
| /** Compatibility re-export for the workflow bundled by the recap CLI package. */ | ||
| export { PR_VISUAL_RECAP_WORKFLOW_YML } from "@agent-native/recap-cli"; | ||
| /** Canonical PR Visual Recap workflow bundled by the CLI installer. */ | ||
| export const PR_VISUAL_RECAP_WORKFLOW_YML = |
There was a problem hiding this comment.
🟡 Core bundled workflow is stale
This copied workflow string is not byte-identical to the canonical .github/workflows/pr-visual-recap.yml; the core regression test fails on the new shell assertion and byte-identity check. Regenerate the bundled artifact from the canonical workflow so core installers receive the same current workflow.
Follow-up to #2038 and #2046.
What changed
@agent-native/coreand@agent-native/recap-cliWhy
The runner configuration accepts GitHub-hosted Windows and macOS labels as well as self-hosted label arrays, but the recap steps use Bash syntax throughout. Without an explicit shell, a valid Windows runner configuration reaches the job and is interpreted by PowerShell. Pinning
shell: bashat the job level makes the workflow contract match the configurations setup and doctor already accept.Verification
pnpm --filter @agent-native/recap-cli exec vitest --run src/pr-visual-recap-workflow.spec.ts(2 tests)pnpm --filter @agent-native/core exec vitest --run src/cli/recap.spec.ts --maxWorkers=25%(199 tests)pnpm --filter @agent-native/recap-cli typecheckpnpm --filter @agent-native/core typecheckpnpm guards(23 checks)pnpm exec oxfmt --check ...pnpm changeset statusactionlinton both configurable workflowsgit diff --check