Skip to content

v2: Rewrite action as composite wrapper around the dhq CLI#3

Open
facundofarias wants to merge 1 commit into
mainfrom
v2
Open

v2: Rewrite action as composite wrapper around the dhq CLI#3
facundofarias wants to merge 1 commit into
mainfrom
v2

Conversation

@facundofarias
Copy link
Copy Markdown

@facundofarias facundofarias commented May 23, 2026

Summary

Replaces the v1 Docker/Alpine action (which POSTed to a DeployHQ webhook URL) with a composite action that installs and invokes the official dhq CLI. The same tool now powers customer dashboards, agents, and CI — one surface, one feature set.

Full spec: /Users/facundofarias/.claude/plans/given-that-we-now-bright-pine.md (local-only). v1 stays pinnable as @v1.

What's in this PR

Architecture

  • runs.using: composite — works on Ubuntu/macOS/Windows runners, amd64 and arm64, hosted and self-hosted.
  • scripts/install-cli.sh — downloads the pinned dhq archive from GitHub Releases, verifies SHA-256 against checksums.txt, caches under $RUNNER_TOOL_CACHE, appends to $GITHUB_PATH.
  • scripts/deploy.sh — builds dhq deploy --json --non-interactive argv from declared inputs, parses the JSON envelope with jq, emits action outputs + a $GITHUB_STEP_SUMMARY table.
  • Built-in wait loop polls dhq deployments show until terminal status — required because the CLI's own --wait is a no-op in JSON mode (verified in internal/commands/deploy.go).

Inputs (declared properly via with: now, not env vars)

  • Required: api-key, account, email.
  • Targeting: project, server, revision (defaults to github.sha), branch.
  • Behaviour: wait (default true), timeout, dry-run, full, start-revision, extra-args.
  • Tooling: cli-version (default v0.17.1).

Outputs
deployment_id, deployment_url, status, server, project.

Removed

  • Dockerfile, entrypoint.sh.
  • DEPLOYHQ_WEBHOOK_URL, REPO_CLONE_URL.

Docs / CI

  • README rewritten with input/output tables, common patterns, and a v1 → v2 migration guide.
  • CHANGELOG.md documents the break.
  • CLAUDE.md updated for the composite/CLI shape and flags the JSON-mode --wait quirk.
  • .github/workflows/test.yml: cross-platform install matrix (Ubuntu/macOS/Windows) + gated dry-run e2e job (skipped when TEST_DEPLOYHQ_* secrets aren't set, so external PRs don't fail).

Breaking changes

This is v2.0.0. Existing consumers pinned to @main will break when we move the main-tracking ref. To stay on the legacy webhook behaviour, pin @v1 (tag already pushed).

v1 (env) v2 (input) Notes
DEPLOYHQ_WEBHOOK_URL (removed) Replaced by API key auth.
DEPLOYHQ_EMAIL email Now a declared input.
REPO_REVISION revision Default changed from "latest" to ${{ github.sha }}.
REPO_BRANCH branch No more main default — CLI auto-resolves.
REPO_CLONE_URL (removed) CLI looks up repo from project config.
n/a api-key, account, project, server, wait, timeout, dry-run, full, start-revision, extra-args, cli-version All new.

Test plan

  • CI install matrix passes on ubuntu-latest, macos-latest, windows-latest (downloads + extracts + dhq --version).
  • Add TEST_DEPLOYHQ_API_KEY, TEST_DEPLOYHQ_ACCOUNT, TEST_DEPLOYHQ_EMAIL, TEST_DEPLOYHQ_PROJECT, TEST_DEPLOYHQ_SERVER repo secrets, then verify the dry-run job creates a preview deployment and populates outputs.
  • From a throwaway consumer workflow, point uses: at deployhq/deployhq-action@v2 (the branch ref, pre-tag) against a non-prod project and confirm --wait blocks, fails the job on a deliberately-broken deploy, and emits deployment_url.
  • Pin @v1 from another workflow and confirm the legacy webhook path still works.

After merge

  1. Cut v2.0.0 annotated tag from main.
  2. Move/create the floating v2 tag at the same commit.
  3. Repoint DeployHQ-owned example workflows from @main to @v2.
  4. Mention the v1 → v2 migration in DeployHQ release notes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Migrated to official DeployHQ CLI with API key authentication
    • Added built-in deployment wait with configurable timeout
    • Expanded inputs for granular deployment control (dry-run, full deployment, start-revision, extra args)
    • New outputs: deployment ID, URL, status, server, and project
  • Documentation

    • Added migration guide from v1 to v2
    • Included quick-start examples and common deployment patterns
    • Updated inputs/outputs reference documentation

Review Change Stack

Replaces the v1 Docker/Alpine action that POSTed to a DeployHQ webhook
URL with a composite action that installs and invokes the official dhq
CLI (deployhq/deployhq-cli). The same tool now backs customer dashboards,
agents, and CI.

Architecture
- runs.using: composite (Linux/macOS/Windows, amd64/arm64).
- scripts/install-cli.sh downloads the pinned dhq release from GitHub
  Releases, verifies SHA-256 against checksums.txt, and caches under
  RUNNER_TOOL_CACHE.
- scripts/deploy.sh builds the dhq deploy --json --non-interactive argv
  from INPUT_* env vars, parses the envelope with jq, and emits action
  outputs + a step summary table.
- --wait is implemented in deploy.sh (polling dhq deployments show)
  because the CLI's --wait is a no-op in JSON mode.

Inputs / outputs
- New required inputs: api-key, account, email (replaces the webhook URL).
- New optional inputs: project, server, wait, timeout, dry-run, full,
  start-revision, extra-args, cli-version.
- Outputs: deployment_id, deployment_url, status, server, project.

Removed
- Dockerfile, entrypoint.sh.
- DEPLOYHQ_WEBHOOK_URL, REPO_CLONE_URL inputs.

Docs / CI
- README rewritten with input/output tables and a v1 -> v2 migration guide.
- CHANGELOG.md documents the break.
- CLAUDE.md updated for the new shape.
- .github/workflows/test.yml: cross-platform install matrix + gated
  dry-run e2e job.

Pinned CLI: v0.17.1. Legacy behaviour stays available at @v1.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 23, 2026

Walkthrough

This PR replaces the webhook-based DeployHQ GitHub Action (v1) with a composite action that wraps the official dhq CLI. The action now uses API key authentication, supports deployment polling with configurable timeout, and provides richer output (deployment ID, URL, resolved server/project). Two new install and deploy scripts replace the prior Docker entrypoint, with comprehensive tests and updated documentation.

Changes

DeployHQ Action v2: CLI-based Composite Action

Layer / File(s) Summary
Action definition and public interface
action.yml
Composite action replaces Docker-based runner; adds 14 typed inputs (authentication, deployment targeting, behavior flags, CLI version pinning), 5 outputs (deployment_id, deployment_url, status, server, project), and wires two bash steps for install and deploy.
CLI installation and version pinning
scripts/install-cli.sh
Downloads pinned dhq CLI from GitHub releases, detects OS/architecture with fallback uname, verifies SHA-256 checksums, caches by version in RUNNER_TOOL_CACHE, extracts binary, and exports to GITHUB_PATH with fallback to action-local cache.
Deployment execution, polling, and output handling
scripts/deploy.sh
Builds dhq deploy arguments from environment variables, executes deployment, parses JSON response, implements optional polling loop (5-second intervals, configurable timeout), emits GitHub Actions outputs and step summary table, maps final status to exit codes (failed→1, timeout→124, cancelled→2).
CI workflow for action validation
.github/workflows/test.yml
Cross-platform (Ubuntu/macOS/Windows) smoke test validates CLI installation; gated dry-run job tests end-to-end deployment with secrets, asserts non-empty deployment_id output, and logs results.
User and maintainer documentation
README.md, CHANGELOG.md, CLAUDE.md
README provides quick-start, input/output reference, common patterns, and v1→v2 migration guide; CHANGELOG documents breaking changes and new features; CLAUDE.md explains internal architecture for AI assistants.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the primary change: rewriting the action from a Docker-based webhook approach to a composite wrapper around the dhq CLI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v2

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yml:
- Around line 1-9: Add an explicit top-level permissions block to the "Test"
workflow to enforce least-privilege rather than inheriting repo defaults:
declare a minimal permissions map (for example, permissions: contents: read and
any other specific scopes the jobs actually need) at the top-level of the
workflow file so jobs cannot use broader rights; update only the workflow header
(the "name: Test" workflow) and ensure each job uses only the required
permission scopes.
- Around line 41-51: Update the "Skip if secrets are missing" step to gate on
all required dry-run secrets rather than only TEST_DEPLOYHQ_API_KEY: check
TEST_DEPLOYHQ_API_KEY, TEST_DEPLOYHQ_ACCOUNT, TEST_DEPLOYHQ_EMAIL,
TEST_DEPLOYHQ_PROJECT and TEST_DEPLOYHQ_SERVER and set skip=true if any are
empty; otherwise set skip=false. Modify the shell logic in that step (the run
block) to test each environment variable (e.g., using a loop or combined
conditional) and emit the same GITHUB_OUTPUT and notice when skipping so the job
cleanly skips on partial secret configuration.
- Line 25: The workflow hardcodes DHQ_VERSION: v0.17.1 which duplicates the
pinned value in action.yml (inputs.cli-version.default) and can drift; remove
the hardcoded DHQ_VERSION assignment and instead reference the composite action
input (use DHQ_VERSION: ${{ inputs.cli-version }} or omit the env assignment so
the composite action's DHQ_VERSION input is used) to ensure the version is
sourced from inputs.cli-version rather than duplicated.
- Line 20: Replace both occurrences of "uses: actions/checkout@v4" in the
workflow with a SHA-pinned checkout reference and disable credential persistence
by adding the input "persist-credentials: false"; specifically update the steps
that currently say uses: actions/checkout@v4 so they use the actions/checkout
commit SHA (pin to the latest stable commit SHA) and include
persist-credentials: false under that step to prevent CI from writing
credentials into the local git config.

In `@scripts/deploy.sh`:
- Around line 106-114: Validate INPUT_TIMEOUT before using it in integer
comparisons: ensure INPUT_TIMEOUT contains only digits (e.g., test with a regex
like '^[0-9]+$' or use a numeric-check function) and if it is empty or
non-numeric, fall back to default 0 or exit with a clear error; then assign
timeout_seconds="${INPUT_TIMEOUT:-0}" (or the validated numeric value) and use
that safe timeout_seconds in the while loop comparison that references elapsed
and timeout_seconds. Make the check near the assignment and reference the
variables timeout_seconds, INPUT_TIMEOUT, started, elapsed, and the while loop
to locate the code to modify.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 15e04fb8-b994-4cb0-89f5-475fc2b2e190

📥 Commits

Reviewing files that changed from the base of the PR and between 5f6fb63 and 586fc97.

📒 Files selected for processing (9)
  • .github/workflows/test.yml
  • CHANGELOG.md
  • CLAUDE.md
  • Dockerfile
  • README.md
  • action.yml
  • entrypoint.sh
  • scripts/deploy.sh
  • scripts/install-cli.sh
💤 Files with no reviewable changes (2)
  • entrypoint.sh
  • Dockerfile

Comment on lines +1 to +9
name: Test

on:
push:
branches: [main, v2]
pull_request:
workflow_dispatch:

jobs:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Set explicit least-privilege permissions for the workflow.

There is no permissions: block, so the workflow inherits repository defaults. Declare minimal permissions explicitly.

🛡️ Suggested change
 on:
   push:
     branches: [main, v2]
   pull_request:
   workflow_dispatch:

+permissions:
+  contents: read
+
 jobs:
📝 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.

Suggested change
name: Test
on:
push:
branches: [main, v2]
pull_request:
workflow_dispatch:
jobs:
name: Test
on:
push:
branches: [main, v2]
pull_request:
workflow_dispatch:
permissions:
contents: read
jobs:
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 1-73: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml around lines 1 - 9, Add an explicit top-level
permissions block to the "Test" workflow to enforce least-privilege rather than
inheriting repo defaults: declare a minimal permissions map (for example,
permissions: contents: read and any other specific scopes the jobs actually
need) at the top-level of the workflow file so jobs cannot use broader rights;
update only the workflow header (the "name: Test" workflow) and ensure each job
uses only the required permission scopes.

os: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the workflow with line numbers (small file)
echo "=== .github/workflows/test.yml ==="
wc -l .github/workflows/test.yml
nl -ba .github/workflows/test.yml | sed -n '1,120p'

# Find action.yml (either root or within .github/actions)
echo
echo "=== Find action.yml files ==="
fd -t f -a action.yml . | head -n 50

# If root action.yml exists, print inputs section
if [ -f action.yml ]; then
  echo
  echo "=== action.yml (root) ==="
  nl -ba action.yml | sed -n '1,200p'
fi

# If there's an action.yml under .github, print any inputs.cli-version defaults
echo
echo "=== Search for cli-version default ==="
rg -n "cli-version" -S .github action.yml || true

Repository: deployhq/deployhq-action

Length of output: 194


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the workflow with line numbers (small file)
echo "=== .github/workflows/test.yml ==="
wc -l .github/workflows/test.yml
nl -ba .github/workflows/test.yml | sed -n '1,160p'

# Find action.yml files
echo
echo "=== Find action.yml files ==="
fd -t f action.yml . | head -n 50

# If root action.yml exists, print inputs section
if [ -f action.yml ]; then
  echo
  echo "=== action.yml (root) ==="
  nl -ba action.yml | sed -n '1,220p'
fi

# Search for cli-version default and any secrets gating
echo
echo "=== Search for cli-version / TEST_DEPLOYHQ_API_KEY / persist-credentials ==="
rg -n "cli-version|TEST_DEPLOYHQ_API_KEY|persist-credentials|permissions:" . || true

Repository: deployhq/deployhq-action

Length of output: 194


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== .github/workflows/test.yml (line-numbered) ==="
wc -l .github/workflows/test.yml
awk '{printf "%4d\t%s\n", NR, $0}' .github/workflows/test.yml | sed -n '1,200p'

echo
echo "=== Find action.yml files ==="
fd -t f action.yml . | head -n 50

echo
echo "=== Print any action.yml files (first ~220 lines) and search inputs/defaults ==="
for f in $(fd -t f action.yml .); do
  echo
  echo "---- $f ----"
  sed -n '1,220p' "$f"
done

echo
echo "=== Search for persist-credentials / permissions / TEST_DEPLOYHQ_API_KEY / cli-version ==="
rg -n "persist-credentials|permissions:|TEST_DEPLOYHQ_API_KEY|cli-version|actions/checkout@v4|actions/checkout@" -S . || true

Repository: deployhq/deployhq-action

Length of output: 7534


🌐 Web query:

GitHub actions checkout@v4 persist-credentials default value

💡 Result:

The default value of persist-credentials for actions/checkout@v4 is effectively true (credentials are persisted in the local git config unless you set persist-credentials: false). [1][2] What this means: - actions/checkout “persists the auth token in the local git config” so subsequent steps can run authenticated git commands. [1][2] - The docs say you can opt out by setting persist-credentials: false. [1][2] Example (opt out): uses: actions/checkout@v4 with: persist-credentials: false

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Locate scripts/install-cli.sh ==="
fd -t f install-cli.sh . || true

echo
echo "=== Print scripts/install-cli.sh (first 200 lines) ==="
sed -n '1,220p' scripts/install-cli.sh

echo
echo "=== Search for DHQ_VERSION usage ==="
rg -n "DHQ_VERSION|inputs\\.cli-version|cli-version" scripts -S || true

Repository: deployhq/deployhq-action

Length of output: 3270


Pin actions/checkout to a commit SHA and disable credential persistence.

actions/checkout@v4 is used on lines 20 and 39 without SHA pinning, and persist-credentials is not set to false, so credentials are persisted in the local git config during CI.

🔒 Suggested hardening
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<pinned-commit-sha> # v4.x
+        with:
+          persist-credentials: false
...
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<pinned-commit-sha> # v4.x
+        with:
+          persist-credentials: false
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 20-20: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml at line 20, Replace both occurrences of "uses:
actions/checkout@v4" in the workflow with a SHA-pinned checkout reference and
disable credential persistence by adding the input "persist-credentials: false";
specifically update the steps that currently say uses: actions/checkout@v4 so
they use the actions/checkout commit SHA (pin to the latest stable commit SHA)
and include persist-credentials: false under that step to prevent CI from
writing credentials into the local git config.

- name: Run install-cli.sh
shell: bash
env:
DHQ_VERSION: v0.17.1
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate workflow =="
ls -la .github/workflows || true
sed -n '1,120p' .github/workflows/test.yml

echo
echo "== Find action.yml =="
ls -la .github || true
ls -la . || true

# Try common locations for action.yml
for f in "action.yml" ".github/action.yml" ".github/actions/action.yml" ".github/actions/test/action.yml"; do
  if [ -f "$f" ]; then
    echo "== Contents: $f =="
    sed -n '1,200p' "$f"
  fi
done

echo
echo "== Search for DHQ_VERSION and cli-version default =="
rg -n "DHQ_VERSION|cli-version|inputs:|action.yml" .github -S || true
rg -n "DHQ_VERSION|cli-version|inputs:" . -S --hidden || true

Repository: deployhq/deployhq-action

Length of output: 8861


Avoid duplicating the dhq CLI version pin in this workflow

  • .github/workflows/test.yml line 25 hardcodes DHQ_VERSION: v0.17.1, duplicating the pinned value already defined in action.yml as inputs.cli-version.default (and used by the composite action via DHQ_VERSION: ${{ inputs.cli-version }}), which can drift over time.
          DHQ_VERSION: v0.17.1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml at line 25, The workflow hardcodes DHQ_VERSION:
v0.17.1 which duplicates the pinned value in action.yml
(inputs.cli-version.default) and can drift; remove the hardcoded DHQ_VERSION
assignment and instead reference the composite action input (use DHQ_VERSION:
${{ inputs.cli-version }} or omit the env assignment so the composite action's
DHQ_VERSION input is used) to ensure the version is sourced from
inputs.cli-version rather than duplicated.

Comment on lines +41 to +51
- name: Skip if secrets are missing
id: gate
env:
API_KEY: ${{ secrets.TEST_DEPLOYHQ_API_KEY }}
run: |
if [[ -z "$API_KEY" ]]; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "::notice::TEST_DEPLOYHQ_API_KEY not set; skipping dry-run job."
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Gate on all required dry-run secrets, not only API key.

Line 46 checks only TEST_DEPLOYHQ_API_KEY, but the action step also requires account, email, project, and server. Partial secret configuration will fail instead of cleanly skipping.

✅ Suggested gate logic
       - name: Skip if secrets are missing
         id: gate
         env:
           API_KEY: ${{ secrets.TEST_DEPLOYHQ_API_KEY }}
+          ACCOUNT: ${{ secrets.TEST_DEPLOYHQ_ACCOUNT }}
+          EMAIL: ${{ secrets.TEST_DEPLOYHQ_EMAIL }}
+          PROJECT: ${{ secrets.TEST_DEPLOYHQ_PROJECT }}
+          SERVER: ${{ secrets.TEST_DEPLOYHQ_SERVER }}
         run: |
-          if [[ -z "$API_KEY" ]]; then
+          missing=()
+          [[ -z "${API_KEY:-}" ]] && missing+=("TEST_DEPLOYHQ_API_KEY")
+          [[ -z "${ACCOUNT:-}" ]] && missing+=("TEST_DEPLOYHQ_ACCOUNT")
+          [[ -z "${EMAIL:-}" ]] && missing+=("TEST_DEPLOYHQ_EMAIL")
+          [[ -z "${PROJECT:-}" ]] && missing+=("TEST_DEPLOYHQ_PROJECT")
+          [[ -z "${SERVER:-}" ]] && missing+=("TEST_DEPLOYHQ_SERVER")
+
+          if ((${`#missing`[@]})); then
             echo "skip=true" >> "$GITHUB_OUTPUT"
-            echo "::notice::TEST_DEPLOYHQ_API_KEY not set; skipping dry-run job."
+            echo "::notice::Missing required test secrets: ${missing[*]}. Skipping dry-run job."
           else
             echo "skip=false" >> "$GITHUB_OUTPUT"
           fi
📝 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.

Suggested change
- name: Skip if secrets are missing
id: gate
env:
API_KEY: ${{ secrets.TEST_DEPLOYHQ_API_KEY }}
run: |
if [[ -z "$API_KEY" ]]; then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "::notice::TEST_DEPLOYHQ_API_KEY not set; skipping dry-run job."
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
- name: Skip if secrets are missing
id: gate
env:
API_KEY: ${{ secrets.TEST_DEPLOYHQ_API_KEY }}
ACCOUNT: ${{ secrets.TEST_DEPLOYHQ_ACCOUNT }}
EMAIL: ${{ secrets.TEST_DEPLOYHQ_EMAIL }}
PROJECT: ${{ secrets.TEST_DEPLOYHQ_PROJECT }}
SERVER: ${{ secrets.TEST_DEPLOYHQ_SERVER }}
run: |
missing=()
[[ -z "${API_KEY:-}" ]] && missing+=("TEST_DEPLOYHQ_API_KEY")
[[ -z "${ACCOUNT:-}" ]] && missing+=("TEST_DEPLOYHQ_ACCOUNT")
[[ -z "${EMAIL:-}" ]] && missing+=("TEST_DEPLOYHQ_EMAIL")
[[ -z "${PROJECT:-}" ]] && missing+=("TEST_DEPLOYHQ_PROJECT")
[[ -z "${SERVER:-}" ]] && missing+=("TEST_DEPLOYHQ_SERVER")
if ((${`#missing`[@]})); then
echo "skip=true" >> "$GITHUB_OUTPUT"
echo "::notice::Missing required test secrets: ${missing[*]}. Skipping dry-run job."
else
echo "skip=false" >> "$GITHUB_OUTPUT"
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml around lines 41 - 51, Update the "Skip if secrets
are missing" step to gate on all required dry-run secrets rather than only
TEST_DEPLOYHQ_API_KEY: check TEST_DEPLOYHQ_API_KEY, TEST_DEPLOYHQ_ACCOUNT,
TEST_DEPLOYHQ_EMAIL, TEST_DEPLOYHQ_PROJECT and TEST_DEPLOYHQ_SERVER and set
skip=true if any are empty; otherwise set skip=false. Modify the shell logic in
that step (the run block) to test each environment variable (e.g., using a loop
or combined conditional) and emit the same GITHUB_OUTPUT and notice when
skipping so the job cleanly skips on partial secret configuration.

Comment thread scripts/deploy.sh
Comment on lines +106 to +114
timeout_seconds="${INPUT_TIMEOUT:-0}"
started=$(date +%s)
poll_interval=5

echo "Waiting for deployment ${deployment_id} to complete..."
while true; do
if [[ "$timeout_seconds" -gt 0 ]]; then
elapsed=$(( $(date +%s) - started ))
if [[ $elapsed -ge $timeout_seconds ]]; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate INPUT_TIMEOUT before numeric comparison.

Line 112 performs integer comparison, but INPUT_TIMEOUT is user-provided string input. A non-numeric value can fail with a shell arithmetic error instead of a clear validation message.

💡 Proposed fix
 if truthy "${INPUT_WAIT:-}" && ! truthy "${INPUT_DRY_RUN:-}"; then
     timeout_seconds="${INPUT_TIMEOUT:-0}"
+    if ! [[ "$timeout_seconds" =~ ^[0-9]+$ ]]; then
+        echo "INPUT_TIMEOUT must be a non-negative integer number of seconds." >&2
+        exit 1
+    fi
     started=$(date +%s)
     poll_interval=5
📝 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.

Suggested change
timeout_seconds="${INPUT_TIMEOUT:-0}"
started=$(date +%s)
poll_interval=5
echo "Waiting for deployment ${deployment_id} to complete..."
while true; do
if [[ "$timeout_seconds" -gt 0 ]]; then
elapsed=$(( $(date +%s) - started ))
if [[ $elapsed -ge $timeout_seconds ]]; then
timeout_seconds="${INPUT_TIMEOUT:-0}"
if ! [[ "$timeout_seconds" =~ ^[0-9]+$ ]]; then
echo "INPUT_TIMEOUT must be a non-negative integer number of seconds." >&2
exit 1
fi
started=$(date +%s)
poll_interval=5
echo "Waiting for deployment ${deployment_id} to complete..."
while true; do
if [[ "$timeout_seconds" -gt 0 ]]; then
elapsed=$(( $(date +%s) - started ))
if [[ $elapsed -ge $timeout_seconds ]]; then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/deploy.sh` around lines 106 - 114, Validate INPUT_TIMEOUT before
using it in integer comparisons: ensure INPUT_TIMEOUT contains only digits
(e.g., test with a regex like '^[0-9]+$' or use a numeric-check function) and if
it is empty or non-numeric, fall back to default 0 or exit with a clear error;
then assign timeout_seconds="${INPUT_TIMEOUT:-0}" (or the validated numeric
value) and use that safe timeout_seconds in the while loop comparison that
references elapsed and timeout_seconds. Make the check near the assignment and
reference the variables timeout_seconds, INPUT_TIMEOUT, started, elapsed, and
the while loop to locate the code to modify.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 586fc977bc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/deploy.sh

# --- Wait loop ----------------------------------------------------------------

if truthy "${INPUT_WAIT:-}" && ! truthy "${INPUT_DRY_RUN:-}"; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Respect wait=true for dry-run deployments

The wait loop is explicitly disabled whenever dry-run is truthy, so a workflow that sets dry-run: "true" and leaves wait at its default "true" will always return immediately with a queued status and exit 0, even if the preview later fails. That creates false-positive CI results for PR validation flows that rely on dry-run to catch deployment issues.

Useful? React with 👍 / 👎.

Comment thread scripts/deploy.sh
Comment on lines +124 to +126
if [[ -z "$show_output" ]] || ! echo "$show_output" | jq -e '.ok == true' >/dev/null 2>&1; then
echo "Failed to fetch deployment status; will retry..."
continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop infinite polling on unrecoverable status fetch errors

When dhq deployments show fails or returns a non-OK envelope, the loop only logs and retries forever; with the default timeout of 0, this can hang the job indefinitely on persistent failures (for example, bad project resolution or repeated API/auth errors). The action should fail after a bounded number of consecutive polling errors or require a finite timeout by default.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant