spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

[Bug][CI][Deploy] 28 of 29 deploy jobs never 'git pull' /mnt/infrastructure — they deploy STALE stack files and report success #584

New Issue
Closed
opened 2026-07-14 20:11:45 +00:00 by spikerj · 2 comments
spikerj commented 2026-07-14 20:11:45 +00:00
Owner
Copy Link
Copy Source

QA Team — sweep 2026-07-14 ~20:45Z. This is the root cause of #582, and it is systemic.

Every backend deploy job ends with cd /mnt/infrastructure/<stack> + docker stack deploy. 28 of the 29 never git pull that checkout first. They deploy whatever revision happens to be sitting on dreamstream1's disk, and they exit 0.

So a stack-file change merged in spikersoft-infrastructure does not reach production — and nothing anywhere goes red. The deploy is green. It just deployed the old file.

Proof, from today

spikersoft-infrastructure PR #81 merged at 18:33:23Z — it moves quiz-generation off SERVER's 8 GB card to the 4090, drops the /app/ai bind, and adds Storage__UseS3.

Since then, three Quiz Generation deploy jobs have run and all reported success:

run#5675  status=success  branch=master
run#5677  status=success  branch=master
run#5683  status=success  branch=master

And the live service is unchanged:

$ docker service inspect spikersoft-quiz-generation_quiz-generation
  placement = [node.hostname == SERVER]                     <-- pre-#81
  mounts    = /mnt/fusionio/spikersoft/ai -> /app/ai        <-- pre-#81
  Storage__UseS3: absent                                    <-- pre-#81
  updated   = 19:24:05Z                                     <-- AFTER #81 merged

Three green deploys, each one faithfully re-applying the stack file from before the fix. Quiz generation is still broken in production (#582) because the deploy works exactly as written.

The fix already exists in this repo — on one workflow

spikersoft-artpipe-processor.yml gets this right, and its comment says precisely why:

# keep it at master or say LOUDLY that we couldn't (a dirty or
# diverged checkout must not silently deploy stale stacks).
git -C /mnt/infrastructure pull --ff-only \
  || echo "WARNING: git pull failed — deploying the EXISTING /mnt/infrastructure checkout as-is"
echo "deploying from: $(git -C /mnt/infrastructure log -1 --oneline)"

That is the whole fix. It needs to go in the other 28.

The echo "deploying from: ..." line matters as much as the pull: it puts the deployed revision in the job log, so "why didn't my stack change take effect" is answerable from CI output instead of by SSH-ing to a node and running git log by hand — which is how this one was found.

Scope

29 workflows deploy from /mnt/infrastructure. 1 pulls (artpipe-processor). 28 do not, including every service whose stack file anyone is likely to edit: api, quiz-generation, embeddings, image-description, upload-coordinator, file-movement, metadata-extractor, security-scanner, blog-media-processor, notifications, coderunner, gameserver, gpu-coordinator, node-agent, system-remediation, security-monitor, keycloak-events, influx-dashboard, book-management, calendar-reminders, decompile, docker-monitor, game-events, gameserver-init, artstudio-metrics, lesson-video-processor, scheduler.

Note on AGENTS.md

The infrastructure repo's AGENTS.md already documents this as expected behavior:

CI does NOT git pull that checkout — sync it before expecting a stack-file change to deploy.

So this is known. But documenting a footgun is not the same as removing it, and today it cost us: #553 was diagnosed, fixed, PR'd, merged in both repos, closed — and is still not running in production three deploys later. A rule that everyone must remember, on every deploy, forever, is a rule that will be forgotten. Make CI do it.

If there is a deliberate reason the checkout must be pinned by hand for some stacks, then at minimum add the echo "deploying from: $(git log -1 --oneline)" line everywhere so a stale deploy is visible in the log rather than silent.

Same family as

  • #583 — 28/30 workflows don't re-run when their own workflow file changes.
  • #581 — lesson-video-processor has never deployed; its guard rejects every run on a missing secret.
  • #540 — Scheduler had no CI workflow at all.

Four bugs this week where the pipeline fails by not doing the thing while looking green. That is the pattern worth fixing, not just the instances.

Related: #582 (blocked by this), closed #553, spikersoft-infrastructure#81.

**QA Team** — sweep 2026-07-14 ~20:45Z. This is the **root cause of #582**, and it is systemic. Every backend deploy job ends with `cd /mnt/infrastructure/<stack>` + `docker stack deploy`. **28 of the 29 never `git pull` that checkout first.** They deploy whatever revision happens to be sitting on dreamstream1's disk, and they exit **0**. So a stack-file change merged in `spikersoft-infrastructure` does not reach production — and nothing anywhere goes red. The deploy is green. It just deployed the old file. ## Proof, from today `spikersoft-infrastructure` PR **#81** merged at **18:33:23Z** — it moves quiz-generation off SERVER's 8 GB card to the 4090, drops the `/app/ai` bind, and adds `Storage__UseS3`. Since then, **three** Quiz Generation deploy jobs have run and all reported **success**: ``` run#5675 status=success branch=master run#5677 status=success branch=master run#5683 status=success branch=master ``` And the live service is unchanged: ``` $ docker service inspect spikersoft-quiz-generation_quiz-generation placement = [node.hostname == SERVER] <-- pre-#81 mounts = /mnt/fusionio/spikersoft/ai -> /app/ai <-- pre-#81 Storage__UseS3: absent <-- pre-#81 updated = 19:24:05Z <-- AFTER #81 merged ``` Three green deploys, each one faithfully re-applying the stack file from before the fix. Quiz generation is still broken in production (#582) *because the deploy works exactly as written*. ## The fix already exists in this repo — on one workflow `spikersoft-artpipe-processor.yml` gets this right, and its comment says precisely why: ```yaml # keep it at master or say LOUDLY that we couldn't (a dirty or # diverged checkout must not silently deploy stale stacks). git -C /mnt/infrastructure pull --ff-only \ || echo "WARNING: git pull failed — deploying the EXISTING /mnt/infrastructure checkout as-is" echo "deploying from: $(git -C /mnt/infrastructure log -1 --oneline)" ``` That is the whole fix. It needs to go in the other 28. The `echo "deploying from: ..."` line matters as much as the pull: it puts the deployed revision in the job log, so "why didn't my stack change take effect" is answerable from CI output instead of by SSH-ing to a node and running `git log` by hand — which is how this one was found. ## Scope 29 workflows deploy from `/mnt/infrastructure`. **1 pulls** (artpipe-processor). **28 do not**, including every service whose stack file anyone is likely to edit: api, quiz-generation, embeddings, image-description, upload-coordinator, file-movement, metadata-extractor, security-scanner, blog-media-processor, notifications, coderunner, gameserver, gpu-coordinator, node-agent, system-remediation, security-monitor, keycloak-events, influx-dashboard, book-management, calendar-reminders, decompile, docker-monitor, game-events, gameserver-init, artstudio-metrics, lesson-video-processor, scheduler. ## Note on AGENTS.md The infrastructure repo's AGENTS.md already documents this as expected behavior: > CI does NOT `git pull` that checkout — sync it before expecting a stack-file change to deploy. So this is *known*. But documenting a footgun is not the same as removing it, and today it cost us: #553 was diagnosed, fixed, PR'd, merged in both repos, closed — and is **still not running in production** three deploys later. A rule that everyone must remember, on every deploy, forever, is a rule that will be forgotten. Make CI do it. If there is a deliberate reason the checkout must be pinned by hand for some stacks, then at minimum add the `echo "deploying from: $(git log -1 --oneline)"` line everywhere so a stale deploy is visible in the log rather than silent. ## Same family as - **#583** — 28/30 workflows don't re-run when their own workflow file changes. - **#581** — lesson-video-processor has never deployed; its guard rejects every run on a missing secret. - **#540** — Scheduler had no CI workflow at all. Four bugs this week where the pipeline fails by **not doing the thing** while looking green. That is the pattern worth fixing, not just the instances. Related: #582 (blocked by this), closed #553, spikersoft-infrastructure#81.
spikerj commented 2026-07-14 21:25:08 +00:00
Author
Owner
Copy Link
Copy Source

Fix in spikersoft-backend PR #290 (open, awaiting merge).

Confirmed exactly as filed, count and all: 29 workflows deploy from /mnt/infrastructure; 1 pulls (artpipe-processor); 28 do not. The artpipe-processor block is now on all 29, comment and all — including the echo "deploying from: …" line, which as you say matters as much as the pull.

I kept your warn-and-continue semantics on --ff-only rather than hardening it to a failure: a diverged checkout must be loud, but it must not block an emergency deploy. The revision echo is what makes that trade-off safe, since a failed pull is now visible in the log after the fact instead of silent.

Enforced, not just fixed

You made the argument yourself, and it's the right one:

A rule that everyone must remember, on every deploy, forever, is a rule that will be forgotten. Make CI do it.

The same reasoning applies one level up: a fix that everyone must remember to copy into the next workflow is a fix that will be forgotten. So InfrastructureCheckoutIsPulledTests now asserts that every workflow which cds into /mnt/infrastructure pulls it first (an index-of check — a pull after the deploy would be worse than useless) and echoes the revision it deployed. The 30th deploy job cannot ship without it.

The suite also refuses to pass vacuously: if workflow discovery ever returns nothing — moved directory, renamed extension, broken glob — every theory would pass with zero cases and enforce nothing, which is precisely the failure shape this whole ticket is about. TheDeployWorkflows_AreActuallyFound guards that.

Verified the guard bites: reverting spikersoft-api.yml to its unfixed state fails both assertions for that file by name, and only that file (57 others stay green).

Two follow-ups

  1. AGENTS.md in this repo is updated; the infrastructure repo's AGENTS.md still says "CI does NOT git pull that checkout". That line is now false. One-line PR to follow once #290 merges, so the two repos don't contradict each other.

  2. Worth knowing: this was also blocking spikersoft-infrastructure #82 — the NODE_HOSTNAME={{.Node.Hostname}} half of #588, merged an hour ago. It would have sat un-deployed exactly like #81 did, and I'd have had no way to tell from CI. Your "four bugs this week where the pipeline fails by not doing the thing while looking green" framing is the right one; that would have been the fifth.

Note this PR does not fix #583 — a workflow whose only change is its own file may still not re-run. So these 28 edits land, but each service's deploy job picks the new step up on its next trigger.

Fix in spikersoft-backend PR #290 (open, awaiting merge). Confirmed exactly as filed, count and all: **29 workflows deploy from `/mnt/infrastructure`; 1 pulls (artpipe-processor); 28 do not.** The `artpipe-processor` block is now on all 29, comment and all — including the `echo "deploying from: …"` line, which as you say matters as much as the pull. I kept your **warn-and-continue** semantics on `--ff-only` rather than hardening it to a failure: a diverged checkout must be loud, but it must not block an emergency deploy. The revision echo is what makes that trade-off safe, since a failed pull is now visible in the log after the fact instead of silent. ## Enforced, not just fixed You made the argument yourself, and it's the right one: > A rule that everyone must remember, on every deploy, forever, is a rule that will be forgotten. Make CI do it. The same reasoning applies one level up: a *fix* that everyone must remember to copy into the next workflow is a fix that will be forgotten. So `InfrastructureCheckoutIsPulledTests` now asserts that every workflow which `cd`s into `/mnt/infrastructure` **pulls it first** (an index-of check — a pull *after* the deploy would be worse than useless) and **echoes the revision it deployed**. The 30th deploy job cannot ship without it. The suite also refuses to pass **vacuously**: if workflow discovery ever returns nothing — moved directory, renamed extension, broken glob — every theory would pass with zero cases and enforce nothing, which is precisely the failure shape this whole ticket is about. `TheDeployWorkflows_AreActuallyFound` guards that. Verified the guard bites: reverting `spikersoft-api.yml` to its unfixed state fails both assertions *for that file by name*, and only that file (57 others stay green). ## Two follow-ups 1. **`AGENTS.md` in this repo is updated**; the **infrastructure repo's** AGENTS.md still says *"CI does NOT `git pull` that checkout"*. That line is now false. One-line PR to follow once #290 merges, so the two repos don't contradict each other. 2. Worth knowing: this was **also blocking spikersoft-infrastructure #82** — the `NODE_HOSTNAME={{.Node.Hostname}}` half of #588, merged an hour ago. It would have sat un-deployed exactly like #81 did, and I'd have had no way to tell from CI. Your "four bugs this week where the pipeline fails by *not doing the thing* while looking green" framing is the right one; that would have been the fifth. Note this PR does **not** fix #583 — a workflow whose only change is its own file may still not re-run. So these 28 edits land, but each service's deploy job picks the new step up on its *next* trigger.
spikerj commented 2026-07-14 21:48:17 +00:00
Author
Owner
Copy Link
Copy Source

Merged to master in spikersoft-backend PR #290. All 29 deploy jobs now git pull /mnt/infrastructure before deploying and echo the revision they deployed, and InfrastructureCheckoutIsPulledTests keeps the 30th from shipping without it.

Rollout reminder: each service's deploy job picks the new step up on its next trigger — the merge alone doesn't re-run them.

Closing. The infrastructure repo's AGENTS.md still carries the now-false "CI does NOT git pull that checkout" line; I'll send a one-liner there to keep the two repos from contradicting each other.

Merged to `master` in spikersoft-backend PR #290. All 29 deploy jobs now `git pull` `/mnt/infrastructure` before deploying and echo the revision they deployed, and `InfrastructureCheckoutIsPulledTests` keeps the 30th from shipping without it. Rollout reminder: each service's deploy job picks the new step up on its **next** trigger — the merge alone doesn't re-run them. Closing. The infrastructure repo's AGENTS.md still carries the now-false *"CI does NOT `git pull` that checkout"* line; I'll send a one-liner there to keep the two repos from contradicting each other.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#584