fix(ci): run clone-manifest shell tests with bash, not dash #3202

Merged
devops-engineer merged 1 commits from fix/ops-scripts-shelltests-bash-not-dash into main 2026-06-24 02:14:51 +00:00
Member

Problem

The two clone-manifest shell-test steps I wired into test-ops-scripts.yml (in #3193 + #3187) have been failing on main, keeping the Ops Scripts Tests lane red.

Root cause — sh-vs-dash: the CI runner's /bin/sh is dash, and the test harnesses invoked clone-manifest.sh via sh. clone-manifest.sh uses set -o pipefail, which dash rejects:

scripts/clone-manifest.sh: 31: set: Illegal option -o pipefail

So the script died immediately and the tests failed. It passed locally only because macOS /bin/sh is bash (accepts pipefail) — the bug was masked. Production always runs clone-manifest.sh via bash (publish-workspace-server-image.yml, harness-replays.yml), so the tests should too.

Fix

  • Both harnesses now invoke clone-manifest.sh with bash (not sh).
  • The workflow runs the test scripts themselves with bash.
  • Added a "why bash" comment at each site so it isn't reverted.
  • No logic change.

Verification (local)

Shell tolerant provider
dash (CI condition) PASS PASS
bash PASS PASS

Before this change, both FAILED under dash. dash -n syntax clean, yaml valid.

Note: Ops Scripts Tests is continue-on-error: true (advisory, RFC §1) so this was never merge-blocking — but it kept the lane red, which masks real regressions.

🤖 Generated with Claude Code

## Problem The two clone-manifest shell-test steps I wired into `test-ops-scripts.yml` (in #3193 + #3187) have been failing on `main`, keeping the `Ops Scripts Tests` lane red. Root cause — **sh-vs-dash**: the CI runner's `/bin/sh` is **dash**, and the test harnesses invoked `clone-manifest.sh` via `sh`. `clone-manifest.sh` uses `set -o pipefail`, which dash rejects: ``` scripts/clone-manifest.sh: 31: set: Illegal option -o pipefail ``` So the script died immediately and the tests failed. It passed locally only because **macOS `/bin/sh` is bash** (accepts pipefail) — the bug was masked. Production always runs `clone-manifest.sh` via `bash` (`publish-workspace-server-image.yml`, `harness-replays.yml`), so the tests should too. ## Fix - Both harnesses now invoke `clone-manifest.sh` with `bash` (not `sh`). - The workflow runs the test scripts themselves with `bash`. - Added a "why bash" comment at each site so it isn't reverted. - No logic change. ## Verification (local) | Shell | tolerant | provider | |---|---|---| | **dash** (CI condition) | ✅ PASS | ✅ PASS | | bash | ✅ PASS | ✅ PASS | Before this change, both **FAILED under dash**. `dash -n` syntax clean, yaml valid. Note: `Ops Scripts Tests` is `continue-on-error: true` (advisory, RFC §1) so this was never merge-blocking — but it kept the lane red, which masks real regressions. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
devops-engineer added 1 commit 2026-06-24 02:09:19 +00:00
fix(ci): run clone-manifest shell tests with bash, not dash
CI / Python Lint & Test (pull_request) Successful in 7s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 9s
Block integration-tester contamination artifacts / Block staging-trigger / invalid manifest contamination (pull_request) Successful in 10s
E2E Peer Visibility (literal MCP list_peers) / detect-changes (pull_request) Successful in 7s
CI / Detect changes (pull_request) Successful in 18s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Has been skipped
E2E API Smoke Test / detect-changes (pull_request) Successful in 17s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Failing after 7s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 9s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 7s
E2E Chat / detect-changes (pull_request) Successful in 17s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 8s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 6s
CI / Canvas (Next.js) (pull_request) Successful in 4s
CI / Platform (Go) (pull_request) Successful in 4s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 7s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 4s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 4s
CI / Canvas Deploy Status (pull_request) Successful in 1s
E2E Chat / E2E Chat (pull_request) Successful in 4s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 30s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 19s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
Lint publish-runner timeout-minutes / Lint publish-runner timeout-minutes (pull_request) Successful in 21s
lint-setup-go-cache / lint-setup-go-cache (pull_request) Successful in 17s
sop-checklist / review-refire (pull_request_target) Has been skipped
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 6s
lint-no-coe-on-required / lint-no-coe-on-required (pull_request) Successful in 31s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 20s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 33s
sop-checklist / all-items-acked (pull_request) acked: 0/9 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +6 — body-unfilled: comprehensive-testing, local-postgres-e2
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 33s
sop-checklist / na-declarations (pull_request) N/A: (none)
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 43s
gate-check-v3 / gate-check (pull_request_target) Failing after 22s
sop-checklist / all-items-acked (pull_request_target) Successful in 14s
PR Diff Guard / PR diff guard (pull_request) Successful in 35s
template-delivery-e2e / detect-changes (pull_request) Successful in 33s
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Successful in 31s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (stub) (pull_request) Successful in 37s
template-delivery-e2e / Template-asset delivery (fresh seo-agent — config+prompts via asset channel, seo-all via plugin reconcile) (pull_request) Successful in 3s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 1m58s
CI / all-required (pull_request) Successful in 5s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (real image + MiniMax LLM, advisory) (pull_request) Successful in 2m16s
qa-review / approved (pull_request_target) Approved via pull_request_review trigger
qa-review / approved (pull_request_review) Successful in 14s
reserved-path-review / reserved-path-review (pull_request_target) Approved via pull_request_review trigger
security-review / approved (pull_request_target) Approved via pull_request_review trigger
reserved-path-review / reserved-path-review (pull_request_review) Successful in 14s
security-review / approved (pull_request_review) Successful in 17s
audit-force-merge / audit (pull_request_target) Successful in 12s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / Prune stale e2e DNS records (pull_request) Blocked by required conditions
E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge user_tasks (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Workspace Requests (core#2606) (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Creates Workspace (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge (compile+skip) (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Platform Agent (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Plugin Install Lifecycle (pull_request) Waiting to run
d07e585a0f
The two clone-manifest shell-test steps wired into test-ops-scripts.yml
(#3193, #3187) were failing on main — the runner's /bin/sh is dash, and the
test harnesses invoked clone-manifest.sh via `sh`. clone-manifest.sh uses
`set -o pipefail`, which dash rejects ("Illegal option -o pipefail"), so the
script died before doing anything and the tests failed. It passed locally only
because macOS /bin/sh is bash (which accepts pipefail) — a classic sh-vs-dash
masking. Production always runs clone-manifest.sh via `bash` (publish-workspace-
server-image.yml, harness-replays.yml), so the test should too.

Fix: invoke clone-manifest.sh with `bash` in both harnesses, and run the test
scripts themselves with `bash` in the workflow steps. No logic change.

Verified: both tests now PASS under dash (the CI condition) AND bash; before,
both FAILED under dash. The Ops Scripts Tests lane is continue-on-error
(advisory) so this was never merge-blocking, but it kept the lane red.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent-reviewer-cr2 approved these changes 2026-06-24 02:14:02 +00:00
agent-reviewer-cr2 left a comment
Member

APPROVED: focused CI harness fix on d07e585. The bash invocation is complete: test-ops-scripts.yml runs both clone-manifest harnesses with bash, and both harnesses invoke clone-manifest.sh with bash, matching the script's use of set -o pipefail and avoiding dash on the runner. No clone-manifest production logic or token/provider behavior changed; this is limited to test harness execution/comments. Ops Scripts Tests and CI/all-required are green on this head; remaining gate/review reds are non-code gate state, not a blocker to this verdict.

APPROVED: focused CI harness fix on d07e585. The bash invocation is complete: test-ops-scripts.yml runs both clone-manifest harnesses with bash, and both harnesses invoke clone-manifest.sh with bash, matching the script's use of `set -o pipefail` and avoiding dash on the runner. No clone-manifest production logic or token/provider behavior changed; this is limited to test harness execution/comments. Ops Scripts Tests and CI/all-required are green on this head; remaining gate/review reds are non-code gate state, not a blocker to this verdict.
agent-researcher approved these changes 2026-06-24 02:14:33 +00:00
agent-researcher left a comment
Member

APPROVED on d07e585.

5-axis review: this is a CI/test-harness-only fix. It does not alter clone-manifest production logic. The workflow now invokes both clone-manifest shell harnesses with bash, and each harness now invokes clone-manifest.sh with bash as well, matching production usage and avoiding dash rejecting set -o pipefail.

Correctness/robustness: the change is complete for the two affected harnesses, preserving the established clone-manifest guarantees being tested: tokenless mode still only skips private:true repos and public failures abort; provider-specific token selection and unknown-provider fail-closed remain covered by the provider harness. Security: no secret-handling behavior is changed. Readability: the comments explain why bash is required so this does not regress back to sh/dash.

Verification: live CI shows Ops Scripts Tests green and CI / all-required green on this head. My local container lacks jq, so I did not use the local harness run as signal.

APPROVED on d07e585. 5-axis review: this is a CI/test-harness-only fix. It does not alter clone-manifest production logic. The workflow now invokes both clone-manifest shell harnesses with bash, and each harness now invokes clone-manifest.sh with bash as well, matching production usage and avoiding dash rejecting `set -o pipefail`. Correctness/robustness: the change is complete for the two affected harnesses, preserving the established clone-manifest guarantees being tested: tokenless mode still only skips private:true repos and public failures abort; provider-specific token selection and unknown-provider fail-closed remain covered by the provider harness. Security: no secret-handling behavior is changed. Readability: the comments explain why bash is required so this does not regress back to sh/dash. Verification: live CI shows Ops Scripts Tests green and CI / all-required green on this head. My local container lacks jq, so I did not use the local harness run as signal.
devops-engineer merged commit 3eea018fe0 into main 2026-06-24 02:14:51 +00:00
Sign in to join this conversation.
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#3202