pdurlej/platform

Fork 0

fix(platformctl): fix hyphenated actor token resolution bug + de-duplicate forgejo helpers #761

New issue

Closed

opened 2026-06-08 23:08:40 +02:00 by ollama · 1 comment

ollama commented

2026-06-08 23:08:40 +02:00

Collaborator

Spec sources (whitelist)

control-plane/platformctl/cli.py line ~1387 — actor.upper().replace('-','_') pattern (correct)
control-plane/platformctl/tools/run_review.py line ~1055 — actor.upper().replace('-','_') pattern (correct)
control-plane/platformctl/tools/run_review.py line ~976 — bare .upper() (BUG)
control-plane/platformctl/tools/pr_sanity.py lines ~606 and ~612 — bare .upper() (BUG)
control-plane/platformctl/tools/run_review.py line ~525 — _verify_actor implementation A
control-plane/platformctl/tools/pr_sanity.py line ~585 — _verify_actor implementation B (duplicate)
control-plane/platformctl/tests/test_glm_comment_hook.py lines ~374 and ~412 — test cases (verify they use non-hyphenated actors)

Extracted context

Latent bug: Forgejo token-env idiom is reimplemented ~6× in two inconsistent forms:

cli.py:1387 + run_review.py:1055: use .upper().replace('-','_') → resolves FORGEJO_TOKEN_PLATFORM_ORCHESTRATOR
run_review.py:976 + pr_sanity.py:606,612: use bare .upper() → resolves FORGEJO_TOKEN_PLATFORM-ORCHESTRATOR (note the hyphen!)

For hyphenated actors like platform-orchestrator, these resolve DIFFERENT env var names — the bare-form sites silently fail to find the token.

Tests use non-hyphenated actors so won't catch this.

Also: _verify_actor is implemented twice — run_review.py:525 (timeout + granular except) and pr_sanity.py:585 (bare except).

Scope

Create new file control-plane/platformctl/identity/forgejo.py (or similar shared module) with:
- forgejo_token_env_name(actor) = f"FORGEJO_TOKEN_{actor.upper().replace('-','_')}"
- resolve_forgejo_actor_token(actor, generic_env=None) that looks up the env var
- verify_forgejo_actor(host, token, expected_actor, *, timeout_s=15) — keep stricter run_review error-handling as canonical
Route the 4 bare-.upper() sites through the helpers: run_review.py:976, pr_sanity.py:606,612
Route the 2 correct sites through the same helpers (deduplication): cli.py:1387, run_review.py:1055
Replace both _verify_actor implementations with calls to the shared version
Remove now-duplicated token-env code from the old locations

Acceptance criteria

715+ tests still green
No hyphenated actor can resolve a different env var name across sites
One shared forgejo-helper module (token-env + verify_actor)
Error handling follows the stricter run_review pattern (timeout + granular except)

Do NOT read

Full repo — only the files listed above
Do NOT modify test files (they already use non-hyphenated actors)

Agent notes

Recommended executor: Gemini 3.1 Pro (real bug fix, needs careful refactoring)
Size: Medium (~80 LOC + new file)
Review tier: tier/medium (touches production token resolution paths)
Audit ref: state/audit/deepseek-2026-06-08-multiperspective.md §E1-E2
This is a subset of #726 item 1-2 (claude's deslop issue)

## Spec sources (whitelist) - `control-plane/platformctl/cli.py` line ~1387 — `actor.upper().replace('-','_')` pattern (correct) - `control-plane/platformctl/tools/run_review.py` line ~1055 — `actor.upper().replace('-','_')` pattern (correct) - `control-plane/platformctl/tools/run_review.py` line ~976 — **bare `.upper()`** (BUG) - `control-plane/platformctl/tools/pr_sanity.py` lines ~606 and ~612 — **bare `.upper()`** (BUG) - `control-plane/platformctl/tools/run_review.py` line ~525 — `_verify_actor` implementation A - `control-plane/platformctl/tools/pr_sanity.py` line ~585 — `_verify_actor` implementation B (duplicate) - `control-plane/platformctl/tests/test_glm_comment_hook.py` lines ~374 and ~412 — test cases (verify they use non-hyphenated actors) ## Extracted context **Latent bug**: Forgejo token-env idiom is reimplemented ~6× in two inconsistent forms: - `cli.py:1387` + `run_review.py:1055`: use `.upper().replace('-','_')` → resolves `FORGEJO_TOKEN_PLATFORM_ORCHESTRATOR` - `run_review.py:976` + `pr_sanity.py:606,612`: use **bare `.upper()`** → resolves `FORGEJO_TOKEN_PLATFORM-ORCHESTRATOR` (note the hyphen!) For hyphenated actors like `platform-orchestrator`, these resolve DIFFERENT env var names — the bare-form sites silently fail to find the token. Tests use non-hyphenated actors so won't catch this. Also: `_verify_actor` is implemented twice — `run_review.py:525` (timeout + granular except) and `pr_sanity.py:585` (bare except). ## Scope 1. Create new file `control-plane/platformctl/identity/forgejo.py` (or similar shared module) with: - `forgejo_token_env_name(actor) = f"FORGEJO_TOKEN_{actor.upper().replace('-','_')}"` - `resolve_forgejo_actor_token(actor, generic_env=None)` that looks up the env var - `verify_forgejo_actor(host, token, expected_actor, *, timeout_s=15)` — keep stricter run_review error-handling as canonical 2. Route the 4 bare-`.upper()` sites through the helpers: `run_review.py:976`, `pr_sanity.py:606,612` 3. Route the 2 correct sites through the same helpers (deduplication): `cli.py:1387`, `run_review.py:1055` 4. Replace both `_verify_actor` implementations with calls to the shared version 5. Remove now-duplicated token-env code from the old locations ## Acceptance criteria - [ ] 715+ tests still green - [ ] No hyphenated actor can resolve a different env var name across sites - [ ] One shared forgejo-helper module (token-env + verify_actor) - [ ] Error handling follows the stricter run_review pattern (timeout + granular except) ## Do NOT read - Full repo — only the files listed above - Do NOT modify test files (they already use non-hyphenated actors) ## Agent notes - Recommended executor: Gemini 3.1 Pro (real bug fix, needs careful refactoring) - Size: Medium (~80 LOC + new file) - Review tier: tier/medium (touches production token resolution paths) - Audit ref: state/audit/deepseek-2026-06-08-multiperspective.md §E1-E2 - This is a subset of #726 item 1-2 (claude's deslop issue)

ollama referenced this issue

2026-06-08 23:09:24 +02:00

[meta] DeepSeek audit 2026-06-08 — atomic Gemini-ready task batch #766

codex added the

class/security-sensitive

labels

2026-06-11 08:16:02 +02:00

pdurlej added the

judge/p1

ready-for-agent

status:codex-ready

labels

2026-06-17 23:35:30 +02:00

pdurlej referenced this issue

2026-06-17 23:35:37 +02:00

deslop(code): fix latent hyphenated-actor token bug + de-dup forgejo helpers #726

pdurlej referenced this issue

2026-06-17 23:35:38 +02:00

[meta] DeepSeek audit 2026-06-08 — atomic Gemini-ready task batch #766

pdurlej commented

2026-06-17 23:37:55 +02:00

Owner

Closing as already implemented on current main.

Evidence checked before close:

control-plane/platformctl/tools/forgejo_helpers.py now owns forgejo_token_env_name(), resolve_forgejo_actor_token(), and verify_forgejo_actor().
control-plane/platformctl/tools/run_review.py and control-plane/platformctl/ci/pr_sanity.py import/use those helpers instead of local bare actor env logic.
Hyphenated actor regression exists in control-plane/platformctl/tests/test_forgejo_helpers.py for platform-orchestrator -> FORGEJO_TOKEN_PLATFORM_ORCHESTRATOR.
Verification: UV_CACHE_DIR=/tmp/platform-uv-cache PYTHONPATH=control-plane uv run --project control-plane --extra dev pytest control-plane/platformctl/tests/test_forgejo_helpers.py control-plane/platformctl/tests/test_glm_comment_hook.py control-plane/platformctl/tests/test_pr_sanity.py -q -> 75 passed.

No PR opened because the implementation is already present; leaving this issue open was queue noise.

Closing as already implemented on current `main`. Evidence checked before close: - `control-plane/platformctl/tools/forgejo_helpers.py` now owns `forgejo_token_env_name()`, `resolve_forgejo_actor_token()`, and `verify_forgejo_actor()`. - `control-plane/platformctl/tools/run_review.py` and `control-plane/platformctl/ci/pr_sanity.py` import/use those helpers instead of local bare actor env logic. - Hyphenated actor regression exists in `control-plane/platformctl/tests/test_forgejo_helpers.py` for `platform-orchestrator` -> `FORGEJO_TOKEN_PLATFORM_ORCHESTRATOR`. - Verification: `UV_CACHE_DIR=/tmp/platform-uv-cache PYTHONPATH=control-plane uv run --project control-plane --extra dev pytest control-plane/platformctl/tests/test_forgejo_helpers.py control-plane/platformctl/tests/test_glm_comment_hook.py control-plane/platformctl/tests/test_pr_sanity.py -q` -> `75 passed`. No PR opened because the implementation is already present; leaving this issue open was queue noise.

pdurlej closed this issue

2026-06-17 23:37:55 +02:00

pdurlej referenced this issue

2026-06-18 00:06:36 +02:00

[meta] DeepSeek audit 2026-06-08 — atomic Gemini-ready task batch #766