pal-e-docs

Review: Revert pal-e-docs-app rename — unblock CI + ship #105/#109

review-1015-2026-04-16 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

The diagnosis is correct and the fix direction is right, but scope is under-specified for a clean dev-agent execution. PR #90's body enumerates every file it touched — that list is the canonical revert checklist and the issue body should reuse it verbatim instead of saying "playwright.config.ts (and elsewhere)." Two values in that list (Keycloak clientId, Harbor image repo) are semantically loaded and need explicit guidance with infra evidence so the dev doesn't second-guess.

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — standalone, regression from PR #90
  • [x] Repo — forgejo_admin/pal-e-app
  • [x] What Broke — clear, with cascade diagram
  • [x] Repro Steps — pipelines #114-#123 named
  • [x] Expected Behavior — present
  • [x] Environment — cluster/namespace, deployed SHA, main SHA, ingress all named
  • [x] Acceptance Criteria — 6 items, mostly testable (see Acceptance Criteria section below)
  • [x] Related — links to dictionary + PR #275 deferral guidance

Traceability

  • [x] story:app-definition label present on board item #1015
  • [ ] story note MISSING — get_section(slug="project-pal-e-docs", anchor_id="user-stories") returns 5 stories (superuser-query, superuser-maintain, agent-read, agent-write, reader-browse). app-definition is not listed. search_notes("story app-definition") returns empty. [SCOPE] Add story:app-definition entry to project-pal-e-docs user-stories table (or to a different project if the story lives elsewhere — clarify which).
  • [x] arch:pal-e-app label present on board item #1015
  • [ ] arch note MISSING — search_notes("arch-pal-e-app") and search_notes("arch pal-e-app frontend") return empty. [SCOPE] Create architecture note arch-pal-e-app describing the frontend component (SvelteKit + Keycloak + Tailscale ingress + Harbor image flow).
  • [x] Forgejo issue — #110 open and well-formed
Note: missing story/arch notes do not block this revert (it's a regression fix, not new architecture). Flagging per skill convention so they get tracked.

File Targets

The issue says "playwright.config.ts (and elsewhere)" — but PR #90's body enumerates the exact file list. Repo-wide grep for pal-e-docs-app confirms PR #90's list matches the live repo state today. Verified files:
  • [x] /home/ldraney/pal-e-app/playwright.config.ts — line 4 (comment), line 20 (baseURL default) — 2 hits
  • [x] /home/ldraney/pal-e-app/.woodpecker.yaml — line 55 (PLAYWRIGHT_BASE_URL), line 69 (Kaniko repo), line 88 (OVERLAY) — 3 hits
  • [x] /home/ldraney/pal-e-app/k8s/deployment.yaml — lines 4, 14, 18, 23, 24 (image repo) — 5 hits
  • [x] /home/ldraney/pal-e-app/k8s/service.yaml — lines 4, 6, 9 — 3 hits
  • [x] /home/ldraney/pal-e-app/src/lib/keycloak.ts — lines 2, 6, 13 (default clientId) — 3 hits
  • [x] /home/ldraney/pal-e-app/.env.example — line 8 (VITE_KEYCLOAK_CLIENT_ID) — 1 hit
  • [x] /home/ldraney/pal-e-app/package.json — line 2 (name field)
  • [x] /home/ldraney/pal-e-app/package-lock.json — lines 2, 8 (name fields)
  • [x] /home/ldraney/pal-e-app/README.md — line 1 (title)
  • [x] /home/ldraney/pal-e-app/CLAUDE.md — lines 1, 72 (title + remote URL)
  • [x] /home/ldraney/pal-e-app/e2e/home.spec.ts — line 4 (comment)
  • [x] /home/ldraney/pal-e-app/e2e/auth.spec.ts — line 4 (comment)
  • [x] /home/ldraney/pal-e-app/e2e/public-readiness.spec.ts — line 8 (comment)
13 files, 27+ hits total. Repo-wide grep for pal-e-app\.tail5b443a returns no matches (confirms no orphan correct refs to preserve).
[BODY] Replace "(and elsewhere)" with the exact PR #90 file enumeration above. Dev shouldn't have to re-derive scope.

Repo Placement

Correct repo. PR #90 was filed and merged on forgejo_admin/pal-e-app (verified via curl to Forgejo API — repository.full_name returns forgejo_admin/pal-e-app). The Forgejo repo itself was NOT renamed; PR #90's premise ("Repo was renamed from pal-e-app to pal-e-docs-app") was wrong. [BODY] Add a one-liner: "PR #90 claimed the repo was renamed — it was not. Repo is and always has been forgejo_admin/pal-e-app. Do not attempt to rename anything in Forgejo as part of this revert."

Dependencies

  • Blocks: PRs #105, #109 deploy (already merged to main, stuck on stale image)
  • Blocks: any future pal-e-app PR (every pipeline since #114 fails the same way)
  • Related but separate: pal-e-platform PR #275 (decouples update-kustomize-tag from test failures). Issue body explicitly says "don't apply here" — that guidance is unambiguous and correct.
  • No in-flight dependency on board (no other pal-e-app revert items in_progress / next_up).

Acceptance Criteria

Mostly testable. Two AC need a small tightening:
  • AC1 (baseURL revert) — testable via grep "pal-e-app.tail5b443a" playwright.config.ts
  • AC2 (repo-wide sweep) — testable via rg pal-e-docs-app returning zero matches. Strong as written.
  • AC3 (CI passes through update-kustomize-tag) — testable via Woodpecker pipeline status
  • AC4 (pal-e-deployments commit) — testable via git log on pal-e-deployments
  • AC5 (ArgoCD rolls to commit ≥ 4454c8d) — [BODY] verify 4454c8d is the actual main HEAD before the dev agent starts. The issue claims this is PR #109's commit; the dev agent should be told either to use the main HEAD at execution time, or this SHA needs to be pinned with provenance.
  • AC6 (manual /notes + role-based routing verification) — needs a how. Is this curl + auth, or browser? [BODY] add the verification recipe (e.g., "open https://pal-e-app.tail5b443a.ts.net/notes in browser after Keycloak login, confirm My Notes view renders").

Blast Radius

Important callouts the issue doesn't make:
  • Keycloak clientId: src/lib/keycloak.ts line 13 sets the default clientId to 'pal-e-docs-app'. .env.example sets VITE_KEYCLOAK_CLIENT_ID=pal-e-docs-app. The deployed pod likely uses an env override, but if not, the running app is asking Keycloak for a client ID that may or may not exist in the realm. [BODY] Add: "If a Keycloak client named pal-e-docs-app was created during PR #90, that's also stale config — the canonical client ID is pal-e-app. Reverting the code defaults is correct; check Keycloak realm for orphan clients as a follow-up (NOT part of this fix)."
  • Harbor image repo: k8s/deployment.yaml line 24 references harbor.tail5b443a.ts.net/pal-e-docs-app/app:e23a1d8c..., but the live overlay (pal-e-deployments/overlays/pal-e-app/prod/kustomization.yaml line 64) uses harbor.tail5b443a.ts.net/pal-e-app/app. The base manifest in this repo is overlay-overridden, so the wrong image path here doesn't break prod — but reverting it to pal-e-app/app is correct AND aligns with .woodpecker.yaml line 69 (Kaniko push target). [BODY] Add: "Harbor project for this image is pal-e-app. Verify by listing harbor.tail5b443a.ts.net/v2/_catalog — do NOT create a new Harbor project. If a pal-e-docs-app Harbor project was auto-created during failed pushes, leave it alone (cleanup is a separate ticket)."
  • Sibling services: grep -r "pal-e-docs-app" /home/ldraney/pal-e-deployments/ returns zero matches. grep -r "pal-e-docs-app" /home/ldraney/pal-e-services/ not checked but should be sweep-verified. [BODY] Add an AC: "Repo-wide grep on pal-e-deployments and pal-e-services returns zero pal-e-docs-app matches" (defensive — confirms no infra cross-contamination).
  • Original Issue #87: PR #90 closed Issue #87. The dev agent should NOT reopen #87 unless it explicitly described the (incorrect) rename request — if so, #87 needs a closing comment explaining the regression. [BODY] Add: "If PR #87 requested the rename, comment on it explaining the revert. Otherwise leave it closed."

Decomposition Assessment

13 files in 1 repo. AC count: 6 (with 2 needing tightening). Estimated agent work: ~10-15 min for the revert (mechanical sed + targeted manual review of keycloak.ts and deployment.yaml) + ~5 min waiting on CI. Total ~20 min agent time.
This exceeds the 5-minute rule on raw time, but the work is mechanical and tightly scoped (one repo, one rename direction). No decomposition needed — splitting would create more coordination overhead than it saves. A single agent can handle the revert + commit + push + monitor CI in one pass.

Recommendation

  • [BODY] Replace "(and elsewhere)" in "What Broke" with the explicit 13-file list from PR #90 (reproduced in File Targets section above).
  • [BODY] Add to "What Broke" or a new "Important context" section: "PR #90 claimed the Forgejo repo was renamed; it was not. Do not rename anything in Forgejo."
  • [BODY] Pin or refresh AC5's reference SHA (≥ 4454c8d) — either confirm it's main HEAD now or instruct dev to use git rev-parse origin/main at execution time.
  • [BODY] Add verification recipe to AC6 (how to manually verify /notes + role routing).
  • [BODY] Add Keycloak clientId callout: revert code defaults to pal-e-app, leave any orphan Keycloak client for follow-up cleanup.
  • [BODY] Add Harbor callout: image goes to pal-e-app/app, do not create new Harbor project, leave any orphan project alone.
  • [BODY] Add new AC: "rg pal-e-docs-app on pal-e-deployments returns zero matches."
  • [BODY] Add Issue #87 guidance (one line on whether to reopen / comment).
  • [SCOPE] Create user story entry on project-pal-e-docs user-stories section for story:app-definition (or clarify if story lives on a different project page).
  • [SCOPE] Create architecture note arch-pal-e-app for the frontend component.
Once the [BODY] fixes land, this is READY. The [SCOPE] items are independent backlog work and should not block this revert.