diff --git a/docs/daily-reviews/2026-05-02.md b/docs/daily-reviews/2026-05-02.md new file mode 100644 index 0000000..aebb4d1 --- /dev/null +++ b/docs/daily-reviews/2026-05-02.md @@ -0,0 +1,75 @@ +--- +date: 2026-05-02 +range: 5ce68c3077907fb998385fa428b3656043db2577..49c60da5611be873713af8e2c36819da373fa3f6 +commits: 4 +blockers: 0 +majors: 0 +minors: 1 +nits: 1 +issue: 189 +--- + +## 2026-05-02 — 49c60da + +Reviewed: 5ce68c3077907fb998385fa428b3656043db2577..49c60da5611be873713af8e2c36819da373fa3f6 (4 commits) +Blockers: 0 | Majors: 0 | Minors: 1 | Nits: 1 + +--- + +- [ ] **[MINOR]** `scripts/create-v1-issues.mjs:91` — POST requests missing `Content-Type: application/json`; issue creation may fail +
details + + **Problem:** `gh()` helper never sets `Content-Type: application/json` on POST requests that pass a JSON-serialised body + + **Why it matters:** Node's native `fetch` does not auto-add `Content-Type` for plain string bodies; if the GitHub API enforces the header (per HTTP spec), all 16 issue-create calls fail with `415 Unsupported Media Type` or the body is silently ignored, leaving no issues created and no actionable error from the validation layer + + **Fix:** + + ```diff + async function gh(path, init = {}) { + const res = await fetch(`https://api.github.com${path}`, { + ...init, + headers: { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${token}`, + 'X-GitHub-Api-Version': '2022-11-28', + + ...(init.body !== undefined ? { 'Content-Type': 'application/json' } : {}), + ...(init.headers ?? {}), + }, + }); + ``` + +
+ +- [ ] **[NIT]** `scripts/create-v1-issues.mjs:18` — `GITHUB_REPOSITORY` validation accepts multi-segment paths silently +
details + + **Problem:** `repository?.includes('/')` only checks for the presence of a slash; `owner/suborg/repo` passes validation, then `const [owner, repo] = repository.split('/')` silently drops the third segment and targets the wrong repo + + **Why it matters:** Issues would be created in `owner/suborg` instead of `owner/suborg/repo`; the error message says "expected owner/repo" but the guard does not enforce that invariant + + **Fix:** + + ```diff + -if (!repository?.includes('/')) { + +const parts = repository?.split('/') ?? []; + +if (parts.length !== 2 || !parts[0] || !parts[1]) { + console.error('Missing/invalid GITHUB_REPOSITORY (expected owner/repo).'); + process.exit(1); + } + -const [owner, repo] = repository.split('/'); + +const [owner, repo] = parts; + ``` + +
+ +--- + +- Reviewed range: `5ce68c3077907fb998385fa428b3656043db2577..49c60da5611be873713af8e2c36819da373fa3f6` (4 commits, 3 files) +- Blockers: 0 +- Majors: 0 +- Minors: 1 +- Nits: 1 +- Counter-argument check: `49c60da.1` tested — GitHub's API is reportedly lenient with Content-Type for JSON bodies, and the post-merge fix commits #187/#188 didn't address this, suggesting the script ran successfully without it. However, relying on undocumented server leniency is fragile. Finding retained as [MINOR]. +- Not reviewed: lockfiles, generated files, prior daily-review docs (doc-only snapshots, out of scope) +- Last reviewed SHA: `49c60da5611be873713af8e2c36819da373fa3f6`