Merge order and three silent conflict hazards across the production-readiness PR batch

[SanabriaRusso]

Context

17 non-draft production-readiness PRs are approved and individually MERGEABLE/CLEAN against main @ 6b72e5b. They are not independent of each other: git merge-tree across all 18 open head refs finds 37 conflicting pairs.

Conflict degree (number of other open PRs each collides with):

#193 -> 10   #190 -> 9   #183 -> 7   #184 #185 #187 #191 -> 6
#194 #195 #196 -> 4   #186 #188 #198 -> 2   #182 #189 -> 1
#134 #192 #197 -> 0

Hub files: docs/getting-started.md, src/server/plugins.ts, src/server/server.ts, package.json / package-lock.json, README.md, .env.example.compose, src/envionment.d.ts.

Most are append-only doc/env collisions — resolve by taking both sides. Three are not, and a careless resolution is silent in all three: no test fails, no error is raised, and a defect that was just fixed comes back.

This issue exists so whoever performs the merges has the order and the hazards written down.

---

Hazard 1 (SILENT) — src/server/server.ts, #190 vs #195

Both edit the same createYoga options object.

• P2: Verify error masking; make masking explicit (#177) #195 adds maskedErrors: { isDev: false } and splits out buildYoga.
• P1: Structured JSON logging with request correlation ids (#172) #190 replaces logging: LOG_LEVEL with an explicit YogaLogger object (its fix for LOG_LEVEL=fatal emitting debug output).

Either one-sided resolution re-breaks the other. Dropping #195's maskedErrors is undetectable in CI: its own test suite still passes, because with NODE_ENV unset the behaviour is identical to main. The only thing that comes back is the NODE_ENV=development leak, where main puts the full Postgres connection string including the password into extensions.originalError — and nothing in CI sets NODE_ENV=development.

Resolution: merge #195 before #190. The merged options object must retain all three changes:

function buildYoga(context: GraphQLContext, plugins: Plugin[]) {
  return createYoga<GraphQLContext>({
    schema,
    logging: {                        // from #190
      debug: yogaLog('debug'), info: yogaLog('info'),
      warn:  yogaLog('warn'),  error: yogaLog('error'),
    },
    graphqlEndpoint: '/',
    landingPage: false,
    healthCheckEndpoint: '/healthcheck',
    graphiql: process.env.ENABLE_GRAPHIQL === 'true' ? true : false,
    maskedErrors: { isDev: false },   // from #195 — do not drop
    plugins,
    cors: { origin: process.env.CORS_ORIGIN ?? '*', methods: ['GET', 'POST'] },
    context,
  });
}

Acceptance check after resolving: with NODE_ENV=development, a resolver that throws an error containing a DSN must return Unexpected error. with no extensions.originalError.

---

Hazard 2 (SILENT) — Dockerfile, #189 vs #194

Both rewrite the same two FROM lines and disagree on the base:

• P1: Upgrade to graphql-yoga 5 and Node 22 LTS (#176) #194 → node:22-alpine@sha256:16e22a55…
• P1: Harden Docker image — tini PID 1, HEALTHCHECK, pinned digest (#171) #189 → node:20-alpine@sha256:fb4cd12c…

Both digests are genuine multi-arch official images. But Node 20 reached EOL on 2026-04-30, and #194 also adds engines: { "node": ">=22.12.0" }, which #189's base does not satisfy. Resolving toward #189 silently enshrines an EOL runtime that contradicts the package's own declared floor.

Resolution: merge #194 first, then rebase #189 onto it. Keep #194's digest, layer #189's hardening on top. The full merged Dockerfile is in the #189 review comment. apk add --no-cache tini and BusyBox wget were both re-tested on the node:22 base and behave identically, so the combination is safe.

Acceptance check: docker run the built image, confirm ps shows 1 tini / 3 node, confirm SIGTERM reaches node, and confirm HEALTHCHECK reports healthy.

---

Hazard 3 (SILENT) — src/server/plugins.ts, #188's signature change

#188 changes buildPlugins() from return plugins to return { plugins, provider } so the entry point can flush OpenTelemetry spans on shutdown, and updates its caller accordingly. Six other PRs edit the same function and still return plugins.

A resolution that keeps the old form drops trace flushing on shutdown with no error — provider is simply undefined and spans are silently discarded.

Resolution: merge #188 on its own, then rebase the remaining plugins.ts PRs onto the new signature. Do not interleave it.

---

Non-hazard (recorded so it is not re-litigated)

#183, #185, #190 and #191 each insert their plugin at index 0 of buildPlugins, each with a comment asserting it "runs ahead of everything else". This is a textual conflict only.

It was initially suspected that ordering #185 (rate limiter) before #191 (metrics) would re-break #191's http_requests_in_flight gauge, since the limiter short-circuits with a 429. It does not. #191's guard is:

const start = startTimes.get(request);
if (start === undefined) return;   // no inc => no dec
startTimes.delete(request);
metrics.inFlight.dec();

No-inc implies no-dec, so both orderings stay balanced. Verified in source and by execution. All four plugins are onRequest/onResponse-only or validate-phase, so the order is semantically free. Resolve textually, keeping all four insertions.

---

Recommended merge order

Wave	PRs	Note
1	#134, #192	zero conflicts — bank them
2	#182	resolve .gitignore toward the fully anchored /db/ + /data/ form
3	#194 then #189	Node 22 before the digest pin (Hazard 2)
4	#195 then #190	maskedErrors before the logger swap (Hazard 1)
5	#188	signature change alone, then rebase the rest (Hazard 3)
6	#191, #185, #183, #184, #187, #193	textual conflicts only
7	#186 (once unblocked), #196, #197, #198	docs that reference everything above

---

Branch-protection prerequisite

Current settings:

required status checks : Run-Tests, Linting
strict                 : TRUE
required approvals     : 1
dismiss_stale_reviews  : TRUE
enforce_admins         : FALSE

With strict: TRUE, every merge puts the remaining PRs BEHIND. The update-branch that fixes that is itself a push, and with dismiss_stale_reviews: TRUE that dismisses the approval. 16 approvals today does not produce 16 merges without re-approving each one after every preceding merge.

Two options before starting:

• (a) Recommended — temporarily set strict: false for the duration of the merge train, keeping the two required checks and the 1-approval rule. Reversible, and it does not weaken what is actually verified.
• (b) Use the admin bypass (enforce_admins: false) for the mechanical update-branch re-approvals.

Real content conflicts still need resolving under either option.

---

Acceptance criteria

• All 17 non-draft PRs merged, or explicitly deferred with a reason
• Hazards 1–3 each verified by their acceptance check above after resolution
• strict restored to true if it was temporarily disabled
• npm run build && npm run test:unit green on main after the final merge