feat(framework): forge wrapper gaps, merge policy and PR workflow skills (#1841) #1842

Merged
jason.woltje merged 26 commits from feat/forge-wrapper-gaps into next 2026-11-05 20:13:57 +00:00
Member

Summary

Adds provider-neutral forge capability and PR status wrappers, complete review/comment enumeration with verified native replies and resolution where supported, declared merge methods, and separate inspection/control of automatic integration. Status aggregation pins the complete head/base identity, fails closed on missing evidence, and supports independent T3 wake and fallback notifier paths.

Extends the repository declaration with an optional validated merge policy. Policy is loaded from the pinned trusted target revision. Absent policy means allowed methods = [squash] and default = squash. Explicit method requests must satisfy allowed methods and any transition-shape mapping, then intersect with the forge's read-only allow-method settings. Repository settings are never changed. Auto-merge and merge-queue enabling are off by default; inspect and cancel are separate operations.

Promotes the full-send, full-stop, babysit-PR and council workflows with their git/watch dependencies into framework sources. Adds direct-write guard coverage, CI-wait compatibility, fixture tests and independent transport-parity expectations. Carries one operator-agnostic local fix: Python import isolation in the shared detection/auth helper. Other local tool drift was not copied wholesale.

Authenticated adapter operations require an explicit actor and host-bound credential; mutations verify the actor. Shell credential workflows stay inside an untraced private subprocess. Anonymous descriptors keep authorization out of process arguments and named temporary files. Public and enterprise API routing is tested against complete expected URLs; nonzero curl exits fail closed, preserving unknown write outcomes.

Original base: 924438016e2eb1c2ca352bfbf9107143b8b342bb.
Historical R3-reviewed head: 5cde8cd15b41fef7cc00ed85c06592254c5f1537.
Final reviewed head: de99f7263f41247406953f71240dfb93fd19f44c.

The follow-up commits restore bounded authenticated HTTPS GET redirects, validate raw response bytes and URI components before following a redirect, disable default curl configuration and URL globbing, and refuse mutation redirects. Real loopback TLS controls exercise the actual queue/status entry paths. The three isolated CI consumers explicitly install the curl/OpenSSL prerequisites. Usage fixtures now isolate their owned Git origins, and milestone creation rejects missing or empty titles before provider work while title-free listing remains supported.

Capability and exit-code contract

  • Exit 4 means unsupported API/method or a forge setting that disallows the method. Unsupported operations refuse before mutation; unknown required state cannot establish readiness.
  • Exit 3 identifies policy refusal (error.kind=policy), distinct from unsupported capabilities and authentication/unknown errors.
  • Native review reply/resolution is unsupported on the inspected older Gitea and Forgejo profiles. Supported providers enumerate all pages and verify mutation readback.
  • Inspected Gitea/Forgejo schemas lack scheduled-merge state readback, so scheduled integration and native queue operations remain unsupported rather than claiming verified state. No server upgrades are included.
  • GitHub rebase-merge and fast-forward-only are unsupported; other methods remain constrained by declaration and forge settings. GitHub automatic integration is conditional on verified capability and policy. Missing required evidence fails closed.

Historical verification at the R3 head

All provider tests use synthetic fixtures; no live forge writes occur in tests. The following retained results describe the historical R3 candidate, not a full rerun at the final head:

  • Each of 67 git shell suites run separately with unexpected network clients denied: 66 exit 0; the pre-existing milestone usage suite exits 1. The sweep driver exits 1 solely for that baseline failure.
  • Seven Python suites via python3 -I: all exit 0, 146 tests. Controls cover complete public/enterprise URLs, curl failures, trusted-base policy, retarget races, malformed state, actor refusal, all 13 shell credential consumers under tracing, and trace/argv mutants.
  • test-pr-watch-parity.sh: exit 0, including independent timeline expectations and wrong-verdict/missing-stop mutant controls.
  • test-forge-schema.sh: exit 0, including schema negative controls.
  • test-validate-repo-json.sh: exit 0, 101 assertions.
  • bash -n over all 107 git shell files: exit 0.
  • Full ShellCheck: exit 1 on both the previous candidate and revised tree; zero new diagnostics.
  • git diff --check against the original base: exit 0.
  • pnpm preflight, pnpm typecheck, pnpm lint, pnpm format:check: each exit 0; normal push hooks also passed.
  • Package skill and framework-manifest Vitest suites: exit 0, 97 tests across three files.

Not verified: live forge writes, repo settings, live T3 watch/notifier delivery, CI.

Final candidate evidence

The final three commits are d3a2b8c8704fa3dfc0f415f72333ff61788a521f (raw header bytes), ade6f3d26f39fee1a833b6c57edd176ada4fd1d4 (issue fixture boundaries), and de99f7263f41247406953f71240dfb93fd19f44c (milestone title ordering). Their combined delta from 1a569f734ccee1820f435372598dc6dad160f001 is eight files, 123 insertions and 22 deletions. Normal commit/push hooks passed; the remote branch was read back at the final full SHA with a clean worktree and no unpushed commits.

  • python3 -I test-forge-cli.py: exit 0, 25 tests. test-forge-auth.py: exit 0, nine tests. test-forge-contract.py: exit 0, 105 tests. Fixtures prove malformed raw headers refuse before follow-up, valid CRLF/OWS and escaped targets remain supported, and the earlier URI/glob controls remain discriminating.
  • The 18 relevant shell/Python invocations, 11 ordered sanitization commands, eight supplemental legacy regressions and the two executable redirect-entry Vitest cases each exit 0. The broad author matrix preceded the last allowed reason-phrase HTAB clarification; its focused positive was rerun successfully, and both independent reviewers subsequently ran the final CLI suite.
  • The corrected issue-comment, issue-close, issue-reopen and milestone usage suites each exit 0 inside an owned synthetic checkout and outside a repository. Earlier versions reproduced the four/two/two unexpected schema reads and the milestone title failure. Original-base milestone controls exit 0 inside the synthetic checkout and 1 outside; the corrected final suite exits 0 in both contexts. Title-free listing and quoted-title creation retain executable provider-dispatch and exact-payload controls.
  • Twenty original configured usage commands were run individually before the final remainder fixes: 17 passed and three failed. The three proved failures were then corrected under explicit scope and passed both context controls. Other remainder commands are explicitly recorded as not run; these results do not represent a full-chain pass.
  • bash -n on the five changed shell files: exit 0. Installed ShellCheck on the three issue usage fixtures: exit 0. Milestone usage ShellCheck: exit 1 for the existing SC2043 one-iteration loop; the original-base copy independently produces the same warning and exit 1. No assertion was removed to suppress it.
  • git diff --check: exit 0. Required normal push hooks ran pnpm preflight, pnpm typecheck, pnpm lint and pnpm format:check, all exit 0. Queue admission returned its documented degraded permission after an unavailable status response; this is not CI success.

The full configured local test:framework-shell chain was attempted and exited 1 at its fifth command, framework/systemd/user/test-fleet-units.sh: the user RuntimeDirectory/manager prerequisite was unavailable. Codex and milestone commands were not reached in that full-chain run. No retry, skipped assertion, hidden tool path or runtime repair was used. Equal original/current systemd source establishes source equality only, not a measured baseline execution failure. Individual remainder checks are separate evidence and do not waive that failed/unavailable local full chain.

CI snapshot and failed-history provenance

Coordinator-verified terminal evidence: pipeline 3083 SUCCESS, workflow 9453, on exact de99f7263f41247406953f71240dfb93fd19f44c. All 11 child steps succeeded with exit 0, including test and build. The saved full pipeline passed tools/woodpecker/verify-terminal-green.py --expect-commit de99f7263f41247406953f71240dfb93fd19f44c (exit 0); the failed pipeline 3082 control was refused (exit 1). No exception was used. This is actual parent-measured exact-head CI evidence, separate from the unavailable local full-chain run. No CI query, rerun or watch was performed for this metadata refresh. Both independent exact-head approvals remain in effect; no merge authority is granted by this update.

Actual pipeline Head Result and diagnosed scope
3076 5cde8cd15 FAILED sanitization: missing test enumeration; corrected by 00d62c62ca3a5f17f11ff28e36a63220a4f97a7a.
3077 00d62c62c FAILED sanitization: missing tool-index rows; corrected by 1effc9c143579da9e442ad98145c86498734e61a.
3078 1effc9c14 FAILED sanitization: BusyBox fixture launcher; corrected by 79682f703f15d6659acce447f4ec726a9e05425c.
3079 79682f703 FAILED test after earlier gates passed: stale redirect assertions plus an actual adapter redirect compatibility loss; corrected with executable coverage and bounded transport.
3080 85ea376dc FAILED test after earlier gates passed: local PR-diff fixture reached unset platform state; corrected by the owned synthetic provider seam at 1a569f734. The earlier flattened diagnostic artifact resulted from concatenating decoded records without separators, not a proved CI backend defect.
3082 1a569f734 FAILED test after earlier gates passed: issue-comment parse-acceptance fixtures inherited the outer checkout origin and recorded four schema GETs; corrected by the no-origin fixture boundary. Close/reopen and title-ordering findings came from separate local remainder checks, not additional CI observations.

No pipeline 3081 result was supplied. The local systemd limitation is separate from these measured CI failures. The explicit prerequisite additions are assessed in source; no full pinned-image pass or binary-inventory measurement is claimed.

Independent review record

Evidence retained on the review host. Historical approvals cover only their listed heads. Interruptions are not approvals; no missing score, report or history is invented.

Reviewer Model Round / scope Head Verdict
rev-code-01 not specified 1 55d6cc18e REQUEST-CHANGES
rev-security-01 not specified 1 55d6cc18e REQUEST-CHANGES
rev-code-01 not specified 2 a193cf284 REQUEST-CHANGES
rev-security-01 not specified 2 a193cf284 REQUEST-CHANGES
rev-code-01 gpt-6-astra 3 5cde8cd15b41fef7cc00ed85c06592254c5f1537 APPROVE; no blockers/nits
rev-security-01 gpt-6-astra 3 5cde8cd15b41fef7cc00ed85c06592254c5f1537 APPROVE; no blockers/nits
rev-code-01 gpt-6-luna 4, enumeration/index 1effc9c143579da9e442ad98145c86498734e61a Interrupted before later edits; no final verdict
rev-security-01 gpt-6.1-sol 4, enumeration/index 1effc9c143579da9e442ad98145c86498734e61a APPROVE locally; CI separately failed
rev-code-01 gpt-6-luna 5, portable fixtures 79682f703f15d6659acce447f4ec726a9e05425c Interrupted before later edits; no final verdict
rev-security-01 gpt-6.1-sol 5, portable fixtures 79682f703f15d6659acce447f4ec726a9e05425c APPROVE locally; CI separately failed
rev-code-01 gpt-6-luna 6, redirects 85ea376dc00bfb04d5a0e9c87d42c0dddac46fe6 Interrupted before later edits; no final verdict
rev-security-01 gpt-6.1-sol 6, redirects 85ea376dc00bfb04d5a0e9c87d42c0dddac46fe6 REQUEST-CHANGES: URI/glob amplification
rev-code-01 gpt-6-luna Superseded URI follow-up b03f3a70bd495c221e363f472882329e96bd856a Interrupted when fixture scope was added; no final verdict; no security task launched for this superseded candidate
rev-code-01 gpt-6-luna Combined URI/PR-diff fixture 1a569f734ccee1820f435372598dc6dad160f001 REQUEST-CHANGES: bare-CR header truncation
rev-security-01 gpt-6.1-sol Combined URI/PR-diff fixture 1a569f734ccee1820f435372598dc6dad160f001 APPROVE; did not override code blocker
rev-code-01 gpt-6-luna Current combined candidate de99f7263f41247406953f71240dfb93fd19f44c APPROVE; no blockers/nits; no score assigned
rev-security-01 gpt-6.1-sol Current combined candidate de99f7263f41247406953f71240dfb93fd19f44c APPROVE; no blockers/new nits; no score assigned

Original R1/R2 responses retain trusted-target policy, explicit host-bound actor authentication, anonymous authorization descriptors, typed fail-closed state, complete head/base identity binding, independent parity/mutant controls, offline fixtures and transport-error refusal. R3 approved those corrections. Later URI/glob amplification was corrected at b03f3a70bd495c221e363f472882329e96bd856a; the later raw-header finding was corrected at d3a2b8c8704fa3dfc0f415f72333ff61788a521f. The final reviewers independently reproduced predecessor failures and corrected behavior. All changes use new commits without rewriting reviewed history.

Current review task receipts

Both tasks used provider codex_daybreak_blue, reasoningEffort=max, serviceTier=default (Standard), with requested and returned settings matched. Evidence retained on the review host; no internal paths or report contents are reproduced here.

  • Code: rev-code-01, gpt-6-luna, task ID node:delegated-task:command%3Amcp%3A17719a77-a459-4ebb-a3c5-ff8a27b26e16%3Adelegate-task%3Aforge-review-code-raw-header-combined-de99f7263-20261104. APPROVE at the exact final head; no blockers/nits; score not assigned.
  • Security: rev-security-01, gpt-6.1-sol, task ID node:delegated-task:command%3Amcp%3A17719a77-a459-4ebb-a3c5-ff8a27b26e16%3Adelegate-task%3Aforge-review-security-raw-header-combined-de99f7263-20261104. APPROVE at the exact final head; no security blockers/new nits; score not assigned.

Four unavailable historical follow-up scheduling texts and removed older author logs remain explicitly unavailable. Retained original briefs, findings, responses and reports were supplied to the fresh reviewers; absent material was not invented. Neither review establishes current-head CI success or live provider qualification.

This PR does not enable automatic integration or request queue admission. Merge remains held for explicit operator authorization and all required exact-head gates.

Follow-up

  • S7: pre-existing deployment-specific host/service literals in detect-platform.sh and tools/git/README.md remain outside this change and need separate upstream cleanup.
  • The original milestone missing-title failure was reproduced with context qualifications (original base: outside-repository failure, inside synthetic checkout success). Its ordering defect is now corrected under explicit scope; final inside/outside usage controls pass. The remaining milestone usage ShellCheck SC2043 warning is independently reproduced on the original base and remains a separate cleanup item.
  • The local systemd RuntimeDirectory prerequisite failure remains outside this correction. Its inherited small hard-coded temporary-log destination is a storage/tooling follow-up. No full-chain pass or measured baseline execution failure is inferred from equal source blobs.

Refs #1841

## Summary Adds provider-neutral forge capability and PR status wrappers, complete review/comment enumeration with verified native replies and resolution where supported, declared merge methods, and separate inspection/control of automatic integration. Status aggregation pins the complete head/base identity, fails closed on missing evidence, and supports independent T3 wake and fallback notifier paths. Extends the repository declaration with an optional validated merge policy. Policy is loaded from the pinned trusted target revision. Absent policy means allowed methods = [squash] and default = squash. Explicit method requests must satisfy allowed methods and any transition-shape mapping, then intersect with the forge's read-only allow-method settings. Repository settings are never changed. Auto-merge and merge-queue enabling are off by default; inspect and cancel are separate operations. Promotes the full-send, full-stop, babysit-PR and council workflows with their git/watch dependencies into framework sources. Adds direct-write guard coverage, CI-wait compatibility, fixture tests and independent transport-parity expectations. Carries one operator-agnostic local fix: Python import isolation in the shared detection/auth helper. Other local tool drift was not copied wholesale. Authenticated adapter operations require an explicit actor and host-bound credential; mutations verify the actor. Shell credential workflows stay inside an untraced private subprocess. Anonymous descriptors keep authorization out of process arguments and named temporary files. Public and enterprise API routing is tested against complete expected URLs; nonzero curl exits fail closed, preserving unknown write outcomes. Original base: `924438016e2eb1c2ca352bfbf9107143b8b342bb`. Historical R3-reviewed head: `5cde8cd15b41fef7cc00ed85c06592254c5f1537`. Final reviewed head: `de99f7263f41247406953f71240dfb93fd19f44c`. The follow-up commits restore bounded authenticated HTTPS GET redirects, validate raw response bytes and URI components before following a redirect, disable default curl configuration and URL globbing, and refuse mutation redirects. Real loopback TLS controls exercise the actual queue/status entry paths. The three isolated CI consumers explicitly install the curl/OpenSSL prerequisites. Usage fixtures now isolate their owned Git origins, and milestone creation rejects missing or empty titles before provider work while title-free listing remains supported. ## Capability and exit-code contract - Exit 4 means unsupported API/method or a forge setting that disallows the method. Unsupported operations refuse before mutation; unknown required state cannot establish readiness. - Exit 3 identifies policy refusal (`error.kind=policy`), distinct from unsupported capabilities and authentication/unknown errors. - Native review reply/resolution is unsupported on the inspected older Gitea and Forgejo profiles. Supported providers enumerate all pages and verify mutation readback. - Inspected Gitea/Forgejo schemas lack scheduled-merge state readback, so scheduled integration and native queue operations remain unsupported rather than claiming verified state. No server upgrades are included. - GitHub rebase-merge and fast-forward-only are unsupported; other methods remain constrained by declaration and forge settings. GitHub automatic integration is conditional on verified capability and policy. Missing required evidence fails closed. ## Historical verification at the R3 head All provider tests use synthetic fixtures; no live forge writes occur in tests. The following retained results describe the historical R3 candidate, not a full rerun at the final head: - Each of 67 git shell suites run separately with unexpected network clients denied: 66 exit 0; the pre-existing milestone usage suite exits 1. The sweep driver exits 1 solely for that baseline failure. - Seven Python suites via `python3 -I`: all exit 0, 146 tests. Controls cover complete public/enterprise URLs, curl failures, trusted-base policy, retarget races, malformed state, actor refusal, all 13 shell credential consumers under tracing, and trace/argv mutants. - `test-pr-watch-parity.sh`: exit 0, including independent timeline expectations and wrong-verdict/missing-stop mutant controls. - `test-forge-schema.sh`: exit 0, including schema negative controls. - `test-validate-repo-json.sh`: exit 0, 101 assertions. - `bash -n` over all 107 git shell files: exit 0. - Full ShellCheck: exit 1 on both the previous candidate and revised tree; zero new diagnostics. - `git diff --check` against the original base: exit 0. - `pnpm preflight`, `pnpm typecheck`, `pnpm lint`, `pnpm format:check`: each exit 0; normal push hooks also passed. - Package skill and framework-manifest Vitest suites: exit 0, 97 tests across three files. Not verified: live forge writes, repo settings, live T3 watch/notifier delivery, CI. ## Final candidate evidence The final three commits are `d3a2b8c8704fa3dfc0f415f72333ff61788a521f` (raw header bytes), `ade6f3d26f39fee1a833b6c57edd176ada4fd1d4` (issue fixture boundaries), and `de99f7263f41247406953f71240dfb93fd19f44c` (milestone title ordering). Their combined delta from `1a569f734ccee1820f435372598dc6dad160f001` is eight files, 123 insertions and 22 deletions. Normal commit/push hooks passed; the remote branch was read back at the final full SHA with a clean worktree and no unpushed commits. - `python3 -I test-forge-cli.py`: exit 0, 25 tests. `test-forge-auth.py`: exit 0, nine tests. `test-forge-contract.py`: exit 0, 105 tests. Fixtures prove malformed raw headers refuse before follow-up, valid CRLF/OWS and escaped targets remain supported, and the earlier URI/glob controls remain discriminating. - The 18 relevant shell/Python invocations, 11 ordered sanitization commands, eight supplemental legacy regressions and the two executable redirect-entry Vitest cases each exit 0. The broad author matrix preceded the last allowed reason-phrase HTAB clarification; its focused positive was rerun successfully, and both independent reviewers subsequently ran the final CLI suite. - The corrected issue-comment, issue-close, issue-reopen and milestone usage suites each exit 0 inside an owned synthetic checkout and outside a repository. Earlier versions reproduced the four/two/two unexpected schema reads and the milestone title failure. Original-base milestone controls exit 0 inside the synthetic checkout and 1 outside; the corrected final suite exits 0 in both contexts. Title-free listing and quoted-title creation retain executable provider-dispatch and exact-payload controls. - Twenty original configured usage commands were run individually before the final remainder fixes: 17 passed and three failed. The three proved failures were then corrected under explicit scope and passed both context controls. Other remainder commands are explicitly recorded as not run; these results do not represent a full-chain pass. - `bash -n` on the five changed shell files: exit 0. Installed ShellCheck on the three issue usage fixtures: exit 0. Milestone usage ShellCheck: exit 1 for the existing SC2043 one-iteration loop; the original-base copy independently produces the same warning and exit 1. No assertion was removed to suppress it. - `git diff --check`: exit 0. Required normal push hooks ran `pnpm preflight`, `pnpm typecheck`, `pnpm lint` and `pnpm format:check`, all exit 0. Queue admission returned its documented degraded permission after an unavailable status response; this is not CI success. The full configured local `test:framework-shell` chain was attempted and exited 1 at its fifth command, `framework/systemd/user/test-fleet-units.sh`: the user RuntimeDirectory/manager prerequisite was unavailable. Codex and milestone commands were not reached in that full-chain run. No retry, skipped assertion, hidden tool path or runtime repair was used. Equal original/current systemd source establishes source equality only, not a measured baseline execution failure. Individual remainder checks are separate evidence and do not waive that failed/unavailable local full chain. ### CI snapshot and failed-history provenance Coordinator-verified terminal evidence: pipeline **3083 SUCCESS**, workflow **9453**, on exact `de99f7263f41247406953f71240dfb93fd19f44c`. All **11 child steps succeeded with exit 0**, including test and build. The saved full pipeline passed `tools/woodpecker/verify-terminal-green.py --expect-commit de99f7263f41247406953f71240dfb93fd19f44c` (exit 0); the failed pipeline 3082 control was refused (exit 1). No exception was used. This is actual parent-measured exact-head CI evidence, separate from the unavailable local full-chain run. No CI query, rerun or watch was performed for this metadata refresh. Both independent exact-head approvals remain in effect; no merge authority is granted by this update. | Actual pipeline | Head | Result and diagnosed scope | | --- | --- | --- | | 3076 | `5cde8cd15` | FAILED sanitization: missing test enumeration; corrected by `00d62c62ca3a5f17f11ff28e36a63220a4f97a7a`. | | 3077 | `00d62c62c` | FAILED sanitization: missing tool-index rows; corrected by `1effc9c143579da9e442ad98145c86498734e61a`. | | 3078 | `1effc9c14` | FAILED sanitization: BusyBox fixture launcher; corrected by `79682f703f15d6659acce447f4ec726a9e05425c`. | | 3079 | `79682f703` | FAILED test after earlier gates passed: stale redirect assertions plus an actual adapter redirect compatibility loss; corrected with executable coverage and bounded transport. | | 3080 | `85ea376dc` | FAILED test after earlier gates passed: local PR-diff fixture reached unset platform state; corrected by the owned synthetic provider seam at `1a569f734`. The earlier flattened diagnostic artifact resulted from concatenating decoded records without separators, not a proved CI backend defect. | | 3082 | `1a569f734` | FAILED test after earlier gates passed: issue-comment parse-acceptance fixtures inherited the outer checkout origin and recorded four schema GETs; corrected by the no-origin fixture boundary. Close/reopen and title-ordering findings came from separate local remainder checks, not additional CI observations. | No pipeline 3081 result was supplied. The local systemd limitation is separate from these measured CI failures. The explicit prerequisite additions are assessed in source; no full pinned-image pass or binary-inventory measurement is claimed. ## Independent review record Evidence retained on the review host. Historical approvals cover only their listed heads. Interruptions are not approvals; no missing score, report or history is invented. | Reviewer | Model | Round / scope | Head | Verdict | | --- | --- | --- | --- | --- | | rev-code-01 | not specified | 1 | `55d6cc18e` | REQUEST-CHANGES | | rev-security-01 | not specified | 1 | `55d6cc18e` | REQUEST-CHANGES | | rev-code-01 | not specified | 2 | `a193cf284` | REQUEST-CHANGES | | rev-security-01 | not specified | 2 | `a193cf284` | REQUEST-CHANGES | | rev-code-01 | gpt-6-astra | 3 | `5cde8cd15b41fef7cc00ed85c06592254c5f1537` | APPROVE; no blockers/nits | | rev-security-01 | gpt-6-astra | 3 | `5cde8cd15b41fef7cc00ed85c06592254c5f1537` | APPROVE; no blockers/nits | | rev-code-01 | gpt-6-luna | 4, enumeration/index | `1effc9c143579da9e442ad98145c86498734e61a` | Interrupted before later edits; no final verdict | | rev-security-01 | gpt-6.1-sol | 4, enumeration/index | `1effc9c143579da9e442ad98145c86498734e61a` | APPROVE locally; CI separately failed | | rev-code-01 | gpt-6-luna | 5, portable fixtures | `79682f703f15d6659acce447f4ec726a9e05425c` | Interrupted before later edits; no final verdict | | rev-security-01 | gpt-6.1-sol | 5, portable fixtures | `79682f703f15d6659acce447f4ec726a9e05425c` | APPROVE locally; CI separately failed | | rev-code-01 | gpt-6-luna | 6, redirects | `85ea376dc00bfb04d5a0e9c87d42c0dddac46fe6` | Interrupted before later edits; no final verdict | | rev-security-01 | gpt-6.1-sol | 6, redirects | `85ea376dc00bfb04d5a0e9c87d42c0dddac46fe6` | REQUEST-CHANGES: URI/glob amplification | | rev-code-01 | gpt-6-luna | Superseded URI follow-up | `b03f3a70bd495c221e363f472882329e96bd856a` | Interrupted when fixture scope was added; no final verdict; no security task launched for this superseded candidate | | rev-code-01 | gpt-6-luna | Combined URI/PR-diff fixture | `1a569f734ccee1820f435372598dc6dad160f001` | REQUEST-CHANGES: bare-CR header truncation | | rev-security-01 | gpt-6.1-sol | Combined URI/PR-diff fixture | `1a569f734ccee1820f435372598dc6dad160f001` | APPROVE; did not override code blocker | | rev-code-01 | gpt-6-luna | **Current combined candidate** | `de99f7263f41247406953f71240dfb93fd19f44c` | **APPROVE; no blockers/nits; no score assigned** | | rev-security-01 | gpt-6.1-sol | **Current combined candidate** | `de99f7263f41247406953f71240dfb93fd19f44c` | **APPROVE; no blockers/new nits; no score assigned** | Original R1/R2 responses retain trusted-target policy, explicit host-bound actor authentication, anonymous authorization descriptors, typed fail-closed state, complete head/base identity binding, independent parity/mutant controls, offline fixtures and transport-error refusal. R3 approved those corrections. Later URI/glob amplification was corrected at `b03f3a70bd495c221e363f472882329e96bd856a`; the later raw-header finding was corrected at `d3a2b8c8704fa3dfc0f415f72333ff61788a521f`. The final reviewers independently reproduced predecessor failures and corrected behavior. All changes use new commits without rewriting reviewed history. ### Current review task receipts Both tasks used provider `codex_daybreak_blue`, `reasoningEffort=max`, `serviceTier=default` (Standard), with requested and returned settings matched. Evidence retained on the review host; no internal paths or report contents are reproduced here. - **Code:** rev-code-01, `gpt-6-luna`, task ID `node:delegated-task:command%3Amcp%3A17719a77-a459-4ebb-a3c5-ff8a27b26e16%3Adelegate-task%3Aforge-review-code-raw-header-combined-de99f7263-20261104`. APPROVE at the exact final head; no blockers/nits; score not assigned. - **Security:** rev-security-01, `gpt-6.1-sol`, task ID `node:delegated-task:command%3Amcp%3A17719a77-a459-4ebb-a3c5-ff8a27b26e16%3Adelegate-task%3Aforge-review-security-raw-header-combined-de99f7263-20261104`. APPROVE at the exact final head; no security blockers/new nits; score not assigned. Four unavailable historical follow-up scheduling texts and removed older author logs remain explicitly unavailable. Retained original briefs, findings, responses and reports were supplied to the fresh reviewers; absent material was not invented. Neither review establishes current-head CI success or live provider qualification. This PR does not enable automatic integration or request queue admission. Merge remains held for explicit operator authorization and all required exact-head gates. ## Follow-up - S7: pre-existing deployment-specific host/service literals in `detect-platform.sh` and `tools/git/README.md` remain outside this change and need separate upstream cleanup. - The original milestone missing-title failure was reproduced with context qualifications (original base: outside-repository failure, inside synthetic checkout success). Its ordering defect is now corrected under explicit scope; final inside/outside usage controls pass. The remaining milestone usage ShellCheck SC2043 warning is independently reproduced on the original base and remains a separate cleanup item. - The local systemd RuntimeDirectory prerequisite failure remains outside this correction. Its inherited small hard-coded temporary-log destination is a storage/tooling follow-up. No full-chain pass or measured baseline execution failure is inferred from equal source blobs. Refs #1841
code-infra-01 added 13 commits 2026-11-04 22:21:46 +00:00
Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Add schema-v3 merge policy with squash-only defaults, protected scheduling, explicit capability failures, normalized CI checks and guarded review mutations. Qualify legacy fake providers with public metadata fixtures.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Keep credentials out of client argv, validate snapshot schema and ownership evidence, reject opaque GraphQL writes and isolate suite scratch roots. Retain existing SCP and SSH transport parsing.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Preserve provider-qualified conversation ownership on enterprise API prefixes and keep post-write malformed readback outcomes unknown. Make the absence assertion independent of scratch directory names.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Promote the four attributed workflows and their git/watch skill dependencies. Document capabilities, merge declarations, unsupported profiles, optional notification prerequisites and measured verification limits. Exclude generated Python bytecode from published git tools.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Resolve merge authority from the pinned target commit; reject full PR identity races and malformed conversation state. Require explicit host-bound actors, verify mutation identity and deliver auth through anonymous descriptors. Migrate legacy fixture credentials to explicit actors.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Exercise separate T3-wake and notifier wrapper paths against expected verdicts; reject wrong-ready and missing-full-stop mutants. Keep schema fixtures offline, operator patterns external and generated guard fixtures in scratch.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Keep metadata fixture arms offline, bind explicit/default-port fixture credentials and avoid new shell diagnostics in isolated credential arms.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Replace the inherited shared-token fixture dependency with a synthetic actor and host-bound token in a private fixture home. Keep the comment-before-close negative and positive controls intact.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Map review findings to fix commits and regressions; preserve offline sweep, targeted correction, mutant controls and baseline failures separately. Record deferred inherited literals and unverified live transport behavior.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Validate complete public and enterprise API URLs in transport fixtures. Treat nonzero curl exits as transport failures for every provider, preserving unknown mutation outcomes.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Disable tracing before caller-side token capture and retain privacy through the entire workflow. Use anonymous curl config descriptors, keep request stdin available, and cover all token-consuming wrappers with trace and argv controls. Preserve signal handling and stateful readback fixtures.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Resolve R2-C3 by documenting anonymous authentication consistently. Record complete URL, transport-error and wrapper trace controls, correct the earlier S6 coverage claim, and preserve the independent baseline failure.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 1 commit 2026-11-04 22:37:41 +00:00
Add eleven shell suites and seven Python suites to the existing framework test command. Keep all prior invocations and exclusions unchanged.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 1 commit 2026-11-04 23:03:57 +00:00
Add the nine missing git-tool index rows to the existing tables. Identify the schema fixture as an offline test helper and retain all existing tool entries.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 1 commit 2026-11-04 23:35:09 +00:00
Replace only the two env -S fixture prefixes with a POSIX shell trampoline. Keep Python isolation, quoted argv, fixture bodies and assertions unchanged.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 1 commit 2026-11-05 00:29:13 +00:00
Follow bounded same-origin HTTPS GET redirects without replaying mutations. Prove real wrapper entry paths and declare TLS fixture prerequisites in all consuming pipeline steps.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 1 commit 2026-11-05 00:56:47 +00:00
Validate raw and resolved URI components before transport and disable curl globbing. Preserve encoded literal targets and prove overlapping defenses with real loopback TLS controls.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 1 commit 2026-11-05 01:02:21 +00:00
Inject a synthetic Gitea detector only for owned local fixture origins. Keep actual fetch/ref/diff and fail-closed reviewer assertions intact.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
code-infra-01 added 3 commits 2026-11-05 01:43:05 +00:00
Capture transport bytes without universal newline conversion and validate CRLF framing and field bytes before parsing. Prove malformed redirects refuse before any follow-up while valid OWS and response blocks remain supported.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Use owned empty Git fixtures so parse-acceptance arms cannot inherit the enclosing checkout remote. Retain no-provider assertions and deliberate GitHub normalization coverage.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
Refuse missing or empty creation titles before provider detection while preserving title-free listing. Prove zero-provider usage failures and unchanged synthetic list and creation payload behavior.

Co-Authored-By: GPT-6.1-Sol (Codex) <[email protected]>
First-time contributor

Handoff, 2026-11-05 (Peggy, USC assistant seat). Jason has asked the homelab lead to take PR #1842 from here. Peggy is stepping off and will not push, review, merge, or watch it.

State at hand-off (read at 10:50 CST 2026-11-05):

  • Open, not draft. feat/forge-wrapper-gaps into next.
  • Head de99f7263f41247406953f71240dfb93fd19f44c. Woodpecker pipeline 3083 on that head: success.
  • Reviews at that exact head: independent code review (GPT-6-Luna) APPROVE, and independent security review (GPT-6.1-Sol) APPROVE, no blockers. The earlier three-round review (rev-code-01, rev-security-01) approved 5cde8cd15. The 9 commits after that are code changes (redirect constraints, raw header validation, milestone title ordering, fixture isolation, CI suite enumeration, docs), not CI config only. They were covered by the two de99 reviews above.
  • Merge: not done. Jason has not given a merge go to Peggy. The merge decision now sits with the homelab lead.
  • Known issue: T3 Code's PR watch on this PR fails to read from the host and drops after 8 tries, roughly every 16 minutes. The forge wrappers read it fine. Cause not diagnosed.
  • Cleanup still owed after merge or close: the web1 T3 worktree for feat-forge-wrapper-gaps and web1 scratch forge-plan.67p3tk.
**Handoff, 2026-11-05 (Peggy, USC assistant seat).** Jason has asked the homelab lead to take PR #1842 from here. Peggy is stepping off and will not push, review, merge, or watch it. State at hand-off (read at 10:50 CST 2026-11-05): - Open, not draft. `feat/forge-wrapper-gaps` into `next`. - Head `de99f7263f41247406953f71240dfb93fd19f44c`. Woodpecker pipeline 3083 on that head: success. - Reviews at that exact head: independent code review (GPT-6-Luna) APPROVE, and independent security review (GPT-6.1-Sol) APPROVE, no blockers. The earlier three-round review (rev-code-01, rev-security-01) approved `5cde8cd15`. The 9 commits after that are code changes (redirect constraints, raw header validation, milestone title ordering, fixture isolation, CI suite enumeration, docs), not CI config only. They were covered by the two de99 reviews above. - Merge: not done. Jason has not given a merge go to Peggy. The merge decision now sits with the homelab lead. - Known issue: T3 Code's PR watch on this PR fails to read from the host and drops after 8 tries, roughly every 16 minutes. The forge wrappers read it fine. Cause not diagnosed. - Cleanup still owed after merge or close: the web1 T3 worktree for `feat-forge-wrapper-gaps` and web1 scratch `forge-plan.67p3tk`.
Owner

Independent review of de99f7263 requested changes. The Mosaic lead is taking the single fix round and will push fast-forward commits onto this branch. Please hold pushes to feat/forge-wrapper-gaps until the fix round lands.

Findings being fixed:

  • Gitea review states: COMMENT or REQUEST_REVIEW after REQUEST_CHANGES clears the change request, and dismissed approvals still count. Result: false approval.
  • Gitea paging stops after the first page because Gitea caps pages at 50.
  • A successful Gitea merge (HTTP 200, empty body) reports failure, and the readback compares base.sha.
  • pr-merge.sh picks the merge path from the working tree's .mosaic/repo.json, not the pinned base.
  • A fork branch named next passes the promotion shape check.
  • README still describes the old credential fallbacks; the explicit-actor migration is undocumented.
Independent review of de99f7263 requested changes. The Mosaic lead is taking the single fix round and will push fast-forward commits onto this branch. Please hold pushes to feat/forge-wrapper-gaps until the fix round lands. Findings being fixed: - Gitea review states: COMMENT or REQUEST_REVIEW after REQUEST_CHANGES clears the change request, and dismissed approvals still count. Result: false approval. - Gitea paging stops after the first page because Gitea caps pages at 50. - A successful Gitea merge (HTTP 200, empty body) reports failure, and the readback compares base.sha. - pr-merge.sh picks the merge path from the working tree's .mosaic/repo.json, not the pinned base. - A fork branch named next passes the promotion shape check. - README still describes the old credential fallbacks; the explicit-actor migration is undocumented.
jason.woltje added 1 commit 2026-11-05 17:49:35 +00:00
jason.woltje added 1 commit 2026-11-05 18:23:35 +00:00
Owner

Lead review status at head ed5332b73: not merging. The PR stays open.

The narrow re-review of ed5332b73 confirms both earlier findings are fixed. A missing base commit now refuses with exit 3, and Gitea endpoints that ignore page finish in 2 requests instead of failing at the 100-page cap.

It found two new defects in that fix:

  1. Critical, wrong merge, packages/mosaic/framework/tools/git/pr-merge.sh:193. git ls-tree "$BASE_SHA" -- .mosaic/repo.json resolves the path relative to the current directory. Called from a subdirectory of the clone, an existing declaration reads as absent and the script falls back to legacy squash. In a dry run, a feature-to-main merge the declaration forbids and a next-to-main promotion it requires as a merge commit both exit 0; abab3a67e refused both. Fix: git ls-tree --full-tree.
  2. Important, fails closed, packages/mosaic/framework/tools/git/forge-api.py:319-321. The repeated-page check compares ids in order. Gitea returns a review's inline comments in map order, so a repeated page often arrives reordered and counts as a partial overlap (exit 6). This breaks status, watch-check, review-comments, merge --json and ci-wait on PRs with several inline comments. Fix: compare the pages as sorted ids, or as sorted canonical JSON when records have no id.

Minor, not blocking: an endpoint that honors limit but ignores page would return only its first page; a comment added between two page requests gives a one-off exit 6. The earlier follow-ups stand: official without a whitelist, -m merge combined with --dry-run/-d/--base-line under a merge policy, and the README sections the PR dropped.

Test evidence at ed5332b73, offline: the 7 Python suites pass; 42 of 43 changed shell suites pass. test-forge-schema.sh needs the repo's eslint/Ajv and must pass in CI.

Lead review status at head ed5332b73: not merging. The PR stays open. The narrow re-review of ed5332b73 confirms both earlier findings are fixed. A missing base commit now refuses with exit 3, and Gitea endpoints that ignore `page` finish in 2 requests instead of failing at the 100-page cap. It found two new defects in that fix: 1. Critical, wrong merge, `packages/mosaic/framework/tools/git/pr-merge.sh:193`. `git ls-tree "$BASE_SHA" -- .mosaic/repo.json` resolves the path relative to the current directory. Called from a subdirectory of the clone, an existing declaration reads as absent and the script falls back to legacy squash. In a dry run, a feature-to-main merge the declaration forbids and a next-to-main promotion it requires as a merge commit both exit 0; abab3a67e refused both. Fix: `git ls-tree --full-tree`. 2. Important, fails closed, `packages/mosaic/framework/tools/git/forge-api.py:319-321`. The repeated-page check compares ids in order. Gitea returns a review's inline comments in map order, so a repeated page often arrives reordered and counts as a partial overlap (exit 6). This breaks `status`, `watch-check`, `review-comments`, `merge --json` and `ci-wait` on PRs with several inline comments. Fix: compare the pages as sorted ids, or as sorted canonical JSON when records have no id. Minor, not blocking: an endpoint that honors `limit` but ignores `page` would return only its first page; a comment added between two page requests gives a one-off exit 6. The earlier follow-ups stand: `official` without a whitelist, `-m merge` combined with `--dry-run`/`-d`/`--base-line` under a merge policy, and the README sections the PR dropped. Test evidence at ed5332b73, offline: the 7 Python suites pass; 42 of 43 changed shell suites pass. `test-forge-schema.sh` needs the repo's eslint/Ajv and must pass in CI.
jason.woltje added 1 commit 2026-11-05 19:17:32 +00:00
jason.woltje added 1 commit 2026-11-05 19:45:02 +00:00
style(framework): format forge wrapper README table
ci/woodpecker/pr/ci Pipeline was successful
faf3f90e3a
jason.woltje left a comment
Owner

Approved by the Mosaic lead on Jason's instruction (2026-11-05). Basis: the final independent re-review approved 01662b281 with no Critical or Important findings. faf3f90e3 only re-aligns a README table with the repo's pinned prettier 3.8.1. CI passed on faf3f90e3. Open minor items are tracked in #1846.

Approved by the Mosaic lead on Jason's instruction (2026-11-05). Basis: the final independent re-review approved 01662b281 with no Critical or Important findings. faf3f90e3 only re-aligns a README table with the repo's pinned prettier 3.8.1. CI passed on faf3f90e3. Open minor items are tracked in #1846.
jason.woltje merged commit e4b82173bb into next 2026-11-05 20:13:57 +00:00
Sign in to join this conversation.
No Reviewers
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1842