Skip to content

fix(ci): record a FAILED deployment when the preview deploy aborts - #658

Merged
JSONbored merged 1 commit into
mainfrom
fix/preview-record-failed-deployment
Jun 13, 2026
Merged

fix(ci): record a FAILED deployment when the preview deploy aborts#658
JSONbored merged 1 commit into
mainfrom
fix/preview-record-failed-deployment

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Robustly closes the 'eternal spinner' gap (reviewbot #92).

When the UI Preview Deploy job fails after a successful build — artifact validation rejects the bundle (the .zip regression), the wrangler upload errors, etc. — it produces no preview and records no Deployment. Reviewbot's before/after 'after' cell then sits on an eternal Rendering preview… spinner with no signal that a preview is never coming. Reviewbot can't see this: there's no Deployment to read, and the deploy runs on workflow_run so it isn't a PR check.

Fix: add a failure step (if: failure() && steps.cfg.outputs.ready == 'true') that resolves the PR the same fork-safe way as the success step and records a failure deployment_status (payload {pr, head_sha}). Reviewbot already maps a failed deployment_status to a terminal 'Preview deploy failed — review manually' card (reviewbot #80), so the spinner now resolves to an honest terminal state on any deploy abort, while the source bug is still surfaced by the failing job.

No change to the success path. YAML validated.

When the deploy job fails after the build (artifact validation rejects the bundle,
wrangler upload errors, etc.), it produced no preview and recorded no Deployment —
so Reviewbot's before/after 'after' cell sat on an eternal 'Rendering preview…'
spinner with no signal it would never come (exactly the .zip-allowlist regression).

Add a failure step (if: failure() && CF creds present) that resolves the PR the
same fork-safe way and records a 'failure' deployment_status. Reviewbot already
maps a failed deployment_status to a terminal 'preview deploy failed' card, so the
spinner now resolves to an honest terminal state on any deploy abort.
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #658 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 13, 2026
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 1 changed file(s) — two independent AI reviewers.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR adds a new GitHub Actions step that records a failed preview deployment status when the UI preview job aborts, ensuring Reviewbot shows a terminal failure instead of an endless spinner. The implementation follows existing patterns and does not introduce security or privacy concerns.

Suggestions

  • Consider guarding the steps.cfg.outputs.ready reference (e.g., using steps.cfg?.outputs?.ready) to avoid potential undefined errors if the cfg step is skipped due to an earlier failure.
  • Add a brief comment linking this step to the corresponding success step for future maintainers.

Worth double-checking

  • If the cfg step fails before setting its outputs, the condition may error; ensure the step safely handles missing outputs.
  • Potential duplicate deployments if both success and failure steps run under edge cases (unlikely but worth monitoring).

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR adds a failure-handling step to the UI preview deploy workflow to record a failed deployment when the preview deploy aborts, ensuring Reviewbot updates its UI to show a failure state instead of waiting indefinitely. The change mirrors the existing success step's logic for PR resolution and deployment recording, is gated appropriately on job failure and config readiness, and uses safe practices (workflow_run-sourced SHA, fork-resistant PR resolution).

Suggestions

  • Consider adding continue-on-error: true to the step to avoid logging extraneous failures if the GitHub API calls fail (though the job is already failed, this keeps logs cleaner).
  • Verify that steps.cfg.outputs.ready is reliably set to 'true' only when the deploy attempt begins (to avoid recording failures for unrelated early job failures).

Worth double-checking

  • If the GitHub API calls fail (e.g., rate limits, permissions), the failure status won't be recorded, leaving Reviewbot in a spinner state—but this is no worse than the current behavior and the step already logs a warning if PR resolution fails.
  • Ensure the failure() trigger doesn't capture unrelated job failures (e.g., linting) before the deploy attempt; the steps.cfg.outputs.ready == 'true' gate mitigates this by requiring config readiness.

@ghost ghost added the gittensory-review label Jun 13, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit e4b24d9 into main Jun 13, 2026
10 checks passed
@JSONbored
JSONbored deleted the fix/preview-record-failed-deployment branch June 13, 2026 23:07
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant