Skip to content

Agent-flows Phase 0: fail-loud metrics, real SRE trigger, AlitaBot docs, orphan cleanup#274

Merged
Szer merged 1 commit into
mainfrom
fix/agent-flows-phase-0
Jul 26, 2026
Merged

Agent-flows Phase 0: fail-loud metrics, real SRE trigger, AlitaBot docs, orphan cleanup#274
Szer merged 1 commit into
mainfrom
fix/agent-flows-phase-0

Conversation

@Szer

@Szer Szer commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

Phase 0 of the approved agent-flows redesign (.github/AGENT-FLOWS-REDESIGN.md, §4 "Phase 0 — stop the lying"). Four mechanical fixes:

  1. scripts/gather-metrics.sh fails loud. Removed the || echo '{"data":{"result":[]}}' fallback that made an unreachable Prometheus/Loki/ArgoCD render as "zero restarts, zero errors, zero 5xx" — the same class of bug that blinded the product agent for 21 runs (Fix: product agent's DB queries silently time out over split-tunnel VPN #264). Every source is now probed up front with retry-then-exit 1 (matching gather-product-data.sh's style), and the report leads with a machine-readable {"sources":{"prometheus":"ok","loki":"ok","argocd":"ok"}} manifest line for a later automated gate to key off.
  2. SRE agent rewired so it actually runs. sre.yml's issues: [labeled] trigger has fired 0/6 times in practice — the deploy-failure issue is created with secrets.GITHUB_TOKEN, and GitHub Actions never triggers workflows from default-token-authored events (anti-recursion). Replaced with _sre-agent.yml, a reusable workflow called directly from _bot-deploy.yml when the deploy or its verification fails, with bot identity (bot, argocd-app-name, container-name, docker-image, commit, run-url, optional issue-number) passed as workflow inputs instead of read from an issue body. Added sre-manual.yml (workflow_dispatch) as a hand-triggered entry point, since workflow_call workflows can't also declare workflow_dispatch. sre.md: bot identity now comes from the prompt; the 5xx-based health checks are replaced with Loki level="Error" rate (the /bot webhook always returns HTTP 200 to Telegram regardless of internal exceptions, so 5xx was structurally dead logic); added an if: always() step that re-enables ArgoCD auto-sync so a killed run can't leave it permanently off.
    • Also threaded AZURE_OPENAI_API_KEY/AZURE_OPENAI_BASE_URL through _bot-deploy.yml's own secrets: contract and its three callers (vahter-deploy.yml, coupon-deploy.yml, alita-deploy.yml) — required so secrets: inherit on the new sre: job actually has those secrets to inherit, since a reusable workflow only receives what its own caller explicitly passed it.
  3. AlitaBot added to AGENTS.md. Repository structure, DB, its three workflows, command/feature summary, and corrected the opening line that described this as a two-bot repo.
  4. Deleted the orphaned .github/prompts/review.md (invoked by no workflow; references MCP tools the codex-action runtime doesn't provide) and its one reference in project.md.

Full rationale: .github/AGENT-FLOWS-REDESIGN.md (§1, §3.5, §4 Phase 0, §9).

Scope note — if: failure() interpretation in _bot-deploy.yml

The new sre job uses needs: [deploy, verify-deploy, notify-failure], if: ${{ failure() && inputs.sre-enabled }} — same shape as the existing notify-failure job, so the SRE agent is guaranteed to run whenever deploy or verify-deploy fails (GitHub's failure() covers the whole transitive needs chain, so it stays true even though notify-failure itself typically succeeds). notify-failure is also a need so the new job can consume its outputs.issue-number (captured from gh issue create's output URL) and pass it through to _sre-agent.yml; if notify-failure didn't run or its output is empty, issue-number is simply blank and the prompt tells the agent to skip issue comment/close steps.

Deviations

  • Neither shellcheck nor actionlint are installed in this environment — validated with bash -n (all touched scripts) and python3 -c "import yaml..." (all touched/created workflow YAML) instead, per the brief's documented fallback.
  • The brief's "run locally with unreachable hosts (the *.internal endpoints will not resolve without VPN)" assumption didn't hold in this sandbox — it turns out to have live routes to the real *.internal infra (confirmed: prometheus.internal/argo.internal both actually resolve and respond here). Used a genuinely-nonexistent hostname (prometheus.bogus-host-that-does-not-exist.internal, confirmed via getent hosts → exit 2) to faithfully reproduce "unreachable" and prove the fail-loud path; also exercised a real-but-wrong-credential ArgoCD 401 as a second failure mode. Both produce curl's real diagnostic text and a non-zero exit with no report printed — see the PR description checks below / test-plan.
  • src/CouponHubBot/docs/PROJECT-AGENT.md still documents the old sre.yml/issues: labeled mechanism — left untouched per "do not modify anything under src/"; flagging as a known follow-up for whoever picks up Phase 1.
  • Not done, and out of scope per the brief: adding AlitaBot to the BotNotAvailable/ErrorLogsDetected Grafana alert rules (Phase 0 item 3 in the redesign doc) — that's Grafana provisioning config, not something editable in this repo, and wasn't one of the four listed changes.

Test plan

  • bash -n on scripts/gather-metrics.sh — clean
  • python3 -c "import yaml..." on every created/edited workflow YAML — clean
  • Ran gather-metrics.sh against a genuinely-unreachable Prometheus host: exits 1, prints the real curl: (6) Could not resolve host: ... error, prints no report
  • Ran gather-metrics.sh against a real ArgoCD endpoint with a bad token: exits 1, prints curl: (22) The requested URL returned error: 401, prints no report
  • Not run: full success path (all three sources reachable + valid ArgoCD token) — no valid prod credentials in this environment
  • Not run: an actual CI dispatch of the new _sre-agent.yml/sre-manual.yml — this PR is not merged, per instructions

🤖 Generated with Claude Code

https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU

…cs, orphan cleanup

Four mechanical fixes from .github/AGENT-FLOWS-REDESIGN.md's Phase 0:

- scripts/gather-metrics.sh: remove the `|| echo '{"data":{"result":[]}}'`
  failure-swallowing fallback (same class of bug fixed in #264's
  gather-product-data.sh). Every source (Prometheus/Loki/ArgoCD) is now
  probed up front with retry-then-exit-1, and the report leads with a
  machine-readable `{"sources":{...}}` manifest line.
- Rewire the SRE agent so it actually runs: sre.yml's `issues: [labeled]`
  trigger never fired (GITHUB_TOKEN anti-recursion — 6/6 runs skipped,
  7 real deploy-failure issues closed with zero SRE comments). Replaced
  with a reusable _sre-agent.yml called directly from _bot-deploy.yml on
  deploy/verify failure, bot identity passed as workflow inputs instead of
  read from an issue body. Added sre-manual.yml for hand-triggered
  investigations. sre.md: bot identity now comes from the prompt, not an
  issue read; 5xx-based health checks replaced with Loki level="Error"
  rate (the webhook always returns 200, so 5xx was structurally dead);
  added an if: always() ArgoCD auto-sync re-enable step so a killed run
  can't leave it disabled. Also threaded AZURE_OPENAI_API_KEY/BASE_URL
  through _bot-deploy.yml and its three callers, required for
  `secrets: inherit` to actually reach the nested reusable workflow.
- AGENTS.md: documented AlitaBot (repo structure, DB, workflows,
  command/feature summary) and corrected the "two-bot repo" opening line.
- Deleted the orphaned .github/prompts/review.md (invoked by no workflow,
  references MCP tools codex-action doesn't provide) and its one
  reference in project.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
@Szer
Szer merged commit b6fd29b into main Jul 26, 2026
4 checks passed
@Szer
Szer deleted the fix/agent-flows-phase-0 branch July 26, 2026 23:53
Szer added a commit that referenced this pull request Jul 26, 2026
…275)

PR #274 wired _sre-agent.yml (which requests `contents: write` for the SRE
one-liner-mitigation PR path) into _bot-deploy.yml. All three *-deploy.yml
callers grant `contents: read`. A called reusable workflow cannot escalate
permissions beyond its caller, so GitHub rejected the whole run at load time:
`Alita: Deploy` run 30226209727 returned startup_failure with zero jobs.

Only Alita surfaced it because alita-deploy.yml is the sole caller whose path
filter includes its own file. Vahter and Coupon were equally broken and would
have failed on their next deploy.

sre-manual.yml had no permissions block at all, so it inherited the repo
default and would have failed the same way; given an explicit one.


Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Szer added a commit that referenced this pull request Jul 27, 2026
* CI: add actionlint gate for .github/workflows/**

Runs actionlint (pinned v1.7.12, commit 914e7df21a07ef503a81201c76d2b11c789d3fca)
on PRs touching .github/** plus workflow_dispatch, failing the PR on any
actionlint error.

Verified against a minimal repro of tonight's PR #274 bug (caller declaring
`permissions: contents: read` calling a reusable workflow that declares
`permissions: contents: write`): actionlint v1.7.12 reports 0 errors, so it
would NOT have caught that specific class — GitHub's permission-escalation
check happens at run-load time, not in actionlint's static analysis. Adding
the gate regardless since it catches many other real workflow bug classes
(shellcheck issues in run: blocks, unknown inputs/outputs, bad expressions).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU

* verify-deploy: classify failures as infra vs app, thread through to SRE agent

verify-deploy.sh now emits failure-class=infra|app|unknown to $GITHUB_OUTPUT
(guarded for local runs) at every existing exit-1 path, without changing any
pass/fail semantics — a failure still fails the deploy exactly as today, only
the classification is new:
  - infra:   could not reach ArgoCD (fail_connectivity) — the 2026-07-23
             argo.internal VPN-drop class (issue #251)
  - app:     control plane reachable, app genuinely unhealthy (readiness,
             Loki errors, 5xx)
  - unknown: reachable but inconclusive (sync never converged in time)

_bot-deploy.yml passes the class through: verify-deploy job output -> deploy-
failure issue body ("**Failure class:**" line, "infra" label added when
applicable) -> _sre-agent.yml call input. _sre-agent.yml gains a matching
optional `failure-class` input, surfaced in the agent's prompt.

sre.md tells the agent that `infra` strongly suggests P3/transient: confirm
ArgoCD actually converged and the app is healthy before opening any
escalation, and prefer closing as transient with a short note — this is
exactly the class of alert that would have fired for the SRE agent's very
first real run tonight (issue #251, VPN drop, deploy had actually succeeded).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU

* CI: suppress pre-existing shellcheck findings surfaced by the new actionlint gate

The new lint-workflows.yml gate (previous commit) surfaced 4 pre-existing
shellcheck findings that were invisible before actionlint was added:

- _bot-build.yml SC2086 x3: printf '  %s\n' $CHANGED/$NON_DOC/$RELEVANT rely
  on intentional word splitting (each var holds a whitespace-separated file
  list; the unquoted splat is what makes printf emit one filename per line).
  Quoting would collapse each list onto a single summary line and silently
  degrade the build-skip decision logging — the linter's suggested fix is
  wrong here, so this is a targeted `disable=SC2086` with a why-comment, not
  a code change.
- vahter-upstream-sync.yml SC2012: `ls vahter-ban-bot.sln* | head -1` is a
  minor, genuine fragility, but switching to `find` touches the upstream
  mirror sync path and is out of scope for a CI-hardening PR — suppressed
  with a comment noting the glob is a fixed, known-safe, non-alphanumeric-free
  pattern, and flagging `find` as a possible follow-up.

Re-ran actionlint v1.7.12 with shellcheck v0.11.0 on PATH (matching the CI
runner) over all of .github/workflows/**: 0 errors in 16 files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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