Skip to content

ci: route reviews via Dario/Hermes on logan-gl502vs#17

Open
falkoro wants to merge 5 commits into
masterfrom
ci/dario-hermes-logan-gl502vs
Open

ci: route reviews via Dario/Hermes on logan-gl502vs#17
falkoro wants to merge 5 commits into
masterfrom
ci/dario-hermes-logan-gl502vs

Conversation

@falkoro
Copy link
Copy Markdown
Owner

@falkoro falkoro commented Jun 7, 2026

Claude via Dario proxy; Grok via Hermes. Runner label logan-gl502vs for personal reviews.

Copilot AI review requested due to automatic review settings June 7, 2026 10:52
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Claude and Grok review GitHub Actions workflows to run on a specific self-hosted runner (logan-gl502vs) and to resolve the Dario/Hermes proxy endpoints via shared scripts checked out from spot-techno/.github.

Changes:

  • Route both review jobs to runs-on: [self-hosted, logan-gl502vs].
  • Add a sparse checkout of shared proxy-resolution scripts from spot-techno/.github and execute them to compute proxy settings.
  • Configure Claude to use Dario-provided ANTHROPIC_BASE_URL / ANTHROPIC_API_KEY and Grok to use Hermes-provided URL output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
.github/workflows/grok-review.yml Switch runner label and replace inline Hermes discovery with a shared resolver script checkout + execution.
.github/workflows/claude-review.yml Switch runner label and add Dario resolver script checkout + execution; wire resolved base URL/API key into Claude action env.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 29 to 32
- uses: actions/checkout@v5
with:
fetch-depth: 0

Comment on lines 26 to 29
- uses: actions/checkout@v5
with:
fetch-depth: 0

Comment thread .github/workflows/grok-review.yml Outdated
Comment on lines +21 to +27
- uses: actions/checkout@v5
with:
repository: spot-techno/.github
path: .spot-github
sparse-checkout: |
scripts/resolve-hermes-proxy.sh
sparse-checkout-cone-mode: false
Comment thread .github/workflows/claude-review.yml Outdated
Comment on lines +18 to +24
- uses: actions/checkout@v5
with:
repository: spot-techno/.github
path: .spot-github
sparse-checkout: |
scripts/resolve-dario-proxy.sh
sparse-checkout-cone-mode: false
@falkoro
Copy link
Copy Markdown
Owner Author

falkoro commented Jun 7, 2026

🧠 Grok Composer review (grok-composer-2.5)

Review

1. Brand consistency

No issues. The diff touches only GitHub Actions workflows and runner helper scripts — no logos, favicons, og:image, schema markup, or marketing UI.

2. Spot Suite architecture

No Spot Suite runtime issues (no Workers, D1, tenant scoping, OAuth flows, migrations, or Pages deploys).

CI credential handling — worth a look:

  • claude-review.yml:32anthropic_api_key: dario is hardcoded in the workflow instead of using a GitHub secret or the script output.
  • resolve-dario-proxy.sh:18-19 — writes api_key=dario to GITHUB_OUTPUT, but the workflow ignores steps.dario.outputs.api_key and duplicates the value at claude-review.yml:32.
  • resolve-dario-proxy.sh:14 — probe uses the same bearer token, so the key is committed in two places.

If dario is only a local-proxy passphrase on an isolated self-hosted host, risk is low; it still matches “credential in git” and will drift if the proxy key changes.

Ops / portability (not checklist violations):

  • claude-review.yml:16 and grok-review.yml:19 — runner label changed from spot-tech-ci to logan-gl502vs, so reviews only run where Dario/Hermes are reachable on the host.

3. Correctness + security

No auth bypass, injection, tenant isolation, or customer-copy issues.

  • Replacing secrets.CLAUDE_CODE_OAUTH_TOKEN with a host-local Dario proxy (claude-review.yml:29-32) is coherent for a machine-bound self-hosted setup; OAuth stays on the host, not in the workflow.
  • resolve-hermes-proxy.sh and the grok workflow refactor (grok-review.yml:27) — health-check extraction looks correct; behavior should match the previous inline script.
  • resolve-dario-proxy.sh:11-16 — POST probe with short timeout is a reasonable reachability check beyond /status.

Positive notes

  • Shared scripts for proxy discovery are a clean DRY improvement (grok-review.yml:27).
  • set -euo pipefail and bounded curl timeouts are appropriate for runner scripts.

Verdict: Clean on brand and Spot Suite product architecture; approve with the minor note that the Dario proxy key is hardcoded in git and should stay host-local or move to a GitHub secret if reused elsewhere.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 7, 2026

🔎 Grok review (Hermes · grok-4.3)

SECURITY: Hardcoded credential dario committed in multiple places.

  • .github/workflows/claude-review.yml:30: anthropic_api_key: dario (also passed via env).
  • .github/scripts/resolve-dario-proxy.sh:13: authorization: Bearer dario and api_key=dario written to $GITHUB_OUTPUT.

These tokens are now stored in the repo and will be visible in workflow logs/runs.

Verdict: Security regression from committing proxy credentials; do not merge.

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.

2 participants