Re-review: var.services entry for notion-mcp-remote (ArgoCD discovery)

review-1043-2026-04-21-v2 Review

review ready

Project

Verdict: APPROVED

Board item: #1043 on board-notion-mcp-remote — Forgejo issue forgejo_admin/pal-e-services#57. Type: Feature. Points: 3. Labels: type:feature,story:ops-deploy-gitops,arch:argocd.

Prior review: review-1043-2026-04-21 (NEEDS_REFINEMENT) flagged four fixes. This re-review verifies each landed.

#	Fix Requested	Landed?	Evidence
1	Schema: <code>source_repo</code> must be <code>forgejo_admin/pal-e-deployments</code> (owner prefix)	YES	Context section documents owner-prefixed value. Schema note block explains why (services.tf URL interpolation as <code>http://forgejo-http.forgejo.svc.cluster.local/${coalesce(source_repo, forgejo_repo)}.git</code>). AC2 asserts the exact value. Verified against live <code>terraform/variables.tf</code> (services object type: <code>source_repo = optional(string)</code>), <code>terraform/services.tf</code> (argocd_application.service repo_url coalesces source_repo/forgejo_repo), and <code>terraform/k3s.tfvars.example</code> (example uses owner-prefixed form).
2	Remove speculative "Possibly <code>terraform/services.tf</code>" File Target	YES	File Targets reduced to <code>terraform/k3s.tfvars</code> only. <code>terraform/services.tf</code> moved under "Files NOT to touch" with justification: module already supports per-service <code>funnel=true</code> via <code>kubernetes_ingress_v1.service_funnel</code> with <code>for_each = { for k, v in var.services : k => v if v.funnel }</code> — verified in live services.tf.
3	Workflow: pal-e-services is plan-and-apply-BEFORE-merge per <code>sop-platform-tf-changes</code> (NOT apply-on-merge)	YES	Constraints section leads with "Workflow: plan-and-apply-BEFORE-merge" bullet, cites <code>sop-platform-tf-changes</code>, explicitly contrasts with pal-e-platform's CI-driven apply-on-merge, and prescribes the sequence (open PR → review → plan → apply → verify → merge). Matches <code>sop-platform-tf-changes</code> § standard-workflow §§ pal-e-services block exactly.
4	Blast-radius guard: verify <code>tofu plan</code> diff isolated to new resources; halt on drift	YES	Constraints section has dedicated "Blast-radius guard" bullet. Names expected resources (namespace, Harbor project + robots, pull secret, ArgoCD application, funnel ingress). Instructs STOP/do-not-apply/escalate on drift. Cites precedent <code>review-1064-2026-04-20</code> pal-e-services state-drift incident. Duplicated in Checklist as "tofu plan reviewed; diff isolated to notion-mcp-remote resources".

Template Completeness

[x] Type (Feature)
[x] Lineage
[x] Repo
[x] User Story
[x] Context (with Schema note and explicit key values)
[x] File Targets (+ explicit "Files NOT to touch")
[x] Acceptance Criteria (5 items, all verifiable)
[x] Test Expectations (with run command)
[x] Constraints (workflow + blast-radius + style)
[x] Checklist
[x] Related Architecture (references arch-deployment-notion-mcp-remote)
[x] Related

Traceability

[x] story:ops-deploy-gitops label — maps to story-notion-mcp-remote-ops-deploy-gitops
[x] story note verified — listed in project-notion-mcp-remote user-stories table (confirmed in prior review)
[x] arch:argocd label — component: ArgoCD
[ ] arch note arch-argocd MISSING — [SCOPE] carried over from prior review. get_note(slug="arch-argocd") returns 404. This remains a backing-note gap but does not block ticket advancement (per skill-review-ticket, [SCOPE] items flag follow-up work without requiring NEEDS_REFINEMENT). Related Architecture pointer arch-deployment-notion-mcp-remote is present and verified.
[x] arch note arch-deployment-notion-mcp-remote verified — exists, active, documents the service topology including pal-e-services var.services --> ACD edge this ticket implements
[x] Forgejo issue — forgejo_admin/pal-e-services#57, open

File Targets

[x] terraform/k3s.tfvars — correct. File is gitignored (expected; plan-and-apply-before-merge pattern).
[x] "Files NOT to touch" list — correctly enumerates terraform/services.tf, network-policies.tf, other service entries. Prevents scope creep.

Repo Placement

Correct. Issue on forgejo_admin/pal-e-services where schema and tfvars live.

Dependencies

Unchanged from prior review. Upstream blocker pal-e-deployments#132 (overlay, board #1044) correctly documented. Downstream pal-e-platform#296 (board #1045) no longer required per the corrected workflow — apply is manual on pal-e-services, so pal-e-platform is not implicated.

Acceptance Criteria

Five ACs, all verifiable:

AC1 (entry present) — grep-verifiable
AC2 (source_repo owner-prefixed) — NEW; grep-verifiable and directly addresses prior review's fix #1
AC3 (plan diff shows expected resources, no unexpected diff) — runnable; resource list explicit
AC4 (values match pre-deploy-validation checklist) — references service-onboarding-sop
AC5 (PR reviewed and merged) — process criterion

Blast Radius

Now explicitly documented in the ticket itself with STOP/escalate procedure and precedent citation. The original concern is now the agent's responsibility to verify in-flight, as it should be.

Decomposition Assessment

1 file edit (one map entry in k3s.tfvars), 5 acceptance criteria, estimated agent work ~5-10 min including plan review. No decomposition needed. 3 points remains reasonable.

Recommendation

All four refinement fixes landed and verified against live codebase. Ticket is ready to advance from backlog.

No action needed on the ticket body.
[SCOPE] carry-over: arch-argocd note still missing. Recommend filing separately — do not block #1043 on it. Same pattern as arch-tailscale-funnel gap flagged in review-788-2026-04-03; these platform-level arch notes are a documentation backlog concern, not a per-ticket blocker.

review-1043-2026-04-21 — prior review (NEEDS_REFINEMENT)
forgejo_admin/pal-e-services#57 — issue
sop-platform-tf-changes — workflow authority
review-1064-2026-04-20 — state-drift precedent cited in blast-radius guard
arch-deployment-notion-mcp-remote — related architecture