pdurlej/platform

Fork 0

refactor(forgejo): share actor token helpers #747

Merged

pdurlej merged 1 commit from codex/issues/726-forgejo-helper-cleanup into main

2026-06-08 15:04:46 +02:00

codex commented

2026-06-08 14:36:17 +02:00

Collaborator

Canary status: missing — fire canary 3+3 manually before merge

Stacked on #746 (codex/openai-saver-pilot). Do not merge before #746 lands, or rebase/retarget this branch onto main after #746 is merged.

Canary Context Pack

Product story

Forgejo service-actor tooling should resolve one token env name per actor everywhere. This removes the latent failure where platform-orchestrator can work in one control-plane path and silently miss its token in another.

What changed

Added platformctl.tools.forgejo_helpers with forgejo_token_env_name, resolve_forgejo_actor_token, and shared verify_forgejo_actor.
Routed run_review.py, pr_sanity.py, CLI identity doctor, and identity doctor CLI through the shared helper.
Collapsed the dead _emit no-op branch, reused EXIT_UNKNOWN_PARTIAL as the CLI unknown-state code, and imported fcntl once in telemetry.
Added regression tests for platform-orchestrator resolving FORGEJO_TOKEN_PLATFORM_ORCHESTRATOR in PR sanity and review comment paths.

Why it changed

Issue #726 identified inconsistent .upper() vs .upper().replace('-', '_') token lookup across Forgejo write helpers.

Files touched

control-plane/platformctl/tools/forgejo_helpers.py
control-plane/platformctl/tools/run_review.py
control-plane/platformctl/ci/pr_sanity.py
control-plane/platformctl/tools/identity_doctor.py
control-plane/platformctl/cli.py
control-plane/platformctl/telemetry.py
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

Relevant context

Forgejo issue #726
state/waves/2026-06-08-openai-saver-deslop/codex/codex-packet.md
Stacked base PR #746

Runtime evidence

No runtime mutation. Local test evidence only.

Known constraints

This touches Forgejo token resolution and actor verification, so route as class/security-sensitive and require full canary.

Explicit out-of-scope

Runtime credential reads or secret-store inspection
Forgejo merge/automerge behavior
#727-#731 deslop issues
Retargeting or merging #746

Requested decision

Review the stacked diff after #746, then rebase/retarget to main or merge in order.

Merge blockers

#746 not merged or this branch not rebased onto main
Full canary 3+3 not run
Any regression in Forgejo actor verification/token lookup tests

Spec sources read

Forgejo issue #726: task scope and acceptance
state/waves/2026-06-08-openai-saver-deslop/codex/codex-packet.md: pilot execution packet
control-plane/platformctl/tools/run_review.py: token lookup and actor verification call sites
control-plane/platformctl/ci/pr_sanity.py: PR sanity comment/review token paths
control-plane/platformctl/cli.py: identity doctor token lookup and _emit
control-plane/platformctl/tools/identity_doctor.py: Forgejo actor check
control-plane/platformctl/telemetry.py: duplicated local fcntl import
control-plane/platformctl/tests/test_pr_sanity.py: regression coverage
control-plane/platformctl/tests/test_glm_comment_hook.py: regression coverage

Refs #726

Canary status: missing — fire canary 3+3 manually before merge Stacked on #746 (`codex/openai-saver-pilot`). Do not merge before #746 lands, or rebase/retarget this branch onto `main` after #746 is merged. ## Canary Context Pack ### Product story Forgejo service-actor tooling should resolve one token env name per actor everywhere. This removes the latent failure where `platform-orchestrator` can work in one control-plane path and silently miss its token in another. ### What changed - Added `platformctl.tools.forgejo_helpers` with `forgejo_token_env_name`, `resolve_forgejo_actor_token`, and shared `verify_forgejo_actor`. - Routed `run_review.py`, `pr_sanity.py`, CLI identity doctor, and identity doctor CLI through the shared helper. - Collapsed the dead `_emit` no-op branch, reused `EXIT_UNKNOWN_PARTIAL` as the CLI unknown-state code, and imported `fcntl` once in telemetry. - Added regression tests for `platform-orchestrator` resolving `FORGEJO_TOKEN_PLATFORM_ORCHESTRATOR` in PR sanity and review comment paths. ### Why it changed Issue #726 identified inconsistent `.upper()` vs `.upper().replace('-', '_')` token lookup across Forgejo write helpers. ### Files touched - `control-plane/platformctl/tools/forgejo_helpers.py` - `control-plane/platformctl/tools/run_review.py` - `control-plane/platformctl/ci/pr_sanity.py` - `control-plane/platformctl/tools/identity_doctor.py` - `control-plane/platformctl/cli.py` - `control-plane/platformctl/telemetry.py` - `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` ### Relevant context - Forgejo issue #726 - `state/waves/2026-06-08-openai-saver-deslop/codex/codex-packet.md` - Stacked base PR #746 ### Runtime evidence No runtime mutation. Local test evidence only. ### Known constraints This touches Forgejo token resolution and actor verification, so route as `class/security-sensitive` and require full canary. ### Explicit out-of-scope - Runtime credential reads or secret-store inspection - Forgejo merge/automerge behavior - #727-#731 deslop issues - Retargeting or merging #746 ### Requested decision Review the stacked diff after #746, then rebase/retarget to `main` or merge in order. ### Merge blockers - #746 not merged or this branch not rebased onto `main` - Full canary 3+3 not run - Any regression in Forgejo actor verification/token lookup tests ## Spec sources read - Forgejo issue #726: task scope and acceptance - `state/waves/2026-06-08-openai-saver-deslop/codex/codex-packet.md`: pilot execution packet - `control-plane/platformctl/tools/run_review.py`: token lookup and actor verification call sites - `control-plane/platformctl/ci/pr_sanity.py`: PR sanity comment/review token paths - `control-plane/platformctl/cli.py`: identity doctor token lookup and `_emit` - `control-plane/platformctl/tools/identity_doctor.py`: Forgejo actor check - `control-plane/platformctl/telemetry.py`: duplicated local `fcntl` import - `control-plane/platformctl/tests/test_pr_sanity.py`: regression coverage - `control-plane/platformctl/tests/test_glm_comment_hook.py`: regression coverage Refs #726

codex added 1 commit

2026-06-08 14:36:17 +02:00

refactor(forgejo): share actor token helpers

canary-required / collect-diff (pull_request) Successful in 4s

Details

infra-docs-drift / docs-drift (pull_request) Successful in 4s

Details

pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 18s

Details

python-ci / Python 3.11 (pull_request) Successful in 45s

Details

python-ci / Python 3.12 (pull_request) Successful in 46s

Details

python-ci / Python 3.13 (pull_request) Successful in 44s

Details

workflow-lint / lint (pull_request) Successful in 5s

Details

base-is-main / guard (pull_request) Failing after 1s

Details

patchwarden-client-dry-run / collect-diff (pull_request) Successful in 4s

Details

patchwarden-pr-sanity / collect-diff (pull_request) Successful in 4s

Details

canary-required / canary (pull_request) Successful in 13s

Details

patchwarden-client-dry-run / dry-run (pull_request) Successful in 22s

Details

patchwarden-pr-sanity / sanity (pull_request) Successful in 3m39s

Details

088448211e

codex added the

class/security-sensitive

label

2026-06-08 14:36:25 +02:00

patchwarden commented

2026-06-08 14:44:03 +02:00

First-time contributor

Patchwarden PR sanity

Status: advisory_findings
PR: 747
Commit: 088448211e9e5c8527be034685501b792bfb1a12
Security-sensitive label: present
Authority: advisory model review plus deterministic blockers only
3+3 canary: still alive; this does not replace it

Deterministic findings

No deterministic findings.

Model reviewers

`global-glm` / `glm-5.1:cloud`

Status: ok
Verdict: NOT_OK
high Breaking change: pr_sanity.py env var name normalization differs from prior behavior
- Evidence: pr_sanity.py lines 628,640 previously used f"FORGEJO_TOKEN_{actor.upper()}"(no replace), now usesresolve_forgejo_actor_tokenwhich applies.replace('-', '_'). For actor='platform-orchestrator', old env was FORGEJO_TOKEN_PLATFORM-OR`
- Next: Document migration path in PR or add fallback lookup for old-style env var names during transition. Verify all CI secrets use underscore-form names before merge.
high Canary 3+3 not run on security-sensitive token resolution change
- Evidence: PR description states 'Canary status: missing — fire canary 3+3 manually before merge' and 'class/security-sensitive'. Token resolution is security-critical path.
- Next: Block merge until canary 3+3 is executed and results posted to PR.
low identity_doctor success detail now uses actor param instead of API-returned login
- Evidence: identity_doctor.py line 84: old code returned f"login={login}"using the value from API response, new code returnsf"login={actor}" using the parameter. While equivalent after verification passes, this loses the defense-in-depth propert
- Next: Consider returning the actual login from verify_forgejo_actor's detail string or passing it through for diagnostic clarity.

`global-deepseek` / `deepseek-v4-pro:cloud`

Status: ok
Verdict: OK
low Exit code constant may differ from original value
- Evidence: control-plane/platformctl/cli.py: removed local EXIT_UNKNOWN_STATE = 8and importedEXIT_UNKNOWN_PARTIAL as EXIT_UNKNOWN_STATEfrom.plan. The value of EXIT_UNKNOWN_PARTIAL is not visible in the diff; if it differs from 8, scripts r
- Next: Verify that EXIT_UNKNOWN_PARTIAL in plan.py equals 8, or document the change if intentional.
low Actor verification timeout increased in run_review.py
- Evidence: control-plane/platformctl/tools/run_review.py: old local _verify_actordefaulted totimeout_s=10; new shared verify_forgejo_actordefaults to 15. Call sites inrun_review.py do not override, so timeout changes from 10s to 15s.
- Next: Confirm that the 15s timeout is acceptable for review posting paths; adjust if needed.

`redteam` / `kimi-k2.6:cloud`

Status: ok
Verdict: OK
Findings: none

Policy notes

GLM 5.1 + DeepSeek V4 Pro are the operator-required model mix for this bot.
Optional red-team model is enabled only when PLATFORMCTL_PR_SANITY_REDTEAM_MODEL is configured.
Auto-merge is not enabled here.

# Patchwarden PR sanity - Status: `advisory_findings` - PR: `747` - Commit: `088448211e9e5c8527be034685501b792bfb1a12` - Security-sensitive label: `present` - Authority: advisory model review plus deterministic blockers only - 3+3 canary: still alive; this does not replace it ## Deterministic findings No deterministic findings. ## Model reviewers ### `global-glm` / `glm-5.1:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`high`** Breaking change: pr_sanity.py env var name normalization differs from prior behavior - Evidence: `pr_sanity.py lines 628,640 previously used `f"FORGEJO_TOKEN_{actor.upper()}"` (no replace), now uses `resolve_forgejo_actor_token` which applies `.replace('-', '_')`. For actor='platform-orchestrator', old env was `FORGEJO_TOKEN_PLATFORM-OR` - Next: Document migration path in PR or add fallback lookup for old-style env var names during transition. Verify all CI secrets use underscore-form names before merge. - **`high`** Canary 3+3 not run on security-sensitive token resolution change - Evidence: `PR description states 'Canary status: missing — fire canary 3+3 manually before merge' and 'class/security-sensitive'. Token resolution is security-critical path.` - Next: Block merge until canary 3+3 is executed and results posted to PR. - **`low`** identity_doctor success detail now uses actor param instead of API-returned login - Evidence: `identity_doctor.py line 84: old code returned `f"login={login}"` using the value from API response, new code returns `f"login={actor}"` using the parameter. While equivalent after verification passes, this loses the defense-in-depth propert` - Next: Consider returning the actual login from verify_forgejo_actor's detail string or passing it through for diagnostic clarity. ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `OK` - **`low`** Exit code constant may differ from original value - Evidence: `control-plane/platformctl/cli.py: removed local `EXIT_UNKNOWN_STATE = 8` and imported `EXIT_UNKNOWN_PARTIAL as EXIT_UNKNOWN_STATE` from `.plan`. The value of `EXIT_UNKNOWN_PARTIAL` is not visible in the diff; if it differs from 8, scripts r` - Next: Verify that `EXIT_UNKNOWN_PARTIAL` in `plan.py` equals 8, or document the change if intentional. - **`low`** Actor verification timeout increased in run_review.py - Evidence: `control-plane/platformctl/tools/run_review.py: old local `_verify_actor` defaulted to `timeout_s=10`; new shared `verify_forgejo_actor` defaults to 15. Call sites in `run_review.py` do not override, so timeout changes from 10s to 15s.` - Next: Confirm that the 15s timeout is acceptable for review posting paths; adjust if needed. ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `OK` - Findings: none ## Policy notes - GLM 5.1 + DeepSeek V4 Pro are the operator-required model mix for this bot. - Optional red-team model is enabled only when `PLATFORMCTL_PR_SANITY_REDTEAM_MODEL` is configured. - Auto-merge is not enabled here.