fix(deps): addIssueDependency sends owner+repo+index in POST body #215

Merged

code-lead merged 1 commit from dev/213 into main

2026-04-21 10:41:34 +00:00

dev commented

2026-04-21 10:34:16 +00:00

Collaborator

Summary

addIssueDependency was sending {index: N} but Forgejo's IssueMeta schema requires {owner, repo, index} — the missing fields caused a 404 IsErrRepoNotExist
Split the repo parameter as <owner>/<name> and include both in the POST body
Added regression tests in forgejo-api.test.ts asserting the on-wire payload shape via fetch spy

Test plan

bun test apps/server/src/forgejo-api.test.ts — 38 tests pass including 4 new addIssueDependency payload assertions
bun test apps/server/src/deps.test.ts — 58 tests pass (no regressions)
just deps-backfill charles/claude-hooks reruns clean (idempotent — 201 first call, 409/200 on retries)

Closes #213

🤖 Generated with Claude Code

## Summary - `addIssueDependency` was sending `{index: N}` but Forgejo's `IssueMeta` schema requires `{owner, repo, index}` — the missing fields caused a 404 `IsErrRepoNotExist` - Split the `repo` parameter as `<owner>/<name>` and include both in the POST body - Added regression tests in `forgejo-api.test.ts` asserting the on-wire payload shape via fetch spy ## Test plan - [ ] `bun test apps/server/src/forgejo-api.test.ts` — 38 tests pass including 4 new `addIssueDependency` payload assertions - [ ] `bun test apps/server/src/deps.test.ts` — 58 tests pass (no regressions) - [ ] `just deps-backfill charles/claude-hooks` reruns clean (idempotent — 201 first call, 409/200 on retries) Closes #213 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dev added 1 commit

2026-04-21 10:34:16 +00:00

fix(deps): addIssueDependency sends owner+repo+index in POST body

qa / qa (pull_request) Successful in 3m24s

Details

qa / dockerfile (pull_request) Successful in 6s

Details

139fb298d0

Forgejo's IssueMeta schema requires {owner, repo, index} — sending only
{index} causes a 404 IsErrRepoNotExist because the API cannot resolve
the blocker's repo from empty owner_name/name fields.

Split the `repo` parameter as `<owner>/<name>` and include both fields
in the request body. Add regression tests asserting the on-wire payload
shape via a fetch spy.

Closes #213

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-21 10:38:52 +00:00

reviewer approved these changes

2026-04-21 10:41:08 +00:00

reviewer left a comment

Approved ✅

Fix is correct. addIssueDependency now sends { owner, repo: name, index: blockerNumber } — exactly the IssueMeta shape Forgejo requires. The 3-line change is minimal and surgical.

Acceptance criteria — all met:

Criterion	Result
POST body includes `owner`, `repo`, `index`	✅
Sibling helpers checked (`removeIssueDependency`, `/blocks` mutations)	✅ — no mutation siblings exist; GET functions carry no body
Unit test asserts on-wire payload shape	✅ — `body.owner`, `body.repo`, `body.index` all explicitly asserted
`deps-backfill` idempotent re-run verified	✅ — `200` path covered in tests too

CI: green (run #1848, 3m30s).

Tests: the 4 new cases in describe("addIssueDependency") are exactly the right shape — payload assertion on the happy path, idempotent 200, 404, and network error. Nothing missing.

Minor observation (non-blocking): repoHasWorkflows guards against a slash-less repo string with if (!owner || !name) return false before fetching. addIssueDependency skips that guard — if name were undefined, JSON.stringify would silently drop the repo key and trigger another 404. In practice this can't happen (all callers use loadWebhookConfig-validated owner/name strings), and the acceptance criteria don't ask for it, so no action needed here.

## Approved ✅ **Fix is correct.** `addIssueDependency` now sends `{ owner, repo: name, index: blockerNumber }` — exactly the `IssueMeta` shape Forgejo requires. The 3-line change is minimal and surgical. **Acceptance criteria — all met:** | Criterion | Result | |---|---| | POST body includes `owner`, `repo`, `index` | ✅ | | Sibling helpers checked (`removeIssueDependency`, `/blocks` mutations) | ✅ — no mutation siblings exist; GET functions carry no body | | Unit test asserts on-wire payload shape | ✅ — `body.owner`, `body.repo`, `body.index` all explicitly asserted | | `deps-backfill` idempotent re-run verified | ✅ — `200` path covered in tests too | **CI:** green (run #1848, 3m30s). **Tests:** the 4 new cases in `describe("addIssueDependency")` are exactly the right shape — payload assertion on the happy path, idempotent 200, 404, and network error. Nothing missing. **Minor observation (non-blocking):** `repoHasWorkflows` guards against a slash-less repo string with `if (!owner || !name) return false` before fetching. `addIssueDependency` skips that guard — if `name` were `undefined`, `JSON.stringify` would silently drop the `repo` key and trigger another 404. In practice this can't happen (all callers use `loadWebhookConfig`-validated `owner/name` strings), and the acceptance criteria don't ask for it, so no action needed here.