Skip to content

Close ArgoCD change_context double-fetch bug; raise project agent effort#297

Merged
Szer merged 1 commit into
mainfrom
fix/argocd-history-populate
Jul 27, 2026
Merged

Close ArgoCD change_context double-fetch bug; raise project agent effort#297
Szer merged 1 commit into
mainfrom
fix/argocd-history-populate

Conversation

@Szer

@Szer Szer commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Investigated the claimed "ArgoCD deploy history never populates" bug in monitor.yml. The stated leading hypothesis (change_context running after VPN disconnect) does not hold on current main — see evidence below. Instead found and closed a real, latent bug: the change_context step fetched the ArgoCD application JSON twice per bot (once via probe_optional(), once more unguarded), and the second unguarded curl can abort the whole gather loop under set -e/pipefail on a transient failure, starving later bots in the loop of their change_context_manifest.json — producing exactly the guard's "emitted no valid manifest" fallback text.
  • scripts/gather/lib.sh: added probe_optional_body() — same status/log semantics as probe_optional(), but captures the response body to a file in the same request, so the caller never needs a second network round-trip.
  • .github/workflows/monitor.yml: change_context step now does one fetch (probe_optional_body) and reads history from the captured file instead of re-curling.
  • .github/workflows/project.yml:101: effort: loweffort: medium (owner-approved). Model (gpt-5-mini) unchanged. product.yml/monitor.yml untouched.

Evidence the step-ordering hypothesis is false

Verbatim ordered step names in the gather job, before (origin/main) and after this change — identical, Gather runtime + baseline evidence (which contains the ArgoCD-history fetch) already runs before Disconnect VPN (post gather):

Ensure labels exist (bot:<name>, monitor, anomaly, priority-*)
Alita daily-cadence gate
Fetch or bootstrap agent-state branch
Setup WireGuard VPN
Gather runtime + baseline evidence (bash loop, not matrix — single VPN session)
Disconnect VPN (post gather)
Guard — evaluate each bot's evidence independently
Append this run + compute baseline stats + fetch fingerprints (bots that passed the guard only)
Detect mechanical P1 (bash, NOT the agent's judgment — see monitor.md "P1" section)
Commit + push agent-state
Upload evidence artifact for the agent job

Confirmed independently via the GitHub Actions API for run 30233672995 (head sha 93361b89, identical to what's on main): step 7 "Gather runtime + baseline evidence" ran 03:04:47Z03:05:47Z; step 8 "Disconnect VPN (post gather)" started at 03:05:47Z — the ArgoCD fetch runs while the VPN is still up.

Also: the specific "binding fact" that run 30233672995's manifest shows degraded(optional): ... emitted no valid manifest does not match what I found. I downloaded that run's monitor-evidence artifact directly (gh run download 30233672995 -n monitor-evidence) and every bot's change_context_manifest.json reads {"argocd_history":"ok"}, with real deploy-history entries populated in change_context.md (e.g. coupon: 4 entries with real id/revision/deployedAt). The next scheduled run at the same commit (30240150429) shows the same — ok for both bots that ran. I could not reproduce the "never populates" symptom on the current main commit across the two most recent runs. The double-fetch bug fixed here is a real, verifiable robustness gap (a transient failure on the redundant second curl would produce exactly the fallback text quoted in the brief for bots processed later in the loop) even though it did not happen to trigger in the two runs I could inspect.

Fixture proof: optional failure still yields degraded(optional) + clean_bots inclusion

Ran a fixture script that sources the real scripts/gather/lib.sh and re-runs the guard step's BAD-computation logic verbatim against a bot whose ArgoCD-history fetch is wired to an always-failing fake curl:

=== Step 1: run probe_optional_body against a curl that always fails ===
[09:34:10] PROBE DEGRADED (optional) [argocd_history] (exit 7): curl: (7) Failed to connect to argo.internal port 80: Connection refused
ARGOCD_HISTORY_STATUS=[degraded(optional): curl: (7) Failed to connect to argo.internal port 80: Connection refused]
change_context_manifest.json:
{"argocd_history":"degraded(optional): curl: (7) Failed to connect to argo.internal port 80: Connection refused"}

APP_JSON_FILE contents (must be empty, not stale/partial): []

=== Step 3: run the guard step's logic, VERBATIM from monitor.yml ===
::warning::bot=testbot: evidence clean (required sources ok) but OPTIONAL source(s) degraded — proceeding anyway: argocd_history: degraded(optional): curl: (7) Failed to connect to argo.internal port 80: Connection refused
run_testbot=true

=== RESULTS ===
SKIPPED=[]
clean_bots=["testbot"]

=== ASSERTIONS ===
PASS: ARGOCD_HISTORY_STATUS starts with 'degraded(optional):'
PASS: SKIPPED is empty — testbot was not skipped
PASS: clean_bots == ["testbot"] — bot still included despite ArgoCD-history failure
FIXTURE TEST: PASSED

PR #293's required-vs-optional split is preserved: an ArgoCD-history failure degrades change_context only, never skips the bot.

Validation gates

  • actionlint (v1.7.12, shellcheck 0.11.0 on PATH) over .github/workflows/**: EXIT=0, zero findings.
  • bash -n scripts/gather/lib.sh: EXIT=0.
  • shellcheck -x scripts/gather/lib.sh: EXIT=0.

Deviations from the brief

  • The brief's leading hypothesis (step-ordering) and its cited "binding fact" about run 30233672995 do not match the current repository state or that run's actual artifact — reported above with verbatim evidence rather than silently re-deriving a different story.
  • Did not touch the VPN session count, .github/prompts/sre.md, src/, tests/, or any deploy workflow.
  • Not merging this PR per the brief.

🤖 Generated with Claude Code

https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU

… agent effort

monitor.yml's change_context step already issued the ArgoCD-history probe
BEFORE "Disconnect VPN (post gather)" (confirmed via source + actual step
timestamps on runs 30233672995/30240150429) — the step-ordering hypothesis
does not hold on current main. What the code did do wrong: it fetched the
ArgoCD application JSON TWICE per bot — once via probe_optional() (body
discarded) and again, unguarded, to build the displayed history. Under this
step's `set -e`/`pipefail`, a transient failure on that second, unguarded
curl (even after the probe itself succeeded) aborts the whole gather loop,
silently starving every bot processed afterwards of its
change_context_manifest.json — exactly the guard's "emitted no valid
manifest" fallback text.

Add probe_optional_body() to lib.sh: same status/log semantics as
probe_optional(), but captures the response body to a file in the same
request, so callers needing the body don't issue a second, unguarded fetch.
monitor.yml's change_context step now fetches the ArgoCD application once
and reads history from the captured file. PR #293's required-vs-optional
split is unchanged: probe failure still yields "degraded(optional): ..."
and the bot still lands in clean_bots (see fixture test in the PR
description).

Also raises project.yml's codex-action effort from low to medium per owner
approval (PR #295 made a silent-summary miss visible; this addresses the
next lever). product.yml and monitor.yml agent effort are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
@Szer
Szer merged commit c90d5fb into main Jul 27, 2026
5 checks passed
@Szer
Szer deleted the fix/argocd-history-populate branch July 27, 2026 09:37
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