fix(connections): mount connection routes in the HTTP server

[mrautela365]

The connection service is wired in the DI container (PR #9) but was never mounted in the HTTP muxer, so every /projects/{projectId}/connection-* route returned 404 in the running binary even though the handlers existed.

This mounts the generated connection server, guarded on cont.Connections (which is non-nil even without a database — nil repo → typed 503 rather than a bare 404).

Why it slipped through: an unmounted Goa service still compiles as dead code, so go build/tests pass. Introduced when PR #9 was reconstructed onto main and server.go was not among the cherry-picked files.

LFXV2-2556

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a wiring gap where the connection service was registered in the DI container (from PR #9) but never mounted in the HTTP muxer, causing all /projects/{projectId}/connection-* routes to return a bare 404 in the running binary. It mounts the generated connections Goa server, guarded on cont.Connections, so the routes are served (returning the typed 503 when the DB is unavailable rather than 404).

Changes:

• Build connection endpoints from cont.Connections and apply the debug payload-logging middleware when cfg.Debug is set.
• Thread connEndpoints through handleHTTPServer and mount the generated connections server on the shared muxer, reusing the existing error handler.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[mrautela365]

Rebased onto main to pick up #24's MegaLinter/gitleaks fix — the previous red MegaLinter run predated that merge. Single commit, no conflicts; required checks (License Header, DCO, Build and Test) green. Ready for review.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[MRashad26]

Inline findings below.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[dealako]

Hi @mrautela365 👋 — reviewed the final state of this PR after the full round of iteration with @MRashad26 and Copilot.

Overall impression: Tight, well-scoped fix for a genuine wiring bug — the connection service was DI-wired but never mounted, so every /projects/{projectId}/connection-* route 404'd in the running binary despite compiling. The mount is correct, and the PR thread shows careful, evidence-driven iteration: the always-true nil-guard was replaced with a fail-loud posture end-to-end, debug.LogPayloads() was removed entirely (it would have leaked bearer tokens + plaintext provider credentials), and the debug.HTTP() comment was corrected to match clue v1.2.1's actual behavior. I independently verified all three claims against the code.

Verification I ran:

• connsvcsvr.New(connEndpoints, mux, decoder, encoder, eh, nil) matches the generated 6-arg constructor; the koHTTPDir args the svc server needs are correctly omitted (connections serves no OpenAPI files).
• NewContainer sets Connections non-nil in both the DB (container.go:91) and no-DB (container.go:57) paths; nil-repo mode returns a typed 503 via ensureAvailable(), not a 404 — covered by TestNilRepo_ReturnsServiceUnavailable.
• Auth isn't bypassed by mounting: JWT is baked into the generated endpoints (a.JWTAuth), not the HTTP layer, and ConnectionService implements the Auther interface.
• debug.LogPayloads() has zero live invocations repo-wide (comment-only). go build ./cmd/... clean; all CI checks green; mergeable.

Structured issue count:

• 🔴 Blocking: 0
• 🟡 Minor: 1 — no automated test locks in the route mount (the exact regression class this PR fixes).
• ⚪ Nit: 1 — the connEndpoints == nil guard in handleHTTPServer is statically unreachable.
• ❔ Question: 0

Final decision: ✅ Approved with minor comments. Nothing here blocks merge — CI is green, every prior reviewer thread is resolved, and the security posture is clean. The one item worth a follow-up (not this PR) is a lightweight mount test; details inline.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[dealako]

Thanks for the fast turnaround, @mrautela365 👏 — you closed out both items from the prior round cleanly, and in the best possible way.

👏 Nice work:

• Extracted the exact buildMux seam I suggested and added TestConnectionRoutesAreMounted, which asserts POST /projects/{id}/connection-google-ads (and /livez) resolve to a real handler — any non-404 proves the mount. I verified this genuinely locks the invariant: stubbing out connsvcsvr.Mount with a no-op makes the test fail with a 404, and restoring it passes. That's exactly the regression guard the PR was missing.
• Turned my dead-guard nit into a tested invariant rather than deleting it. Moving the connEndpoints == nil check into buildMux (which the new TestBuildMuxNilConnEndpointsFailsLoud calls directly) means the fail-loud guard is now exercised instead of statically unreachable — a genuinely better resolution than removing it.

Revision tracking:

• ✅ Resolved — [minor] no automated mount test → buildMux seam + TestConnectionRoutesAreMounted (82d6b0a). Mutation-verified.
• ✅ Resolved — [nit] unreachable nil-guard → relocated into buildMux and now covered by TestBuildMuxNilConnEndpointsFailsLoud (82d6b0a).

New-diff review: The buildMux extraction is behavior-identical — same mux, same mount order, same middleware chain (RequestID → Debug-gated debug.HTTP() → otelhttp) applied by handleHTTPServer to the returned mux, same fail-loud posture. ctx/cfg/endpoints/connEndpoints all still used; no orphaned imports or params (go vet clean). The test uses only synthetic data (proj-123, {} body) — no PII, no secrets, and it makes no positive auth claim, so it can't mask an auth gap. go build ./cmd/..., go test ./cmd/campaign-service/..., and go vet ./cmd/... all pass; all CI checks green; still mergeable. Both my security and code-quality passes came back clean, and Copilot's review of this commit generated no new comments.

Structured issue count (all open items):

• 🔴 Blocking: 0
• 🟡 Minor: 0
• ⚪ Nit: 0 (a couple of purely cosmetic, take-it-or-leave-it observations inline — not worth a revision on their own)

Final decision: ✅ Approved. Both prior findings are fully resolved, the fix is well-tested, and there are no new issues. Ship it.

[dealako]

[nit] Cosmetic only — take-it-or-leave-it. The table sets a JSON body (strings.NewReader("{}")) and Content-Type: application/json for every case, including the GET /livez health row, which ignores the body. Harmless (it still returns 200), just slightly incongruous for a GET probe. Optional: set the body/content-type per-case, or add a one-line comment noting connsvcsvr.Mount is all-or-nothing so a single sentinel route is sufficient to detect a dropped mount. Neither is worth a revision.