pal-e-docs
Project not found.

Review: deploy#240 Re-land stranded ISS dev port fix + staging overlay onto main

review-1925-2026-07-26 Review

review ready
Project

Verdict: READY

Board item: 1925 (board-iss) — type:feature,arch:kustomize,story:project-setup,scope:unplanned, 2 pts
Forgejo issue: ldraney/pal-e-deployments#240 (open)
Issue type: Feature → validated against template-issue-feature
Reviewed: 2026-07-26
Every substantive claim in this ticket was independently reproduced, including a full test cherry-pick onto current origin/main. The scope is accurate, the root-cause diagnosis is correct, and the work is atomic. Three cosmetic [BODY] nits are listed below; none of them would cause an implementing agent to do the wrong thing.

Template Completeness

All 12 sections of template-issue-feature present. No gaps.
  • [x] ### Type — Feature
  • [x] ### Lineage — standalone, discovered validating pal-e-services#192/#193
  • [x] ### Repo — ldraney/pal-e-deployments
  • [x] ### User Story — As/I want/So that, complete
  • [x] ### Context — root cause stated inline, not referenced
  • [x] ### File Targets — both "modify or create" and "should NOT touch" lists
  • [x] ### Feature Flag — "none" with justification (infra overlay, no user-visible workflow)
  • [x] ### Acceptance Criteria — 7 criteria
  • [x] ### Test Expectations — 3 expectations + run command
  • [x] ### Constraints — worktree-workflow, no-rebase, out-of-scope list
  • [x] ### Checklist — standard 3
  • [x] ### Related — project + deliberately excluded follow-ups

Traceability

  • [x] story:project-setup label — present
  • [x] story note verified — project-setup row found in project-iss user-stories table (backing: README + docs/architecture.md + docs/adoption-plan.md)
  • [x] arch:kustomize label — present
  • [x] arch note verified — arch-kustomize exists in pal-e-docs (id 2341)
  • [x] Forgejo issue — pal-e-deployments#240, open, valid
Observation (non-blocking): arch-kustomize is scoped to the prediction-assistant project and documents the kalshi-assistant prod/dev overlay pattern, not ISS. It is the generic pattern note ISS follows, and prior ISS board item 1784 (#5 Deployment overlay in pal-e-deployments) used the identical label, so the precedent is consistent. The skill's existence check passes.

File Targets

All five verified by test cherry-pick of effec9e + 3cd11e1 onto origin/main (458b6fd) in a throwaway clone. Result: CLEAN, zero conflicts, exactly 5 files — matching the File Targets list byte for byte.
  • [x] overlays/intelligentstaffingsystems/dev/deployment.yaml — exists on main; UPSTREAM currently "100.110.151.59:9999", cherry-pick changes it to :8888 as claimed
  • [x] overlays/intelligentstaffingsystems/dev/README.md — exists on main; two 9999 refs (Upstream bullet + mermaid node) fixed by the cherry-pick. Stale purpose statement confirmed verbatim: "enabling mobile testing without Tailscale on the device" — the cherry-pick does not touch this sentence, so AC #5 correctly identifies extra work beyond the replay
  • [x] overlays/intelligentstaffingsystems/staging/kustomization.yaml — new (65 lines); namespace: intelligentstaffingsystems-staging; mirrors prod (same ../../../bases/standard ref, same rename-patch structure)
  • [x] overlays/intelligentstaffingsystems/staging/deployment-patch.yaml — new (132 lines)
  • [x] overlays/intelligentstaffingsystems/staging/secrets.enc.yaml — new (32 lines); genuinely SOPS/age-encrypted, lastmodified 2026-07-21, same age recipient as prod
  • [x] DO NOT TOUCH overlays/intelligentstaffingsystems/prod/kustomization.yaml — instruction to drop 241567c is correct. Main's copy carries no cache-bust line (c566ec6 removed the earlier one), and 241567c adds only # cache-bust 1784333337, which collides with the image-updater's newTag: bump. Confirmed this is the sole conflict source
  • [x] bases/standard/ — exists on main (6 files); the staging overlay's relative ref resolves

Root-cause claims — all confirmed

  • [x] effec9e is not an ancestor of origin/main (git merge-base --is-ancestor → false)
  • [x] 3cd11e1 is not an ancestor of origin/main
  • [x] PR #239 reports merged: true with base.ref = palinks-dev-overlay and merge_commit_sha = effec9e436d0b... — the wrong-base merge is real
  • [x] intelligentstaffingsystems#99 is closed — auto-closed by that stranded PR, as described
  • [x] pal-e-services#193 closed with only its own slice landed; no pal-e-deployments PR was ever opened for the overlay
  • [x] Commit 3cd11e1 sits on 193-iss-staging-environment, stacked on effec9e and 241567c — branch topology matches the description exactly

Urgency claim — confirmed

k3s.tfvars registers intelligentstaffingsystems-staging with source_path = "overlays/intelligentstaffingsystems/staging" and cmp_plugin = kustomize-sops. That path does not exist on main. The next tofu apply would create an ArgoCD app pointing at a missing path. The ticket's framing of this as a must-land-before-apply blocker is accurate.

Repo Placement

OK. All five file targets live in ldraney/pal-e-deployments and the issue is filed on that repo. Single-repo change — no split or companion issue needed. The multi-repo follow-ups (pal-e-platform DNS, pal-e-services Caddy and Harbor) are correctly carved out into the Related section rather than absorbed here.

Dependencies

Nothing blocks this ticket; this ticket blocks the next tofu apply.
  • pal-e-services#193 (board 1893, validation) — this ticket is its missing pal-e-deployments slice. Documented in Lineage.
  • pal-e-services#192 (board 1892, validation) — dev-environment registration, closed. Documented.
  • intelligentstaffingsystems#99 (board 1887, done) — the issue the stranded PR auto-closed. Documented.
  • pal-e-services#198 (Caddy staging vhost) and #199 (Harbor cross-project pull) — both verified open, correctly listed as deliberately excluded follow-ups.
  • Open PRs on pal-e-deployments (7 total) — none other than #241 touches any ISS overlay. No collision risk.
  • Board hygiene drift (unrelated to this ticket): board item 1904 sits in in_progress, but its issue pal-e-services#200 is closed and the ISS repo already merged the promote-to-prod pipeline step (PR #109). Worth a board sweep, does not gate this work.

Acceptance Criteria

Seven criteria, all machine-verifiable, each mapping to a concrete command. This is unusually strong — AC #6 even names the exact API endpoint and JSON field (GET /pulls/{n} → .base.ref == "main"), which is precisely the defect being fixed being turned into its own gate. AC #3 (git diff --name-only lists exactly 5 files) is a tight regression net that catches an accidental 241567c replay.
AC #1 pins the base as e6423a3 or later; origin/main has since advanced to 458b6fd. The "or later" wording absorbs this correctly.
AC #5 is the only criterion requiring prose judgment rather than a command, and it is the one place an agent could stall — see the first [BODY] recommendation, which supplies the missing evidence chain. No missing criteria identified.

Blast Radius

  • Swept every source_path in k3s.tfvars against pal-e-deployments main. overlays/intelligentstaffingsystems/staging is the only path targeting pal-e-deployments that is missing. Two other apparent misses (k8s, k8s/overlays/prod) resolve to different source repos (ldraney/pal-e-api and kalshi-assistant) and are false positives. The exposure is contained to this one service.
  • Swept ~150 closed PRs on pal-e-deployments for merged PRs with base != main. #239 is the only occurrence. The stranded-merge defect class is isolated — there is no second re-land hiding elsewhere in this repo.
  • Latent inconsistency, out of scope, worth its own ticket: k3s.tfvars still registers the ISS Keycloak client with http://localhost:9999/auth/keycloak/callback and web origin http://localhost:9999, while the repo has moved local dev to 8888 (CLAUDE.md, and docs/architecture.md:154 lists http://localhost:8888/*). Landing this ticket does not break anything — the overlay's UPSTREAM is a cluster-side proxy target, entirely separate from browser redirect URIs — but the tfvars/docs disagreement is real and belongs in a pal-e-services ticket.
  • Prod overlay is untouched by the cherry-pick set; the running prod ArgoCD app is unaffected.

Decomposition Assessment

No decomposition needed.
  • File targets: 5 files across 1 repo — under the ">3 files across >2 repos" threshold.
  • Acceptance criteria: 7, which literally exceeds the >5 heuristic. It should not trigger a split here: 5 of the 7 are verification gates on a single atomic change (branch base, cherry-pick set, diff file list, PR base ref, PR body wording), not separate units of work. Only two AC (#4 port value, #5 README purpose) describe content changes. The two cherry-picks are stacked and must land in one commit set — splitting them would recreate the exact stranded-branch failure this ticket exists to fix.
  • Standing directive: feedback_no_decomposition — write inclusive tickets, review them whole.
  • Estimated agent work: clone, checkout -B main origin/main, two cherry-picks, one README paragraph edit, two kustomize builds, push, open PR. Well under 5 minutes. 2 points is right-sized.

Recommendation

Verdict is READY. The three items below are polish that would make an already-strong ticket easier to execute; none blocks implementation.
  • [BODY] Add the evidence chain for AC #5. AC #5 asserts the dev overlay's purpose is "making the local Docker dev server reachable on the public URL dev.intelligentstaffingsystems.ai" — but nothing inside the overlay mentions that domain. Its ingress is a Tailscale Funnel ingress serving iss-dev (→ iss-dev.tail5b443a.ts.net), so an agent reading only the overlay could reasonably conclude the AC is wrong and skip it. I verified the AC is correct, and the proof lives outside the repo: /home/ldraney/pal-e-platform/salt/pillar/caddy.sls lines 37-40 define vhost iss-dev: domain dev.intelligentstaffingsystems.ai → proxy_target iss-dev.tail5b443a.ts.net, and terraform/dns.tf line 64 (iss_dev) points the CNAME at the Hetzner edge. Chain: dev.intelligentstaffingsystems.ai → GoDaddy CNAME → Hetzner edge → Caddy → Tailscale Funnel → this overlay's nginx → local Docker dev server on :8888. Citing that in the issue lets the agent write an accurate README instead of guessing. (Corroborated by intelligentstaffingsystems/docs/local-dev-setup.md:121,141.)
  • [BODY] Fix the divergence count in Context. The ticket says "main is 17 commits ahead of the branch, and all 17 are bot image-bump commits." It is now 19, and 3 are not bot bumps: c566ec6 (docs: add ISS overlay to repo inventory, #238), 7458cbe (fix: replace ISS placeholder secrets with real production values, #236), 19d8a02 (chore: force ArgoCD re-render for ISS SOPS plugin). I re-verified the load-bearing part — prod/kustomization.yaml is still the only overlapping file, and the cherry-pick is still clean — so the conclusion holds. Because the image-updater bot pushes continuously, suggest dropping the fixed count entirely: "main has advanced past the branch; the only overlapping file is prod/kustomization.yaml."
  • [BODY] Fix the test run command. kustomize is not installed on this host (which kustomize → not found; kubectl and sops are present), so the stated run command fails with command not found. Use kubectl kustomize overlays/intelligentstaffingsystems/dev && kubectl kustomize overlays/intelligentstaffingsystems/staging. I ran both against the cherry-picked tree: both exit 0, and staging renders namespace: intelligentstaffingsystems-staging across 4 resources, satisfying test expectation #3. Note the SOPS caveat in the ticket is moot for this path — kubectl kustomize passes the encrypted Secret through as an opaque resource without attempting decryption, so the build is genuinely clean and the PR body can say so honestly.
  • [LABEL] Optional: consider story:dev-environment over (or alongside) story:project-setup. The dev-environment row in the project-iss user-stories table reads "#192 / #193 / #194 + docs/pipeline.md — Dev → staging → prod pipeline", which is exactly this work, and every sibling item (1892, 1893, 1904) carries that key. Both keys are valid table entries, so this is precision, not correctness.

Verification Method

Findings were produced against a throwaway clone at scratchpad/ped cut fresh from the Forgejo origin — not the shared ~/pal-e-deployments checkout, which was inspected read-only and left untouched. No branches were created, rebased, force-pushed, or deleted on the remote; the test cherry-pick lived only in the local throwaway clone.
Incidental note: the ticket states the shared clone is checked out on palinks-dev-overlay. As of this review it is on main at the stale 2bf7b6b with a stale cached origin/main (458b6fd upstream). The stale-local-main hazard the Constraints section warns about is therefore still live, and the git fetch origin && git checkout -B main origin/main instruction remains necessary.
  • skill-review-ticket — workflow followed
  • template-issue-feature — template validated against
  • project-iss — user-stories backing for story:project-setup
  • arch-kustomize — arch note backing arch:kustomize