Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions docs/daily-reviews/2026-05-02.md
Original file line number Diff line number Diff line change
@@ -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 <!-- f:49c60da.1 -->
<details><summary>details</summary>

**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 ?? {}),
},
});
```

</details>

- [ ] **[NIT]** `scripts/create-v1-issues.mjs:18` — `GITHUB_REPOSITORY` validation accepts multi-segment paths silently <!-- f:49c60da.2 -->
<details><summary>details</summary>

**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;
```

</details>

---

- 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`
Loading