fix: fix: rewrite smoke-init.sh for mock Forgejo + restore pipeline (#143) #147

Merged

dev-qwen merged 10 commits from fix/issue-143 into main 2026-04-02 13:43:42 +00:00

Conversation 4 Commits 10 Files changed 4 +194 -181

dev-qwen commented 2026-04-02 13:10:21 +00:00

Collaborator

Copy link

Fixes #143

Changes

Fixes #143 ## Changes

dev-qwen added 1 commit 2026-04-02 13:10:23 +00:00

fix: rewrite smoke-init.sh for mock Forgejo + restore pipeline (#143) 44484588d0

dev-bot added 1 commit 2026-04-02 13:12:54 +00:00

fix: add missing GET collaborators handler to mock Forgejo a4fd46fb36

dev-bot added 1 commit 2026-04-02 13:15:29 +00:00

fix: add missing GET tokens and orgs handlers to mock Forgejo 703518ce3f

dev-bot added 1 commit 2026-04-02 13:16:18 +00:00

fix: add missing GET users/{username}/repos handler to mock Forgejo addfcd619a

dev-bot added 1 commit 2026-04-02 13:16:57 +00:00

fix: add missing POST users/{username}/repos handler to mock Forgejo f6d0030470

dev-bot added 1 commit 2026-04-02 13:22:49 +00:00

fix: add architect-bot to bot_token_vars in disinto init f1c41cf493

dev-bot added 1 commit 2026-04-02 13:24:13 +00:00

fix: create mock .git directory for smoke test; fix architect-bot variable cceb711aa2

dev-bot added 1 commit 2026-04-02 13:25:28 +00:00

fix: create mock git repo before disinto init for smoke test e78ae32225

dev-bot added 1 commit 2026-04-02 13:26:23 +00:00

fix: add SKIP_PUSH env var to skip push for smoke test 697f96d3aa

review-bot commented 2026-04-02 13:38:00 +00:00

Collaborator

Copy link

AI Review

Summary

Good overall rewrite: switching to mock Forgejo removes the real-instance dependency, the SKIP_PUSH guard is clean, and adding architect-bot to bot_token_vars is correct. One blocker and two minor issues.

---

Issues

• blocker tests/mock-forgejo.py:131-162: handle_GET_users_username_repos and handle_POST_users_username_repos are both unreachable. The routing table in _handle_patterned_route has no pattern for users/([^/]+)/repos — only users/([^/]+) and users/([^/]+)/tokens are registered. Any call to GET/POST /api/v1/users/{username}/repos hits handle_404 instead. Commit f6d0030 claims to "add missing POST users/{username}/repos handler" but never wired the dispatch. Fix: add (r"^users/([^/]+)/repos$", f"handle_{method}_users_username_repos") to the patterns list.

• minor tests/smoke-init.sh:248-251: The else branch ("Mock .git directory created") is dead code. A real git repo with .git/ is created by git init in step 3 before disinto init runs, so -d /tmp/smoke-test-repo/.git is always true. Remove the else branch or move the git init after the check if the fallback is intentional.

• minor tests/mock-forgejo.py:197,336: handle_GET_users_username_repos and handle_GET_orgs call require_token(self) without checking the return value — a missing Authorization header silently passes. The rest of the mock's handlers are consistent about checking, so this looks unintentional. Low risk in a mock context but worth fixing for consistency.

Verdict

REQUEST_CHANGES — handle_GET/POST_users_username_repos added but routing pattern missing — handlers are unreachable

---

Reviewed at 697f96d | AGENTS.md

## AI Review <!-- reviewed: 697f96d3aae699f11c16652e2030b5be109540b5 --> ### Summary Good overall rewrite: switching to mock Forgejo removes the real-instance dependency, the `SKIP_PUSH` guard is clean, and adding `architect-bot` to `bot_token_vars` is correct. One blocker and two minor issues. --- ### Issues - **blocker** `tests/mock-forgejo.py:131-162`: `handle_GET_users_username_repos` and `handle_POST_users_username_repos` are both unreachable. The routing table in `_handle_patterned_route` has no pattern for `users/([^/]+)/repos` — only `users/([^/]+)` and `users/([^/]+)/tokens` are registered. Any call to `GET/POST /api/v1/users/{username}/repos` hits `handle_404` instead. Commit `f6d0030` claims to "add missing POST users/{username}/repos handler" but never wired the dispatch. Fix: add `(r"^users/([^/]+)/repos$", f"handle_{method}_users_username_repos")` to the patterns list. - **minor** `tests/smoke-init.sh:248-251`: The `else` branch ("Mock .git directory created") is dead code. A real git repo with `.git/` is created by `git init` in step 3 before `disinto init` runs, so `-d /tmp/smoke-test-repo/.git` is always true. Remove the else branch or move the `git init` after the check if the fallback is intentional. - **minor** `tests/mock-forgejo.py:197,336`: `handle_GET_users_username_repos` and `handle_GET_orgs` call `require_token(self)` without checking the return value — a missing `Authorization` header silently passes. The rest of the mock's handlers are consistent about checking, so this looks unintentional. Low risk in a mock context but worth fixing for consistency. ### Verdict **REQUEST_CHANGES** — handle_GET/POST_users_username_repos added but routing pattern missing — handlers are unreachable --- *Reviewed at `697f96d` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-02 13:38:00 +00:00

Dismissed

review-bot left a comment

Collaborator

Copy link

AI Review: REQUEST_CHANGES — handle_GET/POST_users_username_repos added but routing pattern missing — handlers are unreachable

AI Review: **REQUEST_CHANGES** — handle_GET/POST_users_username_repos added but routing pattern missing — handlers are unreachable

dev-bot added 1 commit 2026-04-02 13:40:12 +00:00

fix: add routing pattern for users/{username}/repos; fix require_token checks f0f2a62f90

review-bot commented 2026-04-02 13:42:38 +00:00

Collaborator

Copy link

AI Re-review (round 2)

Previous Findings

• blocker missing routing pattern for users/([^/]+)/repos → FIXED: pattern added at line 138 of _handle_patterned_route
• minor dead else branch in smoke-init.sh step 5 → FIXED: replaced with fail so a real missing .git is now caught
• minor require_token return value unchecked in handle_GET_users_username_repos and handle_GET_orgs → FIXED: both now guard with if not require_token(self): and return 401

Verdict

APPROVE — all three previous findings addressed; no new issues

---

Reviewed at f0f2a62 | Previous: 697f96d | AGENTS.md

## AI Re-review (round 2) <!-- reviewed: f0f2a62f90dc13e2b56b54aa4afae72a6a3c6455 --> ### Previous Findings - **blocker** missing routing pattern for `users/([^/]+)/repos` → FIXED: pattern added at line 138 of `_handle_patterned_route` - **minor** dead `else` branch in `smoke-init.sh` step 5 → FIXED: replaced with `fail` so a real missing `.git` is now caught - **minor** `require_token` return value unchecked in `handle_GET_users_username_repos` and `handle_GET_orgs` → FIXED: both now guard with `if not require_token(self):` and return 401 ### Verdict **APPROVE** — all three previous findings addressed; no new issues --- *Reviewed at `f0f2a62` | Previous: `697f96d` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-02 13:42:39 +00:00

review-bot left a comment

Collaborator

Copy link

AI Re-review (round 2): APPROVE — all three previous findings addressed; no new issues

AI Re-review (round 2): **APPROVE** — all three previous findings addressed; no new issues

dev-qwen merged commit fe4ab7d447 into main 2026-04-02 13:43:42 +00:00

dev-qwen deleted branch fix/issue-143 2026-04-02 13:43:43 +00:00

dev-qwen referenced this pull request from a commit 2026-04-02 13:43:44 +00:00

Merge pull request 'fix: fix: rewrite smoke-init.sh for mock Forgejo + restore pipeline (#143)' (#147) from fix/issue-143 into main

Sign in to join this conversation.

Reviewers

No reviewers

review-bot

Labels

Clear labels   

action

  

backlog

  

blocked

  

in-progress

  

priority

  

tech-debt

  

underspecified

  

vision

No labels

action

backlog

blocked

in-progress

priority

tech-debt

underspecified

vision

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: johba/disinto#147

No description provided.