84 lines
10 KiB
Markdown
84 lines
10 KiB
Markdown
# PR merge squash message field
|
||
|
||
- **Charter:** `/home/hermes/agent-work/CHARTER-PRMERGE-MESSAGE-FIELD.md`
|
||
- **Owner:** `be-coder-08`
|
||
- **Branch:** `fix/pr-merge-message-field`
|
||
- **Base:** remote `main` / local `origin/main` at `85d2108e4ed15c744ad3b87a5b629e7b2d39405a`
|
||
- **Estate:** HOMELAB tooling shared by HOMELAB and USC
|
||
|
||
## Objective
|
||
|
||
Add an optional, identity-checked Gitea squash message to `pr-merge.sh` so genuine multi-author PRs retain non-poster branch authors without weakening hardcoded squash behavior.
|
||
|
||
## Binding requirements
|
||
|
||
1. `Do` remains hardcoded to `squash`; no provider/repository default may select merge style.
|
||
2. A verified trailer uses a PR commit's linked `author.login` and that same commit's author email. No `/users/{login}` primary-email lookup occurs. Recorded rationale: this asks only what the provider can answer.
|
||
3. A commit with `author.login` null blocks before merge, prints both the null provider fact and commit email fact, and names the escalation principal.
|
||
4. The BLOCK arm must be observed firing; a normal single-author API payload remains exactly `{ "Do": "squash" }`.
|
||
5. Every provider mutation is read back from the provider; no real PR is merged during tests.
|
||
|
||
## Derived interface decisions
|
||
|
||
- Add `--co-author-trailers` rather than accepting arbitrary message text. The wrapper enumerates PR commits and constructs trailers, making an unchecked `Co-authored-by` line unexpressible.
|
||
- Require `--escalate-to PRINCIPAL` with `--co-author-trailers`, so the BLOCK diagnostic always names a principal rather than a generic role.
|
||
- Do not expose `MergeTitleField` separately. When trailers exist, set it from the provider PR title and set `MergeMessageField` only to construction-generated trailers. This preserves one provider source for the title and avoids an unrelated caller-controlled degree of freedom.
|
||
- Preserve first-commit order and emit one trailer per distinct non-poster `author.login`, using that first linked commit's own email.
|
||
|
||
## Plan
|
||
|
||
1. Version the currently deployed wrapper byte-for-byte under `infra/fleet/tools/git/pr-merge.sh`.
|
||
2. Pre-register verified, null-login BLOCK, unchanged single-author, and hardcoded-squash tests; observe RED before implementation.
|
||
3. Implement authenticated commit enumeration, construction-only trailers, message fields on REST, and force REST when trailers are requested.
|
||
4. Copy the exact final versioned bytes to the deployed wrapper; verify hashes match.
|
||
5. Run focused and baseline checks, static/security review, identity-bound commit, queue guard plus direct Woodpecker terminal enumeration, push, self-post PR, and provider poster read-back. Stop at push/PR; do not merge.
|
||
|
||
## Evidence
|
||
|
||
- RED against the byte-identical deployed baseline (`sha256 08a65e8584c5…`): rc 1 with eight named failures. The wrapper rejected `--co-author-trailers`; the null-login path emitted none of the required BLOCK facts/principal; and both verified/ordinary API paths failed the stdin-config credential assertion (ordinary path exposed the fixture token through curl argv). Log: `/home/hermes/agent-work/be-coder-08/evidence/prmerge-message-field-red.log`.
|
||
- GREEN after implementation: verified linked multi-author payload, null-login BLOCK, required named principal, unchanged ordinary `{ "Do": "squash" }` payload, hard non-squash refusal, stdin-config token transport, and absence of `/users` lookup all passed. Log: `/home/hermes/agent-work/be-coder-08/evidence/prmerge-message-field-green.log`.
|
||
- Provider capability probe against `git.mosaicstack.dev`: authenticated `be-coder-08` POST to deliberately nonexistent PR `2147483647` with both message fields returned JSON HTTP 404; the unauthenticated same request returned JSON HTTP 401 (not the charter's predicted 403). The authenticated-vs-unauthenticated differential proves write authorization resolved while no mergeable subject existed. `tl-mosaic` ruled the literal non-load-bearing: preserve the observed 404/401 pair and do not manufacture a 403 case. No cause was inferred and no real PR was targeted.
|
||
- Provider-generated trailer behavior is not treated as exclusive or absent. The wrapper's VERIFIED/BLOCK decision binds each requested non-poster trailer to commit `author.login` plus that commit's email; it does not assume `MergeMessageField` is the squash's only trailer source. The poster is omitted from the constructed list because the resulting squash author already records the poster; any additional provider-generated trailer is outside this change's unmeasured mechanism.
|
||
- An early candidate SHA-256 `5de32876990e4f26920448cb3220cc7f1146d558b4dd2bc1ee1a2abee2f2cbe6` passed the initial harness, then author-side review found credential-fallback and argv-exposure defects. The live deployed wrapper was atomically restored to baseline SHA-256 `08a65e8584c52c6d41ea1c686f8b95585c21e4b37320a2447eba09359a0e02c1`; the remediated candidate remains only in the worktree.
|
||
|
||
## Remediation and current review state
|
||
|
||
1. Token and Basic Auth now use stdin curl configuration, not argv. PR title, contributor email, and the JSON payload also remain out of child argv.
|
||
2. Each credential attempt binds commit inspection and merge. A token failure during either inspection or mutation causes Basic fallback to repeat inspection before mutation; the payload pins the inspected `head_commit_id`.
|
||
3. Focused tests cover both fallback seams, metadata/credential argv absence, null-login BLOCK, explicit squash, unchanged ordinary payload, and retained log-safe provider diagnostics.
|
||
4. Codex review rounds 3–5 requested retained provider error text, log-safe provider diagnostics, fail-closed credential fallback, stable value-option parsing, and PR-title trailer-injection prevention. These are remediated with regression assertions. A post-remediation independent review is still required.
|
||
5. **Accepted linkage limitation:** `author.login` resolution proves that the commit address maps to a registered provider account. It does not prove that the named principal authored the commit because Git author metadata is self-asserted. This gate checks attribution linkage, not authorship; commit signing is out of scope and currently unadopted. Coordinators explicitly ruled that this does not add a third state.
|
||
6. Codex's sandbox could not execute the harness because its checkout was read-only; that environmental limitation is recorded separately from host-side test results.
|
||
|
||
## Disposable provider fixture still required
|
||
|
||
- Use a retained scratch repository only, with two branch authors and `author != committer` on at least one commit.
|
||
- Arm A supplies a message-field trailer and classifies the landed squash object as `APPENDS`, `OVERWRITES`, or `REPLACES`; its absence control must also be demonstrated.
|
||
- Arm B includes a registered foreign branch trailer and classifies it as `SURVIVES` or `DROPPED`; verify that identity through an existing commit whose `author.login` resolves. Demonstrate an absence control.
|
||
- Parse landed trailers key-agnostically with `^[A-Za-z-]+-[Bb]y:` and record generated poster pair presence/absence plus resulting poster attribution.
|
||
- Record `/users/<login>` status and raw email only as non-gating estate telemetry. Never read `active`, `visibility`, or any profile field as an identity gate.
|
||
- Use distinct principals: poster `be-coder-08`, merger `Mos`, Arm A `be-coder-07`, and Arm B `be-coder-06`. Capture every trailer-shaped line verbatim and in order. Zero trailer lines means the generator did not fire and the run is `VOID`, not evidence that either arm dropped.
|
||
- Report the same read-back evidence to `mos-claude` on socket `default` and `tl-mosaic` on socket `mosaic-fleet`. Stop on `OVERWRITES`, `REPLACES`, or any poster-attribution regression.
|
||
|
||
## Fixture preflight
|
||
|
||
- Retained public repository: `mosaicstack/prmerge-trailer-fixture`; PR `#1`, posted by `be-coder-08` and reserved for merge by `Mos`.
|
||
- Existing `mosaicstack/stack` commits resolve `be-coder-07` and `be-coder-06` through `author.login`; exact addresses are `[email protected]` and `[email protected]`.
|
||
- Non-gating HOMELAB telemetry for authenticated reader `be-coder-08`: `/api/v1/users/be-coder-06` returned HTTP 200 with raw `email` value `[email protected]`.
|
||
- Provider preflight showed PR commit enumeration is newest-first. A new RED test proved that deriving `head_commit_id` from the final array element selected the wrong commit. The candidate now reads `.head.sha` from the authenticated PR endpoint before enumeration, verifies it appears in the commit set, and atomically pins that SHA in the explicit squash payload. RED: `/home/hermes/agent-work/be-coder-08/evidence/prmerge-head-order-red.log`.
|
||
- Fixture PR head `f6ba6e5105031fa21f5ff7bd8e4379d99c16e1de` has `author.login=be-coder-07`, `committer.login=be-coder-08`, and branch-message trailer `Co-authored-by: be-coder-06 <[email protected]>`.
|
||
|
||
## Fixture result
|
||
|
||
- `Mos` merged retained fixture PR `#1` through staged candidate SHA-256 `60e779a85fd13b729d859ea7c986d1e9b1641b97991611329226c1b3113ffb6e`; resulting squash commit: `3f550715d9bc716426fd355a65fe997b3a90fa7d` with one parent.
|
||
- Provider read-back: poster/commit author `be-coder-08`, committer/merger `Mos`. The run is non-void.
|
||
- Trailer-shaped lines, verbatim and in order:
|
||
1. `Co-authored-by: be-coder-07 <[email protected]>`
|
||
2. `Co-authored-by: be-coder-08 <[email protected]>`
|
||
- Arm A supplied field value (`be-coder-07`) landed. Arm B branch trailer (`be-coder-06`) dropped. Both fabricated absence controls remained absent. No `Co-committed-by:` line landed.
|
||
- The candidate payload construction explicitly excludes the poster and supplied only the Arm A `be-coder-07` line. Therefore the landed poster line was provider-generated, not candidate-composed. The raw result supports `FIELD LANDS`, `BRANCH DROPS`, and `POSTER GENERATED`; it does not support a claim that candidate code supplied the poster. Evidence: `/home/hermes/agent-work/be-coder-08/evidence/prmerge-fixture-readback.log` and the retained provider object.
|
||
|
||
## Current hold point
|
||
|
||
No delivery branch push or PR exists. The deployed wrapper remains on its original baseline bytes. Implementation and disposable end-to-end read-back are green; final review disposition, commit/rebase, and delivery gates remain.
|