feat: Slack channel canvas shows deployment status - #33
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No Slack app needed: CI renders both environments statelessly from the GitHub Deployments API and POSTs to a member-created Slack workflow whose Update-a-canvas step replaces the channel canvas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
decentraland-bot
left a comment
There was a problem hiding this comment.
Review: PR #33 — feat: Slack channel canvas shows deployment status
Well-crafted PR. The stateless, idempotent design is the right call — every run reconstructs full state from the Deployments API, so any event self-heals the canvas without needing persistent storage. Secrets are properly scoped, permissions are least-privilege, the BASH_SOURCE guard correctly supports both sourcing (for tests) and direct execution, and jq --arg handles all untrusted API data safely. CI is green (all 5 checks pass). No P0 or P1 issues found.
Findings
[P2] echo-embedded fmt_time failure silently swallowed in render_last_deploy_line (update-canvas.sh:73)
render_last_deploy_line wraps fmt_time in echo "... $(fmt_time ...) ..." — set -e does not catch failures of command substitutions embedded in a simple command's arguments (the echo itself succeeds). In contrast, render_running_line uses line+="... $(fmt_time ...)", where the assignment does propagate the failure. In practice this only matters if the GitHub API returns a malformed timestamp (extremely unlikely on ubuntu-latest with GNU date), but the asymmetry is worth noting. A local ts; ts="$(fmt_time ...)" before the echo would make both functions consistent.
[P2] Superseded v1 plan doc ships in the PR (docs/superpowers/plans/)
The plan (774 lines) describes the abandoned Slack-app design and references secrets that won't exist (SLACK_BOT_TOKEN, SLACK_PULSE_CHANNEL_ID). The "SUPERSEDED" banner is clear, but shipping stale planning docs that name non-existent secrets is a potential confusion vector for future readers or automated agents. Consider whether these should live in git history only (omit from this PR) rather than as shipped documentation. The spec (docs/superpowers/specs/) is fine — it has a v2 banner that accurately describes the merged implementation.
[P2] No test coverage for statuses-endpoint failure path
The gh_error scenario only exercises the deployments endpoint returning HTTP 500. A separate scenario where a specific deployment's statuses endpoint fails would confirm the latest_status → exit 1 → set -e propagation chain works end-to-end (it does — I verified manually — but a regression test would be valuable).
[P2] actions/checkout@v4 pinned by tag, not SHA (slack-canvas.yml:30)
Standard supply-chain hardening pins third-party actions to a commit SHA. Low risk given this is the official actions/checkout, but worth aligning with best practices.
[P2] No --max-time on curl calls
A hung connection to the GitHub API or Slack webhook could stall the job until the 5-minute timeout-minutes kills the whole workflow run. Adding --max-time 30 (or similar) to gh_get and the webhook POST would fail faster and produce a clearer error.
[P2] Minor: all-deployments-without-status edge case
If every deployment on the page has zero statuses (all skipped via continue), the canvas shows RUNNING_LINE="⚠️ no recent success" alongside LAST_LINE="—", which is slightly contradictory. Extremely unlikely in practice (statuses arrive almost immediately), but a comment in collect_env would clarify the intent.
Security
No security issues found. Secrets are step-scoped env vars (never written to disk or echoed). Webhook URL is a GitHub Actions secret with automatic log redaction. All API data flows through jq --arg for safe JSON escaping — no shell injection vectors. Workflow permissions are minimal (contents: read, deployments: read). The ref: main checkout prevents a malicious deploy branch from smuggling modified action code.
Architecture
Sound design. The stateless full-render approach with cancel-in-progress: true is correct for this use case — any run is self-consistent and the newest one always wins. The MAX_DEPLOYMENTS=30 ceiling is reasonable (the loop breaks early on the first success, so the common case is 1–2 API calls per environment). Test coverage is thorough: 5 scenarios covering normal, empty, API error, webhook rejection, and degraded states.
Git conventions (ADR-6) ✅
- PR title
feat: Slack channel canvas shows deployment status— correct semantic commit format - Branch
feat/slack-canvas-deploy-status— correct<type>/<summary>pattern
Reviewed by Jarvis 🤖 · Requested by Mikhail Agapov (<@U04A7TYN13L>) via Slack
…tusless edge - --max-time 30 on GitHub API and webhook curls - shopt -s inherit_errexit so failures inside $(render/jq) assignments propagate - all-statusless page keeps both placeholder lines instead of contradicting itself - new flow scenarios: statuses-endpoint failure, statusless; superseded v1 plan doc untracked Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review addressed in 13ae07e — all six P2s:
Flow suite is now 7 scenarios; render + flow green locally and in the 🤖 Generated with Claude Code |
WebTransport (#25) + Slack canvas (#33). Resolutions: metrics files keep both worlds (per-transport counters + latency histograms); drain-cycle and RTT histograms stay top-level on TransportSnapshot (ENet-side measurements); ENetHostedService keeps main's class shape (no ITransport) plus the continent resolver; five hand-built test snapshots gained ByTransport.
Keeps the Pulse Slack channel's canvas in sync with deployments (dev and prd): which branch/tag and commit is running, the outcome of the latest deploy attempt, since when — updated automatically on every deployment, with no Slack app (delivery is a member-created Workflow Builder webhook).
How it works
deployment_status— every deploy flow already ends indcl-deploy-action, which creates a GitHub Deployment whose statusdecentraland-botdrives through the real container rollout. Filtered totask == "dcl/container-deployment"in statesin_progress|success|failure|error; no existing deploy workflow is modified.⚠️ no recent success) and POSTs nine plain-text variables to a Slack Workflow Builder webhook whose Update a canvas step replaces the channel canvas wholesale.Setup required after merge
SLACK_CANVAS_WEBHOOK_URL.Testing
test-render.sh+test-flow.sh(mock GitHub API + webhook; 5 scenarios incl. failed-newest/succeeded-older, all-failures page, API error, webhook rejection) — wired into CI as theslack-canvas-testsjob.🤖 Generated with Claude Code