[MEDIUM] Decide createCoreFetch default responseShape ('full' vs 'body') and align README + call sites

uwe.admin commented

2026-04-25 13:39:55 +02:00

Owner

From audit #1.

Problem

src/core-fetch.ts:104 defaults responseShape to 'full' but the README example shows 'body'. Derived projects have been observed using both; the docs/code mismatch is a footgun.

Remediation

Pick one default and align everything:

Either flip the default to 'body' and add a deprecation pass on the old 'full' callers, OR
Keep 'full' and update the README + every derived project's call site to pass responseShape: 'body' explicitly where they want the body shape.

Acceptance

One sentence in the README states the chosen default unambiguously.
All derived projects (fewo-webapp, palibu, webapp-template) match.

Effort: small (decision-driven).

From audit #1. ## Problem `src/core-fetch.ts:104` defaults `responseShape` to `'full'` but the README example shows `'body'`. Derived projects have been observed using both; the docs/code mismatch is a footgun. ## Remediation Pick one default and align everything: - Either flip the default to `'body'` and add a deprecation pass on the old `'full'` callers, OR - Keep `'full'` and update the README + every derived project's call site to pass `responseShape: 'body'` explicitly where they want the body shape. ## Acceptance - One sentence in the README states the chosen default unambiguously. - All derived projects (fewo-webapp, palibu, webapp-template) match. Effort: small (decision-driven).

uwe.admin added the

new

label 2026-04-25 13:39:55 +02:00

uwe.admin referenced this issue

2026-04-25 13:40:05 +02:00

Code Audit Report — 2026-04-23 #1

uwe.admin referenced this issue

2026-04-25 13:46:08 +02:00

[MEDIUM] Add vitest suite for createCoreFetch + createI18n; wire into prepare #4

uwe.admin commented

2026-04-25 13:47:30 +02:00

Author

Owner

Agent Evaluation

Note on the audit framing: The current shapes in core-fetch.ts are 'wrapped' and 'body' (not 'full' as the audit body says). Default in code is 'wrapped' (line 104). Treating that as the issue: the docstring at the top of the file calls 'wrapped' the default, the README example shows the responseShape: 'body' form, and downstream usage is split:

Repo	What it does
fewo-webapp	passes `responseShape: 'body'` explicitly (matches its orval config)
palibu	uses default → `'wrapped'`
webapp-template	uses default → `'wrapped'` (no override)

So the runtime split today is fewo-webapp on 'body', everyone else on 'wrapped'.

Feasibility: Trivial code change either direction. The friction is in downstream PRs if we flip the default.

Impact: Medium. The footgun is real — Orval's generator emits the body-shape mutator signature, so a consumer wiring createCoreFetch without thinking gets the wrapped shape and their generated client misbehaves silently (the response object becomes {data, status, headers} where the generator expected data). Forgetting responseShape: 'body' on a fresh project is exactly the kind of bug that doesn't surface until a deeply-nested response handler tries to read a property and gets undefined.

Effort: Small (the change), Small-Medium (the downstream sweep).

Recommendation: Accept.

Implementation plan

Pick a default (decision below).
Update src/core-fetch.ts — the ?? default on line 104 + the comment block on lines 4-9 + the README example.
Bump package.json version to flag this as a behaviour change to downstreams (semver: minor if the new default doesn't break existing callers, major if it does).
Sweep downstream repos (fewo-webapp, palibu, webapp-template): adjust each core-fetch.ts to either drop the now-redundant responseShape arg or add it explicitly so the previous behaviour is preserved. One PR per repo.
Add a one-line README assertion ("Default is 'X' — pass responseShape: 'Y' to get the other shape") to remove ambiguity.

Decision needed

Check one (edit this comment):

Option A — Default 'body' — matches Orval's generator expectation, makes the no-arg form correct out of the box, fewo-webapp's existing override becomes redundant. Palibu + webapp-template flip behaviour silently — needs explicit testing of their existing fetches.
Option B — Keep default 'wrapped' — backwards compatible for palibu + webapp-template; fewo-webapp's responseShape: 'body' override stays. Update README + docstring to make the default explicit so future projects don't trip over it. No silent behaviour flips downstream.

Default recommendation: A. The whole point of the scaffold is that the no-arg form should be the right answer for the typical Orval-driven derived project. If 'wrapped' is the default, every new project hits the same footgun the audit identified.

## Agent Evaluation **Note on the audit framing:** The current shapes in `core-fetch.ts` are `'wrapped'` and `'body'` (not `'full'` as the audit body says). Default in code is `'wrapped'` (line 104). Treating that as the issue: the docstring at the top of the file calls `'wrapped'` the default, the README example shows the `responseShape: 'body'` form, and downstream usage is split: | Repo | What it does | |-----------------|-----------------------------------------------------------------------| | fewo-webapp | passes `responseShape: 'body'` explicitly (matches its orval config) | | palibu | uses default → `'wrapped'` | | webapp-template | uses default → `'wrapped'` (no override) | So the runtime split today is **fewo-webapp on 'body', everyone else on 'wrapped'**. **Feasibility:** Trivial code change either direction. The friction is in downstream PRs if we flip the default. **Impact:** Medium. The footgun is real — Orval's generator emits the body-shape mutator signature, so a consumer wiring `createCoreFetch` without thinking gets the wrapped shape and their generated client misbehaves silently (the response object becomes `{data, status, headers}` where the generator expected `data`). Forgetting `responseShape: 'body'` on a fresh project is exactly the kind of bug that doesn't surface until a deeply-nested response handler tries to read a property and gets `undefined`. **Effort:** Small (the change), Small-Medium (the downstream sweep). **Recommendation:** Accept. ### Implementation plan 1. **Pick a default** (decision below). 2. **Update `src/core-fetch.ts`** — the `??` default on line 104 + the comment block on lines 4-9 + the README example. 3. **Bump `package.json` version** to flag this as a behaviour change to downstreams (semver: minor if the new default doesn't break existing callers, major if it does). 4. **Sweep downstream repos** (fewo-webapp, palibu, webapp-template): adjust each `core-fetch.ts` to either drop the now-redundant `responseShape` arg or add it explicitly so the previous behaviour is preserved. One PR per repo. 5. **Add a one-line README assertion** ("Default is `'X'` — pass `responseShape: 'Y'` to get the other shape") to remove ambiguity. ### Decision needed Check one (edit this comment): - [ ] **Option A — Default `'body'`** — matches Orval's generator expectation, makes the no-arg form correct out of the box, fewo-webapp's existing override becomes redundant. Palibu + webapp-template flip behaviour silently — needs explicit testing of their existing fetches. - [ ] **Option B — Keep default `'wrapped'`** — backwards compatible for palibu + webapp-template; fewo-webapp's `responseShape: 'body'` override stays. Update README + docstring to make the default explicit so future projects don't trip over it. No silent behaviour flips downstream. Default recommendation: **A**. The whole point of the scaffold is that the no-arg form should be the right answer for the typical Orval-driven derived project. If `'wrapped'` is the default, every new project hits the same footgun the audit identified.

uwe.admin added the

effort:medium

evaluated

labels 2026-04-25 13:47:31 +02:00

uwe.admin commented

2026-04-25 13:47:31 +02:00

Author

Owner

Evaluated webapp-scaffold#5 — Small/Medium, recommend accept; A/B decision on default (body vs wrapped). Note: shapes in code are wrapped/body, audit said full/body.

u.schuster commented

2026-04-25 21:28:00 +02:00

I choose Option A

uwe.admin commented

2026-04-25 21:31:20 +02:00

Author

Owner

Re-evaluation

Owner selected Option A — Default 'body'.

Acknowledged. Implementation pass:

core-fetch.ts: change the default from 'wrapped' to 'body' (factory ctor arg + docstring + the inline default at the destructure).
README.md + top-of-file docstring: update the default-behaviour example so the no-arg form returns the body directly; mention responseShape: 'wrapped' for callers who want {response, body}.
Downstream silent-flip mitigation:
- palibu: audit every createCoreFetch() call site; either add explicit responseShape: 'wrapped' (preserve behaviour) or migrate consumers to expect plain body. PR alongside the scaffold bump.
- webapp-template: same — add explicit responseShape: 'wrapped' if any view depends on the wrapped shape; otherwise migrate.
- fewo-webapp: drop the now-redundant responseShape: 'body' override (separate PR after scaffold bump lands).
Bump scaffold version (minor — behaviour change for default).
CHANGELOG entry under BREAKING (default flipped).

Apply the accepted label to trigger implementation. The scaffold-side change is small; the downstream coordination is what makes this Medium effort.

## Re-evaluation Owner selected **Option A — Default `'body'`**. **Acknowledged.** Implementation pass: 1. `core-fetch.ts`: change the default from `'wrapped'` to `'body'` (factory ctor arg + docstring + the inline default at the destructure). 2. `README.md` + top-of-file docstring: update the default-behaviour example so the no-arg form returns the body directly; mention `responseShape: 'wrapped'` for callers who want `{response, body}`. 3. **Downstream silent-flip mitigation:** - `palibu`: audit every `createCoreFetch()` call site; either add explicit `responseShape: 'wrapped'` (preserve behaviour) or migrate consumers to expect plain body. PR alongside the scaffold bump. - `webapp-template`: same — add explicit `responseShape: 'wrapped'` if any view depends on the wrapped shape; otherwise migrate. - `fewo-webapp`: drop the now-redundant `responseShape: 'body'` override (separate PR after scaffold bump lands). 4. Bump scaffold version (minor — behaviour change for default). 5. CHANGELOG entry under BREAKING (default flipped). Apply the `accepted` label to trigger implementation. The scaffold-side change is small; the downstream coordination is what makes this Medium effort.

uwe.admin commented

2026-04-25 21:31:20 +02:00

Author

Owner

Re-evaluated #5 — owner picked Option A (default body); awaiting accepted label.

uwe.admin added the

accepted

label 2026-04-25 21:32:07 +02:00

uwe.admin referenced this issue from a commit

2026-04-25 21:47:41 +02:00

#5: createCoreFetch default responseShape flipped to 'body'

uwe.admin closed this issue