fix: sync functional tests openshell version with shared pin by ralphbean · Pull Request #2535 · fullsend-ai/fullsend

ralphbean · 2026-06-22T21:12:08Z

Summary

The functional-tests workflow hardcoded openshell 0.0.38 while the rest of the repo pins 0.0.63 via .github/scripts/openshell-version.sh. Sources the shared script instead.
Adds an early version-mismatch guard in eval/run-functional.sh so drift is caught regardless of how tests are invoked (CI or locally).
Uses ${OPENSHELL_SHA} for the supervisor image tag instead of a hardcoded SHA.

Spotted in #2534.

Test plan

Functional tests workflow picks up 0.0.63 from the shared script
eval/run-functional.sh fails fast if an old openshell is installed locally

🤖 Generated with Claude Code

The functional-tests workflow hardcoded openshell 0.0.38 while the rest of the repo pins 0.0.63 via .github/scripts/openshell-version.sh. Source the shared script instead of hardcoding the version, and add an early version-mismatch check in eval/run-functional.sh so any drift is caught regardless of how the tests are invoked. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

qodo-code-review · 2026-06-22T21:13:02Z

PR Summary by Qodo

Fix functional tests to use shared OpenShell version pin
🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Description

• Source the shared OpenShell version pin in the functional-tests GitHub workflow.
• Tag the OpenShell supervisor image using the pinned OPENSHELL_SHA.
• Fail fast in the local functional runner when the installed OpenShell version drifts.

Diagram

graph TD
  wf["functional-tests.yml"] --> vs["openshell-version.sh"] --> cli["OpenShell CLI"]
  wf --> img["Supervisor image tag"]
  rf["run-functional.sh"] --> vs --> chk{"Version matches pin?"}
  chk -- "yes" --> h["Functional harness"]
  chk -- "no" --> err["Exit with error"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Extract full OpenShell setup into a reusable composite action/script

➕ Eliminates duplication beyond just the version pin (install, gateway start, env wiring).
➕ Makes future workflow consumers consistent by construction.
➖ More moving parts and review surface than this targeted fix.
➖ Requires designing inputs/outputs and updating multiple call sites.

2. Add a dedicated CI drift-check job (version/SHA consistency)

➕ Catches mismatches without relying on the functional run path.
➕ Can be extended to validate image tags, gateway binary, etc.
➖ Additional CI job to maintain.
➖ Doesn’t improve local developer feedback unless mirrored in scripts.

Recommendation: The PR’s approach (sourcing a single pinned version script in both CI and the local runner, plus a fail-fast guard) is the best minimal change: it fixes the immediate drift and prevents recurrence in the highest-friction path (local runs). Consider a follow-up to extract the full OpenShell setup into a shared composite action/script if duplication continues to grow.

Files changed (2) +16 / -6

Bug fix (1) +14 / -0

run-functional.shAdd early OpenShell version mismatch guard +14/-0

Add early OpenShell version mismatch guard

• Adds a preflight check that requires openshell to be installed and verifies its version matches the repo-pinned OPENSHELL_VERSION from .github/scripts/openshell-version.sh. Exits with a clear error message before running the functional harness when drift is detected.

eval/run-functional.sh

Other (1) +2 / -6

functional-tests.ymlSource shared OpenShell pin and use pinned supervisor image SHA +2/-6

Source shared OpenShell pin and use pinned supervisor image SHA

• Replaces the hardcoded OpenShell version with sourcing the shared .github/scripts/openshell-version.sh. Updates the supervisor image reference to use ${OPENSHELL_SHA} instead of a hardcoded digest, keeping CI aligned with the repository pin.

.github/workflows/functional-tests.yml

github-actions · 2026-06-22T21:13:27Z

Site preview

Preview: https://3fdb8a3b-site.fullsend-ai.workers.dev

Commit: f300b6bdcec870704e774488d32b08d625c603a3

qodo-code-review · 2026-06-22T21:14:37Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

fullsend-ai-review · 2026-06-22T21:15:39Z

🤖 Finished Review · ✅ Success · Started 9:15 PM UTC · Completed 9:25 PM UTC
Commit: de1c02e · View workflow run →

codecov · 2026-06-22T21:19:36Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

fullsend-ai-review · 2026-06-22T21:25:39Z

Review

Findings

Medium

[protected-path] .github/workflows/functional-tests.yml — This file is under the .github/ protected path. The PR links to ci(functional-tests): use pull_request_target for fork PR support #2534 (as the origin of the observation) and the description explains the rationale for syncing the hardcoded openshell version with the shared pin script. Human approval is required for protected-path changes regardless of context.
[claimed-vs-actual-scope] .github/workflows/functional-tests.yml — PR body claims "Uses ${OPENSHELL_SHA} for the supervisor image tag instead of a hardcoded SHA," but the diff deletes the OPENSHELL_SUPERVISOR_IMAGE export entirely (along with the entire gateway start step) rather than replacing it with ${OPENSHELL_SHA}. The PR description is inaccurate on this point.
[commit-prefix-mismatch] .github/workflows/functional-tests.yml — PR title uses fix: prefix, which per COMMITS.md is reserved for "Bug fix visible to users" and appears in release notes under Bug Fixes. COMMITS.md explicitly states: "Upgrading a dependency or vendored tool version → chore" and "When in doubt, prefer refactor or chore over feat or fix." Syncing CI version pins is infrastructure maintenance — the correct prefix is chore(ci): or ci:.

Low

[incomplete-duplication-removal] .github/workflows/functional-tests.yml — The diff removes the TODO comment that identified duplication between functional-tests.yml and action.yml. The PR resolves version pin and CLI install duplication, and removes gateway download/start (which was never in action.yml). Podman setup (Install Podman, Configure rootless Podman, Start Podman API) remains duplicated. Removing the TODO is slightly premature but the remaining duplication is lower-risk boilerplate.

Previous run

Review

Findings

Medium

[protected-path] .github/workflows/functional-tests.yml — This file is under the .github/ protected path. The PR links to ci(functional-tests): use pull_request_target for fork PR support #2534 (as the origin of the observation) and the description explains the rationale for syncing the hardcoded openshell version with the shared pin script. Human approval is required for protected-path changes regardless of context.
[claimed-vs-actual-scope] .github/workflows/functional-tests.yml:121 — PR body claims "Uses ${OPENSHELL_SHA} for the supervisor image tag instead of a hardcoded SHA," but the diff deletes the OPENSHELL_SUPERVISOR_IMAGE export entirely rather than replacing it with ${OPENSHELL_SHA}. The gateway will fall back to its built-in default supervisor image. This aligns with action.yml behavior (which also does not set OPENSHELL_SUPERVISOR_IMAGE), so the functional effect is correct, but the PR description is inaccurate.
[incomplete-duplication-removal] .github/workflows/functional-tests.yml:64 — The diff removes the TODO comment that called out duplication between functional-tests.yml and action.yml (version, CLI, gateway, Podman, gateway start). Only the version pin duplication is addressed by sourcing openshell-version.sh. The remaining duplicated steps (CLI install, gateway download, Podman setup, gateway start) are still duplicated. Removing the TODO suggests the problem is fully resolved when it is not.

Low

[naming-consistency] .github/workflows/functional-tests.yml:64 — The workflow step name "Set OpenShell version" does not reflect that the sourced script also sets OPENSHELL_SHA via GITHUB_ENV. Consider updating to "Set OpenShell version and SHA."
[edge-case] eval/run-functional.sh:55 — The version extraction openshell --version | awk '{print $NF}' assumes the last whitespace-delimited field is the version string. If the output format were to include additional tokens, $NF would return the wrong value. Risk is theoretical — standard Python CLI tools output toolname X.Y.Z where $NF works correctly.

Previous run (2)

Review

Findings

Medium

[protected-path] .github/workflows/functional-tests.yml — This file is under the .github/ protected path. The PR links to ci(functional-tests): use pull_request_target for fork PR support #2534 and the description explains the rationale (syncing hardcoded openshell version with the shared pin script). Human approval is required for protected-path changes regardless of context.

Low

[edge-case] eval/run-functional.sh:55 — The version extraction openshell --version | awk '{print $NF}' assumes the last whitespace-delimited field is the version string. If the output format were to include additional tokens (e.g., build metadata), $NF would return the wrong value. Risk is theoretical — standard Python CLI tools output toolname X.Y.Z where $NF works correctly.
[error-handling-gap] eval/run-functional.sh:53 — The source of openshell-version.sh is not guarded by a file-existence check. The script's set -euo pipefail ensures it exits on failure, but unlike the other pre-flight checks in this file, there is no user-friendly error message directing the user to the fix.
[implementation-detail] .github/workflows/functional-tests.yml:64 — The workflow step name "Set OpenShell version" is slightly misleading since the sourced script also sets OPENSHELL_SHA. Consider updating to "Set OpenShell version and SHA."

Labels: PR modifies CI workflow files (.github/workflows/) and eval scripts

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-06-22T21:25:40Z


+# Fail fast if openshell version doesn't match the repo pin
+if command -v openshell >/dev/null 2>&1; then
+  source "${REPO_ROOT}/.github/scripts/openshell-version.sh"


[low] edge-case

The version extraction openshell --version | awk '{print $NF}' assumes the last whitespace-delimited field is the version string. If the output format were to include additional tokens, $NF would return the wrong value. Risk is theoretical — standard Python CLI tools output toolname X.Y.Z where $NF works correctly.

fullsend-ai-review · 2026-06-22T21:25:40Z

  exit 1
 fi

+# Fail fast if openshell version doesn't match the repo pin


[low] error-handling-gap

The source of openshell-version.sh is not guarded by a file-existence check. The script's set -euo pipefail ensures it exits on failure, but unlike the other pre-flight checks in this file, there is no user-friendly error message.

Suggested fix: Add a guard before sourcing: if [[ ! -f "${REPO_ROOT}/.github/scripts/openshell-version.sh" ]]; then echo "ERROR: ..."; exit 1; fi

fullsend-ai-review · 2026-06-22T21:25:40Z

-      # gateway start) is duplicated from action.yml. Extract into a
-      # shared script (e.g. .github/scripts/setup-openshell.sh) so the
-      # version and config stay in sync across both places.
      - name: Set OpenShell version


[low] implementation-detail

The workflow step name 'Set OpenShell version' is slightly misleading since the sourced script also sets OPENSHELL_SHA. Consider updating to 'Set OpenShell version and SHA'.

action.yml doesn't set OPENSHELL_SUPERVISOR_IMAGE — it lets the gateway use its built-in default. The functional tests had an old hardcoded SHA that doesn't correspond to the openshell release SHA. Remove the override so the 0.0.63 gateway uses its own default supervisor image. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

fullsend-ai-review · 2026-06-22T21:34:10Z

🤖 Finished Review · ❌ Failure · Started 9:34 PM UTC · Completed 9:45 PM UTC
Commit: 4a92735 · View workflow run →

The functional tests manually downloaded the gateway binary, started it in the background, and registered it with the CLI — an approach that worked with 0.0.38 but broke with 0.0.63 (gateway port 8080 never came up). Replace the manual setup with the same pattern action.yml uses: - Write gateway.env config - Install via the shared install-openshell.sh script - Let openshell manage its own gateway This matches the changes made in PR #2315 when openshell was bumped from 0.0.54 to 0.0.63. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

fullsend-ai-review · 2026-06-22T22:50:08Z

🤖 Review · ⚠️ Cancelled · Started 10:50 PM UTC · Ended 10:52 PM UTC
Commit: 4e21a60 · View workflow run →

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

fullsend-ai-review · 2026-06-22T22:56:12Z

🤖 Finished Review · ✅ Success · Started 10:56 PM UTC · Completed 11:07 PM UTC
Commit: f300b6b · View workflow run →

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-06-22T23:07:37Z

-        run: echo "OPENSHELL_VERSION=0.0.38" >> "${GITHUB_ENV}"
-
-      - name: Install OpenShell CLI
+      - name: Configure OpenShell gateway


[low] incomplete-duplication-removal

The diff removes the TODO comment that identified duplication between functional-tests.yml and action.yml. The PR resolves version pin and CLI install duplication, but Podman setup remains duplicated. Removing the TODO is slightly premature.

fullsend-ai-retro · 2026-06-23T09:35:43Z

🤖 Finished Retro · ✅ Success · Started 9:35 AM UTC · Completed 9:42 AM UTC
Commit: f300b6b · View workflow run →

fullsend-ai-retro · 2026-06-23T09:42:44Z

Retro: PR #2535 — sync functional tests openshell version

Verdict: Workflow performed well. No new proposals — all improvement areas are covered by existing open issues.

Timeline

2026-06-22 21:12 — ralphbean opened PR to sync functional test openshell version with shared pin (spotted in ci(functional-tests): use pull_request_target for fork PR support #2534)
21:12–22:52 — 4 commits iterated from a simple version-pin fix to full alignment of functional test setup with action.yml (+19/−50, 2 files)
Review bot ran 4 times: success → 422 failure → cancellation (superseded) → success
2026-06-23 09:20 — Human reviewer (rh-hemartin) approved cleanly
09:30 — PR merged

Review quality assessment

The review bot performed well on this PR:

Good catches: Flagged an inaccurate PR description (claimed ${OPENSHELL_SHA} replacement but diff actually deletes the export) and a commit prefix mismatch (fix: vs chore(ci): per COMMITS.md).
Reasonable low-severity findings: 4 inline comments on edge-case handling, error guards, step naming, and incomplete duplication removal — all valid observations appropriately rated as low severity.
No false positives that would waste reviewer time.

Inefficiencies identified (all covered by existing issues)

Issue	Existing coverage
422 failure on review submission — Run 2 failed because an inline comment targeted a line outside the diff hunk, causing a GitHub API 422	#1067 (post-review: handle 422 on inline comment submission gracefully)
Multiple review runs on rapid commits — 4 review runs for 4 commits within 2 hours; 1 failed, 1 cancelled, only 2 produced useful output	#1418, #1014, and ~10 other open issues on deduplication/debouncing
Commit prefix enforcement — `fix:` prefix flagged but not escalated	#2105 (escalate prefix mismatches to changes_requested)
PR description accuracy — Mismatch flagged but only as info-level	#2280 (escalate description-vs-diff discrepancies above info severity)

The concurrency group cancellation worked correctly (run 3 was properly superseded by run 4 within seconds). The existing issue backlog for review deduplication is extensive, suggesting this is a known and actively tracked area.

…rsion-sync fix: sync functional tests openshell version with shared pin

github-actions Bot deployed to site-preview June 22, 2026 21:13 View deployment

ralphbean enabled auto-merge June 22, 2026 21:13

ralphbean mentioned this pull request Jun 22, 2026

ci(functional-tests): use pull_request_target for fork PR support #2534

Merged

3 tasks

fullsend-ai-review Bot reviewed Jun 22, 2026

View reviewed changes

fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/ci CI pipelines and checks labels Jun 22, 2026

github-actions Bot deployed to site-preview June 22, 2026 21:32 View deployment

github-actions Bot deployed to site-preview June 22, 2026 22:48 View deployment

fix: quote $HOME and simplify heredoc to satisfy actionlint

f300b6b

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>

github-actions Bot deployed to site-preview June 22, 2026 22:53 View deployment

fullsend-ai-review Bot reviewed Jun 22, 2026

View reviewed changes

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 22, 2026

rh-hemartin approved these changes Jun 23, 2026

View reviewed changes

ralphbean added this pull request to the merge queue Jun 23, 2026

Merged via the queue into main with commit 468dce4 Jun 23, 2026
17 checks passed

ralphbean deleted the fix/openshell-version-sync branch June 23, 2026 09:30

ifireball pushed a commit to ifireball/fullsend that referenced this pull request Jun 24, 2026

Merge pull request fullsend-ai#2535 from fullsend-ai/fix/openshell-ve…

f87b72c

…rsion-sync fix: sync functional tests openshell version with shared pin

qodo-code-review Bot mentioned this pull request Jul 11, 2026

chore(deps): bump wrangler to 4.110.0 to match workers-types v5 #4102

Merged

7 tasks

Uh oh!

Conversation

ralphbean commented Jun 22, 2026

Summary

Test plan

Uh oh!

qodo-code-review Bot commented Jun 22, 2026

PR Summary by Qodo

Uh oh!

github-actions Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

qodo-code-review Bot commented Jun 22, 2026

Code Review by Qodo

Great, no issues found!

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jun 22, 2026

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-retro Bot commented Jun 23, 2026

Retro: PR #2535 — sync functional tests openshell version

Timeline

Review quality assessment

Inefficiencies identified (all covered by existing issues)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

github-actions Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 22, 2026 •

edited

Loading

fullsend-ai-retro Bot commented Jun 23, 2026 •

edited

Loading