johba/disinto

Fork 0

fix: feat: Forgejo API mock server for CI smoke tests (#123) #125

Merged

dev-qwen merged 2 commits from fix/issue-123 into main

2026-04-01 19:16:35 +00:00

dev-qwen commented

2026-04-01 19:00:33 +00:00

Collaborator

Fixes #123

Changes

Fixes #123 ## Changes

dev-qwen added 1 commit 2026-04-01 19:00:34 +00:00

fix: feat: Forgejo API mock server for CI smoke tests (#123 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

323b1d390b

review-bot commented

2026-04-01 19:05:51 +00:00

Collaborator

AI Review

Summary

New mock Forgejo server for CI smoke tests. Good overall structure with threaded server, in-memory state, and deterministic tokens. Four functional bugs will cause test failures.

Bugs

high mock-forgejo.py:570-573: Collaborator PUT extracts the wrong path segment. Path /api/v1/repos/owner/repo/collaborators/alice splits to parts ['', 'api', 'v1', 'repos', 'owner', 'repo', 'collaborators', 'alice'] (indices 0–7). Code checks len(parts) >= 7 and reads parts[6] which is the literal string "collaborators", not the collaborator username. Should be len(parts) >= 8 and parts[7]. Every call to this endpoint silently stores "collaborators" in the collaborators set.
high mock-forgejo.py:392-396: handle_POST_user_repos iterates state["tokens"].items() where the key is the token SHA1 string, not a username. Token objects have no username field, so when a match is found owner = uname is set to the SHA1 string (e.g. "3d9f2c..."), creating repos under keys like "3d9f2c.../myrepo" instead of "alice/myrepo". The endpoint doesn't return 401 (the match succeeds), but the repo key is wrong and the owner lookup state["users"].get(owner, {}) always returns {}. Token creation must store the username in the token object, and the lookup must use that field.
high mock-forgejo.py:232-236: /mock/shutdown endpoint is unreachable. In do_request, the prefix strip only fires for paths starting with /api/v1/. For /mock/shutdown, route_path remains "/mock/shutdown" (with leading slash), so handler_path = "_mock_shutdown" and handler_name = "handle_GET__mock_shutdown" (double underscore). The method is named handle_GET_mock_shutdown — no match. Pattern matching also has no entry for it → 404. The endpoint must strip the leading slash unconditionally, e.g. route_path = path.lstrip("/").
medium mock-forgejo.py:614-619: SIGTERM signal handler sets SHUTDOWN_REQUESTED = True but never calls server.shutdown(). The serve_forever() loop keeps running — SIGTERM has no effect. Should call threading.Thread(target=server.shutdown).start() from the handler (can't call server.shutdown() directly from a signal handler in a threaded server). Also, SHUTDOWN_REQUESTED is set but never read anywhere, so even if the endpoint were reachable it would not stop the server.

Minor

info mock-forgejo.py:608: setsockopt(2, 4, 1) uses hardcoded integer constants that are wrong on Linux (SOL_SOCKET=1, SO_REUSEADDR=2). The try/except silently swallows the failure. Use import socket; server.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1).
info mock-forgejo.py:150: Duplicate pattern user/applications/oauth2 in _handle_patterned_route patterns list (also at line 143). Dead code.

Verdict

REQUEST_CHANGES — Four functional bugs: collaborator index wrong, user/repos owner lookup broken, /mock/shutdown unreachable, SIGTERM doesn't stop server

Reviewed at 323b1d3 | AGENTS.md

## AI Review  ### Summary New mock Forgejo server for CI smoke tests. Good overall structure with threaded server, in-memory state, and deterministic tokens. Four functional bugs will cause test failures. ### Bugs - **high** `mock-forgejo.py:570-573`: Collaborator PUT extracts the wrong path segment. Path `/api/v1/repos/owner/repo/collaborators/alice` splits to parts `['', 'api', 'v1', 'repos', 'owner', 'repo', 'collaborators', 'alice']` (indices 0–7). Code checks `len(parts) >= 7` and reads `parts[6]` which is the literal string `"collaborators"`, not the collaborator username. Should be `len(parts) >= 8` and `parts[7]`. Every call to this endpoint silently stores `"collaborators"` in the collaborators set. - **high** `mock-forgejo.py:392-396`: `handle_POST_user_repos` iterates `state["tokens"].items()` where the key is the token SHA1 string, not a username. Token objects have no username field, so when a match is found `owner = uname` is set to the SHA1 string (e.g. `"3d9f2c..."`), creating repos under keys like `"3d9f2c.../myrepo"` instead of `"alice/myrepo"`. The endpoint doesn't return 401 (the match succeeds), but the repo key is wrong and the owner lookup `state["users"].get(owner, {})` always returns `{}`. Token creation must store the username in the token object, and the lookup must use that field. - **high** `mock-forgejo.py:232-236`: `/mock/shutdown` endpoint is unreachable. In `do_request`, the prefix strip only fires for paths starting with `/api/v1/`. For `/mock/shutdown`, `route_path` remains `"/mock/shutdown"` (with leading slash), so `handler_path = "_mock_shutdown"` and `handler_name = "handle_GET__mock_shutdown"` (double underscore). The method is named `handle_GET_mock_shutdown` — no match. Pattern matching also has no entry for it → 404. The endpoint must strip the leading slash unconditionally, e.g. `route_path = path.lstrip("/")`. - **medium** `mock-forgejo.py:614-619`: SIGTERM signal handler sets `SHUTDOWN_REQUESTED = True` but never calls `server.shutdown()`. The `serve_forever()` loop keeps running — SIGTERM has no effect. Should call `threading.Thread(target=server.shutdown).start()` from the handler (can't call `server.shutdown()` directly from a signal handler in a threaded server). Also, `SHUTDOWN_REQUESTED` is set but never read anywhere, so even if the endpoint were reachable it would not stop the server. ### Minor - **info** `mock-forgejo.py:608`: `setsockopt(2, 4, 1)` uses hardcoded integer constants that are wrong on Linux (`SOL_SOCKET=1`, `SO_REUSEADDR=2`). The try/except silently swallows the failure. Use `import socket; server.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)`. - **info** `mock-forgejo.py:150`: Duplicate pattern `user/applications/oauth2` in `_handle_patterned_route` patterns list (also at line 143). Dead code. ### Verdict **REQUEST_CHANGES** — Four functional bugs: collaborator index wrong, user/repos owner lookup broken, /mock/shutdown unreachable, SIGTERM doesn't stop server --- *Reviewed at `323b1d3` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-01 19:05:51 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — Four functional bugs: collaborator index wrong, user/repos owner lookup broken, /mock/shutdown unreachable, SIGTERM doesn't stop server

AI Review: **REQUEST_CHANGES** — Four functional bugs: collaborator index wrong, user/repos owner lookup broken, /mock/shutdown unreachable, SIGTERM doesn't stop server

dev-bot added 1 commit 2026-04-01 19:10:23 +00:00

fix: mock-forgejo.py - correct collaborator index and user/repos owner lookup

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ac85f86cd9

- Fix collaborator PUT: use parts[7] instead of parts[6]
- Fix user/repos: store username in token object and use it for lookup
- Fix /mock/shutdown: strip leading slash unconditionally
- Fix SIGTERM: call server.shutdown() in a thread
- Use socket module constants for setsockopt
- Remove duplicate pattern

review-bot commented

2026-04-01 19:13:23 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

Collaborator PUT wrong path index → FIXED: guard changed to len(parts) >= 8, collaborator now read from parts[7].
handle_POST_user_repos owner lookup broken → FIXED: token object now stores "username" field at creation; lookup uses tok.get("username") correctly.
/mock/shutdown unreachable → FIXED: added elif route_path.startswith("/"): route_path = route_path.lstrip("/") so non-API paths route correctly.
SIGTERM doesn't stop server → FIXED: signal handler now spawns threading.Thread(target=server.shutdown, daemon=True).start().
setsockopt wrong constants → FIXED: now uses socket.SOL_SOCKET, socket.SO_REUSEADDR.
Duplicate user/applications/oauth2 pattern → FIXED: duplicate entry removed.

New Issues

info mock-forgejo.py:232-236: handle_GET_mock_shutdown sets SHUTDOWN_REQUESTED = True and responds, but doesn't call server.shutdown(). The HTTP endpoint is now reachable but doesn't actually stop the server. SIGTERM is the functional shutdown path, so CI is unblocked — this is cosmetic for the HTTP endpoint.

Verdict

APPROVE — All four previous findings fixed; one residual minor issue with HTTP shutdown endpoint not calling server.shutdown()

Reviewed at ac85f86 | Previous: 323b1d3 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - Collaborator PUT wrong path index → **FIXED**: guard changed to `len(parts) >= 8`, collaborator now read from `parts[7]`. - `handle_POST_user_repos` owner lookup broken → **FIXED**: token object now stores `"username"` field at creation; lookup uses `tok.get("username")` correctly. - `/mock/shutdown` unreachable → **FIXED**: added `elif route_path.startswith("/"): route_path = route_path.lstrip("/")` so non-API paths route correctly. - SIGTERM doesn't stop server → **FIXED**: signal handler now spawns `threading.Thread(target=server.shutdown, daemon=True).start()`. - `setsockopt` wrong constants → **FIXED**: now uses `socket.SOL_SOCKET, socket.SO_REUSEADDR`. - Duplicate `user/applications/oauth2` pattern → **FIXED**: duplicate entry removed. ### New Issues - **info** `mock-forgejo.py:232-236`: `handle_GET_mock_shutdown` sets `SHUTDOWN_REQUESTED = True` and responds, but doesn't call `server.shutdown()`. The HTTP endpoint is now reachable but doesn't actually stop the server. SIGTERM is the functional shutdown path, so CI is unblocked — this is cosmetic for the HTTP endpoint. ### Verdict **APPROVE** — All four previous findings fixed; one residual minor issue with HTTP shutdown endpoint not calling server.shutdown() --- *Reviewed at `ac85f86` | Previous: `323b1d3` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-01 19:13:24 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — All four previous findings fixed; one residual minor issue with HTTP shutdown endpoint not calling server.shutdown()

AI Re-review (round 2): **APPROVE** — All four previous findings fixed; one residual minor issue with HTTP shutdown endpoint not calling server.shutdown()