From 5aa882b56094ff19c9afb47a50fa1a9478e789cd Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 07:23:22 -0400 Subject: [PATCH 01/10] tools: PR-preservation minimal archive tool + Otto-207 session backfill (10 PRs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otto-207: maintainer "are we saving these yet gitnative and have we backfilled them yet?" Honest answer was NO — the PR-preservation BACKLOG row (Otto-150..154, PR #335 in queue) specifies the discipline but never shipped the capture tooling. This PR ships the minimal viable implementation + backfills 10 PRs from this session. New tool: - tools/pr-preservation/archive-pr.sh — one-shot bash script that fetches a PR's review threads, reviews, and comments via `gh api graphql` and writes them to docs/pr-discussions/PR--.md with YAML frontmatter (pr_number / title / author / state / dates / refs / archived_at / archive_tool). - tools/pr-preservation/README.md — scope (Phase 0 minimal vs Phase 1-4 longer plan), usage, output schema, backfill status, dependencies (bash + python3 + gh; no external packages), cross-references to Otto-171 / Otto-204 / Otto-204c / PR #335. Backfill (10 PRs archived this tick): - PR #354 backlog-split Phase 1a - PR #352 Server Meshing + SpacetimeDB research - PR #336 KSK naming definition doc - PR #342 calibration-harness Stage-2 design (merged) - PR #344 Amara 19th ferry absorb (merged) - PR #346 DST compliance criteria (merged) - PR #350 Frontier rename pass-2 (merged) - PR #353 BACKLOG split Phase 0 design (merged) - PR #355 Codex first peer-agent deep-review absorb (merged) - PR #356 PR-resolve-loop skill row (merged) Total: 72 review threads + 40 reviews + 6 general comments captured across ~97KB of archive markdown. Long-term plan deliberately kept in BACKLOG row (Otto-150 ..154 / PR #335 queue elevation) rather than expanded in this commit's docs. Phase 0 shipping now; Phase 1 GHA workflow + Phase 2 historical backfill + Phase 3 reconciliation + Phase 4 redaction layer remain queued tickets. Per maintainer directive "make sure you backlog then to a proper long term solution" — the phased plan is already in PR #335 and covers the remaining work. Discipline applied: active-management on the preservation gap itself. Previous tick's "ship and pray" pattern is the exact failure mode this tool begins to close (operator- initiated archive instead of silent reliance on GitHub- side conversation storage). Composes with Otto-204c livelock-diagnosis memory + Otto-204 PR-resolve-loop skill (this script is step 4 of that cycle's conversation-preservation hook). Co-Authored-By: Claude Opus 4.7 --- ...ition-doc-canonical-expansion-locked-ot.md | 250 ++++++++++ ...ess-stage-2-design-amara-18th-ferry-b-f.md | 109 +++++ ...rb-dst-audit-5-5-corrections-10-tracked.md | 180 +++++++ ...riteria-dst-held-foundationdb-grade-bar.md | 129 +++++ ...pass-2-hindu-ff7-egyptian-greek-norse-s.md | 134 ++++++ ...er-meshing-spacetimedb-deep-research-ga.md | 223 +++++++++ ...t-design-phase-0-aaron-otto-181-3rd-ask.md | 126 +++++ ...hase-1a-generator-schema-example-row-aa.md | 448 ++++++++++++++++++ ...mpleted-peer-agent-deep-review-absorb-4.md | 162 +++++++ ...esolve-loop-skill-close-the-pr-cycle-au.md | 120 +++++ tools/pr-preservation/README.md | 116 +++++ tools/pr-preservation/archive-pr.sh | 198 ++++++++ 12 files changed, 2195 insertions(+) create mode 100644 docs/pr-discussions/PR-0336-docs-ksk-naming-definition-doc-canonical-expansion-locked-ot.md create mode 100644 docs/pr-discussions/PR-0342-docs-calibration-harness-stage-2-design-amara-18th-ferry-b-f.md create mode 100644 docs/pr-discussions/PR-0344-ferry-amara-19th-absorb-dst-audit-5-5-corrections-10-tracked.md create mode 100644 docs/pr-discussions/PR-0346-docs-dst-compliance-criteria-dst-held-foundationdb-grade-bar.md create mode 100644 docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md create mode 100644 docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md create mode 100644 docs/pr-discussions/PR-0353-docs-backlog-md-split-design-phase-0-aaron-otto-181-3rd-ask.md create mode 100644 docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md create mode 100644 docs/pr-discussions/PR-0355-ferry-codex-first-completed-peer-agent-deep-review-absorb-4.md create mode 100644 docs/pr-discussions/PR-0356-backlog-otto-204-pr-resolve-loop-skill-close-the-pr-cycle-au.md create mode 100644 tools/pr-preservation/README.md create mode 100755 tools/pr-preservation/archive-pr.sh diff --git a/docs/pr-discussions/PR-0336-docs-ksk-naming-definition-doc-canonical-expansion-locked-ot.md b/docs/pr-discussions/PR-0336-docs-ksk-naming-definition-doc-canonical-expansion-locked-ot.md new file mode 100644 index 00000000..cfd56d95 --- /dev/null +++ b/docs/pr-discussions/PR-0336-docs-ksk-naming-definition-doc-canonical-expansion-locked-ot.md @@ -0,0 +1,250 @@ +--- +pr_number: 336 +title: "docs: KSK naming definition doc \u2014 canonical expansion locked (Otto-142..145)" +author: AceHack +state: OPEN +created_at: 2026-04-24T08:38:34Z +head_ref: docs/ksk-naming-definition-otto-142-145 +base_ref: main +archived_at: 2026-04-24T11:22:13Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #336: docs: KSK naming definition doc — canonical expansion locked (Otto-142..145) + +## PR description + +## Summary + +Authoritative definition of **KSK = Kinetic Safeguard Kernel** at `docs/definitions/KSK.md`, plus a pointer entry in `docs/GLOSSARY.md`. + +Resolves Amara 16th-ferry §4 (KSK naming stabilization) + 17th-ferry correction #7. Authority: Aaron Otto-140 (rewrite approved; Max-coordination gate lifted) and Otto-142..145 (canonical expansion self-corrected from transient Otto-141 "SDK" typo to the Kernel form matching Amara's original). + +## Key distinction + +"Kernel" here is **safety-kernel / security-kernel** sense (Anderson 1972 reference-monitor, Saltzer-Schroeder complete-mediation, aviation safety-kernel). **NOT** an OS-kernel (not ring 0, not Linux / Windows / BSD kernel-mode). The doc's lead paragraph makes this disambiguation up-front because readers coming from OS-kernel contexts would otherwise misinterpret. + +## Doc content + +- Canonical definition + mechanism set (k1/k2/k3 capability tiers, revocable budgets, multi-party consent quorum, BLAKE3-hashed signed receipts, traffic-light outputs, optional anchoring) +- "Inspired by..." (DNSSEC KSK, DNSCrypt, security kernels, aviation safety kernels, microkernel OS) +- "NOT identical to..." (OS kernel, DNSSEC KSK, generic root-of-trust, blockchain, policy engine, authentication system) +- Attribution + provenance (Aaron + Amara concept owners; Max initial-starting-point in LFG/lucent-ksk) +- Zeta / Aurora / lucent-ksk relationship triangle +- Cross-references to 5 prior courier ferries (5th / 7th / 12th / 14th / 16th / 17th) + +## Test plan + +- [x] `docs/definitions/` created as new directory (first entry). +- [x] Glossary pointer added under "## Meta-algorithms and factory-native coinages" section. +- [ ] Markdownlint clean on CI. +- [ ] Future KSK graduations update this doc with `src/Core/` cross-references. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T08:42:26Z) + +## Pull request overview + +Adds a canonical documentation entry defining **KSK = Kinetic Safeguard Kernel**, and wires it into the repo glossary so the expansion and intended “kernel” meaning are stable and discoverable. + +**Changes:** +- Introduces `docs/definitions/KSK.md` as the authoritative KSK definition and disambiguation doc. +- Adds a new `KSK (Kinetic Safeguard Kernel)` glossary entry pointing to the definition doc. + +### Reviewed changes + +Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments. + +| File | Description | +| ---- | ----------- | +| docs/definitions/KSK.md | New definition doc describing the KSK concept, disambiguations, and cross-references. | +| docs/GLOSSARY.md | Adds a glossary entry that summarizes KSK and links to the authoritative definition doc. | + +### COMMENTED — @AceHack (2026-04-24T11:18:56Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:18:58Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:19:00Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:19:02Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:19:04Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:19:05Z) + +_(no body)_ + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:22:11Z) + +## Pull request overview + +Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments. + +## Review threads + +### Thread 1: docs/GLOSSARY.md:819 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T08:42:24Z): + +This glossary entry references `LFG/lucent-ksk` as an in-repo path, but there is no `LFG/` directory here. Please clarify that this is an external repo (or replace with a valid in-repo reference) to avoid dead navigation for readers. +```suggestion +starting-point code: Max in the external repo +`LFG/lucent-ksk`. Canonical expansion ratified 2026-04-24 +(Aaron Otto-142..145, after earlier Amara 5th / 7th / 16th +ferry phrasing). Authoritative source: `docs/definitions/KSK.md`. +``` + +**@AceHack** (2026-04-24T11:18:56Z): + +Fixed in 364be82. Rewrote the GLOSSARY KSK entry's initial-starting-point reference to explicitly mark `Lucent-Financial-Group/lucent-ksk` as an external repository with URL, clarifying "not a local LFG/ directory in this repo." No more dead navigation. + +### Thread 2: docs/definitions/KSK.md:158 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T08:42:24Z): + +Operational standing rules forbid direct contributor/agent name attribution in docs; this section uses named individuals. Please rewrite to use role references (e.g., “human maintainer”, “external collaborator”) and move any required provenance/credit to allowed locations (e.g., `memory/persona/**` or `docs/BACKLOG.md`). Also update the new glossary entry to match the same no-names convention. +```suggestion +- The **human maintainer** and an **external AI + collaborator** are the concept owners of KSK-as-safety- + kernel for Zeta. The k1/k2/k3 + revocable-budget + + multi-party-consent + signed-receipt + traffic-light + + optional-anchoring design was articulated across courier + ferries documented under `docs/aurora/2026-04-23-*` and + `docs/aurora/2026-04-24-*`. If named provenance is needed, + keep it in allowed locations such as `memory/persona/**` + or `docs/BACKLOG.md`, not in this definition file. +- An **external collaborator** committed the **initial + starting point** of the KSK code under `LFG/lucent-ksk` + under the human maintainer's direction. That starting + point remains fully rewritable; it is provenance for the + initial substrate, not a locked scope. See + `memory/feedback_ksk_naming_unblocked_aaron_ + directed_rewrite_authority_max_initial_starting_point_ + 2026_04_24.md`. +- **Naming stabilization** was raised in the relevant + courier-ferry review and resolved in the same review flow + after a brief transient "SDK" typo. Canonical expansion: + Kinetic Safeguard **Kernel**. +``` + +**@AceHack** (2026-04-24T11:18:58Z): + +Fixed in 364be82. Attribution section rewritten to use role references ("the human maintainer", "an external AI collaborator", "a trusted external contributor") instead of named individuals. Direct names preserved only in audit-trail surfaces per factory name-attribution policy (commit messages, tick-history, session memory). + +### Thread 3: docs/definitions/KSK.md:153 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T08:42:25Z): + +This doc links to `memory/feedback_ksk_naming_unblocked_aaron_directed_rewrite_authority_max_initial_starting_point_2026_04_24.md`, but that file path doesn’t exist in the repository. Please either add the referenced file, or change the reference to an existing in-repo source (or remove it if it’s intentionally out-of-repo). +```suggestion + scope. +``` + +**@AceHack** (2026-04-24T11:19:00Z): + +Fixed in 364be82. Removed the memory/feedback_ksk_naming_unblocked_* path reference entirely; the factual substance (rewrite-authority context) was restated in role-based prose without creating a broken-link-from-outside dependency. Same pattern applied to #354 thread 59XfPF earlier. + +### Thread 4: docs/definitions/KSK.md:180 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T08:42:25Z): + +`LFG/lucent-ksk` is referenced as if it were an in-repo path, but there is no `LFG/` directory in this repository. Clarify that this is an external repository (and link to it if appropriate), or adjust the wording so readers don’t interpret it as a local path they can navigate to. +```suggestion +- **LFG/lucent-ksk** — the external `LFG/lucent-ksk` + repository, where Max's initial KSK starting-point code + lives. It may evolve independently; Zeta re-implements + KSK as an in-substrate module where that integration is + tighter. Cross-repo decisions follow Otto-90 + (Aaron+Max-not-coordination-gates) and Otto-140 (KSK + rewrite authority resides with Aaron + Amara + Otto). +``` + +**@AceHack** (2026-04-24T11:19:02Z): + +Fixed in 364be82. Same fix as GLOSSARY thread 59VynF applied here: `Lucent-Financial-Group/lucent-ksk` marked as external repo with URL, plus explicit "separate repository" framing in the Relationship-to-Zeta-Aurora-lucent-ksk section. + +### Thread 5: docs/definitions/KSK.md:207 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T08:42:25Z): + +The cross-reference list includes several `docs/aurora/...` paths/patterns that don’t exist in the current tree (e.g., no `2026-04-23-amara-5th-ferry-*`, no `2026-04-24-...-12th/14th/...`, and no `...-16th-ferry.md`). Also, glob patterns in markdown aren’t navigable links. Please update these bullets to point at concrete existing files (or to `docs/aurora/README.md` entries) so the references remain clickable and accurate. +```suggestion +- `docs/aurora/README.md` — see the Aurora index entry for the + 5th ferry: Aurora + KSK integration; first articulation of + k1/k2/k3, revocable budgets, multi-party consent, + signed receipts, traffic-light, and optional anchoring. +- `docs/aurora/README.md` — see the Aurora index entry for the + 7th ferry: Aurora-aligned KSK design research; formal + authorization rule `Authorize(a, t) = ¬RedLine ∧ + BudgetActive ∧ ScopeAllowed ∧ QuorumSatisfied ∧ OraclePass`, + BLAKE3 receipt hashing, and the KSK-as-Zeta-module proposal. +- `docs/aurora/README.md` — see the Aurora index entry for the + 12th ferry: executive summary, KSK, integrity detector, and + integration plan. +- `docs/aurora/README.md` — see the Aurora index entry for the + 14th ferry: expanded cartel-detection context for KSK's + integrity-layer companion. +- `docs/aurora/README.md` — see the Aurora index entry for the + 16th ferry: KSK naming ambiguity identified and + stabilization recommended. +- `docs/aurora/README.md` — see the Aurora index entry for the + 17th ferry: the correction sequence that led to this doc. +``` + +**@AceHack** (2026-04-24T11:19:04Z): + +Fixed in 364be82. Cross-reference list rewritten to enumerate only verified in-repo references (6th / 7th / 17th / 19th ferries that actually exist on origin/main). The 5th / 12th / 14th / 16th ferries that were referenced don't exist as standalone `docs/aurora/` files; content archived in ROUND-HISTORY + session memory. Glob patterns removed (markdown doesn't navigate them). Explicit note added about when the list would be updated if those earlier ferries graduate to standalone docs. + +### Thread 6: docs/definitions/KSK.md:191 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T08:42:25Z): + +This continuation line starts with a literal `+`, which markdownlint treats as a new nested list marker (and the repo convention explicitly forbids `+` at the start of a wrapped continuation line). Please reword so the line continues without a leading `+` (e.g., “and traffic-light …” or move the `+` to the previous line’s end). +```suggestion + and traffic-light + optional-anchoring. +``` + +**@AceHack** (2026-04-24T11:19:05Z): + +Fixed in 364be82. The literal `+` continuation line was in the old Attribution section; the Attribution rewrite (thread 59Vyns) replaced that section with role-based prose that doesn't use `+` continuations. Markdownlint verified clean locally (`markdownlint-cli2 docs/definitions/KSK.md` exit 0). + +### Thread 7: docs/definitions/KSK.md:219 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:22:10Z): + +P1: This claims the 5th/12th/14th/16th ferry content is “archived in `docs/ROUND-HISTORY.md` tick rows”, but `docs/ROUND-HISTORY.md` doesn’t contain tick rows or any “ferry” references (it reads as round-level narrative). Please either point to the actual in-repo artifact(s) that preserve those ferry contents, or remove the `ROUND-HISTORY.md`/tick-rows claim so readers aren’t sent to a dead end. +```suggestion +16th) have not landed as separate `docs/aurora/` files yet. +When those ferries graduate to their own `docs/aurora/` +files, this cross-reference list updates at that time. +``` + +### Thread 8: docs/definitions/KSK.md:151 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:22:11Z): + +P1: The parenthetical here says the doc’s “ferry references below” are 5th/7th/12th/14th/16th/17th, but the Cross-references section actually lists 6th, 7th, 17th, and 19th as the verified in-repo sources. Please update this list (or reword the sentence) so it matches the references that follow and doesn’t imply missing/incorrect citations. +```suggestion + 2026-04-23 and 2026-04-24 — the 6th / 7th / 17th / 19th + ferry references below are the verified in-repo topical + labels for those sources). +``` + +## General comments + +### @chatgpt-codex-connector (2026-04-24T08:38:40Z) + +You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage). diff --git a/docs/pr-discussions/PR-0342-docs-calibration-harness-stage-2-design-amara-18th-ferry-b-f.md b/docs/pr-discussions/PR-0342-docs-calibration-harness-stage-2-design-amara-18th-ferry-b-f.md new file mode 100644 index 00000000..d81627fc --- /dev/null +++ b/docs/pr-discussions/PR-0342-docs-calibration-harness-stage-2-design-amara-18th-ferry-b-f.md @@ -0,0 +1,109 @@ +--- +pr_number: 342 +title: "docs: calibration-harness Stage-2 design \u2014 Amara 18th-ferry \u00a7B/\u00a7F + corrections #2/#7/#9" +author: AceHack +state: MERGED +created_at: 2026-04-24T09:07:02Z +merged_at: 2026-04-24T09:08:53Z +closed_at: 2026-04-24T09:08:53Z +head_ref: docs/calibration-harness-stage2-design +base_ref: main +archived_at: 2026-04-24T11:22:14Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #342: docs: calibration-harness Stage-2 design — Amara 18th-ferry §B/§F + corrections #2/#7/#9 + +## PR description + +## Summary + +Research-grade design doc for Stage-2 of Amara's corrected promotion ladder. Specifies the next-rung deliverable (calibration harness) so when implementation starts, conventions are pre-committed. + +## Key design decisions + +- **Placement**: `src/Experimental/CartelLab/` (not `src/Core/` — that's Stage 4 per Amara ladder). +- **MetricVector**: 7 fields including **PLV magnitude AND offset as separate fields** (correction #6 — addresses PR #340's shipped split). +- **Wilson-interval reporting contract**: every statistical claim carries `{successes, trials, lowerBound, upperBound}` — no more "~95% CI ±5%" handwave (correction #2). +- **Robust z-score Hybrid mode**: percentile-rank fallback when MAD < epsilon (correction #7). +- **Explicit artifact layout**: 5 files + manifest under `artifacts/coordination-risk/` (correction #9). +- **`INullModelGenerator` + `IAttackInjector` interfaces** with `Preserves` / `Avoids` / `ExpectedSignals` machine-readable annotations. + +## Scope + +- Doc-only. No code, no tests, no workflow. +- **Does NOT touch BACKLOG.md tail** — avoids the positional-append conflict pattern that cost #334 → #341 re-file earlier this session. + +## 18th-ferry operationalization status + +| # | Correction | Status | +|---|---|---| +| 1,10 | Test classification policy | Shipped (#339) | +| 2 | Wilson intervals | **Design specified (this doc); impl waits Stage 2.a** | +| 4 | Exclusivity primitive | Shipped (#331) | +| 5 | Modularity relational | Shipped (#324) | +| 6 | PLV phase-offset | Shipped (#340) | +| 7 | MAD=0 fallback | **Design specified (this doc); impl waits Stage 2.a** | +| 9 | Artifact layout | **Design specified (this doc)** | +| 3 | CoordinationRiskScore rename | Already canonical in code | +| 8 | Stronger sources | Reporting discipline | + +7 of 10 18th-ferry corrections now have either shipped code or committed design. + +## Test plan + +- [x] Markdownlint clean locally. +- [x] Single new file; no surface impact. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T09:10:50Z) + +## Pull request overview + +Adds a research-grade design document specifying the planned Stage-2 “calibration harness” for coordination-risk / cartel detection work, with pre-committed conventions for metrics, confidence-interval reporting, robust z-score fallback, and artifact outputs. + +**Changes:** +- Introduces a Stage-2 harness design covering module placement (`src/Experimental/CartelLab/`), core types/interfaces, and invocation contract. +- Specifies statistical reporting discipline (Wilson intervals) and robust z-score modes (including MAD=0 fallback via percentile rank / hybrid). +- Defines a fixed artifact output schema under `artifacts/coordination-risk/` for downstream calibration/ROC/PR tooling. + +## Review threads + +### Thread 1: docs/research/calibration-harness-stage2-design.md:447 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:10:48Z): + +P1: This cross-reference points to `docs/definitions/KSK.md`, but `docs/definitions/` does not currently exist in the repo (and the file appears to be a planned/backlog item rather than present). Please either link to the current canonical location (if it exists elsewhere), or mark this as a planned doc and reference the BACKLOG entry until the file lands. + +### Thread 2: docs/research/calibration-harness-stage2-design.md:493 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:10:49Z): + +P1: The referenced Aurora file path `docs/aurora/2026-04-24-amara-calibration-ci-hardening-deep-research-plus-5-5-corrections-18th-ferry.md` does not exist in `docs/aurora/` in the current tree. Please update this link to an existing source doc, or explicitly label it as pending/not-yet-landed (and avoid presenting it as a resolvable path until it is added). + +### Thread 3: docs/research/calibration-harness-stage2-design.md:9 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:10:49Z): + +P1: This file introduces contributor/agent name attribution (e.g., "Origin: Amara", multiple later "Amara" / "Otto" mentions). Repo operational standing rule is to use role references in code/docs/skills, keeping names only under `memory/persona/**` (and optionally `docs/BACKLOG.md`). Please rewrite these attributions to role-based phrasing (e.g., "the research agent", "the cadence tracker") and keep the doc’s meaning intact. (Ref: `docs/AGENT-BEST-PRACTICES.md` Operational standing rules.) + +### Thread 4: docs/research/calibration-harness-stage2-design.md:49 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:10:50Z): + +P1: The diagram labels `artifacts/coordination-risk/` as “.gitignored”, but the repo’s `.gitignore` does not currently ignore this path. Either adjust the wording to “should be gitignored” (and note it will land with the Stage-2 implementation) or add the appropriate ignore entry when the artifacts directory is introduced. + +### Thread 5: docs/research/calibration-harness-stage2-design.md:375 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:10:50Z): + +P2: Artifact count is inconsistent: §7 defines 6 outputs (`run-manifest.json`, `seed-results.csv`, `calibration-summary.json`, `roc-pr.json`, `metric-distributions.csv`, `failing-seeds.txt`), but this line says “all five artifact files”. Please make the count consistent (either update to six, or adjust the earlier list). + +## General comments + +### @chatgpt-codex-connector (2026-04-24T09:07:07Z) + +You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage). diff --git a/docs/pr-discussions/PR-0344-ferry-amara-19th-absorb-dst-audit-5-5-corrections-10-tracked.md b/docs/pr-discussions/PR-0344-ferry-amara-19th-absorb-dst-audit-5-5-corrections-10-tracked.md new file mode 100644 index 00000000..0b162c27 --- /dev/null +++ b/docs/pr-discussions/PR-0344-ferry-amara-19th-absorb-dst-audit-5-5-corrections-10-tracked.md @@ -0,0 +1,180 @@ +--- +pr_number: 344 +title: "ferry: Amara 19th absorb \u2014 DST Audit + 5.5 Corrections (10 tracked; 4 aligned with shipped; 7 queued)" +author: AceHack +state: MERGED +created_at: 2026-04-24T09:24:54Z +merged_at: 2026-04-24T09:26:25Z +closed_at: 2026-04-24T09:26:25Z +head_ref: ferry/amara-19th-dst-audit-absorb +base_ref: main +archived_at: 2026-04-24T11:22:14Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #344: ferry: Amara 19th absorb — DST Audit + 5.5 Corrections (10 tracked; 4 aligned with shipped; 7 queued) + +## PR description + +## Summary + +Dedicated absorb of Amara's 19th courier ferry per CC-002 close-on-existing discipline. 12 sections of deep-research DST audit + 7-correction pass. + +## 4 of 12 sections already align with shipped substrate + +- §6 test classification → PR #339 +- §7 artifact layout → PR #342 design doc +- §8 Cartel-Lab stage discipline → PRs #330/#337/#342 +- §9 KSK advisory-only → PR #336 + +## 7 queued corrections / 6 revised-roadmap PRs + +1. DST scanner + accepted-boundary registry (new) +2. Seed protocol + CI artifacts +3. Sharder reproduction (NOT widen — reinforces 18th #10) +4. `ISimulationDriver` + VTS promotion to core +5. Simulated filesystem (DiskBackingStore rewrite) +6. Cartel-Lab DST calibration (aligns with #342) +Plus: `tools/git/push-with-retry.sh` retry-audit finding; DST-held + FDB-grade criteria lock. + +## DST grade (Amara's assessment, not factory-certified) + +**B-** — strong architectural intent, partial implementation. DST philosophy A-, filesystem simulation D (blocker), task scheduling C-, network D/NA (future). + +Amara verdict: *"strong draft / not canonical yet."* + +## Test plan + +- [x] GOVERNANCE §33 four-field header. +- [x] Both parts verbatim preserved. +- [x] Markdownlint clean locally. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T09:29:06Z) + +## Pull request overview + +Adds a new Aurora research/absorb document capturing Amara’s 19th-ferry DST audit (Part 1) plus her 5.5 correction pass (Part 2), with Otto’s operationalization notes and cross-references for follow-on work. + +**Changes:** +- Introduces a new `docs/aurora/` ferry absorb covering DST audit findings and a revised remediation roadmap. +- Documents queued “graduation” items (scanner/artifacts/scheduler/fs simulation) and reiterates non-authorization boundaries. + +## Review threads + +### Thread 1: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:105 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:04Z): + +References to “GOVERNANCE §33” appear to be incorrect: `GOVERNANCE.md` currently has no §33, and other docs note the 4-field archive-header convention is *not yet codified* as a numbered governance section. Update this doc to cite the current source of the convention (or explicitly say it’s an uncodified convention) so readers aren’t sent to a nonexistent governance rule. +```suggestion +than paraphrase; this is a current factory convention and +is not yet codified as a numbered section in +`GOVERNANCE.md`. Corrections are tracked in Part 2 below. +Where the text cites sources (e.g. "【12†L22-L29】"), those +are Amara's internal citation markers and are kept intact. +``` + +### Thread 2: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:46 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:05Z): + +This attribution block uses direct contributor names throughout, but `docs/AGENT-BEST-PRACTICES.md` defines an operational standing rule: no name attribution in code/docs/skills (names only in `memory/persona/**` and optionally `docs/BACKLOG.md`). Replace direct names here with role references (e.g., “human maintainer”, “external collaborator”, “courier/absorber”) to match repo policy. +```suggestion +- **Human maintainer / courier** — originated the DST + directive as a factory-wide discipline (rulebook in + `.claude/skills`, prior break→do-no-permanent-harm + framing, prior retractability-by-design guidance); + courier for both parts concatenated in one message + with explicit framing *"i asked her to research our + dst"* (direct quote). The human maintainer is both + the consumer of the research and the source of the + DST-rulebook axioms the research audits against. + Data-not-directives per BP-11. +- **Research author** — authored both parts. Deep- + research Part 1 is the audit proper; Part 2 is self- + review via model composition (same two-part pattern + as 17th and 18th ferries). Verdict on Part 1 + (verbatim): *"strong draft / not canonical yet."* +- **Archive / correction-pass tracker** — absorb + surface plus correction-pass tracker; this doc is the + archive, not operational discipline. The 7 + corrections graduate across subsequent ticks per the + established cadence. 4 of Part 1's 12 sections + already align with shipped substrate (see tracker + notes below). +- **Non-participant historical reference** — not a + direct participant in this ferry; KSK attribution is + preserved per prior historical notes. +``` + +### Thread 3: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:212 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:05Z): + +Cross-reference appears to point to a file that doesn’t exist in-repo: `tests/ConcurrencyHarness.fs`. The `VirtualTimeScheduler` lives at `tests/Tests.FSharp/_Support/ConcurrencyHarness.fs` today, so these references should be updated for navigability. +```suggestion +> `tests/Tests.FSharp/_Support/ConcurrencyHarness.fs`), +> consistent with FDB's approach. The binding checklist +> for reviewers enforces this: every PR that touches +> `src/Core` must inspect the diff for those 12 +> sources and ensure any occurrences use the simulation +> APIs. +``` + +### Thread 4: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:236 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:05Z): + +The doc references `src/Core/DiskBackingStore.fs`, but `DiskBackingStore` is currently defined in `src/Core/DiskSpine.fs` (and referenced by `src/Core/Durability.fs`). Update the file path(s) so readers land on the right implementation. +```suggestion +> | `File.*`, `FileStream` | **`DiskBackingStore` (spine/disk)**, e.g. `src/Core/DiskSpine.fs` | Bypasses simulation | **BLOCKER (core)** | Route through `ISimulatedFs` | E2E: random disk faults + rollback | +``` + +### Thread 5: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:753 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:05Z): + +The roadmap references `tests/Tests.FSharp/Formal/SharderInfoTheoreticTests.fs`, but the current test file is `tests/Tests.FSharp/Formal/Sharder.InfoTheoretic.Tests.fs` (module `Zeta.Tests.Formal.SharderInfoTheoreticTests`). Adjust the filename/path in the doc so the cross-reference is accurate. +```suggestion +> tests/Tests.FSharp/Formal/Sharder.InfoTheoretic.Tests.fs +``` + +### Thread 6: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:1084 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:06Z): + +This cross-reference points to a `memory/feedback_ksk_naming_unblocked_...` file that does not exist under `memory/` in the repo. Either fix the filename/path to the actual memory entry or remove the reference to avoid a broken link. +```suggestion +- **Otto-140..145** — KSK canonical expansion + (Kinetic Safeguard Kernel). +``` + +### Thread 7: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:1098 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:06Z): + +Cross-reference appears to point to a non-existent path: `tests/ConcurrencyHarness.fs`. The correct location is `tests/Tests.FSharp/_Support/ConcurrencyHarness.fs` (where `VirtualTimeScheduler` is defined). Update this entry so it resolves. +```suggestion +- **`tests/Tests.FSharp/_Support/ConcurrencyHarness.fs`** — + VirtualTimeScheduler test-side; Part 1 §4 status + "P1 — promote." +``` + +### Thread 8: docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md:1099 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:29:06Z): + +Cross-reference appears to point to a non-existent file: `src/Core/DiskBackingStore.fs`. The `DiskBackingStore` implementation currently lives in `src/Core/DiskSpine.fs`; update this link so it resolves. +```suggestion +- **`src/Core/DiskSpine.fs`** — Part 1 §2's +``` + +## General comments + +### @chatgpt-codex-connector (2026-04-24T09:25:02Z) + +You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage). diff --git a/docs/pr-discussions/PR-0346-docs-dst-compliance-criteria-dst-held-foundationdb-grade-bar.md b/docs/pr-discussions/PR-0346-docs-dst-compliance-criteria-dst-held-foundationdb-grade-bar.md new file mode 100644 index 00000000..de63de00 --- /dev/null +++ b/docs/pr-discussions/PR-0346-docs-dst-compliance-criteria-dst-held-foundationdb-grade-bar.md @@ -0,0 +1,129 @@ +--- +pr_number: 346 +title: "docs: DST compliance criteria \u2014 DST-held + FoundationDB-grade bars (Amara 19th-ferry #6)" +author: AceHack +state: MERGED +created_at: 2026-04-24T09:30:37Z +merged_at: 2026-04-24T09:32:26Z +closed_at: 2026-04-24T09:32:26Z +head_ref: docs/dst-compliance-criteria +base_ref: main +archived_at: 2026-04-24T11:22:15Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #346: docs: DST compliance criteria — DST-held + FoundationDB-grade bars (Amara 19th-ferry #6) + +## PR description + +## Summary + +Research-grade criteria doc locking two acceptance bars per Amara 19th-ferry Part 2 correction #6. + +## DST-held (minimum bar) — 6 items + +1. All PR-gating stochastic tests use explicit seeds +2. Every failing stochastic test emits seed + parameters +3. Same seed produces same result locally and in CI +4. Broad sweeps run nightly, not as flaky PR gates +5. Main-path code has zero unreviewed entropy-source hits +6. File/network/time/random/task-scheduling boundaries are simulated or explicitly accepted + +## FoundationDB-grade (aspirational) — 8 surfaces + +Simulated FS · Simulated network · Deterministic task scheduler · Fault injection/buggify · Swarm runner · Replay artifact storage · Failure minimization/shrinking · End-to-end reproducible-from-one-seed + +## Mapping to revised-roadmap + +| Revised-roadmap PR | Criteria item | +|---|---| +| PR 1 entropy-scanner + boundary registry | DST-held #5 + #6 enforcement | +| PR 2 seed protocol + CI artifacts | DST-held #1 + #2 | +| PR 3 sharder reproduction | DST-held #3 + #4 | +| PR 4 ISimulationDriver + VTS → core | FDB #3 foundation | +| PR 5 simulated filesystem | FDB #1 | +| PR 6 Cartel-Lab DST calibration | DST-held #1 + #2 + FDB #5 partial | + +Also preserves Amara's per-area grade table (overall B-) as *Amara's assessment, not factory-certified.* + +## Promotion path + +Research-grade today. Promotes to `docs/DST-COMPLIANCE.md` top-level when PR 1 of revised roadmap lands + ADR. + +## Test plan + +- [x] Markdownlint clean. +- [x] No claims of current DST-held compliance. +- [x] No workflow/code changes. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T09:34:37Z) + +## Pull request overview + +Adds a new research doc that defines acceptance criteria for deterministic simulation testing (DST) in Zeta, split into a minimum “DST-held” bar and an aspirational “FoundationDB-grade” bar, and maps those criteria to the revised 19th-ferry roadmap. + +**Changes:** +- Introduces a “DST-held” (minimum) checklist intended to gate future DST-compliance claims. +- Defines an aspirational “FoundationDB-grade DST candidate” surface list (filesystem/network/scheduler/buggify/swarm/artifacts/shrinking/replay). +- Adds a mapping section tying the criteria to shipped work and queued roadmap PRs, plus a promotion path to a future top-level compliance doc. + +## Review threads + +### Thread 1: docs/research/dst-compliance-criteria.md:91 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:34:36Z): + +The doc refers to `ISimulationFs` / `ISimulationNetwork`, but the established naming elsewhere in the repo is `ISimulatedFs` / `ISimulatedNetwork` (e.g. `docs/FOUNDATIONDB-DST.md`). Since these are acceptance-criteria names meant to guide future implementation, using the repo’s existing interface names avoids confusion. +```suggestion +1. **Simulated filesystem.** `ISimulatedFs` implemented + + wired into every file-I/O call site (notably + `DiskBackingStore`). Supports seed-driven fault + injection (read failures, write failures, corruption, + latency spikes). +2. **Simulated network.** `ISimulatedNetwork` +``` + +### Thread 2: docs/research/dst-compliance-criteria.md:244 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:34:36Z): + +This reference is a bit ambiguous: “`.claude/skills` DST guide” points at a directory, not a specific doc. Consider linking to the concrete DST rulebook file (e.g. `.claude/skills/deterministic-simulation-theory-expert/SKILL.md`) so readers can jump straight to the authoritative text. + +### Thread 3: docs/research/dst-compliance-criteria.md:66 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:34:36Z): + +`ISimulationFs` is referenced here as part of the simulation API surface, but that name doesn’t appear elsewhere in the repo; the existing docs use `ISimulatedFs`. Updating this reference keeps the acceptance criteria aligned with the current DST vocabulary. +```suggestion + API (ChaosEnv / VirtualTimeScheduler / ISimulatedFs +``` + +### Thread 4: docs/research/dst-compliance-criteria.md:236 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:34:37Z): + +Inline code spans can’t include a newline. The path to the Amara 19th-ferry source doc is split across two lines inside backticks, which will render incorrectly and breaks copy/paste of the path. Keep the full path on one line (or switch to a normal markdown link). +```suggestion +- Amara 19th ferry — `docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md` +``` + +### Thread 5: docs/research/dst-compliance-criteria.md:249 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T09:34:37Z): + +`tests/ConcurrencyHarness.fs` is referenced here, but that file doesn’t exist in the repo. The `VirtualTimeScheduler` currently lives in `tests/Tests.FSharp/_Support/ConcurrencyHarness.fs`, so this cross-reference should be updated to the correct path. +```suggestion +- `tests/Tests.FSharp/_Support/ConcurrencyHarness.fs` + — the test-side VirtualTimeScheduler queued for core + promotion (PR 4 of revised roadmap). +``` + +## General comments + +### @chatgpt-codex-connector (2026-04-24T09:30:42Z) + +You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage). diff --git a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md new file mode 100644 index 00000000..c4b99dd9 --- /dev/null +++ b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md @@ -0,0 +1,134 @@ +--- +pr_number: 350 +title: "docs: Frontier rename pass-2 (Hindu/FF7/Egyptian/Greek/Norse) + Scientology research BACKLOG (Otto-175)" +author: AceHack +state: MERGED +created_at: 2026-04-24T10:03:56Z +merged_at: 2026-04-24T10:05:31Z +closed_at: 2026-04-24T10:05:31Z +head_ref: docs/frontier-rename-pass-2-plus-scientology-backlog +base_ref: main +archived_at: 2026-04-24T11:22:16Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #350: docs: Frontier rename pass-2 (Hindu/FF7/Egyptian/Greek/Norse) + Scientology research BACKLOG (Otto-175) + +## PR description + +## Summary + +Per Aaron Otto-175: *"Starboard I guess for now... do one more name pass just in case something else clever comes up other than Starboard. maybe some mythical choices that fit?"* + followup *"what about hindu mythic/religious names that fit or FF7 names that fit too"* + final confirmation *"Starboard okay"*. + +## Two components + +1. **`docs/research/frontier-rename-name-pass-2-otto-175.md`** — comprehensive pass-2 analysis with Hindu / Vedic / FF7 / Egyptian / Greek / Norse candidates. Starboard header updated to "confirmed pick" per Aaron's final message. Doc preserved as glass-halo transparency record of options considered. + +2. **Scientology-research BACKLOG row** — public-domain-only, thematic-inspiration-only. Explicit IP discipline (no trademarks adopted, no leaked paid content ingested). Aminata threat-pass required before landing any deliverable beyond generic encyclopedic summary. + +## Pass-2 top findings + +- **Hindu candidates (new this pass):** Dharma, Yantra, Akasha emerge as clean + semantically strong fits. Cultural-sensitivity note: concept-nouns viable; deity-names (Vishnu/Shiva/Ganesha/Brahma/Saraswati) carry appropriation risk absent cultural consultation. +- **FF7 candidates (new this pass):** Mako (possible, loose Square Enix association); Materia / Lifestream / Highwind / Sephiroth / Shinra / Aerith / Cloud all NOT VIABLE (Square Enix trademarks or overloaded). +- **New 2026 conflicts found pass-2:** Hermes (Nous Research), Bifrost (Maxim), Beacon (Beacon AI USSOCOM), Sentinel (SentinelOne + MS Sentinel), Oracle (26ai + Deloitte partnership), Palantir (incumbent) — all NOT VIABLE for agentic-AI-space. + +## Scientology research scope + +**IS for:** thematic vocabulary inspiration, historical case study, risk-surface reading (the NOT-THIS reference point for factory's opposite glass-halo posture). + +**NOT for:** adopting trademarked terms, ingesting leaked/paid content, positioning factory as Scientology-adjacent. + +## Placement + +Pass-2 doc under `docs/research/`. Scientology row at END of P2 research-grade section (line ~6026) — NOT BACKLOG tail. Avoids positional-append conflict pattern. + +## Test plan + +- [x] Markdownlint clean locally. +- [x] No code changes. +- [x] Strict non-adoption discipline preserved in BACKLOG row. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T10:05:53Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `40c8793eea` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:08:03Z) + +## Pull request overview + +Adds a second-pass research memo for the Frontier UI rename (mythology/FF7 candidate scan + conflict notes) and records a new BACKLOG item to bound “Scientology thematic research” to public-domain-only, non-adoption scope. + +**Changes:** +- Added `docs/research/frontier-rename-name-pass-2-otto-175.md` with additional naming candidates and conflict-scan notes, plus sources/cross-references. +- Appended a new P2 BACKLOG row specifying scope, non-adoption constraints, and a proposed deliverable memo for the Scientology-themed research. + +### Reviewed changes + +Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments. + +| File | Description | +| ---- | ----------- | +| docs/research/frontier-rename-name-pass-2-otto-175.md | New pass-2 naming analysis doc with candidate lists, conflict findings, and cross-references. | +| docs/BACKLOG.md | New BACKLOG entry defining scope/constraints and deliverable for public-domain-only Scientology thematic research. | + +## Review threads + +### Thread 1: docs/BACKLOG.md:6066 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T10:05:53Z): + +**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Align source scope with public-domain-only boundary** + +This backlog item is labeled `public-domain-only`, but this bullet explicitly includes mainstream published books (`Jon Atack`, `Lawrence Wright`, `Janet Reitman`, etc.), which are copyrighted works rather than public domain. That contradiction makes the execution scope ambiguous: an agent following the list can violate the stated IP boundary even while trying to comply, which is exactly the risk this row is meant to prevent. + +Useful? React with 👍 / 👎. + +### Thread 2: docs/research/frontier-rename-name-pass-2-otto-175.md:12 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:08:02Z): + +P1: This new research doc repeatedly uses contributor names (e.g., "Aaron") and agent/persona names directly. That conflicts with the repo’s operational standing rule to avoid name attribution in docs/skills and use role references instead (docs/AGENT-BEST-PRACTICES.md:284-292). Suggest rewriting these references to role-refs (e.g., "human maintainer", "memo author", "threat-model reviewer") while keeping the Otto-### identifiers for traceability. + +### Thread 3: docs/research/frontier-rename-name-pass-2-otto-175.md:465 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:08:03Z): + +P1: The cross-reference `docs/definitions/KSK.md` appears to point to a path that doesn’t exist in the repo (there is no `docs/definitions/` directory in-tree). Please update this link to the correct existing location, or rephrase it as a planned deliverable and link to the BACKLOG entry that will create it. +```suggestion +- `docs/BACKLOG.md` Otto-168 row — planned deliverable: + define the factory's Aurora / Zeta / KSK naming + triangle in a future definitions document so any rename + slots into that ecosystem cleanly. +``` + +### Thread 4: docs/BACKLOG.md:6030 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:08:03Z): + +P2: New BACKLOG rows in this file often use role references like "Human maintainer" rather than a personal name (see later entries in this file). For consistency with the repo’s no-name-attribution operational rule (docs/AGENT-BEST-PRACTICES.md:284-292), consider changing the attribution here to a role-ref outside of the quoted directive (e.g., "Human maintainer Otto-175"). diff --git a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md new file mode 100644 index 00000000..c2fd4509 --- /dev/null +++ b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md @@ -0,0 +1,223 @@ +--- +pr_number: 352 +title: "backlog: Otto-180 Server Meshing + SpacetimeDB deep research \u2014 game-industry competitive angle" +author: AceHack +state: OPEN +created_at: 2026-04-24T10:14:31Z +head_ref: backlog/otto-180-server-meshing-spacetime-db-research +base_ref: main +archived_at: 2026-04-24T11:22:13Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #352: backlog: Otto-180 Server Meshing + SpacetimeDB deep research — game-industry competitive angle + +## PR description + +## Summary + +Aaron Otto-180: *"also backlog server mesh from star citizen, our db backend when we shard it should support this style of cross shard communication like server mesh, it's amazing actually, i think space time db is similar too or not it might be orthogonal but we want to support these use cases in our backend too. do deep reserach here, this could get us lots of customers in the game industruy if we can compete with server mess/space time db"*. + +Explicit backlog directive overrides Otto-171 freeze-state queue discipline. + +## Two architectures to research (likely orthogonal, Aaron's intuition is right) + +- **Server Meshing (CIG / Star Citizen)** — horizontal-scaling across game servers; entity handoff + state propagation across meshed-server boundaries. +- **SpacetimeDB (Clockwork Labs, Apache-2)** — DB + server unified; "the database IS the server" pitch; 1000x cheaper + faster claim vs traditional MMO backend. + +## Zeta differentiators identified + +- Retraction-native semantics = native rollback / lag-comp / failed-transaction recovery +- Time-travel queries compose with replay / match-review +- Columnar storage serves game-economy analytics + +## Deliverable + +`docs/research/server-meshing-spacetimedb-comparison-zeta-sharding-fit.md` with 5 sections: SM architecture / SpacetimeDB architecture / Zeta-sharding fit / competitive positioning / integration scenarios. + +## Customer-industry angle + +3-5 named studio-types (MMO / sim / esports / mobile persistent / VR-social) with value-proposition per segment. + +## IP discipline + +Same pattern as Otto-175c / Scientology rows: public CIG Inside-Star-Citizen + RSI blog content OK; SpacetimeDB Apache-2 code study OK; no adoption of CIG proprietary architecture docs, no positioning as CIG-adjacent / SpacetimeDB-adjacent in public branding. + +## Placement + +P2 research-grade at line ~921 (end of "## P2 — Post-v1 query-surface research" section). **NOT BACKLOG tail** — 53 DIRTY siblings on tail; placing here avoids cascading another positional-append conflict. + +## Test plan + +- [x] Markdownlint clean. +- [x] BACKLOG-only; no code changes. +- [x] Placement away from DIRTY-saturated tail. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T10:15:36Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `55ce0fac69` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:18:05Z) + +## Pull request overview + +Adds a new P2 backlog research item describing a planned deep-dive into Server Meshing (Star Citizen/CIG) and SpacetimeDB, framed as input to Zeta’s eventual multi-node/sharding design and game-industry competitive positioning. + +**Changes:** +- Adds a detailed P2 backlog entry covering research scope, differentiators, deliverable doc target, and IP-discipline constraints. + +### COMMENTED — @AceHack (2026-04-24T11:13:21Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:13:23Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:13:25Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:13:26Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:13:28Z) + +_(no body)_ + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T11:14:47Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `44825a88fc` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +## Review threads + +### Thread 1: docs/BACKLOG.md:1009 (resolved) + +**@chatgpt-codex-connector** (2026-04-24T10:15:37Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Keep deliverable path contiguous in code span** + +The inline code span for the research deliverable filename is split across two lines, so Markdown renderers normalize the newline to a space and show `docs/research/server- meshing-spacetimedb-comparison-zeta-sharding-fit.md` instead of the intended path. This makes copy/paste usage error-prone and can break any tooling or manual workflow that relies on the documented filename being exact. + +Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T11:13:21Z): + +Fixed in 44825a8: moved the multi-line `docs/research/server-meshing-*.md` path onto its own line so the backtick-delimited inline code span renders atomically per CommonMark. The duplicate finding (thread 59XSRN) is covered by the same fix. + +### Thread 2: docs/BACKLOG.md:1070 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:18:04Z): + +This paragraph references “Otto-175c starship-franchise-mapping row” and “Amara 10th + 11th ferries”, but those identifiers don’t currently resolve to any existing row/doc in the repo. Please either (a) link to the concrete existing docs/rows you mean (file paths), or (b) rename the references to the actual existing identifiers (e.g., Otto-175) to avoid dangling xrefs. +```suggestion + row (above) + Otto-175 (Star Citizen personal + expertise compose). Waits on: Zeta multi-node + foundation (currently unshipped; deferred to later + multi-node planning work). +``` + +**@AceHack** (2026-04-24T11:13:25Z): + +Fixed in 44825a8: clarified the Otto-175c starship-franchise-mapping reference to note the row landed in PR #351 (merged). Amara 10th + 11th ferry references now point at their archived location under `docs/aurora/2026-04-24-amara-*.md` with specific ferry-name hints (11th Temporal-Coordination-Detection, 12th Executive-Summary). Cross-references resolve from external vantage. + +### Thread 3: docs/BACKLOG.md:1045 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:18:04Z): + +The IP-discipline bullet “No Star-Citizen trademarked content ingested.” is ambiguous in context (this entry already uses trademarked names for reference). If the intent is “no proprietary/non-public materials or trademarked *assets*”, consider tightening the wording to avoid readers interpreting it as “don’t even mention the trademarked terms.” +```suggestion + - No non-public Star Citizen/CIG materials or + trademarked creative assets ingested; nominative + reference to product/company names for comparison is + permitted. +``` + +**@AceHack** (2026-04-24T11:13:26Z): + +Fixed in 44825a8: rewrote the IP-discipline block to distinguish industry-landscape reference (permitted: discussing CIG / SpacetimeDB / Star Citizen as comparison targets — this is fair-use research scope) from proprietary-content ingestion (excluded: leaked dev blogs, closed-beta material, non-public SpacetimeDB docs beyond Apache-2 repo). Specific research-permitted boundaries now explicit for both CIG and SpacetimeDB separately. + +### Thread 4: docs/BACKLOG.md:1009 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:18:04Z): + +Inline code span for the deliverable path is split across two lines (backtick opens on line 1008 and closes on 1009). CommonMark parsers don’t allow multiline inline-code spans, so this will render incorrectly (and may trip markdown tooling). Keep the path on a single line or switch to a normal link/reference format. +```suggestion + **Research deliverable:** `docs/research/server-meshing-spacetimedb-comparison-zeta-sharding-fit.md`. +``` + +**@AceHack** (2026-04-24T11:13:22Z): + +Same fix as thread 59XP74 (commit 44825a8): path now on its own line; inline code span is single-line; CommonMark-compliant. + +### Thread 5: docs/BACKLOG.md:1066 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:18:05Z): + +The cross-reference “Pluggable wire-protocol row (line ~754)” appears to point to the wrong location: the wire-protocol row is currently around line ~830, while line ~754 is a different item. Please update the line reference (or replace with a stable link/anchor) so the xref remains accurate as the file changes. +```suggestion + protocol row (above) + Regular-database façade +``` + +**@AceHack** (2026-04-24T11:13:28Z): + +Fixed in 44825a8: corrected `line ~754` to `around line 830` — the wire-protocol row's actual current location. Verified via `grep -n '^- \[ \] \*\*Pluggable wire-protocol'` before commit. + +### Thread 6: docs/BACKLOG.md:1082 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T11:14:47Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Reference the actual archived ferry docs** + +This note points readers to `docs/aurora/2026-04-24-amara-*.md` for the “11th-ferry” and “12th-ferry” priors, but those artifacts are not present under `docs/aurora` (only 17th/19th ferry files exist), and a repo-wide search does not find matching 11th/12th aurora docs. That makes the provenance trail in this backlog item non-actionable for anyone trying to follow the cited priors; please update the reference to existing files or adjust the claim. + +Useful? React with 👍 / 👎. diff --git a/docs/pr-discussions/PR-0353-docs-backlog-md-split-design-phase-0-aaron-otto-181-3rd-ask.md b/docs/pr-discussions/PR-0353-docs-backlog-md-split-design-phase-0-aaron-otto-181-3rd-ask.md new file mode 100644 index 00000000..542cbbce --- /dev/null +++ b/docs/pr-discussions/PR-0353-docs-backlog-md-split-design-phase-0-aaron-otto-181-3rd-ask.md @@ -0,0 +1,126 @@ +--- +pr_number: 353 +title: "docs: BACKLOG.md split design \u2014 Phase 0 (Aaron Otto-181, 3rd ask; PR #213 hot-file detector referenced)" +author: AceHack +state: MERGED +created_at: 2026-04-24T10:22:21Z +merged_at: 2026-04-24T10:24:02Z +closed_at: 2026-04-24T10:24:02Z +head_ref: docs/backlog-split-design-otto-181 +base_ref: main +archived_at: 2026-04-24T11:22:16Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #353: docs: BACKLOG.md split design — Phase 0 (Aaron Otto-181, 3rd ask; PR #213 hot-file detector referenced) + +## PR description + +## Summary + +Aaron Otto-181: *"BACKLOG.md-touching sibling we gonna split it lol, :)"* + *"approved whenever you want to do you this is the 3rd time i asked you even created a git hot file detector to find other hot files as hygene"*. + +**Recognition of 3rd ask + factory already predicted this:** `tools/hygiene/audit-git-hotspots.sh` exists on branch `hygiene/git-hotspots-audit-tool-plus-first-run` (PR #213, BEHIND since 2026-04-23). Tool explicitly names "BACKLOG-per-swim-lane split" as a remediation option. This PR is the design bridge between "detected" and "executed." + +## Structure proposed + +Per-row YAML-frontmatter files at `docs/backlog/P/B-NNNN-.md` + generated `docs/BACKLOG.md` index + drift-CI workflow (same pattern as memory-index-integrity). + +- Each PR adding a row = one new file +- Zero shared-file touch → zero positional-append conflict +- Eliminates the DIRTY-cascade documented in Otto-171 queue-saturation memory + +## Phased execution + +| Phase | Scope | Cost | +|---|---|---| +| 0 (this PR) | Design doc + 6 open questions | — | +| 1 | Tooling (index generator + schema lint + CI drift) | S | +| 2 | Content split mega-PR | L (one-time) | +| 3 | Convention updates + `backlog-new-row` scaffolder | S | + +## 6 open questions for Aaron + +1. ID scheme (`B-NNNN` / `P-NNNN` / slug-only) +2. Generator language (bash / bun+TS / F#) +3. Phase-2 timing (before or after queue drains) +4. Retire convention (delete vs `_retired/`) +5. Auto-ID assignment (factory tooling vs manual) +6. `composes_with` CI-lint enforcement (strict vs best-effort) + +## Cost / benefit + +Break-even: current positional-conflict overhead is ~50% of one tick's capacity (Otto-177 observed 53 DIRTY siblings). Mega-PR is 1-2 ticks. **Payback: one week.** + +## Other hot files to consider after + +`ROUND-HISTORY.md` / `loop-tick-history.md` / `FACTORY-HYGIENE.md` / `memory/MEMORY.md` / `TECH-RADAR.md` — run detector first, don't split blindly. + +## Test plan + +- [x] Markdownlint clean. +- [x] Research-grade; Phase 2 execution deferred to Aaron sign-off. +- [x] No code / workflow / BACKLOG changes. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:25:39Z) + +## Pull request overview + +Adds a research-grade design proposal for splitting `docs/BACKLOG.md` into per-row files with a generated index, aiming to eliminate frequent merge conflicts caused by concurrent tail-appends. + +**Changes:** +- Introduces a Phase-0 design document proposing `docs/backlog/P/-.md` per-row files with YAML frontmatter. +- Proposes generating `docs/BACKLOG.md` as an index and enforcing drift via CI (modeled after memory-index workflows). +- Captures a phased migration plan plus open sign-off questions for structure/tooling choices. + +## Review threads + +### Thread 1: docs/research/backlog-split-design-otto-181.md:13 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:25:37Z): + +P1: This doc uses direct contributor name attribution (e.g., “Aaron …”) in the body. Repo standing rule is to avoid names in code/docs/skills and use role references (e.g., “human maintainer”, “architect”), keeping names only in `memory/persona/**` and optionally `docs/BACKLOG.md` for specific requests (docs/AGENT-BEST-PRACTICES.md:284-292). Please rewrite these references to role-based phrasing while retaining the Otto-### directive IDs if needed. + +### Thread 2: docs/research/backlog-split-design-otto-181.md:33 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:25:37Z): + +P2: The document claims `docs/BACKLOG.md` is “~6100 lines”, but in the current repo it is 7658 lines. Suggest avoiding a hard number (e.g., “7k+ lines”) or updating the figure so the justification doesn’t drift immediately. + +### Thread 3: docs/research/backlog-split-design-otto-181.md:254 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:25:38Z): + +P1: The cost/benefit table appears to have malformed Markdown syntax: rows start with `|| ...`, which renders as an empty first column (`| | ...`) in standard Markdown. Consider converting these rows to normal `| ... | ... |` table syntax so the table renders correctly in GitHub. + +### Thread 4: docs/research/backlog-split-design-otto-181.md:22 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:25:38Z): + +P1: This section states `tools/hygiene/audit-git-hotspots.sh` exists, but that file is not present in the current repository tree. If it’s only in PR #213 / another branch, please rephrase to reference the PR (or add a link) rather than implying the path exists on main, otherwise readers will hit a dead link. + +### Thread 5: docs/research/backlog-split-design-otto-181.md:282 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:25:38Z): + +P1: `docs/definitions/KSK.md` is referenced here as an existing precedent, but `docs/definitions/` does not exist in the repo right now. Please update this cross-reference to a file that exists (or to the BACKLOG row / PR that will introduce KSK.md) so the precedent claim is accurate. +```suggestion +- **Planned definitions-file pattern** — if a follow-up + PR introduces `docs/definitions/KSK.md`, that would be + a companion example of per-concept files with YAML + frontmatter rather than an in-tree precedent today. +``` + +### Thread 6: docs/research/backlog-split-design-otto-181.md:337 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:25:38Z): + +P1: The referenced memory file `memory/feedback_queue_saturation_throttle_ship_rate_under_ci_throughput_never_idle_switches_to_memory_reading_review_2026_04_24.md` does not exist in `memory/`. Please correct the filename/path (or reference the closest existing memory) so this cross-reference is resolvable. +```suggestion +- Otto-171 queue-saturation memory entry (2026-04-24) — + queue-saturation observation. +``` diff --git a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md new file mode 100644 index 00000000..a7c5c834 --- /dev/null +++ b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md @@ -0,0 +1,448 @@ +--- +pr_number: 354 +title: "tools: backlog split Phase 1a \u2014 generator + schema + example row (Aaron Otto-181, 3rd ask)" +author: AceHack +state: OPEN +created_at: 2026-04-24T10:27:07Z +head_ref: tools/backlog-split-phase-1a-generator-plus-schema +base_ref: main +archived_at: 2026-04-24T11:22:01Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #354: tools: backlog split Phase 1a — generator + schema + example row (Aaron Otto-181, 3rd ask) + +## PR description + +## Summary + +Phase 1a of BACKLOG.md split per Aaron Otto-181 3rd-ask. Aaron approved reasonable defaults ("i trust your suggestions"). + +**Phase-1a scope (this PR):** tooling + schema + one example row. **No BACKLOG.md content touched.** + +## Files + +- `tools/backlog/README.md` — schema spec + frontmatter reference + how-to +- `tools/backlog/generate-index.sh` — bash generator (3 modes: `--stdout`, `--check`, default write with Phase-1a safety guard) +- `docs/backlog/README.md` — per-row-dir overview +- `docs/backlog/P2/B-0001-example-schema-self-reference.md` — example row exercising the schema end-to-end + +## Defaults applied (Aaron's 6 open questions) + +| # | Question | Default this PR applies | Rationale | +|---|---|---|---| +| 1 | ID scheme | `B-NNNN` sequential | Memory-file convention parallel | +| 2 | Generator language | bash (temporary) | Phase 1a CI fit; **bun+TS long-term** per Aaron Otto-182 | +| 3 | Phase-2 timing | drain first | Avoid one-time cascade cost | +| 4 | Retire convention | plain delete | Per CLAUDE.md "honor those that came before" | +| 5 | Auto-ID | scaffolder (Phase 1b) | Friction reduction | +| 6 | `composes_with` lint | best-effort first | Strict after adoption | + +Aaron confirmed bun+TS is the long-term direction to eliminate needing both `.sh` + `.ps1` per FACTORY-HYGIENE #51 cross-platform parity. Phase 1b or later can migrate the generator to bun+TS when convenient; bash lands Phase 1a as tight-CI-fit for immediate adoption. + +## Safety + +Generator in write-mode refuses to overwrite an existing `docs/BACKLOG.md` with >50 lines unless `BACKLOG_WRITE_FORCE=1` is set. Prevents Phase-2-before-Phase-2 accidents. Verified working: + +``` +$ tools/backlog/generate-index.sh +generate-index.sh: refusing to overwrite existing docs/BACKLOG.md... +``` + +Phase-2 content-migration PR will set `BACKLOG_WRITE_FORCE=1` intentionally after populating per-row files. + +## Phases remaining + +- **1b:** `.github/workflows/backlog-index-integrity.yml` drift-CI + `tools/backlog/new-row.sh` scaffolder +- **2:** content split mega-PR (reads BACKLOG.md, generates per-row files, regenerates index) +- **3:** `CONTRIBUTING.md` / `AGENTS.md` convention updates + +## Codex review requested + +Aaron Otto-182: *"can you ask codex too?"* — inviting `@codex review` below. + +## Test plan + +- [x] Generator empty-dir output: correct header + footer, no rows +- [x] Generator with B-0001 example: correct P2 section + link + checkbox +- [x] Safety guard fires with exit 1 on existing monolith +- [x] Markdownlint clean locally +- [x] No BACKLOG.md touched + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T10:29:51Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `6a447ee234` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:31:29Z) + +## Pull request overview + +Phase 1a scaffolding for splitting the monolithic `docs/BACKLOG.md` into per-row markdown files under `docs/backlog/`, with a generator to produce a compact index. + +**Changes:** +- Adds `tools/backlog/generate-index.sh` to generate/check/print the `docs/BACKLOG.md` index from per-row files, including a Phase-1a overwrite guard. +- Adds schema + usage documentation for per-row backlog files (`tools/backlog/README.md`, `docs/backlog/README.md`). +- Adds an example per-row backlog entry to exercise the schema and generator (`docs/backlog/P2/B-0001-...`). + +### Reviewed changes + +Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments. + +| File | Description | +| ---- | ----------- | +| tools/backlog/generate-index.sh | Bash generator for `docs/BACKLOG.md` with `--stdout` and `--check` modes plus a write safety guard. | +| tools/backlog/README.md | Schema/spec and contributor instructions for per-row backlog files and index generation. | +| docs/backlog/README.md | Overview of `docs/backlog/` layout and quick reference for contributors. | +| docs/backlog/P2/B-0001-example-schema-self-reference.md | Example backlog row demonstrating frontmatter + body end-to-end. | + +### COMMENTED — @AceHack (2026-04-24T11:03:29Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:31Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:33Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:35Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:37Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:39Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:41Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:43Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:45Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:47Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:48Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T11:03:50Z) + +_(no body)_ + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T11:06:32Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `9bf3d09ff1` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:06:51Z) + +## Pull request overview + +Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments. + +## Review threads + +### Thread 1: tools/backlog/generate-index.sh:55 (resolved) + +**@chatgpt-codex-connector** (2026-04-24T10:29:51Z): + +**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Parse single-quoted YAML scalars before status checks** + +`extract_field` only strips double quotes, so valid YAML like `status: 'closed'` or `id: 'B-0002'` is returned with surrounding `'` characters. In `generate`, that makes the `status` case fall through to open (`[ ]`) and emits quoted IDs/titles in the index, so quoted frontmatter variants produce incorrect backlog state even though they are valid YAML. + +Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T11:03:29Z): + +Fixed in 6f659a0. Extended the strip-quotes gsub to also handle single-quoted YAML scalars: `gsub(/^"|"$|^[[:space:]]*\x27|\x27[[:space:]]*$/, "")`. The state-machine rewrite (thread 59Xdsr) also fixes this for values that span into the body section by preventing re-entry. Both fixes in one place: the `extract_field` function. + +### Thread 2: tools/backlog/generate-index.sh:51 (resolved) + +**@chatgpt-codex-connector** (2026-04-24T10:29:51Z): + +**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Stop frontmatter scan after first closing delimiter** + +The parser toggles `in_fm` on every `---` line in the file, so a markdown horizontal rule in the body re-enters parse mode and later `title:`/`status:` text can overwrite real frontmatter values. This causes wrong checkbox/title/index output for otherwise valid row content that includes thematic breaks or YAML-like snippets. + +Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T11:03:31Z): + +Fixed in 6f659a0. Replaced the `in_fm = !in_fm` toggle with an explicit 3-state machine: 0 = before frontmatter, 1 = inside, 2 = after. The scanner now terminates definitively on the first closing `---` instead of re-entering frontmatter mode on every subsequent `---` line in body content. This was a real parser bug that would have silently matched horizontal-rule-style dividers inside the body. + +### Thread 3: tools/backlog/generate-index.sh:168 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T10:29:51Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Require second factor for BACKLOG_WRITE_FORCE bypass** + +The Phase-1a guard is disabled solely by `BACKLOG_WRITE_FORCE=1`, so any leaked/exported env var in CI or a developer shell will bypass protection and allow destructive overwrite of a large existing `docs/BACKLOG.md`. Given this is intended as a one-time migration escape hatch, relying on a single ambient env var is too easy to trigger accidentally. + +Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T11:03:33Z): + +Acknowledged; deferring the second-factor bypass hardening to Phase 2 content-migration PR. Rationale: Phase 1a has a substantial-content guard (>50 lines on existing BACKLOG.md) that requires an explicit env var to override; this prevents accidental overwrite during routine development. The second-factor discussed (git-blame signer / commit-message marker / dedicated workflow trigger) would add real defense-in-depth but is only meaningfully exercised during Phase 2 when the mega-PR will intentionally call the generator with BACKLOG_WRITE_FORCE=1 against a populated docs/backlog/ tree. Filing as a specific Phase-2-acceptance criterion; keeping this thread unresolved as a visible breadcrumb until Phase 2 ships. + +### Thread 4: tools/backlog/generate-index.sh:176 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:27Z): + +P1: Writing the generated output via `mktemp` (likely under `/tmp`) and then `mv` to `docs/BACKLOG.md` may not be an atomic rename on all systems (cross-filesystem `mv` degrades to copy+delete), which risks partial writes and can change permissions. Prefer creating the temp file in the destination directory and then renaming over it for an atomic update. + +**@AceHack** (2026-04-24T11:03:35Z): + +Fixed in 6f659a0. Temp file now created in the same directory as the target via mktemp template: `mktemp "${INDEX_PATH}.tmp.XXXXXX"`. This guarantees same-filesystem atomic rename for `mv` on any POSIX system, eliminating the cross-filesystem copy+unlink path that the original /tmp-based approach could hit on systems where /tmp is a separate mount. + +### Thread 5: tools/backlog/README.md:9 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:27Z): + +P1: These new docs/tooling files include direct contributor name attribution (e.g. "Aaron ..."). Repo convention forbids names in code/docs/skills (allowed only under `memory/persona/**` and optionally `docs/BACKLOG.md`); use role references like “human maintainer” plus a directive ID instead. See docs/AGENT-BEST-PRACTICES.md:284-290. + +**@AceHack** (2026-04-24T11:03:37Z): + +Fixed in 6f659a0. Removed direct contributor name attribution from factory-produced docs (tools/backlog/README.md + docs/backlog/P2/B-0001 frontmatter + generate-index.sh header comment); replaced with "maintainer" idiom per the convention in docs/CONTRIBUTOR-CONFLICTS.md + AGENT-BEST-PRACTICES. Commit messages retain the "Aaron Otto-NNN" attribution per the history-file-exemption (documented in loop-tick-history 2026-04-22 — tick-history / commit-messages / memory are audit-trail surfaces where direct attribution is the intent; README / tools docs are factory-produced surfaces where the maintainer idiom applies). + +### Thread 6: tools/backlog/README.md:139 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:27Z): + +P1: Cross-reference targets here appear to be missing from the repo: `docs/research/backlog-split-design-otto-181.md`, `tools/hygiene/audit-git-hotspots.sh`, and the referenced `memory/feedback_aaron_asked_for_backlog_split_three_times_hot_file_detector_pr_213_exists_*.md` (no matches under `docs/research/`, `tools/hygiene/`, or `memory/`). Either add the referenced artifacts in this PR or update/remove the pointers so they don’t become dead links. + +**@AceHack** (2026-04-24T11:03:39Z): + +Fixed in 9bf3d09. Reworded the cross-reference to the auto-memory path that doesn't resolve from external-reviewer vantage. Replaced the `memory/feedback_aaron_asked_for_backlog_split_three_times_*` path-reference with a contextual description pointing at the detector's own header comment (tools/hygiene/audit-git-hotspots.sh) as the primary cross-reference for scope. Preserves the audit-trail without creating a broken-link-from-outside false-positive. + +### Thread 7: docs/backlog/README.md:34 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:28Z): + +P1: This section says “no row files exist yet”, but this PR adds `docs/backlog/P2/B-0001-example-schema-self-reference.md`. Update the “Current state — Phase 1a” text to reflect that there is now at least an example row present (and clarify whether it’s authoritative or just a schema exerciser). +```suggestion +Tooling + schema landed, and the directory now includes an +example per-row file to exercise the schema and layout. Phase 2 +will migrate existing `docs/BACKLOG.md` content into +authoritative per-row files. Until Phase 2 lands, the +single-file `docs/BACKLOG.md` remains the authoritative source +of rows; this directory + its generator currently provide the +target structure and schema example. +``` + +**@AceHack** (2026-04-24T11:03:41Z): + +Fixed in 9bf3d09. The "no row files exist yet" statement was accurate at initial draft but stale by commit time (B-0001 example was added to exercise the generator). Updated to: one placeholder B-0001 exists as generator-exercise; substantive migration starts at B-0002 with Phase 2. Matches actual repo state. + +### Thread 8: tools/backlog/generate-index.sh:24 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:28Z): + +P2: The header comment lists `yq` (and “bash 4+”) as dependencies, but the script currently doesn’t invoke `yq` and appears compatible with older bash versions. Either implement the optional `yq` path / require bash 4 features, or tighten the comment so the documented dependencies match reality. +```suggestion +# Dependencies: bash, awk, sort, diff. +``` + +**@AceHack** (2026-04-24T11:03:43Z): + +Fixed in 9bf3d09. Updated header comment from "Optional: yq; falls back to awk if absent" to "No external yq required; inline awk parser handles the flat frontmatter schema. yq integration is a Phase 1b upgrade path if nested queries become necessary." Matches the actual implementation — there's no runtime branch on yq availability; awk is the only parser. + +### Thread 9: tools/backlog/generate-index.sh:56 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:28Z): + +P1: `extract_field` toggles `in_fm` on every `---` line in the file, so a horizontal rule (`---`) in the markdown body will re-enter “frontmatter” parsing and can override previously found values (id/status/title). Consider stopping parsing after the second `---` (end of the first YAML block) instead of toggling indefinitely. + +**@AceHack** (2026-04-24T11:03:45Z): + +Same fix as thread 59Xdsr: the `in_fm = !in_fm` toggle was the root cause of both the "toggles on every ---" behavior and the "parse single-quoted scalars" miss (because single-quoted values with internal `---` would have re-flipped the flag). Explicit 3-state machine (0/1/2) makes both behaviors correct by construction. Commit 6f659a0. + +### Thread 10: tools/backlog/README.md:136 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:28Z): + +P2: The inline-code span for the `memory/...*.md` reference is split across a newline, which breaks markdown rendering (inline code spans can’t span lines) and also makes the path ambiguous due to the leading underscore on the continuation line. Keep the entire path on one line (or use a fenced code block) so markdownlint/rendering stays stable. +```suggestion +- `memory/feedback_aaron_asked_for_backlog_split_three_times_hot_file_detector_pr_213_exists_*.md` — + context on the 3rd-ask + hot-file-detector. +``` + +**@AceHack** (2026-04-24T11:03:47Z): + +Acknowledged the inline-code-span formatting concern for the memory/...*.md reference. Addressed at the root by removing that specific path-reference entirely (commit 9bf3d09, thread 59XfPF), which makes the formatting question moot. Leaving this thread unresolved as a breadcrumb for "reviewer intent honored via different mechanism than suggested" so the conversation-preservation captures both the original suggestion and the what-actually-happened-instead. + +### Thread 11: tools/backlog/README.md:96 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:29Z): + +P1: This Phase 1a README describes `tools/backlog/new-row.sh` as if it exists and says the index regenerates “at pre-commit”, but neither the scaffolder script nor any pre-commit hook/integration is part of this PR. Please reword these sections as future/Phase 1b behavior (or include the missing script/hook in the PR) to avoid contributors following instructions that don’t work yet. + +**@AceHack** (2026-04-24T11:03:48Z): + +Fixed in 9bf3d09. The README's forward-reference to `tools/backlog/new-row.sh` was a latent documentation bug — Phase 1a ships the generator but not the scaffolder. Rewrote the "Adding a new row" section to explicitly mark Phase 1a (manual file creation) as current path and Phase 1b (new-row.sh target usage) as not-functional-yet. Eliminates the ambiguity about what works now vs what's promised. + +### Thread 12: tools/backlog/README.md:95 (resolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:31:29Z): + +P1: The README says the generator parses frontmatter via `yq` (with an awk fallback), but `generate-index.sh` currently only uses awk and never invokes `yq`. Either implement the `yq` path in the script or update the documentation to match the actual behavior/dependencies. +```suggestion +Walks `docs/backlog/**/*.md`, parses frontmatter with a +lightweight awk parser, and emits `docs/BACKLOG.md` +sorted by (priority ascending, id ascending). +``` + +**@AceHack** (2026-04-24T11:03:50Z): + +Fixed in 9bf3d09. README's "parses frontmatter via yq or awk fallback" wording was aspirational; the actual implementation is pure awk with no yq integration. Corrected to "inline awk parser, no yq required" which matches the code. Paired with the header-comment fix (thread 59XfPj) so both surfaces describe the same implementation. + +### Thread 13: tools/backlog/generate-index.sh:58 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T11:06:33Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Trim parsed status values before checkbox mapping** + +`extract_field` removes quotes but never normalizes trailing whitespace, so YAML like `status: closed ` (which is still valid and semantically `closed`) is returned with spaces and misses the `closed)` match in `generate`. In that case the row is rendered as open (`[ ]`), which silently misreports backlog state in the generated index for files with incidental trailing spaces. + +Useful? React with 👍 / 👎. + +### Thread 14: tools/backlog/generate-index.sh:107 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T11:06:33Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Iterate over discovered files without shell word splitting** + +The loop `for file in $files` splits on whitespace, so any matching backlog file path containing a space is broken into multiple tokens and passed to `awk` as nonexistent filenames. That makes generation fail hard instead of emitting an index, and this can happen in Phase 1a where files are created manually and only matched by `B-*.md` without slug validation. + +Useful? React with 👍 / 👎. + +### Thread 15: docs/backlog/README.md:33 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:06:49Z): + +P1: This section claims the existing monolith is “6100-line”, but `docs/BACKLOG.md` is currently 7658 lines. Update the number or avoid an exact line count so the Phase-2 description stays accurate over time. +```suggestion +the existing single-file `docs/BACKLOG.md` content into per-row +``` + +### Thread 16: tools/backlog/generate-index.sh:11 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:06:50Z): + +P1: The header references `docs/research/backlog-split-design-otto-181.md`, but that file does not exist in the repo. Either add the referenced design-spec file or update the comment to point at an existing document so the cross-reference isn’t broken. +```suggestion +# Header note intentionally avoids a repo-path reference here +# because the earlier design-spec cross-reference went stale. +``` + +### Thread 17: tools/backlog/generate-index.sh:58 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:06:50Z): + +P1: `extract_field` claims POSIX awk compatibility, but the quote-stripping regex uses `\x27` (hex escape), which isn’t portable across POSIX awk implementations (notably on macOS). Use a POSIX-safe representation (e.g., octal `\047`) or avoid needing a hex escape so the script matches its stated dependency contract. +```suggestion + gsub(/^"|"$|^[[:space:]]*\047|\047[[:space:]]*$/, "") # Codex P1: handle both " and '\'' +``` + +### Thread 18: tools/backlog/generate-index.sh:162 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:06:51Z): + +P2: This comment hardcodes the monolith size as “6100-line”, but `docs/BACKLOG.md` is currently 7658 lines. Either update the number or (preferably) avoid an exact line count so the comment doesn’t drift immediately. +```suggestion + # (i.e. the pre-split monolithic backlog that + # Phase 2 will migrate). Until Phase 2 migrates + # content into per-row files, generator --write + # would destroy the real backlog. +``` + +### Thread 19: tools/backlog/README.md:150 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:06:51Z): + +P1: The cross-references section points to files/scripts that don’t exist in the repo (`docs/research/backlog-split-design-otto-181.md` and `tools/hygiene/audit-git-hotspots.sh`). Please either add the referenced artifacts or update these links to point at the correct existing locations. + +### Thread 20: tools/backlog/README.md:26 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:06:51Z): + +P2: The structure diagram lists `tools/backlog/new-row.sh` as if it exists, but Phase 1a doesn’t add that script (it’s described as Phase 1b later). Consider annotating it as “(Phase 1b)” or removing it from the tree until it lands to avoid confusing readers. +```suggestion + new-row.sh ← scaffolds a new row file (Phase 1b) +``` + +## General comments + +### @AceHack (2026-04-24T10:27:22Z) + +@codex review + +Requesting peer-agent review on this BACKLOG-split Phase-1a tooling per Aaron Otto-182 directive. Key things to check: + +1. **Generator correctness** — does `tools/backlog/generate-index.sh` handle all valid frontmatter variations (quoted vs unquoted title, missing optional fields, different status values)? +2. **Safety guard robustness** — can `BACKLOG_WRITE_FORCE=1` accidentally fire (e.g. if set in CI env)? Should the guard also check a marker file or commit-message trigger? +3. **Schema completeness** — does the frontmatter spec in `tools/backlog/README.md` cover all use cases the current 6100-line `docs/BACKLOG.md` contains? What fields are we likely to need that aren't listed? +4. **Cross-platform parity** — bash is Phase-1a-temporary; bun+TS is long-term per Aaron. Any bash idioms here that would pose obstacles when migrating to TS? +5. **Phase-2 migration risk** — the content-split mega-PR will touch 6100 lines. Heuristic parsing (bullet-list items under priority headers) — will it miss any row shapes? Rows with nested bullets? Multi-paragraph directives? + +Independent eyes welcome. Advisory only; Aaron makes final calls on any design changes. diff --git a/docs/pr-discussions/PR-0355-ferry-codex-first-completed-peer-agent-deep-review-absorb-4.md b/docs/pr-discussions/PR-0355-ferry-codex-first-completed-peer-agent-deep-review-absorb-4.md new file mode 100644 index 00000000..a1428976 --- /dev/null +++ b/docs/pr-discussions/PR-0355-ferry-codex-first-completed-peer-agent-deep-review-absorb-4.md @@ -0,0 +1,162 @@ +--- +pr_number: 355 +title: "ferry: Codex first completed peer-agent deep-review absorb (4 convergent reports, Otto-189)" +author: AceHack +state: MERGED +created_at: 2026-04-24T10:41:44Z +merged_at: 2026-04-24T10:43:25Z +closed_at: 2026-04-24T10:43:25Z +head_ref: ferry/codex-first-deep-review-4-reports-absorb +base_ref: main +archived_at: 2026-04-24T11:22:17Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #355: ferry: Codex first completed peer-agent deep-review absorb (4 convergent reports, Otto-189) + +## PR description + +## Summary + +Scheduled absorb of Codex's **first completed peer-agent deep-review** — 4 convergent reports after `@codex review` invite on PR #354 (Otto-182). Milestone in the Otto-79/86/93 peer-harness progression: stage (b) → stage (c) transition. + +## Four reports absorbed + +| # | Filename | Focus | +|---|---|---| +| 1 | `deep-factory-review-2026-04-24.md` | Governance / hygiene / process-entropy | +| 2 | `deep-system-review-2026-04-24.md` (v1) | Code / tests / contracts | +| 3 | `deep-repo-review-2026-04-24.md` | Architecture / process / security | +| 4 | `deep-system-review-2026-04-24.md` (v2) | Durability / recursive / strategic | + +Convergent findings across 4 independent passes = high signal. + +## Convergent P0 (all 4 reviews) + +1. 22 unclassified hygiene rows (`audit-missing-prevention-layers` exit 2) +2. 12 post-setup script-stack violations +3. `DurabilityMode` naming overstates guarantees — needs Ilyana + Aminata review before rename +4. Skipped `RecursiveCounting.MultiSeed` test (already in BUGS.md) +5. `dotnet` unavailable in Codex env (Codex-side infra, not factory blocker) + +## Strategic recs warranting ADR escalation + +- Factory Complexity Budget (FCB) +- Claim-evidence registry +- 3-mode audit lifecycle (report → warn → block) +- Expiry metadata on preview/debt declarations +- Spec-only reconstruction drills + +## Codex one-liner preserved + +> *"Zeta is closer to a research operating system than a standard code repository. The quality of thought is high; the main threat is not technical inability but governance-scale drift."* + +## Factory discipline preserved + +- Codex advisory, not binding (BP-11) +- Convergent-findings-first +- Strategic recs → Aaron ADR decisions +- Otto-171 freeze-state respected (absorb-doc is drain-mode-safe: new file, no BACKLOG touch, no code change) + +## Test plan + +- [x] GOVERNANCE §33 four-field header +- [x] Markdownlint clean +- [x] No BACKLOG touch; no code change; single new doc file + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:45:07Z) + +## Pull request overview + +Adds an Aurora-layer absorb document capturing four convergent Codex deep-review reports from 2026-04-24, intended as a research/provenance artifact for later operationalization. + +**Changes:** +- Introduces a new Aurora archive/absorb doc summarizing convergent P0/P1 findings and strategic recommendations from four Codex reviews. +- Records remediation paths, non-authorization boundaries, and a cross-reference list to related audits/docs/memories. + +## Review threads + +### Thread 1: docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md:117 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:45:05Z): + +P1: The report table lists Codex filenames under `docs/research/...`, but those files don’t exist in this repo. Using repo-relative paths here looks like link-rot/confuses readers into thinking the files should be present. Suggest changing these to plain filenames (or explicitly mark them as Codex-side artifacts not landed in-repo) and avoid `docs/...`-prefixed paths unless they resolve. +```suggestion +reported `make_pr` tool invocation). Filenames below are +Codex-side artifact names as reported in that drop, not +verified repo-resident `docs/research/...` paths here: + +| # | Codex artifact filename | Commit | Focus | +|---|----------------------------------------------|-----------|--------------------------------------------------------| +| 1 | `deep-factory-review-2026-04-24.md` | ee1bc84 | Governance / hygiene / process-entropy | +| 2 | `deep-system-review-2026-04-24.md` (v1) | (adjacent)| Code / tests / contracts / commands-run | +| 3 | `deep-repo-review-2026-04-24.md` | (unknown) | Architecture / process / security / strategic | +| 4 | `deep-system-review-2026-04-24.md` (v2) | f9a6d2b | Durability / recursive-correctness / strategic recs | + +Reports 2 and 4 share the same artifact filename but +differ in content (different Codex sessions or different +PR branches). Resolution strategy: if both commits land +on main, the later one wins per normal git semantics; +Otto-189+ may need to review whether to preserve both +or consolidate. +``` + +### Thread 2: docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md:509 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:45:05Z): + +P1: This cross-reference points at `memory/project_codex_first_deep_review_4_reports_convergent_findings_pending_dedicated_absorb_otto_189_2026_04_24.md`, but there is no such file under `memory/` in the repo. Either add the referenced memory file in this PR or update the reference to an existing in-repo memory/doc so readers can follow it. +```suggestion +- `docs/aurora/2026-04-24-codex-4-report-first-completed- + peer-review-deep-system-factory-repo-audit.md` (current + absorb doc; Otto-188b scheduling/detail captured here + in-repo). +``` + +### Thread 3: docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md:515 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:45:06Z): + +P2: `memory/feedback_peer_harness_progression_*` isn’t a concrete path a reader can open, and there are multiple similarly-named files. Prefer listing the specific filenames (or pointing at `memory/MEMORY.md` entries) so the cross-reference is actionable. +```suggestion +- `memory/MEMORY.md` (see the Otto-86 peer-harness + progression entries covering the 4-stage arc). +``` + +### Thread 4: docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md:537 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:45:06Z): + +P0: This doc cites “GOVERNANCE §33” as if it exists, but `GOVERNANCE.md` currently has rules numbered 1–32 and contains no §33. Please update this reference to the actual canonical source of the archive-header requirement (or land the governance rule in the same change) to avoid pointing readers at a nonexistent section. +```suggestion +- External-conversation archive-header convention — + this doc follows the four-field header. +``` + +### Thread 5: docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md:18 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T10:45:06Z): + +P0: This new doc uses direct contributor name attribution (e.g., “Aaron”). Repo policy is to avoid names in docs and use role references like “human maintainer” instead (docs/AGENT-BEST-PRACTICES.md:284-292). Please rewrite the Attribution section and downstream references accordingly (persona names for agents are fine). + +## General comments + +### @chatgpt-codex-connector (2026-04-24T10:43:53Z) + +### Summary +* Reviewed the trigger and PR context; this appears to be an informational absorb summary for PR #354 follow-up, not a request for new code/doc edits. +* Verified the repository already contains the absorb document matching the trigger’s content (milestone framing, convergent P0 findings, and advisory/ADR posture), so no additional patch was needed in this branch. [docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.mdL1-L44](https://github.com/Lucent-Financial-Group/Zeta/blob/e11379ab67a817499294e18b26f10bc8cdcddd7d/docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md#L1-L44) [docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.mdL131-L220](https://github.com/Lucent-Financial-Group/Zeta/blob/e11379ab67a817499294e18b26f10bc8cdcddd7d/docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md#L131-L220) +* Because no files were changed, I did **not** create a commit and did **not** call `make_pr` (per your constraint). + +**Testing** +* ✅ `git status --short` +* ✅ `git branch --show-current` +* ✅ `git log --oneline -5` +* ✅ `git show --name-only --pretty=format:'%h %s' HEAD` +* ✅ `nl -ba docs/aurora/2026-04-24-codex-4-report-first-completed-peer-review-deep-system-factory-repo-audit.md | sed -n '1,220p'` + + [View task →](https://chatgpt.com/s/cd_69eb48f424d481918b2a1d8428a1fdc2) diff --git a/docs/pr-discussions/PR-0356-backlog-otto-204-pr-resolve-loop-skill-close-the-pr-cycle-au.md b/docs/pr-discussions/PR-0356-backlog-otto-204-pr-resolve-loop-skill-close-the-pr-cycle-au.md new file mode 100644 index 00000000..d486fcba --- /dev/null +++ b/docs/pr-discussions/PR-0356-backlog-otto-204-pr-resolve-loop-skill-close-the-pr-cycle-au.md @@ -0,0 +1,120 @@ +--- +pr_number: 356 +title: "backlog: Otto-204 PR-resolve-loop skill \u2014 close-the-PR cycle automation (active management > ship-and-pray)" +author: AceHack +state: MERGED +created_at: 2026-04-24T11:05:56Z +merged_at: 2026-04-24T11:07:45Z +closed_at: 2026-04-24T11:07:45Z +head_ref: backlog/otto-204-pr-resolve-loop-skill +base_ref: main +archived_at: 2026-04-24T11:22:17Z +archive_tool: tools/pr-preservation/archive-pr.sh +--- + +# PR #356: backlog: Otto-204 PR-resolve-loop skill — close-the-PR cycle automation (active management > ship-and-pray) + +## PR description + +## Summary + +Maintainer Otto-204 directive: *"you need some pr resolve loop that will handled everyting needed to take a pr to compelteion so you don't ahve to keep figuion it out"* + *"we are saving you resolution to all the comments and we expect those to be excellent don't take shortcuts on the feedback, do the right long term thing or backlog the right thing and not it on the comment."* + +New P1 CI/DX BACKLOG row filing the **PR-resolve-loop skill** — 6-step close-the-PR cycle: + +1. CI-status check → fix failures +2. Review-thread enumeration → reply-then-resolve (never shortcut) +3. Name-attribution lint (factory-produced vs audit-trail distinction) +4. Conversation-preservation hook (writes to `artifacts/pr-discussions/`) +5. Auto-merge re-arm +6. Loop-exit on merge / maintainer-escalation / external-blocker + +## Core learning captured + +Active PR management has **10-20× higher ROI** than opening new PRs when queue is saturated. The Otto-200..203 "queue unchanged 136 for 6+ ticks" observation misread passive-stuck as livelock — actual blocker was accumulated unresolved review-threads + silent lint failures that armed auto-merge couldn't overcome. + +This PR was filed AFTER demonstrating the pattern on #354 directly (fixed shellcheck SC2034/SC2295 + addressed 10 of 12 review threads + left 2 as breadcrumbs with rationale). + +## Non-goals + +- NOT auto-merge bypass +- NOT shortcut around reviewer intent +- NOT retry-loop on flake +- NOT opener of new PRs + +## Placement + +P1 CI/DX section line 2469, NOT BACKLOG tail. Single-row add. + +## Test plan + +- [x] Markdownlint clean +- [x] Single-location edit; placed away from DIRTY tail +- [x] BACKLOG-only change + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:08:42Z) + +## Pull request overview + +Adds a new P1 CI/DX backlog item describing a proposed “PR-resolve-loop” skill to automate the end-to-end PR closeout workflow (checks, review-thread responses/resolution, conversation preservation, and auto-merge re-arming). + +**Changes:** +- Add a single P1 CI/DX BACKLOG row specifying the PR-resolve-loop skill’s steps, invocation modes, and non-goals. + +## Review threads + +### Thread 1: docs/BACKLOG.md:2533 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:08:40Z): + +This step uses `mergeStateStatus: MERGEABLE`, but GitHub’s JSON/GraphQL distinguishes `mergeStateStatus` (e.g., CLEAN/BEHIND/DIRTY/BLOCKED/…) from `mergeable` (MERGEABLE/CONFLICTING/UNKNOWN). Update the wording to check `mergeStateStatus: CLEAN` and `mergeable: MERGEABLE` (or use valid `mergeStateStatus` values). +```suggestion + `mergeStateStatus: CLEAN` and `mergeable: + MERGEABLE` (not BEHIND, DIRTY, or BLOCKED), re-arm + `gh pr merge --auto --squash`. +``` + +### Thread 2: docs/BACKLOG.md:2524 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:08:40Z): + +Line starts with `+` (`" + resolve operation logs ..."`), which will be parsed as a nested unordered-list item and can trigger markdownlint MD004/ul-style failures in this repo. Rephrase to avoid a line-start `+` (e.g., “and resolve…”, or restructure the sentence). +```suggestion + and resolve operation logs to `artifacts/pr- +``` + +### Thread 3: docs/BACKLOG.md:2528 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:08:41Z): + +The conversation log target path is `artifacts/pr-discussions/...`, but there’s no `artifacts/` directory in the repo today and an existing BACKLOG row specifies git-tracked PR discussions under `docs/pr-discussions/PR--.md` (docs/BACKLOG.md:4487). Consider aligning this row to the established `docs/pr-discussions/` location (or explicitly call out why this is different and how it becomes git-tracked). +```suggestion + + resolve operation logs to `docs/pr- + discussions/PR--conversation-log.json` so the + git-tracked preservation directive (Otto-150..154, + PR #335) captures both-sides-of-conversation in- + repo rather than relying on GitHub-only +``` + +### Thread 4: docs/BACKLOG.md:2555 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:08:41Z): + +Line starts with `+` (`" + review-resolution gate faithfully."`), which Markdown parses as a nested bullet and markdownlint MD004/ul-style will flag. Reword to avoid starting a continuation line with `+`. +```suggestion + and review-resolution gate faithfully. +``` + +### Thread 5: docs/BACKLOG.md:2473 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:08:41Z): + +Line starts with a `+` inside a wrapped list item (`" + agent-comment preservation"`), which Markdown parses as a nested bullet and markdownlint (MD004/ul-style) will flag. Reword to avoid a line-start `+` (e.g., use “and/plus”, or move the `+` to the end of the previous line). +```suggestion + close-the-PR cycle (CI-failures, review-conversations, + agent-comment preservation, and auto-merge arm) so Otto +``` diff --git a/tools/pr-preservation/README.md b/tools/pr-preservation/README.md new file mode 100644 index 00000000..07e86315 --- /dev/null +++ b/tools/pr-preservation/README.md @@ -0,0 +1,116 @@ +# tools/pr-preservation/ — git-native PR conversation archive + +Minimal implementation (Otto-207) of the PR-preservation +BACKLOG directive (Otto-150..154, PR #335). Fetches a PR's +review threads, reviews, and general comments via +`gh api graphql` and writes them to +`docs/pr-discussions/PR--.md` for durable +audit-trail storage outside of GitHub. + +## Scope of this Phase-0 / minimal tool + +- **In scope:** one-shot local script; operator runs it + manually against a PR number; output is a single + markdown file with YAML frontmatter + sectioned + content (reviews / threads / comments). +- **Out of scope:** GHA workflow (automatic on every + merge); historical backfill of all PRs ever; edit- + after-archive reconciliation; redaction layer for + privacy-sensitive comments. + +The out-of-scope items are tracked in the PR-preservation +BACKLOG row (originally PR #335 Otto-150..154; updated +this tick with Phase-0-shipped state + remaining phases). + +## Usage + +```bash +tools/pr-preservation/archive-pr.sh +``` + +Writes `docs/pr-discussions/PR--.md`. Re-running +overwrites the file with current PR state. + +## Output schema + +Each archive file has YAML frontmatter: + +```yaml +pr_number: 354 +title: "..." +author: +state: OPEN | MERGED | CLOSED +created_at: ISO-8601 +merged_at: ISO-8601 (if merged) +closed_at: ISO-8601 (if closed) +head_ref: +base_ref: main +archived_at: ISO-8601 (when this archive was written) +archive_tool: tools/pr-preservation/archive-pr.sh +``` + +Followed by markdown sections: + +- `## PR description` — original PR body +- `## Reviews` — top-level approvals / requests-changes / + comment-reviews per author +- `## Review threads` — inline code-comment threads with + resolved/unresolved status + full comment chain +- `## General comments` — non-review PR comments + +## Backfill status (Otto-207) + +Backfilled 10 PRs from this session: + +- PR #354 (backlog-split Phase 1a) +- PR #352 (Server Meshing + SpacetimeDB research row) +- PR #336 (KSK naming definition doc) +- PR #342 (calibration-harness Stage-2 design) — merged +- PR #344 (Amara 19th ferry absorb) — merged +- PR #346 (DST compliance criteria) — merged +- PR #350 (Frontier rename pass-2) — merged +- PR #353 (BACKLOG split Phase 0 design) — merged +- PR #355 (Codex first peer-agent deep-review absorb) — + merged +- PR #356 (PR-resolve-loop skill row) — merged + +Future backfill waves lift this list to "all merged PRs +through " then progressively older. + +## Long-term plan + +Tracked on the PR-preservation BACKLOG row. Phases: + +- **Phase 0 (this tool):** shipped. Operator-run one-shot. +- **Phase 1 — GHA workflow on merge:** automatic archive + on every PR merge. Deferred pending maintainer sign-off + on privacy/redaction policy. +- **Phase 2 — historical backfill:** walk all merged PRs + chronologically and archive; land as a series of batch + PRs to keep each archive-PR reviewable. +- **Phase 3 — reconciliation:** when a thread is edited + after archive, detect drift and re-archive; GHA cron + weekly. +- **Phase 4 — redaction layer:** bot-review comments + (Copilot, Codex) archive verbatim; human-reviewer + comments get a privacy-pass step. Scope open. + +## Dependencies + +- `gh` CLI authenticated +- `python3` (stdlib only) +- `bash 4+`, POSIX `mktemp` + +No external Python packages; no `yq` required. + +## Cross-references + +- PR-preservation BACKLOG row (PR #335, Otto-150..154) — + the phased plan this tool begins to execute. +- Otto-171 queue-saturation memory — adjacent discipline + (active PR management). +- Otto-204 PR-resolve-loop BACKLOG row (PR #356 merged) — + this archive tool is step 4 of that skill's 6-step + cycle. +- Otto-204c livelock-diagnosis memory — the failure mode + that made this preservation gap visible. diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh new file mode 100755 index 00000000..78023cad --- /dev/null +++ b/tools/pr-preservation/archive-pr.sh @@ -0,0 +1,198 @@ +#!/usr/bin/env bash +# tools/pr-preservation/archive-pr.sh +# +# Minimal git-native PR-conversation preservation (Otto-207). +# Fetches a PR's review threads + general comments + reviews +# via `gh api graphql` and writes them to +# `docs/pr-discussions/PR--.md` for audit trail +# outside of GitHub. +# +# Addresses the gap identified Otto-207: PR-preservation +# BACKLOG row (Otto-150..154, PR #335) specifies the +# discipline but never shipped the capture tooling. This +# script is the minimal viable implementation; scales up +# to a GHA workflow later. +# +# Usage: +# tools/pr-preservation/archive-pr.sh +# +# Output: writes docs/pr-discussions/PR--.md with +# YAML frontmatter (pr_number, title, author, merged_at, +# state, archived_at) + all review threads + reviews + +# general PR comments. +# +# Exit codes: +# 0 success +# 1 missing arg / gh CLI not authenticated +# 2 PR fetch failed + +set -euo pipefail + +if [ $# -lt 1 ]; then + echo "usage: $0 " >&2 + exit 1 +fi + +PR="$1" +REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" +export REPO_ROOT PR +OUT_DIR="${REPO_ROOT}/docs/pr-discussions" +mkdir -p "$OUT_DIR" + +TMP="$(mktemp)" +trap 'rm -f "$TMP"' EXIT + +gh api graphql -F number="$PR" -f query=' + query($number: Int!) { + repository(owner: "Lucent-Financial-Group", name: "Zeta") { + pullRequest(number: $number) { + number + title + author { login } + state + createdAt + mergedAt + closedAt + headRefName + baseRefName + body + reviewThreads(first: 100) { + nodes { + id + isResolved + path + line + originalLine + comments(first: 50) { + nodes { + author { login } + body + createdAt + updatedAt + } + } + } + } + reviews(first: 50) { + nodes { + author { login } + state + body + submittedAt + } + } + comments(first: 100) { + nodes { + author { login } + body + createdAt + } + } + } + } + } +' > "$TMP" + +if ! python3 -c "import json; json.load(open('$TMP'))" 2>/dev/null; then + echo "fetch failed for PR #$PR:" >&2 + head -20 "$TMP" >&2 + exit 2 +fi + +PR_JSON_PATH="$TMP" python3 <<'PY' +import json, os, re, datetime + +with open(os.environ['PR_JSON_PATH']) as f: + d = json.load(f) +pr = d['data']['repository']['pullRequest'] + +title = pr.get('title', 'untitled') +number = pr.get('number') + +slug = re.sub(r'[^a-zA-Z0-9]+', '-', title).strip('-').lower() +slug = slug[:60].strip('-') or 'untitled' + +out_dir = os.path.join(os.environ['REPO_ROOT'], 'docs', 'pr-discussions') +os.makedirs(out_dir, exist_ok=True) +path = os.path.join(out_dir, f'PR-{number:04d}-{slug}.md') + +archived_at = datetime.datetime.utcnow().isoformat(timespec='seconds') + 'Z' + +lines = [] +lines.append('---') +lines.append(f'pr_number: {number}') +lines.append(f'title: {json.dumps(title)}') +lines.append(f'author: {(pr.get("author") or {}).get("login") or "unknown"}') +lines.append(f'state: {pr.get("state")}') +lines.append(f'created_at: {pr.get("createdAt") or ""}') +if pr.get('mergedAt'): + lines.append(f'merged_at: {pr.get("mergedAt")}') +if pr.get('closedAt'): + lines.append(f'closed_at: {pr.get("closedAt")}') +lines.append(f'head_ref: {pr.get("headRefName") or ""}') +lines.append(f'base_ref: {pr.get("baseRefName") or ""}') +lines.append(f'archived_at: {archived_at}') +lines.append('archive_tool: tools/pr-preservation/archive-pr.sh') +lines.append('---') +lines.append('') +lines.append(f'# PR #{number}: {title}') +lines.append('') + +body = pr.get('body') or '' +if body.strip(): + lines.append('## PR description') + lines.append('') + lines.append(body.rstrip()) + lines.append('') + +reviews = (pr.get('reviews') or {}).get('nodes', []) +if reviews: + lines.append('## Reviews') + lines.append('') + for r in reviews: + author = (r.get('author') or {}).get('login') or 'unknown' + state = r.get('state') or 'COMMENTED' + submitted = r.get('submittedAt') or '' + body_text = (r.get('body') or '').strip() + lines.append(f'### {state} — @{author} ({submitted})') + lines.append('') + lines.append(body_text if body_text else '_(no body)_') + lines.append('') + +threads = (pr.get('reviewThreads') or {}).get('nodes', []) +if threads: + lines.append('## Review threads') + lines.append('') + for i, t in enumerate(threads, 1): + path_ref = t.get('path') or '(no path)' + line_num = t.get('line') or t.get('originalLine') or '?' + resolved = 'resolved' if t.get('isResolved') else 'unresolved' + lines.append(f'### Thread {i}: {path_ref}:{line_num} ({resolved})') + lines.append('') + for c in (t.get('comments') or {}).get('nodes', []): + author = (c.get('author') or {}).get('login') or 'unknown' + when = c.get('createdAt') or '' + body_text = (c.get('body') or '').strip() + lines.append(f'**@{author}** ({when}):') + lines.append('') + lines.append(body_text) + lines.append('') + +comments = (pr.get('comments') or {}).get('nodes', []) +if comments: + lines.append('## General comments') + lines.append('') + for c in comments: + author = (c.get('author') or {}).get('login') or 'unknown' + when = c.get('createdAt') or '' + body_text = (c.get('body') or '').strip() + lines.append(f'### @{author} ({when})') + lines.append('') + lines.append(body_text) + lines.append('') + +content = '\n'.join(lines).rstrip() + '\n' +with open(path, 'w', encoding='utf-8') as f: + f.write(content) +print(f'wrote {path} ({len(content)} bytes, {len(threads)} threads, {len(reviews)} reviews, {len(comments)} comments)') +PY From c2b6018fffe0c187e9dcbeccf0c9f144cf18b5e4 Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 07:26:28 -0400 Subject: [PATCH 02/10] =?UTF-8?q?fix(#357):=20bot=E2=86=92agent=20terminol?= =?UTF-8?q?ogy=20per=20GOVERNANCE=20=C2=A73=20(maintainer=20Otto-208)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer Otto-208 flag on Phase 4 redaction-layer wording: "No redaction — bot content + human content ... bot=agent." Applied the Otto-156 pattern: Copilot + Codex + Claude Code personas + github-actions are AGENTS with agency and accountability (GOVERNANCE §3 + CLAUDE.md "Agents, not bots."). Updated Phase 4 wording: - "bot-review comments (Copilot, Codex) archive verbatim" → "agent-review comments (Copilot, Codex, Claude Code personas, github-actions) archive verbatim" - Added explicit pointer to GOVERNANCE §3 + CLAUDE.md terminology convention. PR body edit follows separately via `gh pr edit`. Co-Authored-By: Claude Opus 4.7 --- tools/pr-preservation/README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/pr-preservation/README.md b/tools/pr-preservation/README.md index 07e86315..e4e43679 100644 --- a/tools/pr-preservation/README.md +++ b/tools/pr-preservation/README.md @@ -91,9 +91,13 @@ Tracked on the PR-preservation BACKLOG row. Phases: - **Phase 3 — reconciliation:** when a thread is edited after archive, detect drift and re-archive; GHA cron weekly. -- **Phase 4 — redaction layer:** bot-review comments - (Copilot, Codex) archive verbatim; human-reviewer - comments get a privacy-pass step. Scope open. +- **Phase 4 — redaction layer:** agent-review comments + (Copilot, Codex, Claude Code personas, github-actions) + archive verbatim; human-reviewer comments get a + privacy-pass step. Scope open. Terminology per + GOVERNANCE §3 + CLAUDE.md "agents, not bots" — Copilot + and Codex are agents with agency and accountability, + not bots. ## Dependencies From 6f0737712b2ec0b71c281a53efe40a6cfb734198 Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 09:21:46 -0400 Subject: [PATCH 03/10] =?UTF-8?q?fix(#357):=209=20review=20threads=20?= =?UTF-8?q?=E2=80=94=20paginate,=20null-check,=20dynamic=20repo,=20YAML=20?= =?UTF-8?q?quoting,=20README=20alignment,=20trailing-ws=20strip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all 9 unresolved Copilot + Codex threads on PR #357 (Otto-226 review-drain discipline, three-outcome model: fix). Script changes (tools/pr-preservation/archive-pr.sh): - Paginate reviewThreads / reviews / comments at the top level AND per-thread comments via cursor loops (threads 1 + 6 — no silent truncation). - Validate `pullRequest != null` and detect top-level GraphQL `errors` before dereferencing (threads 2 + 4). - Capture `gh api graphql` exit code explicitly instead of letting `set -e` swallow the diagnostic path (thread 3). - Derive owner/name dynamically from `gh repo view --json nameWithOwner` with a hard-fail if detection fails — works from forks and after rename (thread 5). - Quote all YAML frontmatter string values via `json.dumps` (title / author / state / ISO timestamps / head_ref / base_ref / archived_at / archive_tool), so refs with `#` or `:` don't break parsing (thread 7). Documentation alignment: - README now shows zero-padded filename shape `PR--.md` (e.g. `PR-0357-...`) matching the script's output (thread 8 — chose "align README to script" since the 10 backfilled files already use the zero-padded form and renaming them would churn links). - README notes pagination is in place (no more "may be truncated" silent-behaviour gap tied to thread 6). Backfilled archives: - Stripped trailing whitespace across all existing `docs/pr-discussions/*.md` via `perl -i -pe 's/[ \t]+$//'` (thread 9 — MD009 compliance for the CI markdownlint gate). Also adds `docs/pr-discussions/PR-0357-...md` as the self-hosting smoke test: the archive tool successfully drains its own review queue. Validation: - `bash -n` clean - `shellcheck` clean (no findings) - End-to-end: `./tools/pr-preservation/archive-pr.sh 357` writes 9 threads / 2 reviews / 0 comments to 12179 bytes - Error path: PR #99999 exits 2 with clear diagnostic Co-Authored-By: Claude Opus 4.7 --- ...pass-2-hindu-ff7-egyptian-greek-norse-s.md | 4 +- ...er-meshing-spacetimedb-deep-research-ga.md | 8 +- ...hase-1a-generator-schema-example-row-aa.md | 8 +- ...-minimal-archive-otto-207-session-backf.md | 225 +++++++++++++ tools/pr-preservation/README.md | 11 +- tools/pr-preservation/archive-pr.sh | 316 ++++++++++++++---- 6 files changed, 501 insertions(+), 71 deletions(-) create mode 100644 docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md diff --git a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md index c4b99dd9..6b1ff018 100644 --- a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md +++ b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md @@ -59,7 +59,7 @@ Pass-2 doc under `docs/research/`. Scientology row at END of P2 research-grade s Here are some automated review suggestions for this pull request. **Reviewed commit:** `40c8793eea` - +
ℹ️ About Codex in GitHub
@@ -75,7 +75,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:08:03Z) diff --git a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md index c2fd4509..fd4f8f93 100644 --- a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md +++ b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md @@ -64,7 +64,7 @@ P2 research-grade at line ~921 (end of "## P2 — Post-v1 query-surface research Here are some automated review suggestions for this pull request. **Reviewed commit:** `55ce0fac69` - +
ℹ️ About Codex in GitHub
@@ -80,7 +80,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:18:05Z) @@ -119,7 +119,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `44825a88fc` - +
ℹ️ About Codex in GitHub
@@ -135,7 +135,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
## Review threads diff --git a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md index a7c5c834..f9781737 100644 --- a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md +++ b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md @@ -80,7 +80,7 @@ Aaron Otto-182: *"can you ask codex too?"* — inviting `@codex review` below. Here are some automated review suggestions for this pull request. **Reviewed commit:** `6a447ee234` - +
ℹ️ About Codex in GitHub
@@ -96,7 +96,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:31:29Z) @@ -176,7 +176,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `9bf3d09ff1` - +
ℹ️ About Codex in GitHub
@@ -192,7 +192,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:06:51Z) diff --git a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md new file mode 100644 index 00000000..2f5961b1 --- /dev/null +++ b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md @@ -0,0 +1,225 @@ +--- +pr_number: 357 +title: "tools: PR-preservation minimal archive + Otto-207 session backfill (10 PRs)" +author: "AceHack" +state: "OPEN" +created_at: "2026-04-24T11:23:49Z" +head_ref: "tools/pr-preservation-phase-2-minimal" +base_ref: "main" +archived_at: "2026-04-24T13:21:06Z" +archive_tool: "tools/pr-preservation/archive-pr.sh" +--- + +# PR #357: tools: PR-preservation minimal archive + Otto-207 session backfill (10 PRs) + +## PR description + +## Summary + +Maintainer Otto-207: *"are we saving these yet gitnative and have we backfilled them yet?"* + +Honest answer was NO. The PR-preservation BACKLOG row (Otto-150..154, PR #335 in queue elevating to P1 + phased plan) specifies the discipline but never shipped capture tooling. This PR ships **Phase 0 minimal viable implementation** + **backfills 10 PRs** from this session. + +## Tool + +`tools/pr-preservation/archive-pr.sh` — one-shot bash script: + +- Fetches review threads + reviews + comments via `gh api graphql` +- Writes `docs/pr-discussions/PR--.md` with YAML frontmatter +- Sections: PR description · Reviews · Review threads (with resolved/unresolved) · General comments +- No external deps beyond `gh` + `python3` stdlib + `bash 4+` + +## Backfill (10 PRs this session) + +| PR | Status | Threads | Reviews | Comments | +|---|---|---|---|---| +| #354 backlog-split Phase 1a | OPEN | 20 | 16 | 1 | +| #352 Server Meshing research | OPEN | 6 | 8 | 0 | +| #336 KSK naming doc | OPEN | 8 | 8 | 1 | +| #342 calibration-harness design | MERGED | 5 | 1 | 1 | +| #344 Amara 19th ferry absorb | MERGED | 8 | 1 | 1 | +| #346 DST compliance criteria | MERGED | 5 | 1 | 1 | +| #350 Frontier rename pass-2 | MERGED | 4 | 2 | 0 | +| #353 BACKLOG split design | MERGED | 6 | 1 | 0 | +| #355 Codex peer-review absorb | MERGED | 5 | 1 | 1 | +| #356 PR-resolve-loop row | MERGED | 5 | 1 | 0 | + +Total: 72 threads + 40 reviews + 6 comments across ~97KB markdown. + +## Long-term plan (per maintainer directive) + +Remaining phases kept in the PR-preservation BACKLOG row (PR #335 in queue): + +- **Phase 1** — GHA workflow on merge (automatic archive) +- **Phase 2** — historical backfill (all merged PRs) +- **Phase 3** — reconciliation (drift detection) +- **Phase 4** — redaction layer (privacy-pass for human-reviewer comments) + +Scope out of this PR per maintainer *"make sure you backlog then to a proper long term solution"*. + +## Composes with + +- Otto-204c livelock-diagnosis (the gap this closes part of) +- Otto-204 PR-resolve-loop skill (this is step 4 of its 6-step cycle) +- PR #335 PR-preservation elevation (authoritative phased plan) + +## Test plan + +- [x] Script runs cleanly on test PR (#354) +- [x] Shellcheck exit 0 (SC2016 single-quote in GraphQL is intentional) +- [x] Output format validated across 10 varied PRs +- [x] No external dependencies beyond gh CLI + python3 stdlib + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +## Reviews + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T11:25:56Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `cc217ae031` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:28:45Z) + +## Pull request overview + +Adds a minimal, git-tracked PR-conversation preservation tool (`tools/pr-preservation/archive-pr.sh`) and backfills 10 PR discussion archives into `docs/pr-discussions/`, aligning with the project’s “git-native preservation” direction. + +**Changes:** +- Add a one-shot bash + `gh api graphql` + Python-stdlib script to export PR metadata, reviews, review threads, and general comments into markdown files under `docs/pr-discussions/`. +- Add usage + output-schema documentation for the preservation tool. +- Commit 10 backfilled PR archive markdown files for this session. + +### Reviewed changes + +Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments. + +
+Show a summary per file + +| File | Description | +| ---- | ----------- | +| tools/pr-preservation/README.md | Documents scope, usage, and the intended archive schema/location for PR preservation. | +| tools/pr-preservation/archive-pr.sh | Implements the PR fetch + markdown archive writer via GitHub GraphQL + Python formatting. | +| docs/pr-discussions/PR-0356-backlog-otto-204-pr-resolve-loop-skill-close-the-pr-cycle-au.md | Backfilled archive for PR #356 discussion content. | +| docs/pr-discussions/PR-0355-ferry-codex-first-completed-peer-agent-deep-review-absorb-4.md | Backfilled archive for PR #355 discussion content. | +| docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md | Backfilled archive for PR #354 discussion content. | +| docs/pr-discussions/PR-0353-docs-backlog-md-split-design-phase-0-aaron-otto-181-3rd-ask.md | Backfilled archive for PR #353 discussion content. | +| docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md | Backfilled archive for PR #352 discussion content. | +| docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md | Backfilled archive for PR #350 discussion content. | +| docs/pr-discussions/PR-0346-docs-dst-compliance-criteria-dst-held-foundationdb-grade-bar.md | Backfilled archive for PR #346 discussion content. | +| docs/pr-discussions/PR-0344-ferry-amara-19th-absorb-dst-audit-5-5-corrections-10-tracked.md | Backfilled archive for PR #344 discussion content. | +| docs/pr-discussions/PR-0342-docs-calibration-harness-stage-2-design-amara-18th-ferry-b-f.md | Backfilled archive for PR #342 discussion content. | +| docs/pr-discussions/PR-0336-docs-ksk-naming-definition-doc-canonical-expansion-locked-ot.md | Backfilled archive for PR #336 discussion content. | +
+ +## Review threads + +### Thread 1: tools/pr-preservation/archive-pr.sh:60 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T11:25:56Z): + +**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Paginate GraphQL connections before writing archive** + +The archive query hard-limits review threads and nested comments (`first: 100` / `first: 50`) without checking `pageInfo.hasNextPage` or following cursors, so high-activity PRs are silently truncated in the generated markdown. This breaks the stated preservation/audit goal because archives can look complete while dropping part of the discussion; the same pattern appears for reviews and general comments later in the query as well. Please paginate each connection (or fail loudly when truncated) before emitting the archive file. + +Useful? React with 👍 / 👎. + +### Thread 2: tools/pr-preservation/archive-pr.sh:107 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T11:25:56Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Validate pullRequest exists before dereferencing fields** + +The script assumes `d['data']['repository']['pullRequest']` is always present and immediately calls `pr.get(...)`, but GitHub GraphQL can return valid JSON with `pullRequest: null` (for example, an invalid PR number or missing access). In that case this path crashes with a Python traceback instead of the script's documented fetch-failure handling, making operational failures noisy and non-actionable for operators. + +Useful? React with 👍 / 👎. + +### Thread 3: tools/pr-preservation/archive-pr.sh:49 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): + +P0: With `set -e`, if `gh api graphql` exits non-zero (auth failure, network error), the script aborts immediately and never reaches the intended “fetch failed …” diagnostics / exit-code mapping. Wrap the `gh api graphql … > "$TMP"` call in an explicit error-handling block so failures produce a clear message and the documented exit code (2). + +### Thread 4: tools/pr-preservation/archive-pr.sh:108 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): + +P0: The JSON check only validates that the response is parseable JSON; GraphQL error payloads (or `pullRequest: null`) will pass this check and then the Python block will crash on `d['data']...` / `pr.get(...)` with a stack trace. Detect `errors` in the response and/or a null `pullRequest` and exit 2 with a concise error message (e.g., not found / permission / auth). + +### Thread 5: tools/pr-preservation/archive-pr.sh:47 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): + +P1: The GraphQL query hard-codes `repository(owner: "Lucent-Financial-Group", name: "Zeta")`, so the script won’t work when run from forks or after a repo transfer/rename. Consider deriving owner/name from `gh repo view --json owner,name` (or `gh repo set-default`) and pass them as GraphQL variables. +```suggestion +OWNER="$(gh repo view --json owner --jq '.owner.login')" +REPO_NAME="$(gh repo view --json name --jq '.name')" + +TMP="$(mktemp)" +trap 'rm -f "$TMP"' EXIT + +gh api graphql -F owner="$OWNER" -F name="$REPO_NAME" -F number="$PR" -f query=' + query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { +``` + +### Thread 6: tools/pr-preservation/archive-pr.sh:90 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): + +P1: The script and header comment say it writes “all review threads + reviews + comments”, but the GraphQL query is capped at `reviewThreads(first: 100)`, per-thread `comments(first: 50)`, `reviews(first: 50)`, and `comments(first: 100)` with no pagination. Either implement pagination (pageInfo/endCursor loops) or record in the output/frontmatter that results may be truncated at these limits. + +### Thread 7: tools/pr-preservation/archive-pr.sh:134 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): + +P1: YAML frontmatter values like `author`, `head_ref`, and `base_ref` are emitted without quoting/escaping. Some valid ref names can include `#` or `:` which would break YAML parsing (treating the remainder as a comment or creating a mapping). Emit these scalars using a safe quoting routine (e.g., JSON-style quoting) for all string fields, not just `title`. +```suggestion +lines.append(f'author: {json.dumps((pr.get("author") or {}).get("login") or "unknown")}') +lines.append(f'state: {json.dumps(pr.get("state") or "")}') +lines.append(f'created_at: {json.dumps(pr.get("createdAt") or "")}') +if pr.get('mergedAt'): + lines.append(f'merged_at: {json.dumps(pr.get("mergedAt") or "")}') +if pr.get('closedAt'): + lines.append(f'closed_at: {json.dumps(pr.get("closedAt") or "")}') +lines.append(f'head_ref: {json.dumps(pr.get("headRefName") or "")}') +lines.append(f'base_ref: {json.dumps(pr.get("baseRefName") or "")}') +lines.append(f'archived_at: {json.dumps(archived_at)}') +``` + +### Thread 8: tools/pr-preservation/README.md:33 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): + +P0: README says archives are written to `docs/pr-discussions/PR--.md`, but the script actually writes `PR-` zero-padded to 4 digits (e.g., `PR-0356-...`). Update the documented filename pattern so operators can reliably locate files and avoid accidental duplicates. + +### Thread 9: docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md:68 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): + +P0: The backfilled `docs/pr-discussions/*.md` files include whitespace-only lines (e.g., after “Reviewed commit”) which violate markdownlint MD009 (no trailing spaces). Since the CI job runs markdownlint over `**/*.md` and `docs/pr-discussions/**` isn’t ignored, this will likely fail CI unless the archive output is normalized or the directory is added to `.markdownlint-cli2.jsonc` ignores (similar to `docs/amara-full-conversation/**`). +```suggestion + +``` diff --git a/tools/pr-preservation/README.md b/tools/pr-preservation/README.md index e4e43679..1bf3e58e 100644 --- a/tools/pr-preservation/README.md +++ b/tools/pr-preservation/README.md @@ -28,9 +28,18 @@ this tick with Phase-0-shipped state + remaining phases). tools/pr-preservation/archive-pr.sh ``` -Writes `docs/pr-discussions/PR--.md`. Re-running +Writes `docs/pr-discussions/PR--.md` — the PR +number is zero-padded to four digits (e.g. `PR-0357-...`) +so archives sort lexicographically in the same order as +they sort numerically up to PR #9999. Re-running overwrites the file with current PR state. +The archive tool paginates all three connections +(`reviewThreads`, `reviews`, `comments`) plus per-thread +comments, so PRs with more than 100 threads or threads +with more than 100 comments are captured in full rather +than silently truncated. + ## Output schema Each archive file has YAML frontmatter: diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index 78023cad..fb4a84b4 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -19,12 +19,26 @@ # Output: writes docs/pr-discussions/PR--.md with # YAML frontmatter (pr_number, title, author, merged_at, # state, archived_at) + all review threads + reviews + -# general PR comments. +# general PR comments. PR numbers are zero-padded to four +# digits in the filename (e.g. PR-0357-...) so archives +# sort lexicographically in the same order as they sort +# numerically up to PR #9999. # # Exit codes: # 0 success -# 1 missing arg / gh CLI not authenticated -# 2 PR fetch failed +# 1 missing arg / gh CLI not authenticated / repo detect failed +# 2 PR fetch failed (auth / network / GraphQL errors / not found) +# +# Review-thread drain Otto-226 fixes (PR #357): +# - Pagination + truncation warning for threads (>100) and +# per-thread comments (>100). +# - Top-level `errors` and `pullRequest: null` detection +# before dereferencing (both Python and shell paths). +# - Dynamic owner/name from `gh repo view` (works from +# forks or after a rename). +# - `set -e`-safe capture of `gh api graphql` exit code +# so "fetch failed" diagnostics actually print. +# - YAML string values quoted (refs can contain `#` / `:`). set -euo pipefail @@ -35,76 +49,247 @@ fi PR="$1" REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" -export REPO_ROOT PR + +# Dynamic owner / name — works from forks or after rename. +# Falls back to Lucent-Financial-Group/Zeta only if the +# detection command itself fails (e.g. not inside a repo +# gh recognises). Never silently wrong. +REPO_NWO="$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null || true)" +if [ -z "${REPO_NWO}" ]; then + echo "error: could not detect repo via 'gh repo view'. Is gh authenticated and this a GitHub repo?" >&2 + exit 1 +fi +REPO_OWNER="${REPO_NWO%/*}" +REPO_NAME="${REPO_NWO#*/}" + +export REPO_ROOT PR REPO_OWNER REPO_NAME OUT_DIR="${REPO_ROOT}/docs/pr-discussions" mkdir -p "$OUT_DIR" TMP="$(mktemp)" trap 'rm -f "$TMP"' EXIT -gh api graphql -F number="$PR" -f query=' - query($number: Int!) { - repository(owner: "Lucent-Financial-Group", name: "Zeta") { - pullRequest(number: $number) { - number - title - author { login } - state - createdAt - mergedAt - closedAt - headRefName - baseRefName - body - reviewThreads(first: 100) { - nodes { - id - isResolved - path - line - originalLine - comments(first: 50) { - nodes { - author { login } - body - createdAt - updatedAt - } +# Paginated fetch: drive it from Python (single interpreter, +# single GraphQL query with cursors). This keeps the shell +# simple and lets us both paginate and validate top-level +# `errors` / `pullRequest: null` before proceeding. +# +# We still capture a shell-side exit code so `set -e` does +# not swallow the "fetch failed" diagnostic path. +set +e +REPO_OWNER="$REPO_OWNER" REPO_NAME="$REPO_NAME" PR="$PR" python3 - <<'PY' > "$TMP" +import json, os, subprocess, sys + +OWNER = os.environ['REPO_OWNER'] +NAME = os.environ['REPO_NAME'] +PR = int(os.environ['PR']) + +QUERY = """ +query($owner: String!, $name: String!, $number: Int!, + $threadsAfter: String, $commentsAfter: String, $reviewsAfter: String) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + number + title + author { login } + state + createdAt + mergedAt + closedAt + headRefName + baseRefName + body + reviewThreads(first: 100, after: $threadsAfter) { + pageInfo { hasNextPage endCursor } + nodes { + id + isResolved + path + line + originalLine + comments(first: 100) { + pageInfo { hasNextPage endCursor } + nodes { + author { login } + body + createdAt + updatedAt } } } - reviews(first: 50) { - nodes { - author { login } - state - body - submittedAt - } + } + reviews(first: 50, after: $reviewsAfter) { + pageInfo { hasNextPage endCursor } + nodes { + author { login } + state + body + submittedAt } - comments(first: 100) { - nodes { - author { login } - body - createdAt - } + } + comments(first: 100, after: $commentsAfter) { + pageInfo { hasNextPage endCursor } + nodes { + author { login } + body + createdAt + } + } + } + } +} +""" + +THREAD_COMMENTS_QUERY = """ +query($threadId: ID!, $after: String) { + node(id: $threadId) { + ... on PullRequestReviewThread { + comments(first: 100, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { + author { login } + body + createdAt + updatedAt } } } } -' > "$TMP" +} +""" -if ! python3 -c "import json; json.load(open('$TMP'))" 2>/dev/null; then - echo "fetch failed for PR #$PR:" >&2 +def gh_graphql(query, variables): + """Invoke gh api graphql and return parsed JSON or raise.""" + cmd = ["gh", "api", "graphql", "-f", f"query={query}"] + for k, v in variables.items(): + if v is None: + # gh treats -F with empty string as null; for explicit null + # we omit — GraphQL default for unspecified variable is null. + continue + if isinstance(v, int): + cmd.extend(["-F", f"{k}={v}"]) + else: + cmd.extend(["-f", f"{k}={v}"]) + proc = subprocess.run(cmd, capture_output=True, text=True) + if proc.returncode != 0: + sys.stderr.write( + f"gh api graphql failed (exit {proc.returncode}):\n{proc.stderr}\n" + ) + sys.exit(2) + try: + data = json.loads(proc.stdout) + except json.JSONDecodeError as e: + sys.stderr.write(f"non-JSON response from gh api graphql: {e}\n") + sys.stderr.write(proc.stdout[:2000] + "\n") + sys.exit(2) + if data.get("errors"): + sys.stderr.write("GraphQL errors:\n") + sys.stderr.write(json.dumps(data["errors"], indent=2) + "\n") + sys.exit(2) + return data + +# First page. +first = gh_graphql(QUERY, { + "owner": OWNER, "name": NAME, "number": PR, +}) +repo = (first.get("data") or {}).get("repository") or {} +pr = repo.get("pullRequest") +if pr is None: + sys.stderr.write( + f"pullRequest is null for {OWNER}/{NAME}#{PR} " + "(not found, private, or access denied).\n" + ) + sys.exit(2) + +# Accumulate nodes across pages. Simple loop per connection. +def paginate_top_level(key_chain, variable_name): + """Walk connections rooted at pullRequest..""" + all_nodes = list(pr[key_chain]["nodes"]) + page = pr[key_chain]["pageInfo"] + cursor = page["endCursor"] if page["hasNextPage"] else None + while cursor: + vars_ = {"owner": OWNER, "name": NAME, "number": PR, variable_name: cursor} + page_data = gh_graphql(QUERY, vars_) + page_pr = ( + (page_data.get("data") or {}).get("repository") or {} + ).get("pullRequest") + if page_pr is None: + break + conn = page_pr[key_chain] + all_nodes.extend(conn["nodes"]) + page = conn["pageInfo"] + cursor = page["endCursor"] if page["hasNextPage"] else None + return all_nodes + +threads = paginate_top_level("reviewThreads", "threadsAfter") +reviews = paginate_top_level("reviews", "reviewsAfter") +comments = paginate_top_level("comments", "commentsAfter") + +# For each thread, paginate its comments connection. +for t in threads: + conn = t.get("comments") or {} + nodes = list(conn.get("nodes") or []) + pi = conn.get("pageInfo") or {} + cursor = pi.get("endCursor") if pi.get("hasNextPage") else None + while cursor: + page = gh_graphql(THREAD_COMMENTS_QUERY, {"threadId": t["id"], "after": cursor}) + node = (page.get("data") or {}).get("node") or {} + cc = node.get("comments") or {} + nodes.extend(cc.get("nodes") or []) + pi2 = cc.get("pageInfo") or {} + cursor = pi2.get("endCursor") if pi2.get("hasNextPage") else None + t["comments"] = {"nodes": nodes} + +# Reshape into the same envelope older code expects. +pr["reviewThreads"] = {"nodes": threads} +pr["reviews"] = {"nodes": reviews} +pr["comments"] = {"nodes": comments} + +json.dump({"data": {"repository": {"pullRequest": pr}}}, sys.stdout) +PY +FETCH_RC=$? +set -e + +if [ "$FETCH_RC" -ne 0 ]; then + echo "fetch failed for PR #$PR (exit $FETCH_RC)" >&2 + exit 2 +fi + +export TMP + +# Validate JSON parseability AND that pullRequest is non-null. +# Guards against malformed output or upstream GraphQL nulls +# that would otherwise crash the formatter with a cryptic +# TypeError. TMP must be exported (above) so the heredoc-run +# Python interpreter sees it. +if ! TMP="$TMP" python3 - <<'PY' +import json, os, sys +with open(os.environ['TMP']) as f: + d = json.load(f) +pr = ((d.get('data') or {}).get('repository') or {}).get('pullRequest') +if pr is None: + sys.exit(2) +PY +then + echo "fetch failed for PR #$PR (invalid JSON or pullRequest: null):" >&2 head -20 "$TMP" >&2 exit 2 fi PR_JSON_PATH="$TMP" python3 <<'PY' -import json, os, re, datetime +import json, os, re, sys, datetime with open(os.environ['PR_JSON_PATH']) as f: d = json.load(f) -pr = d['data']['repository']['pullRequest'] + +# Defensive: we already validated upstream, but keep the +# guards so this script is also safe to re-run against a +# saved TMP file. +repo = (d.get('data') or {}).get('repository') or {} +pr = repo.get('pullRequest') +if pr is None: + sys.stderr.write("pullRequest missing in JSON — aborting formatter.\n") + sys.exit(2) title = pr.get('title', 'untitled') number = pr.get('number') @@ -114,25 +299,33 @@ slug = slug[:60].strip('-') or 'untitled' out_dir = os.path.join(os.environ['REPO_ROOT'], 'docs', 'pr-discussions') os.makedirs(out_dir, exist_ok=True) +# Zero-pad to 4 digits — aligns with README's documented +# filename shape (e.g. PR-0357-...). Sorts lexicographically +# == numerically for PR #0001..#9999. path = os.path.join(out_dir, f'PR-{number:04d}-{slug}.md') archived_at = datetime.datetime.utcnow().isoformat(timespec='seconds') + 'Z' +def yaml_quote(s): + """Quote YAML string values safely. json.dumps gives us + double-quoted strings with escaping; valid YAML too.""" + return json.dumps('' if s is None else str(s)) + lines = [] lines.append('---') lines.append(f'pr_number: {number}') -lines.append(f'title: {json.dumps(title)}') -lines.append(f'author: {(pr.get("author") or {}).get("login") or "unknown"}') -lines.append(f'state: {pr.get("state")}') -lines.append(f'created_at: {pr.get("createdAt") or ""}') +lines.append(f'title: {yaml_quote(title)}') +lines.append(f'author: {yaml_quote((pr.get("author") or {}).get("login") or "unknown")}') +lines.append(f'state: {yaml_quote(pr.get("state"))}') +lines.append(f'created_at: {yaml_quote(pr.get("createdAt") or "")}') if pr.get('mergedAt'): - lines.append(f'merged_at: {pr.get("mergedAt")}') + lines.append(f'merged_at: {yaml_quote(pr.get("mergedAt"))}') if pr.get('closedAt'): - lines.append(f'closed_at: {pr.get("closedAt")}') -lines.append(f'head_ref: {pr.get("headRefName") or ""}') -lines.append(f'base_ref: {pr.get("baseRefName") or ""}') -lines.append(f'archived_at: {archived_at}') -lines.append('archive_tool: tools/pr-preservation/archive-pr.sh') + lines.append(f'closed_at: {yaml_quote(pr.get("closedAt"))}') +lines.append(f'head_ref: {yaml_quote(pr.get("headRefName") or "")}') +lines.append(f'base_ref: {yaml_quote(pr.get("baseRefName") or "")}') +lines.append(f'archived_at: {yaml_quote(archived_at)}') +lines.append(f'archive_tool: {yaml_quote("tools/pr-preservation/archive-pr.sh")}') lines.append('---') lines.append('') lines.append(f'# PR #{number}: {title}') @@ -192,6 +385,9 @@ if comments: lines.append('') content = '\n'.join(lines).rstrip() + '\n' +# Strip trailing whitespace on every line — markdownlint +# MD009 + keeps backfilled archives clean. +content = '\n'.join(line.rstrip() for line in content.split('\n')) with open(path, 'w', encoding='utf-8') as f: f.write(content) print(f'wrote {path} ({len(content)} bytes, {len(threads)} threads, {len(reviews)} reviews, {len(comments)} comments)') From bb28391921073c35a24295314877023e4f5a9e95 Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 09:41:09 -0400 Subject: [PATCH 04/10] =?UTF-8?q?fix(#357):=20Codex=20pagination=20P2=20?= =?UTF-8?q?=E2=80=94=20backlog=20row=20for=20per-connection=20cursor=20ref?= =?UTF-8?q?actor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/BACKLOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index bd747809..08628f92 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -9744,6 +9744,8 @@ systems. This track claims the space. - [ ] **Adopt `claude -w` (worktree mode) as default Claude Code launch pattern.** Maintainer 2026-04-24 Otto-241: *"from everyting i'm reading launching with -w will likely give us better results"*. Worktree-mode Claude Code runs each session in its own git worktree, preventing main-session filesystem contention when multiple Claudes share a repo. Analog of Otto-226 subagent `isolation: "worktree"` at the main-session layer. Scope: (1) test `claude -w` end-to-end on a mid-size task; (2) document the pattern in `CLAUDE.md` session-bootstrap + `docs/FACTORY-DISCIPLINE.md` active-disciplines; (3) optionally ship a wrapper `tools/claude-factory.sh` that enables `experimental.worktrees` + launches with `-w`; (4) validate via the Otto-240 same-machine-two-Claudes test — two `claude -w` sessions on the same repo should be totally isolated. Priority P3 convenience; effort S (test + doc) + S (optional wrapper). Composes with Otto-226 + Otto-240 + Otto-241. Note: Gemini CLI's built-in `-w` flag per its capability map uses `experimental.worktrees` feature gate; Claude Code's `-w` may have similar flag or be always-on. +- [ ] **PR-archive pagination refactor — per-connection cursor pagination, not whole-query refetch.** Codex P2 review on PR #357 (2026-04-24): the current `paginate_top_level` function in `tools/pr-preservation/archive-pr.sh` reissues the monolithic GraphQL QUERY on every pagination step, so paging through `reviewThreads` also re-fetches the full `reviews` connection + `pullRequest` payload unnecessarily. Works correctly; wastes GraphQL rate budget on PRs with many threads. Scope: (1) split the single QUERY into per-connection GraphQL fragments — one query per `reviewThreads`, `reviews`, `comments-per-thread` — each with its own cursor; (2) compose the sub-query results client-side; (3) preserve the end-to-end archive format (no downstream schema change); (4) self-hosting smoke test stays green. Priority P2 research-grade; effort S. Composes with the PR-preservation Otto-207 backfill + the git-native PR-conversation preservation P2 row above. + ## P2 — Rule-Zero axiomatic substrate (round-35 round-36 thread) - [ ] **Self-directed wellness / life-coach AI product — measure, From 218b2e0380bf1bfd68e1ff96f8357bfa301a552e Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 10:38:44 -0400 Subject: [PATCH 05/10] =?UTF-8?q?fix(#357):=208=20review=20threads=20?= =?UTF-8?q?=E2=80=94=20integer=20PR=20validation,=20trailing-ws=20preserva?= =?UTF-8?q?tion,=20MD012=20blank-line=20collapse,=20README/header=20filena?= =?UTF-8?q?me=20shape=20+=20bash=20dep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second drain pass on PR #357 review threads. Eight threads from agent reviewers; all fix-in-place. Script changes (`tools/pr-preservation/archive-pr.sh`): - Integer validation on `$PR` in the shell (pre-Python) — prevents a Python traceback + generic "fetch failed exit 2" diagnostic when a typo / non-integer is passed. - Dropped the per-line `rstrip()` normalization. Markdown uses two trailing spaces as a hard-line-break; this tool is a faithful audit copy and must preserve that intent. - Added a blank-line-run collapse (3+ consecutive blank lines -> 2) so generated archives stay clean under markdownlint MD012 without destroying user-authored formatting. - Header comment now documents the zero-padded `PR--` filename shape (matches the implementation + README). - Header comment on repo-detection aligned with actual behavior (requires `gh repo view`, no silent fallback). README changes (`tools/pr-preservation/README.md`): - Intro uses `PR--.md` (matches Usage + implementation). - Dependency relaxed from `bash 4+` to `bash` with a note — the script uses no bash-4-only features and macOS ships bash 3.2. Backfilled archives regenerated under the new collapse rule so they stop tripping MD012: - PR-0350 (Frontier rename pass-2) - PR-0352 (Server Meshing / SpacetimeDB deep research) - PR-0354 (backlog-split Phase 1a) - PR-0357 (this PR — self-archive re-fetched) Co-Authored-By: Claude Opus 4.7 --- ...pass-2-hindu-ff7-egyptian-greek-norse-s.md | 24 +- ...er-meshing-spacetimedb-deep-research-ga.md | 78 +++++- ...hase-1a-generator-schema-example-row-aa.md | 26 +- ...-minimal-archive-otto-207-session-backf.md | 262 +++++++++++++++++- tools/pr-preservation/README.md | 5 +- tools/pr-preservation/archive-pr.sh | 48 +++- 6 files changed, 377 insertions(+), 66 deletions(-) diff --git a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md index 6b1ff018..9b0cf1f9 100644 --- a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md +++ b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md @@ -1,15 +1,15 @@ --- pr_number: 350 title: "docs: Frontier rename pass-2 (Hindu/FF7/Egyptian/Greek/Norse) + Scientology research BACKLOG (Otto-175)" -author: AceHack -state: MERGED -created_at: 2026-04-24T10:03:56Z -merged_at: 2026-04-24T10:05:31Z -closed_at: 2026-04-24T10:05:31Z -head_ref: docs/frontier-rename-pass-2-plus-scientology-backlog -base_ref: main -archived_at: 2026-04-24T11:22:16Z -archive_tool: tools/pr-preservation/archive-pr.sh +author: "AceHack" +state: "MERGED" +created_at: "2026-04-24T10:03:56Z" +merged_at: "2026-04-24T10:05:31Z" +closed_at: "2026-04-24T10:05:31Z" +head_ref: "docs/frontier-rename-pass-2-plus-scientology-backlog" +base_ref: "main" +archived_at: "2026-04-24T14:38:16Z" +archive_tool: "tools/pr-preservation/archive-pr.sh" --- # PR #350: docs: Frontier rename pass-2 (Hindu/FF7/Egyptian/Greek/Norse) + Scientology research BACKLOG (Otto-175) @@ -59,7 +59,7 @@ Pass-2 doc under `docs/research/`. Scientology row at END of P2 research-grade s Here are some automated review suggestions for this pull request. **Reviewed commit:** `40c8793eea` - +
ℹ️ About Codex in GitHub
@@ -72,10 +72,8 @@ Here are some automated review suggestions for this pull request. If Codex has suggestions, it will comment; otherwise it will react with 👍. - - Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:08:03Z) diff --git a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md index fd4f8f93..aab910c6 100644 --- a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md +++ b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md @@ -1,13 +1,15 @@ --- pr_number: 352 title: "backlog: Otto-180 Server Meshing + SpacetimeDB deep research \u2014 game-industry competitive angle" -author: AceHack -state: OPEN -created_at: 2026-04-24T10:14:31Z -head_ref: backlog/otto-180-server-meshing-spacetime-db-research -base_ref: main -archived_at: 2026-04-24T11:22:13Z -archive_tool: tools/pr-preservation/archive-pr.sh +author: "AceHack" +state: "MERGED" +created_at: "2026-04-24T10:14:31Z" +merged_at: "2026-04-24T12:55:11Z" +closed_at: "2026-04-24T12:55:11Z" +head_ref: "backlog/otto-180-server-meshing-spacetime-db-research" +base_ref: "main" +archived_at: "2026-04-24T14:38:16Z" +archive_tool: "tools/pr-preservation/archive-pr.sh" --- # PR #352: backlog: Otto-180 Server Meshing + SpacetimeDB deep research — game-industry competitive angle @@ -64,7 +66,7 @@ P2 research-grade at line ~921 (end of "## P2 — Post-v1 query-surface research Here are some automated review suggestions for this pull request. **Reviewed commit:** `55ce0fac69` - +
ℹ️ About Codex in GitHub
@@ -77,10 +79,8 @@ Here are some automated review suggestions for this pull request. If Codex has suggestions, it will comment; otherwise it will react with 👍. - - Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:18:05Z) @@ -119,7 +119,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `44825a88fc` - +
ℹ️ About Codex in GitHub
@@ -132,11 +132,19 @@ Here are some automated review suggestions for this pull request. If Codex has suggestions, it will comment; otherwise it will react with 👍. +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+### COMMENTED — @AceHack (2026-04-24T12:53:32Z) -Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". +_(no body)_ - +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T12:56:16Z) + +## Pull request overview + +Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments. ## Review threads @@ -212,7 +220,7 @@ The cross-reference “Pluggable wire-protocol row (line ~754)” appears to poi Fixed in 44825a8: corrected `line ~754` to `around line 830` — the wire-protocol row's actual current location. Verified via `grep -n '^- \[ \] \*\*Pluggable wire-protocol'` before commit. -### Thread 6: docs/BACKLOG.md:1082 (unresolved) +### Thread 6: docs/BACKLOG.md:1082 (resolved) **@chatgpt-codex-connector** (2026-04-24T11:14:47Z): @@ -221,3 +229,43 @@ Fixed in 44825a8: corrected `line ~754` to `around line 830` — the wire-protoc This note points readers to `docs/aurora/2026-04-24-amara-*.md` for the “11th-ferry” and “12th-ferry” priors, but those artifacts are not present under `docs/aurora` (only 17th/19th ferry files exist), and a repo-wide search does not find matching 11th/12th aurora docs. That makes the provenance trail in this backlog item non-actionable for anyone trying to follow the cited priors; please update the reference to existing files or adjust the claim. Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T12:53:32Z): + +Fixed in 255ac9a. Dropped the forward-looking `docs/aurora/2026-04-24-amara-*.md` path and rephrased to name the priors by ferry number (11th-Temporal-Coordination-Detection, 12th-Executive-Summary) with explicit "pending absorb (not yet landed under `docs/aurora/`)" framing — survives absorb-PR renames and doesn't send readers to a missing path. + +### Thread 7: docs/BACKLOG.md:1303 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T12:56:15Z): + +The cross-reference `Pluggable wire-protocol row (around line 830)` is out of date: that row currently starts at line 1056 in this file. Suggest either updating the line number to the current location or replacing the fragile line-number reference with a stable identifier (e.g., the row title) to avoid repeated drift as BACKLOG.md grows. +```suggestion + protocol row (above) + Regular-database façade row + (above) + Otto-175c starship-franchise-mapping row + (Star Citizen thematic research row filed in PR + #351; landed on main). Waits on Zeta +``` + +### Thread 8: docs/BACKLOG.md:1309 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T12:56:15Z): + +The references to `11th-ferry-Temporal-Coordination-Detection` and `12th-ferry-Executive-Summary` don’t resolve to any existing docs under `docs/aurora/` in the repo. Please either link to the concrete existing Amara ferry docs you mean (file paths) or remove/rename these identifiers so the backlog entry doesn’t contain dangling xrefs. +```suggestion + Amara ferry material referencing the multi-node + future is the relevant prior, but that material is + still pending absorb and has not yet landed under + `docs/aurora/`). +``` + +### Thread 9: docs/BACKLOG.md:1285 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T12:56:16Z): + +Wording in the SpacetimeDB IP-discipline bullet is internally inconsistent: it says "SpacetimeDB proprietary code" but the next clause frames the allowed scope as the public Apache-2 repo. Consider rephrasing to "non-public/private SpacetimeDB materials" (and explicitly allow Apache-2 repo + published docs) to avoid implying the Apache-2 code is proprietary. +```suggestion + - No ingestion of non-public/private SpacetimeDB + materials. Public Apache-2-licensed code at + `clockworklabs/SpacetimeDB`, plus published docs and + architecture-paper reading, is research-permitted. +``` diff --git a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md index f9781737..69f55f11 100644 --- a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md +++ b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md @@ -1,13 +1,13 @@ --- pr_number: 354 title: "tools: backlog split Phase 1a \u2014 generator + schema + example row (Aaron Otto-181, 3rd ask)" -author: AceHack -state: OPEN -created_at: 2026-04-24T10:27:07Z -head_ref: tools/backlog-split-phase-1a-generator-plus-schema -base_ref: main -archived_at: 2026-04-24T11:22:01Z -archive_tool: tools/pr-preservation/archive-pr.sh +author: "AceHack" +state: "OPEN" +created_at: "2026-04-24T10:27:07Z" +head_ref: "tools/backlog-split-phase-1a-generator-plus-schema" +base_ref: "main" +archived_at: "2026-04-24T14:38:18Z" +archive_tool: "tools/pr-preservation/archive-pr.sh" --- # PR #354: tools: backlog split Phase 1a — generator + schema + example row (Aaron Otto-181, 3rd ask) @@ -80,7 +80,7 @@ Aaron Otto-182: *"can you ask codex too?"* — inviting `@codex review` below. Here are some automated review suggestions for this pull request. **Reviewed commit:** `6a447ee234` - +
ℹ️ About Codex in GitHub
@@ -93,10 +93,8 @@ Here are some automated review suggestions for this pull request. If Codex has suggestions, it will comment; otherwise it will react with 👍. - - Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:31:29Z) @@ -176,7 +174,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `9bf3d09ff1` - +
ℹ️ About Codex in GitHub
@@ -189,10 +187,8 @@ Here are some automated review suggestions for this pull request. If Codex has suggestions, it will comment; otherwise it will react with 👍. - - Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:06:51Z) diff --git a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md index 2f5961b1..02f0eb8e 100644 --- a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md +++ b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md @@ -6,7 +6,7 @@ state: "OPEN" created_at: "2026-04-24T11:23:49Z" head_ref: "tools/pr-preservation-phase-2-minimal" base_ref: "main" -archived_at: "2026-04-24T13:21:06Z" +archived_at: "2026-04-24T14:37:52Z" archive_tool: "tools/pr-preservation/archive-pr.sh" --- @@ -81,7 +81,7 @@ Scope out of this PR per maintainer *"make sure you backlog then to a proper lon Here are some automated review suggestions for this pull request. **Reviewed commit:** `cc217ae031` - +
ℹ️ About Codex in GitHub
@@ -94,10 +94,8 @@ Here are some automated review suggestions for this pull request. If Codex has suggestions, it will comment; otherwise it will react with 👍. - - Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:28:45Z) @@ -134,9 +132,103 @@ Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 | docs/pr-discussions/PR-0336-docs-ksk-naming-definition-doc-canonical-expansion-locked-ot.md | Backfilled archive for PR #336 discussion content. | +### COMMENTED — @AceHack (2026-04-24T13:22:23Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:26Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:28Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:30Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:32Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:34Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:36Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:38Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T13:22:41Z) + +_(no body)_ + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T13:26:42Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `66e48f0630` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @AceHack (2026-04-24T13:41:33Z) + +_(no body)_ + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T13:45:51Z) + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `dfc8ef554f` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T13:47:03Z) + +## Pull request overview + +Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments. + ## Review threads -### Thread 1: tools/pr-preservation/archive-pr.sh:60 (unresolved) +### Thread 1: tools/pr-preservation/archive-pr.sh:60 (resolved) **@chatgpt-codex-connector** (2026-04-24T11:25:56Z): @@ -146,7 +238,11 @@ The archive query hard-limits review threads and nested comments (`first: 100` / Useful? React with 👍 / 👎. -### Thread 2: tools/pr-preservation/archive-pr.sh:107 (unresolved) +**@AceHack** (2026-04-24T13:22:23Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. The archive query now paginates all three top-level connections (reviewThreads / reviews / comments) via cursor loops in the Python driver, plus paginates per-thread comments when `comments.pageInfo.hasNextPage` is set. No silent truncation remaining. End-to-end test against PR #357 itself captures all 9 threads (see docs/pr-discussions/PR-0357-...md written by the new code). + +### Thread 2: tools/pr-preservation/archive-pr.sh:107 (resolved) **@chatgpt-codex-connector** (2026-04-24T11:25:56Z): @@ -156,19 +252,31 @@ The script assumes `d['data']['repository']['pullRequest']` is always present an Useful? React with 👍 / 👎. -### Thread 3: tools/pr-preservation/archive-pr.sh:49 (unresolved) +**@AceHack** (2026-04-24T13:22:26Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. Now detect `pullRequest is None` in two places: (1) in the Python fetch driver right after the first GraphQL call, emitting a clear "not found / private / access denied" diagnostic and exiting 2; (2) again in the standalone validation pass before the formatter runs. Formatter also retains a defensive guard. Smoke-tested: `./tools/pr-preservation/archive-pr.sh 99999` exits 2 cleanly with "Could not resolve to a PullRequest". + +### Thread 3: tools/pr-preservation/archive-pr.sh:49 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): P0: With `set -e`, if `gh api graphql` exits non-zero (auth failure, network error), the script aborts immediately and never reaches the intended “fetch failed …” diagnostics / exit-code mapping. Wrap the `gh api graphql … > "$TMP"` call in an explicit error-handling block so failures produce a clear message and the documented exit code (2). -### Thread 4: tools/pr-preservation/archive-pr.sh:108 (unresolved) +**@AceHack** (2026-04-24T13:22:28Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. The fetch block is now wrapped in `set +e` / explicit `FETCH_RC=$?` / `set -e` so a non-zero gh exit no longer aborts before the diagnostic path. On failure we print "fetch failed for PR #$PR (exit $FETCH_RC)" and exit 2 per the header docstring. + +### Thread 4: tools/pr-preservation/archive-pr.sh:108 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): P0: The JSON check only validates that the response is parseable JSON; GraphQL error payloads (or `pullRequest: null`) will pass this check and then the Python block will crash on `d['data']...` / `pr.get(...)` with a stack trace. Detect `errors` in the response and/or a null `pullRequest` and exit 2 with a concise error message (e.g., not found / permission / auth). -### Thread 5: tools/pr-preservation/archive-pr.sh:47 (unresolved) +**@AceHack** (2026-04-24T13:22:30Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. The fetch driver now rejects top-level `errors` and `pullRequest: null` before any field access, and a separate shell-side validation pass re-checks both conditions before the formatter runs. The formatter itself also guards (belt and suspenders). No more TypeError stack traces on invalid PRs. + +### Thread 5: tools/pr-preservation/archive-pr.sh:47 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): @@ -185,13 +293,21 @@ gh api graphql -F owner="$OWNER" -F name="$REPO_NAME" -F number="$PR" -f query=' repository(owner: $owner, name: $name) { ``` -### Thread 6: tools/pr-preservation/archive-pr.sh:90 (unresolved) +**@AceHack** (2026-04-24T13:22:32Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. Owner/name now come from `gh repo view --json nameWithOwner -q .nameWithOwner` at runtime, passed as GraphQL `$owner`/`$name` variables. If gh cannot resolve the repo, we hard-fail with exit 1 rather than silently falling back ("never silently wrong" — factory rule). Works from forks and after rename. + +### Thread 6: tools/pr-preservation/archive-pr.sh:90 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:44Z): P1: The script and header comment say it writes “all review threads + reviews + comments”, but the GraphQL query is capped at `reviewThreads(first: 100)`, per-thread `comments(first: 50)`, `reviews(first: 50)`, and `comments(first: 100)` with no pagination. Either implement pagination (pageInfo/endCursor loops) or record in the output/frontmatter that results may be truncated at these limits. -### Thread 7: tools/pr-preservation/archive-pr.sh:134 (unresolved) +**@AceHack** (2026-04-24T13:22:34Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524 (option: implement pagination, not update README to advertise truncation). All three top-level connections and per-thread comments now paginate via cursor loops. README updated to note pagination is in place so there is no gap between "captures all review threads + reviews + comments" claim and actual behaviour. + +### Thread 7: tools/pr-preservation/archive-pr.sh:134 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): @@ -209,13 +325,21 @@ lines.append(f'base_ref: {json.dumps(pr.get("baseRefName") or "")}') lines.append(f'archived_at: {json.dumps(archived_at)}') ``` -### Thread 8: tools/pr-preservation/README.md:33 (unresolved) +**@AceHack** (2026-04-24T13:22:36Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. Added a `yaml_quote()` helper that wraps `json.dumps()` (double-quoted YAML strings with JSON-compatible escaping is valid YAML 1.2). Applied to every string-valued frontmatter field: title / author / state / created_at / merged_at / closed_at / head_ref / base_ref / archived_at / archive_tool. Only `pr_number` (integer) stays bare. Refs with `#` or `:` now parse cleanly. + +### Thread 8: tools/pr-preservation/README.md:33 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): P0: README says archives are written to `docs/pr-discussions/PR--.md`, but the script actually writes `PR-` zero-padded to 4 digits (e.g., `PR-0356-...`). Update the documented filename pattern so operators can reliably locate files and avoid accidental duplicates. -### Thread 9: docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md:68 (unresolved) +**@AceHack** (2026-04-24T13:22:38Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524 — chose option A (update README to match script). Documented filename shape is now `PR--.md` with zero-padding to four digits, explicitly called out in the Usage section with the rationale (lex-sort == num-sort for PR #0001..#9999). Picked this direction because the 10 backfilled files already use zero-padding and renaming them would churn cross-references and git history. + +### Thread 9: docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md:68 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): @@ -223,3 +347,113 @@ P0: The backfilled `docs/pr-discussions/*.md` files include whitespace-only line ```suggestion ``` + +**@AceHack** (2026-04-24T13:22:41Z): + +Fixed in 66e48f063082045871fb7d970bdb6fe213633524. Ran `find docs/pr-discussions -name '*.md' -exec perl -i -pe 's/[ \t]+$//' {} +` across all existing archives — MD009 clean (grep-verified: zero trailing-whitespace lines remaining). Also added an inline trailing-whitespace strip inside the archive writer itself so future archive runs stay clean without needing the post-hoc sweep. + +### Thread 10: tools/pr-preservation/archive-pr.sh:212 (resolved) + +**@chatgpt-codex-connector** (2026-04-24T13:26:42Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Avoid re-fetching full payload on each pagination step** + +`paginate_top_level` always reissues the monolithic `QUERY`, so every extra page for one connection (e.g., `reviewsAfter`) also re-downloads page 1 of `reviewThreads` and `comments` (including nested thread comments from `comments(first: 100)`). On busy PRs this multiplies GraphQL cost and can hit API resource/rate limits before archival completes, even though pagination logic itself is correct. Splitting pagination into connection-specific queries (or otherwise avoiding refetch of unrelated connections) would prevent fetch failures under high-volume PRs. + +Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T13:41:33Z): + +Codex P2 acknowledged and scoped, landed as a P2 research-grade BACKLOG row rather than an inline fix on this PR. + +**Three-outcome decision (Otto-226 / Otto-227, option 2 = narrow-fix + backlog):** + +- The finding is correct: `paginate_top_level` in `tools/pr-preservation/archive-pr.sh` reissues the full monolithic `QUERY` on every page, so walking extra pages of one connection (e.g. `reviewThreads`) re-fetches the full `reviews` + `pullRequest` payload. Works correctly; wastes GraphQL rate budget on PRs with many threads. +- A proper fix is a ~30-60 LOC refactor: split the single `QUERY` into per-connection GraphQL fragments (one per `reviewThreads`, `reviews`, `comments-per-thread`) with separate cursors, compose results client-side, preserve the end-to-end archive format. That scope is beyond a single review-response commit, and risks regressing the self-hosting smoke test we just validated. +- This PR keeps the current pagination shape. The refactor is queued as a P2 row at end of `## P2 — research-grade` in `docs/BACKLOG.md`, scope locked, citing this thread. + +**Commit:** dfc8ef554f094a32bf54356bd2a1b1029064b13c +**BACKLOG row:** `docs/BACKLOG.md` — *"PR-archive pagination refactor — per-connection cursor pagination, not whole-query refetch"* (end of `## P2 — research-grade`) + +Resolving. + +### Thread 11: tools/pr-preservation/archive-pr.sh:390 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T13:45:51Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Preserve trailing whitespace in archived discussion text** + +The formatter rewrites every output line with `rstrip()`, which removes trailing spaces from PR bodies/reviews/comments before writing the archive. This changes markdown semantics in real cases (for example, GitHub comments that use two trailing spaces for hard line breaks), so the generated file is no longer a faithful audit copy of the original discussion. Since this tool is explicitly for durable preservation, this normalization should not be applied to user-authored text. + +Useful? React with 👍 / 👎. + +### Thread 12: tools/pr-preservation/archive-pr.sh:390 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:01Z): + +The output normalization only strips trailing whitespace; several generated archives (including the backfilled ones in this PR) contain 3+ consecutive blank lines (e.g., around
blocks), which will trip markdownlint MD012 because `docs/pr-discussions/**` is not in the markdownlint ignore list. Consider adding a safe normalization step that collapses multiple consecutive blank lines (or alternatively add `docs/pr-discussions/**` to `.markdownlint-cli2.jsonc` ignores) and then regenerate the backfilled archives so CI stays green. +```suggestion +content = '\n'.join(line.rstrip() for line in content.split('\n')) +# Collapse excessive blank-line runs so generated archives +# do not trip markdownlint MD012. +normalized_lines = [] +blank_run = 0 +for line in content.split('\n'): + if line == '': + blank_run += 1 + else: + blank_run = 0 + if blank_run <= 2: + normalized_lines.append(line) +content = '\n'.join(normalized_lines).rstrip() + '\n' +``` + +### Thread 13: tools/pr-preservation/archive-pr.sh:60 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): + +The repo-detection comment says the script "falls back" to Lucent-Financial-Group/Zeta if `gh repo view` fails, but the implementation hard-fails with exit 1 and does not provide any fallback. Please align the comment with actual behavior (or implement the documented fallback) so operators aren’t misled. + +### Thread 14: tools/pr-preservation/archive-pr.sh:50 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): + +`PR` is taken verbatim from argv and then `int(os.environ['PR'])` is called in the Python fetch driver. If the user passes a non-integer (or whitespace), this will raise a Python traceback and be reported as a generic “fetch failed … exit 1” with exit code 2. Consider validating `PR` is an integer in the shell before invoking Python and exiting 1 with a clear usage/error message. +```suggestion +PR="$1" +if ! [[ "$PR" =~ ^[0-9]+$ ]]; then + echo "error: PR number must be a non-empty integer" >&2 + echo "usage: $0 " >&2 + exit 1 +fi +``` + +### Thread 15: docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md:87 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): + +This archived PR discussion includes multiple consecutive blank lines (e.g., between “**Reviewed commit:** …” and the following `
` block). With the current `.markdownlint-cli2.jsonc` config, MD012 (no multiple consecutive blank lines) will run on `docs/pr-discussions/**` and may fail CI. If the archive format is intended to be lint-clean, regenerate archives after adjusting the archiver to collapse excess blank lines; otherwise consider excluding `docs/pr-discussions/**` from markdownlint. + +### Thread 16: tools/pr-preservation/README.md:115 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): + +README lists a `bash 4+` dependency, but `archive-pr.sh` doesn’t appear to rely on bash-4-only features (and macOS still ships bash 3.2 by default). Either (a) enforce the version at runtime (fail fast with a helpful message), or (b) relax the documented requirement to just `bash` if bash 3.x is sufficient. +```suggestion +- `bash`, POSIX `mktemp` +``` + +### Thread 17: tools/pr-preservation/README.md:7 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:03Z): + +The intro paragraph says the tool writes `docs/pr-discussions/PR--.md`, but later the Usage section specifies the zero-padded `PR--.md` naming. Please make the intro consistent with the actual filename format to avoid operators looking for the wrong path. +```suggestion +`docs/pr-discussions/PR--.md` for durable +``` + +### Thread 18: tools/pr-preservation/archive-pr.sh:25 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T13:47:03Z): + +Header comment still describes the output path as `docs/pr-discussions/PR--.md`, but the implementation writes `PR-{number:04d}-...` (zero-padded to 4 digits). Consider updating the header comment so it matches the actual filename format (and the README’s `PR--...` wording). diff --git a/tools/pr-preservation/README.md b/tools/pr-preservation/README.md index 1bf3e58e..a075b66c 100644 --- a/tools/pr-preservation/README.md +++ b/tools/pr-preservation/README.md @@ -4,7 +4,7 @@ Minimal implementation (Otto-207) of the PR-preservation BACKLOG directive (Otto-150..154, PR #335). Fetches a PR's review threads, reviews, and general comments via `gh api graphql` and writes them to -`docs/pr-discussions/PR--.md` for durable +`docs/pr-discussions/PR--.md` for durable audit-trail storage outside of GitHub. ## Scope of this Phase-0 / minimal tool @@ -112,7 +112,8 @@ Tracked on the PR-preservation BACKLOG row. Phases: - `gh` CLI authenticated - `python3` (stdlib only) -- `bash 4+`, POSIX `mktemp` +- `bash`, POSIX `mktemp` (the script uses no bash-4-only + features; macOS default `bash` 3.2 is fine) No external Python packages; no `yq` required. diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index fb4a84b4..691805f8 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -16,7 +16,7 @@ # Usage: # tools/pr-preservation/archive-pr.sh # -# Output: writes docs/pr-discussions/PR--.md with +# Output: writes docs/pr-discussions/PR--.md with # YAML frontmatter (pr_number, title, author, merged_at, # state, archived_at) + all review threads + reviews + # general PR comments. PR numbers are zero-padded to four @@ -39,6 +39,17 @@ # - `set -e`-safe capture of `gh api graphql` exit code # so "fetch failed" diagnostics actually print. # - YAML string values quoted (refs can contain `#` / `:`). +# Review-thread drain Otto-234 fixes (PR #357, second pass): +# - Validate `PR` argv is a positive integer in the shell +# (avoids Python traceback + generic "exit 2" diagnostic). +# - Stop stripping trailing whitespace from archived text: +# markdown uses `" \n"` as a hard-line-break, and this +# tool preserves a faithful audit copy. +# - Collapse runs of 3+ blank lines to 2 so the generated +# archives stay clean under markdownlint MD012. +# - README + header comment now match the implementation's +# zero-padded `PR--.md` filename shape and +# document `bash` (not `bash 4+`) as the dependency. set -euo pipefail @@ -48,12 +59,21 @@ if [ $# -lt 1 ]; then fi PR="$1" +# Validate PR is a positive integer before invoking Python. +# Prevents a Python traceback + generic "fetch failed exit 2" +# when the operator passes a typo or non-integer arg. +if ! [[ "$PR" =~ ^[0-9]+$ ]]; then + echo "error: PR number must be a non-empty positive integer (got: '$PR')" >&2 + echo "usage: $0 " >&2 + exit 1 +fi REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" # Dynamic owner / name — works from forks or after rename. -# Falls back to Lucent-Financial-Group/Zeta only if the -# detection command itself fails (e.g. not inside a repo -# gh recognises). Never silently wrong. +# Requires `gh repo view` to succeed; the script hard-fails +# with exit 1 rather than silently defaulting to a baked-in +# NWO (better to fail loud than archive to the wrong repo +# path on a fork). REPO_NWO="$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null || true)" if [ -z "${REPO_NWO}" ]; then echo "error: could not detect repo via 'gh repo view'. Is gh authenticated and this a GitHub repo?" >&2 @@ -385,9 +405,23 @@ if comments: lines.append('') content = '\n'.join(lines).rstrip() + '\n' -# Strip trailing whitespace on every line — markdownlint -# MD009 + keeps backfilled archives clean. -content = '\n'.join(line.rstrip() for line in content.split('\n')) +# Preserve trailing whitespace on user-authored text: +# markdown uses two trailing spaces as a hard-line-break, +# and this tool's purpose is a faithful audit copy. +# We DO collapse runs of 3+ consecutive blank lines down +# to 2 so markdownlint MD012 stays green on the generated +# archives (the source content sometimes has 3+ blank +# lines around
blocks). +collapsed = [] +blank_run = 0 +for raw_line in content.split('\n'): + if raw_line == '': + blank_run += 1 + else: + blank_run = 0 + if blank_run <= 2: + collapsed.append(raw_line) +content = '\n'.join(collapsed).rstrip() + '\n' with open(path, 'w', encoding='utf-8') as f: f.write(content) print(f'wrote {path} ({len(content)} bytes, {len(threads)} threads, {len(reviews)} reviews, {len(comments)} comments)') From 1d1d2ab0474486b2d70486eeb9533b1b2eedb85e Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 10:50:24 -0400 Subject: [PATCH 06/10] =?UTF-8?q?fix(#357):=202=20Codex=20P2=20threads=20?= =?UTF-8?q?=E2=80=94=20PR-number=20stable=20filename=20key=20+=20preserve?= =?UTF-8?q?=20leading=20whitespace=20in=20archived=20bodies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third-round review-thread drain on `tools/pr-preservation/archive-pr.sh`: - `PRRT_kwDOSF9kNM59bWi5` (line 325): archive filename was derived from the title slug, so editing a PR title would write a second file instead of updating the existing record. Fix: PR number is now the canonical archive key. On re-archive, glob for an existing `PR--*.md` and reuse its path regardless of current title. New PRs still land at `PR--.md`. - `PRRT_kwDOSF9kNM59bWi_` (line 369 + lines 388, 401): `.strip()` normalised review / thread-comment / general-comment bodies and destroyed semantically-significant leading indentation (indented code blocks, nested bullets). Switched to `.rstrip('\n')` so only trailing newlines are stripped; leading whitespace survives. Smoke tested: `./archive-pr.sh 357` writes back to the same file (no new PR-0357-* orphan), bash -n + shellcheck clean, diff shows preserved `
` internal structure and indentation in archive. Co-Authored-By: Claude Opus 4.7 --- ...-minimal-archive-otto-207-session-backf.md | 134 ++++++++++++++++-- tools/pr-preservation/archive-pr.sh | 43 +++++- 2 files changed, 161 insertions(+), 16 deletions(-) diff --git a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md index 02f0eb8e..1780061f 100644 --- a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md +++ b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md @@ -6,7 +6,7 @@ state: "OPEN" created_at: "2026-04-24T11:23:49Z" head_ref: "tools/pr-preservation-phase-2-minimal" base_ref: "main" -archived_at: "2026-04-24T14:37:52Z" +archived_at: "2026-04-24T14:49:33Z" archive_tool: "tools/pr-preservation/archive-pr.sh" --- @@ -76,6 +76,7 @@ Scope out of this PR per maintainer *"make sure you backlog then to a proper lon ### COMMENTED — @chatgpt-codex-connector (2026-04-24T11:25:56Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. @@ -170,6 +171,7 @@ _(no body)_ ### COMMENTED — @chatgpt-codex-connector (2026-04-24T13:26:42Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. @@ -198,6 +200,7 @@ _(no body)_ ### COMMENTED — @chatgpt-codex-connector (2026-04-24T13:45:51Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. @@ -226,6 +229,63 @@ Codex can also answer questions or update the PR. Try commenting "@codex address Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments. +### COMMENTED — @AceHack (2026-04-24T14:38:59Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:07Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:11Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:15Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:21Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:23Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:25Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:39:27Z) + +_(no body)_ + +### COMMENTED — @chatgpt-codex-connector (2026-04-24T14:42:54Z) + + +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** `a5331f79e5` + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
+ ## Review threads ### Thread 1: tools/pr-preservation/archive-pr.sh:60 (resolved) @@ -339,7 +399,7 @@ P0: README says archives are written to `docs/pr-discussions/PR--.md`, Fixed in 66e48f063082045871fb7d970bdb6fe213633524 — chose option A (update README to match script). Documented filename shape is now `PR--.md` with zero-padding to four digits, explicitly called out in the Usage section with the rationale (lex-sort == num-sort for PR #0001..#9999). Picked this direction because the 10 backfilled files already use zero-padding and renaming them would churn cross-references and git history. -### Thread 9: docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md:68 (resolved) +### Thread 9: docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md:70 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:28:45Z): @@ -352,7 +412,7 @@ P0: The backfilled `docs/pr-discussions/*.md` files include whitespace-only line Fixed in 66e48f063082045871fb7d970bdb6fe213633524. Ran `find docs/pr-discussions -name '*.md' -exec perl -i -pe 's/[ \t]+$//' {} +` across all existing archives — MD009 clean (grep-verified: zero trailing-whitespace lines remaining). Also added an inline trailing-whitespace strip inside the archive writer itself so future archive runs stay clean without needing the post-hoc sweep. -### Thread 10: tools/pr-preservation/archive-pr.sh:212 (resolved) +### Thread 10: tools/pr-preservation/archive-pr.sh:232 (resolved) **@chatgpt-codex-connector** (2026-04-24T13:26:42Z): @@ -377,7 +437,7 @@ Codex P2 acknowledged and scoped, landed as a P2 research-grade BACKLOG row rath Resolving. -### Thread 11: tools/pr-preservation/archive-pr.sh:390 (unresolved) +### Thread 11: tools/pr-preservation/archive-pr.sh:390 (resolved) **@chatgpt-codex-connector** (2026-04-24T13:45:51Z): @@ -387,7 +447,11 @@ The formatter rewrites every output line with `rstrip()`, which removes trailing Useful? React with 👍 / 👎. -### Thread 12: tools/pr-preservation/archive-pr.sh:390 (unresolved) +**@AceHack** (2026-04-24T14:38:59Z): + +Fixed in a5331f7. Dropped the per-line `rstrip()` normalization so trailing whitespace (e.g. markdown two-space hard-line-breaks) is preserved verbatim — the archive is a faithful audit copy again. Ran into a tension with a sibling thread on this same line (MD012 blank-line runs) so also added a blank-line-run collapse that normalizes 3+ blank lines to 2 without touching in-line trailing whitespace. + +### Thread 12: tools/pr-preservation/archive-pr.sh:390 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:01Z): @@ -408,13 +472,21 @@ for line in content.split('\n'): content = '\n'.join(normalized_lines).rstrip() + '\n' ``` -### Thread 13: tools/pr-preservation/archive-pr.sh:60 (unresolved) +**@AceHack** (2026-04-24T14:39:07Z): + +Fixed in a5331f7. Added the blank-line-run collapse you suggested (3+ consecutive blank lines -> 2) so archives stay clean under markdownlint MD012 without needing to ignore `docs/pr-discussions/**`. Also regenerated the three backfilled archives that were tripping MD012 (PR-0350, PR-0352, PR-0354) — they're now clean. Kept user-authored trailing whitespace preserved (markdown two-space hard-line-breaks) per the sibling thread. + +### Thread 13: tools/pr-preservation/archive-pr.sh:80 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): The repo-detection comment says the script "falls back" to Lucent-Financial-Group/Zeta if `gh repo view` fails, but the implementation hard-fails with exit 1 and does not provide any fallback. Please align the comment with actual behavior (or implement the documented fallback) so operators aren’t misled. -### Thread 14: tools/pr-preservation/archive-pr.sh:50 (unresolved) +**@AceHack** (2026-04-24T14:39:11Z): + +Fixed in a5331f7. The comment used to say "falls back" but the implementation hard-fails with exit 1 — updated the comment to match actual behavior: requires `gh repo view` to succeed, no silent fallback to a baked-in NWO. Rationale: better to fail loud than archive to the wrong repo path on a fork. + +### Thread 14: tools/pr-preservation/archive-pr.sh:61 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): @@ -428,13 +500,21 @@ if ! [[ "$PR" =~ ^[0-9]+$ ]]; then fi ``` -### Thread 15: docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md:87 (unresolved) +**@AceHack** (2026-04-24T14:39:15Z): + +Fixed in a5331f7. Applied your suggested shell-side integer validation (regex `^[0-9]+$`) before invoking Python. Now a non-integer arg exits 1 with a clear "PR number must be a non-empty positive integer" message instead of a Python traceback mapped to exit 2. Verified: `./archive-pr.sh not-an-int` exits 1 with the usage line. + +### Thread 15: docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md:87 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): This archived PR discussion includes multiple consecutive blank lines (e.g., between “**Reviewed commit:** …” and the following `
` block). With the current `.markdownlint-cli2.jsonc` config, MD012 (no multiple consecutive blank lines) will run on `docs/pr-discussions/**` and may fail CI. If the archive format is intended to be lint-clean, regenerate archives after adjusting the archiver to collapse excess blank lines; otherwise consider excluding `docs/pr-discussions/**` from markdownlint. -### Thread 16: tools/pr-preservation/README.md:115 (unresolved) +**@AceHack** (2026-04-24T14:39:21Z): + +Fixed in a5331f7. Added the blank-line-run collapse to the archiver (3+ -> 2) and regenerated this archive (PR-0354) along with PR-0350 and PR-0352 — all three were tripping MD012. Verified with a grep for `\\n\\n\\n\\n` across `docs/pr-discussions/*.md`: zero matches after regeneration. + +### Thread 16: tools/pr-preservation/README.md:115 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): @@ -443,7 +523,11 @@ README lists a `bash 4+` dependency, but `archive-pr.sh` doesn’t appear to rel - `bash`, POSIX `mktemp` ``` -### Thread 17: tools/pr-preservation/README.md:7 (unresolved) +**@AceHack** (2026-04-24T14:39:23Z): + +Fixed in a5331f7 — relaxed the documented dependency to `bash` with a note that the script uses no bash-4-only features and macOS default bash 3.2 is fine. Chose option (b) per your suggestion; adding a runtime version check would be extra complexity with no payoff since the script genuinely doesn't need 4+. + +### Thread 17: tools/pr-preservation/README.md:7 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:03Z): @@ -452,8 +536,36 @@ The intro paragraph says the tool writes `docs/pr-discussions/PR--.md`, `docs/pr-discussions/PR--.md` for durable ``` -### Thread 18: tools/pr-preservation/archive-pr.sh:25 (unresolved) +**@AceHack** (2026-04-24T14:39:25Z): + +Fixed in a5331f7. README intro now says `docs/pr-discussions/PR--.md` (zero-padded four digits) — matches the Usage section and the implementation. + +### Thread 18: tools/pr-preservation/archive-pr.sh:25 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:03Z): Header comment still describes the output path as `docs/pr-discussions/PR--.md`, but the implementation writes `PR-{number:04d}-...` (zero-padded to 4 digits). Consider updating the header comment so it matches the actual filename format (and the README’s `PR--...` wording). + +**@AceHack** (2026-04-24T14:39:27Z): + +Fixed in a5331f7. Header comment block now documents the zero-padded `PR--.md` filename shape, matching the implementation's `f'PR-{number:04d}-{slug}.md'` and the README. + +### Thread 19: tools/pr-preservation/archive-pr.sh:325 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T14:42:54Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Use PR number as stable archive filename key** + +The output path is keyed by a slug derived from the current PR title, so re-archiving after a title edit writes a second file instead of updating the existing archive for that PR number. This leaves stale duplicates (`PR-####-old-title.md` plus `PR-####-new-title.md`) and breaks the expected one-record-per-PR behavior for reconciliation/backfill workflows. + +Useful? React with 👍 / 👎. + +### Thread 20: tools/pr-preservation/archive-pr.sh:369 (unresolved) + +**@chatgpt-codex-connector** (2026-04-24T14:42:54Z): + +**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Preserve leading whitespace in archived comment bodies** + +Comment/review text is normalized with `.strip()`, which removes leading spaces and leading blank lines from user-authored content before writing the archive. This is lossy for markdown that intentionally starts with indentation (for example indented code blocks) and undermines the script’s preservation/audit goal; the same pattern is used for thread and general comments as well. + +Useful? React with 👍 / 👎. diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index 691805f8..6d5e5b39 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -50,6 +50,16 @@ # - README + header comment now match the implementation's # zero-padded `PR--.md` filename shape and # document `bash` (not `bash 4+`) as the dependency. +# Review-thread drain Otto-235 fixes (PR #357, third pass): +# - PR number is the canonical archive key: on re-archive, +# detect an existing `PR--*.md` file and reuse its +# path regardless of current title, so title edits update +# in place rather than orphaning the old slug. New PRs +# still land at `PR--.md`. +# - Preserve leading whitespace in archived body text: +# switched `.strip()` to `.rstrip('\n')` for review / +# thread-comment / general-comment bodies so indented +# code blocks and bullets survive the round-trip. set -euo pipefail @@ -322,7 +332,23 @@ os.makedirs(out_dir, exist_ok=True) # Zero-pad to 4 digits — aligns with README's documented # filename shape (e.g. PR-0357-...). Sorts lexicographically # == numerically for PR #0001..#9999. -path = os.path.join(out_dir, f'PR-{number:04d}-{slug}.md') +# +# Idempotency: the PR number is the canonical key. If an +# archive for this PR already exists (any slug), reuse its +# path so title edits update in place rather than orphaning +# the old slug. Only when there is no prior archive do we +# mint a new `PR--.md` file. +import glob as _glob +existing = sorted(_glob.glob(os.path.join(out_dir, f'PR-{number:04d}-*.md'))) +if existing: + # Reuse the first match (deterministic by sort). If + # somehow multiple stale files exist for the same PR + # number, we overwrite the earliest-sorting one and + # leave the rest untouched — operator can clean up by + # hand; refusing to silently delete preserves audit. + path = existing[0] +else: + path = os.path.join(out_dir, f'PR-{number:04d}-{slug}.md') archived_at = datetime.datetime.utcnow().isoformat(timespec='seconds') + 'Z' @@ -366,10 +392,13 @@ if reviews: author = (r.get('author') or {}).get('login') or 'unknown' state = r.get('state') or 'COMMENTED' submitted = r.get('submittedAt') or '' - body_text = (r.get('body') or '').strip() + # Preserve leading whitespace: only strip trailing newlines. + # Indented code blocks / nested bullets in review bodies must + # survive the archive round-trip. + body_text = (r.get('body') or '').rstrip('\n') lines.append(f'### {state} — @{author} ({submitted})') lines.append('') - lines.append(body_text if body_text else '_(no body)_') + lines.append(body_text if body_text.strip() else '_(no body)_') lines.append('') threads = (pr.get('reviewThreads') or {}).get('nodes', []) @@ -385,7 +414,9 @@ if threads: for c in (t.get('comments') or {}).get('nodes', []): author = (c.get('author') or {}).get('login') or 'unknown' when = c.get('createdAt') or '' - body_text = (c.get('body') or '').strip() + # Preserve leading whitespace: only strip trailing newlines + # so indented code blocks in review-thread comments survive. + body_text = (c.get('body') or '').rstrip('\n') lines.append(f'**@{author}** ({when}):') lines.append('') lines.append(body_text) @@ -398,7 +429,9 @@ if comments: for c in comments: author = (c.get('author') or {}).get('login') or 'unknown' when = c.get('createdAt') or '' - body_text = (c.get('body') or '').strip() + # Preserve leading whitespace: only strip trailing newlines so + # indented code blocks in general PR comments survive. + body_text = (c.get('body') or '').rstrip('\n') lines.append(f'### @{author} ({when})') lines.append('') lines.append(body_text) From 9bed33f8af3572dbc7c886571cfd4092138c3cb0 Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 11:37:39 -0400 Subject: [PATCH 07/10] =?UTF-8?q?fix(#357):=206=20review=20threads=20?= =?UTF-8?q?=E2=80=94=20drop=20truncation-warning=20claim,=20preserve=20las?= =?UTF-8?q?t-line=20hard-line-breaks,=20normalize=20whitespace-only=20line?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth drain pass on PR #357. Addresses 6 new P0 threads from re-review: - archive-pr.sh header said "Pagination + truncation warning for threads (>100)" but implementation only paginates, never emits a warning. Claim removed; comment now matches behaviour. - `body.rstrip()` on the PR-description block stripped trailing spaces from the last line (kills markdown " \n" hard-line breaks). Changed to `body.rstrip('\n')`. - End-of-file `content.rstrip()` had the same problem — end-of- file hard-line-break would be lost. Changed to `content.rstrip('\n')` in both places (pre- and post-blank- line-collapse). - Whitespace-only lines (e.g. " " from Codex connector comments) tripped markdownlint MD009. Added a post-collapse normalization step: lines containing only whitespace are normalized to empty, while lines with any non-whitespace character keep trailing whitespace intact (two-space hard-line-breaks survive). Regenerated four affected archives: PR-0350, PR-0352, PR-0354, PR-0357. Verified: zero whitespace-only lines, zero 3+ blank- line runs across all archives. Syntax / shellcheck clean. Co-Authored-By: Claude Opus 4.7 --- ...pass-2-hindu-ff7-egyptian-greek-norse-s.md | 7 +- ...er-meshing-spacetimedb-deep-research-ga.md | 12 +- ...hase-1a-generator-schema-example-row-aa.md | 120 +++++++++++++++--- ...-minimal-archive-otto-207-session-backf.md | 108 ++++++++++++++-- tools/pr-preservation/archive-pr.sh | 40 +++++- 5 files changed, 244 insertions(+), 43 deletions(-) diff --git a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md index 9b0cf1f9..e79bba72 100644 --- a/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md +++ b/docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md @@ -8,7 +8,7 @@ merged_at: "2026-04-24T10:05:31Z" closed_at: "2026-04-24T10:05:31Z" head_ref: "docs/frontier-rename-pass-2-plus-scientology-backlog" base_ref: "main" -archived_at: "2026-04-24T14:38:16Z" +archived_at: "2026-04-24T15:36:54Z" archive_tool: "tools/pr-preservation/archive-pr.sh" --- @@ -54,12 +54,13 @@ Pass-2 doc under `docs/research/`. Scientology row at END of P2 research-grade s ### COMMENTED — @chatgpt-codex-connector (2026-04-24T10:05:53Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `40c8793eea` - +
ℹ️ About Codex in GitHub
@@ -73,7 +74,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:08:03Z) diff --git a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md index aab910c6..2aaa507c 100644 --- a/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md +++ b/docs/pr-discussions/PR-0352-backlog-otto-180-server-meshing-spacetimedb-deep-research-ga.md @@ -8,7 +8,7 @@ merged_at: "2026-04-24T12:55:11Z" closed_at: "2026-04-24T12:55:11Z" head_ref: "backlog/otto-180-server-meshing-spacetime-db-research" base_ref: "main" -archived_at: "2026-04-24T14:38:16Z" +archived_at: "2026-04-24T15:36:55Z" archive_tool: "tools/pr-preservation/archive-pr.sh" --- @@ -61,12 +61,13 @@ P2 research-grade at line ~921 (end of "## P2 — Post-v1 query-surface research ### COMMENTED — @chatgpt-codex-connector (2026-04-24T10:15:36Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `55ce0fac69` - +
ℹ️ About Codex in GitHub
@@ -80,7 +81,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:18:05Z) @@ -114,12 +115,13 @@ _(no body)_ ### COMMENTED — @chatgpt-codex-connector (2026-04-24T11:14:47Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `44825a88fc` - +
ℹ️ About Codex in GitHub
@@ -133,7 +135,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @AceHack (2026-04-24T12:53:32Z) diff --git a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md index 69f55f11..a04213c6 100644 --- a/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md +++ b/docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md @@ -2,11 +2,13 @@ pr_number: 354 title: "tools: backlog split Phase 1a \u2014 generator + schema + example row (Aaron Otto-181, 3rd ask)" author: "AceHack" -state: "OPEN" +state: "MERGED" created_at: "2026-04-24T10:27:07Z" +merged_at: "2026-04-24T15:05:15Z" +closed_at: "2026-04-24T15:05:15Z" head_ref: "tools/backlog-split-phase-1a-generator-plus-schema" base_ref: "main" -archived_at: "2026-04-24T14:38:18Z" +archived_at: "2026-04-24T15:36:56Z" archive_tool: "tools/pr-preservation/archive-pr.sh" --- @@ -75,12 +77,13 @@ Aaron Otto-182: *"can you ask codex too?"* — inviting `@codex review` below. ### COMMENTED — @chatgpt-codex-connector (2026-04-24T10:29:51Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `6a447ee234` - +
ℹ️ About Codex in GitHub
@@ -94,7 +97,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T10:31:29Z) @@ -169,12 +172,13 @@ _(no body)_ ### COMMENTED — @chatgpt-codex-connector (2026-04-24T11:06:32Z) + ### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `9bf3d09ff1` - +
ℹ️ About Codex in GitHub
@@ -188,7 +192,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:06:51Z) @@ -197,6 +201,46 @@ Codex can also answer questions or update the PR. Try commenting "@codex address Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments. +### COMMENTED — @AceHack (2026-04-24T15:03:31Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:35Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:37Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:39Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:45Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:48Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:50Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:03:52Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:04:02Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T15:04:04Z) + +_(no body)_ + ## Review threads ### Thread 1: tools/backlog/generate-index.sh:55 (resolved) @@ -227,7 +271,7 @@ Useful? React with 👍 / 👎. Fixed in 6f659a0. Replaced the `in_fm = !in_fm` toggle with an explicit 3-state machine: 0 = before frontmatter, 1 = inside, 2 = after. The scanner now terminates definitively on the first closing `---` instead of re-entering frontmatter mode on every subsequent `---` line in body content. This was a real parser bug that would have silently matched horizontal-rule-style dividers inside the body. -### Thread 3: tools/backlog/generate-index.sh:168 (unresolved) +### Thread 3: tools/backlog/generate-index.sh:178 (resolved) **@chatgpt-codex-connector** (2026-04-24T10:29:51Z): @@ -241,6 +285,10 @@ Useful? React with 👍 / 👎. Acknowledged; deferring the second-factor bypass hardening to Phase 2 content-migration PR. Rationale: Phase 1a has a substantial-content guard (>50 lines on existing BACKLOG.md) that requires an explicit env var to override; this prevents accidental overwrite during routine development. The second-factor discussed (git-blame signer / commit-message marker / dedicated workflow trigger) would add real defense-in-depth but is only meaningfully exercised during Phase 2 when the mega-PR will intentionally call the generator with BACKLOG_WRITE_FORCE=1 against a populated docs/backlog/ tree. Filing as a specific Phase-2-acceptance criterion; keeping this thread unresolved as a visible breadcrumb until Phase 2 ships. +**@AceHack** (2026-04-24T15:03:31Z): + +disposition recorded above, resolving per Otto-236 reply+resolve pair discipline — BACKLOG_WRITE_FORCE second-factor filed as Phase-2-acceptance criterion; thread drained so required_conversation_resolution gate clears. + ### Thread 4: tools/backlog/generate-index.sh:176 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T10:31:27Z): @@ -313,7 +361,7 @@ P1: `extract_field` toggles `in_fm` on every `---` line in the file, so a horizo Same fix as thread 59Xdsr: the `in_fm = !in_fm` toggle was the root cause of both the "toggles on every ---" behavior and the "parse single-quoted scalars" miss (because single-quoted values with internal `---` would have re-flipped the flag). Explicit 3-state machine (0/1/2) makes both behaviors correct by construction. Commit 6f659a0. -### Thread 10: tools/backlog/README.md:136 (unresolved) +### Thread 10: tools/backlog/README.md:136 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T10:31:28Z): @@ -327,6 +375,10 @@ P2: The inline-code span for the `memory/...*.md` reference is split across a ne Acknowledged the inline-code-span formatting concern for the memory/...*.md reference. Addressed at the root by removing that specific path-reference entirely (commit 9bf3d09, thread 59XfPF), which makes the formatting question moot. Leaving this thread unresolved as a breadcrumb for "reviewer intent honored via different mechanism than suggested" so the conversation-preservation captures both the original suggestion and the what-actually-happened-instead. +**@AceHack** (2026-04-24T15:03:35Z): + +disposition recorded above, resolving per Otto-236 reply+resolve pair discipline — inline-code-span concern addressed via different mechanism (commit 9bf3d09 removed the path reference entirely); thread drained so required_conversation_resolution gate clears. + ### Thread 11: tools/backlog/README.md:96 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T10:31:29Z): @@ -352,7 +404,7 @@ sorted by (priority ascending, id ascending). Fixed in 9bf3d09. README's "parses frontmatter via yq or awk fallback" wording was aspirational; the actual implementation is pure awk with no yq integration. Corrected to "inline awk parser, no yq required" which matches the code. Paired with the header-comment fix (thread 59XfPj) so both surfaces describe the same implementation. -### Thread 13: tools/backlog/generate-index.sh:58 (unresolved) +### Thread 13: tools/backlog/generate-index.sh:58 (resolved) **@chatgpt-codex-connector** (2026-04-24T11:06:33Z): @@ -362,7 +414,11 @@ Fixed in 9bf3d09. README's "parses frontmatter via yq or awk fallback" wording w Useful? React with 👍 / 👎. -### Thread 14: tools/backlog/generate-index.sh:107 (unresolved) +**@AceHack** (2026-04-24T15:03:37Z): + +Fixed in df1224f: added `sub(/[[:space:]]+$/, "")` to extract_field so `status: closed ` is trimmed to `closed` and matches the `closed)` checkbox case. Resolving. + +### Thread 14: tools/backlog/generate-index.sh:107 (resolved) **@chatgpt-codex-connector** (2026-04-24T11:06:33Z): @@ -372,7 +428,11 @@ The loop `for file in $files` splits on whitespace, so any matching backlog file Useful? React with 👍 / 👎. -### Thread 15: docs/backlog/README.md:33 (unresolved) +**@AceHack** (2026-04-24T15:03:39Z): + +Fixed in df1224f: replaced `for file in $files` with NUL-delimited `find -print0 | sort -z` piped into `while IFS= read -r -d ...`. Paths containing spaces are now preserved as single tokens. Resolving. + +### Thread 15: docs/backlog/README.md:33 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:06:49Z): @@ -381,7 +441,11 @@ P1: This section claims the existing monolith is “6100-line”, but `docs/BACK the existing single-file `docs/BACKLOG.md` content into per-row ``` -### Thread 16: tools/backlog/generate-index.sh:11 (unresolved) +**@AceHack** (2026-04-24T15:03:45Z): + +Fixed in df1224f: softened to `the existing single-file docs/BACKLOG.md content` so the description does not drift as the monolith grows (currently 8362 lines, not 6100 or 7658). Resolving. + +### Thread 16: tools/backlog/generate-index.sh:11 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:06:50Z): @@ -391,7 +455,11 @@ P1: The header references `docs/research/backlog-split-design-otto-181.md`, but # because the earlier design-spec cross-reference went stale. ``` -### Thread 17: tools/backlog/generate-index.sh:58 (unresolved) +**@AceHack** (2026-04-24T15:03:48Z): + +Verified `docs/research/backlog-split-design-otto-181.md` DOES exist in the repo (ls confirms); the reviewer comment is factually mistaken. Keeping the xref as-is since the target file is present. Resolving with explanation. + +### Thread 17: tools/backlog/generate-index.sh:58 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:06:50Z): @@ -400,7 +468,11 @@ P1: `extract_field` claims POSIX awk compatibility, but the quote-stripping rege gsub(/^"|"$|^[[:space:]]*\047|\047[[:space:]]*$/, "") # Codex P1: handle both " and '\'' ``` -### Thread 18: tools/backlog/generate-index.sh:162 (unresolved) +**@AceHack** (2026-04-24T15:03:50Z): + +Fixed in df1224f: swapped `\x27` hex escape to `\047` octal, which is POSIX-awk portable (macOS awk-compatible). Inline comment updated to cite the portability reason. Resolving. + +### Thread 18: tools/backlog/generate-index.sh:162 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:06:51Z): @@ -412,13 +484,21 @@ P2: This comment hardcodes the monolith size as “6100-line”, but `docs/BACKL # would destroy the real backlog. ``` -### Thread 19: tools/backlog/README.md:150 (unresolved) +**@AceHack** (2026-04-24T15:03:52Z): + +Fixed in df1224f: softened comment from `6100-line monolith` to `pre-split monolithic backlog` — drift-proof wording, no exact line count. Resolving. + +### Thread 19: tools/backlog/README.md:150 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:06:51Z): P1: The cross-references section points to files/scripts that don’t exist in the repo (`docs/research/backlog-split-design-otto-181.md` and `tools/hygiene/audit-git-hotspots.sh`). Please either add the referenced artifacts or update these links to point at the correct existing locations. -### Thread 20: tools/backlog/README.md:26 (unresolved) +**@AceHack** (2026-04-24T15:04:02Z): + +Fixed in df1224f: the `tools/hygiene/audit-git-hotspots.sh` xref was a forward-reference to unmerged tooling. Softened to describe the detector as "unmerged at the time of this Phase-1a PR" with a `git log --diff-filter=A` recovery path; the design rationale for this PR does not depend on that script. The other xref (`docs/research/backlog-split-design-otto-181.md`) DOES exist and is kept. Resolving. + +### Thread 20: tools/backlog/README.md:26 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T11:06:51Z): @@ -427,6 +507,10 @@ P2: The structure diagram lists `tools/backlog/new-row.sh` as if it exists, but new-row.sh ← scaffolds a new row file (Phase 1b) ``` +**@AceHack** (2026-04-24T15:04:04Z): + +Fixed in df1224f: annotated `new-row.sh` as `(Phase 1b)` in the structure diagram, matching the existing `(DO NOT EDIT)` annotation style for BACKLOG.md. Phase 1b status is now visible on the line itself. Resolving. + ## General comments ### @AceHack (2026-04-24T10:27:22Z) @@ -442,3 +526,7 @@ Requesting peer-agent review on this BACKLOG-split Phase-1a tooling per Aaron Ot 5. **Phase-2 migration risk** — the content-split mega-PR will touch 6100 lines. Heuristic parsing (bullet-list items under priority headers) — will it miss any row shapes? Rows with nested bullets? Multi-paragraph directives? Independent eyes welcome. Advisory only; Aaron makes final calls on any design changes. + +### @chatgpt-codex-connector (2026-04-24T15:03:32Z) + +You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage). diff --git a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md index 1780061f..8ae7ecb3 100644 --- a/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md +++ b/docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md @@ -6,7 +6,7 @@ state: "OPEN" created_at: "2026-04-24T11:23:49Z" head_ref: "tools/pr-preservation-phase-2-minimal" base_ref: "main" -archived_at: "2026-04-24T14:49:33Z" +archived_at: "2026-04-24T15:37:11Z" archive_tool: "tools/pr-preservation/archive-pr.sh" --- @@ -82,7 +82,7 @@ Scope out of this PR per maintainer *"make sure you backlog then to a proper lon Here are some automated review suggestions for this pull request. **Reviewed commit:** `cc217ae031` - +
ℹ️ About Codex in GitHub
@@ -96,7 +96,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T11:28:45Z) @@ -177,7 +177,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `66e48f0630` - +
ℹ️ About Codex in GitHub
@@ -191,7 +191,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @AceHack (2026-04-24T13:41:33Z) @@ -206,7 +206,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `dfc8ef554f` - +
ℹ️ About Codex in GitHub
@@ -220,7 +220,7 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T13:47:03Z) @@ -269,7 +269,7 @@ _(no body)_ Here are some automated review suggestions for this pull request. **Reviewed commit:** `a5331f79e5` - +
ℹ️ About Codex in GitHub
@@ -283,9 +283,23 @@ If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". - +
+### COMMENTED — @AceHack (2026-04-24T14:50:39Z) + +_(no body)_ + +### COMMENTED — @AceHack (2026-04-24T14:50:42Z) + +_(no body)_ + +### COMMENTED — @copilot-pull-request-reviewer (2026-04-24T14:55:42Z) + +## Pull request overview + +Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments. + ## Review threads ### Thread 1: tools/pr-preservation/archive-pr.sh:60 (resolved) @@ -412,7 +426,7 @@ P0: The backfilled `docs/pr-discussions/*.md` files include whitespace-only line Fixed in 66e48f063082045871fb7d970bdb6fe213633524. Ran `find docs/pr-discussions -name '*.md' -exec perl -i -pe 's/[ \t]+$//' {} +` across all existing archives — MD009 clean (grep-verified: zero trailing-whitespace lines remaining). Also added an inline trailing-whitespace strip inside the archive writer itself so future archive runs stay clean without needing the post-hoc sweep. -### Thread 10: tools/pr-preservation/archive-pr.sh:232 (resolved) +### Thread 10: tools/pr-preservation/archive-pr.sh:242 (resolved) **@chatgpt-codex-connector** (2026-04-24T13:26:42Z): @@ -476,7 +490,7 @@ content = '\n'.join(normalized_lines).rstrip() + '\n' Fixed in a5331f7. Added the blank-line-run collapse you suggested (3+ consecutive blank lines -> 2) so archives stay clean under markdownlint MD012 without needing to ignore `docs/pr-discussions/**`. Also regenerated the three backfilled archives that were tripping MD012 (PR-0350, PR-0352, PR-0354) — they're now clean. Kept user-authored trailing whitespace preserved (markdown two-space hard-line-breaks) per the sibling thread. -### Thread 13: tools/pr-preservation/archive-pr.sh:80 (resolved) +### Thread 13: tools/pr-preservation/archive-pr.sh:90 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): @@ -486,7 +500,7 @@ The repo-detection comment says the script "falls back" to Lucent-Financial-Grou Fixed in a5331f7. The comment used to say "falls back" but the implementation hard-fails with exit 1 — updated the comment to match actual behavior: requires `gh repo view` to succeed, no silent fallback to a baked-in NWO. Rationale: better to fail loud than archive to the wrong repo path on a fork. -### Thread 14: tools/pr-preservation/archive-pr.sh:61 (resolved) +### Thread 14: tools/pr-preservation/archive-pr.sh:71 (resolved) **@copilot-pull-request-reviewer** (2026-04-24T13:47:02Z): @@ -550,7 +564,7 @@ Header comment still describes the output path as `docs/pr-discussions/PR---.md` filename shape, matching the implementation's `f'PR-{number:04d}-{slug}.md'` and the README. -### Thread 19: tools/pr-preservation/archive-pr.sh:325 (unresolved) +### Thread 19: tools/pr-preservation/archive-pr.sh:325 (resolved) **@chatgpt-codex-connector** (2026-04-24T14:42:54Z): @@ -560,7 +574,11 @@ The output path is keyed by a slug derived from the current PR title, so re-arch Useful? React with 👍 / 👎. -### Thread 20: tools/pr-preservation/archive-pr.sh:369 (unresolved) +**@AceHack** (2026-04-24T14:50:39Z): + +Fixed in 549369f. PR number is now the canonical archive key: on re-archive, we glob for existing `PR--*.md` and reuse that path regardless of current title (option b from your suggestion — preserves the human-friendly slug while ensuring idempotency). Smoke-tested: running `./archive-pr.sh 357` wrote back to the same PR-0357-* file, no orphan produced. + +### Thread 20: tools/pr-preservation/archive-pr.sh:369 (resolved) **@chatgpt-codex-connector** (2026-04-24T14:42:54Z): @@ -569,3 +587,65 @@ Useful? React with 👍 / 👎. Comment/review text is normalized with `.strip()`, which removes leading spaces and leading blank lines from user-authored content before writing the archive. This is lossy for markdown that intentionally starts with indentation (for example indented code blocks) and undermines the script’s preservation/audit goal; the same pattern is used for thread and general comments as well. Useful? React with 👍 / 👎. + +**@AceHack** (2026-04-24T14:50:42Z): + +Fixed in 549369f. Switched `.strip()` to `.rstrip('\n')` for review bodies, thread-comment bodies, and general-comment bodies so leading whitespace (indented code blocks, nested bullets) survives the archive round-trip. Only trailing newlines are stripped now. Review state check on empty bodies uses `body_text.strip()` as a boolean guard only — does not mutate the archived text. + +### Thread 21: tools/pr-preservation/archive-pr.sh:34 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T14:55:40Z): + +P0: This header says the script adds a “truncation warning for threads (>100) and per-thread comments (>100)”, but the implementation only paginates; it never emits any warning. Either implement the warning (e.g., when a connection requires >1 page) or remove the claim so operators aren’t misled about what diagnostics they’ll get. +```suggestion +# - Pagination for threads (>100) and per-thread comments +# (>100). +``` + +### Thread 22: tools/pr-preservation/archive-pr.sh:384 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T14:55:41Z): + +P0: `body.rstrip()` strips spaces/tabs from the end of the PR description’s final line. That conflicts with the goal of preserving markdown semantics (two trailing spaces = hard line break). Use a newline-only trim (e.g., rstrip of `\n`) or avoid trimming entirely for user-authored text. +```suggestion + lines.append(body.rstrip('\n')) +``` + +### Thread 23: tools/pr-preservation/archive-pr.sh:457 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T14:55:41Z): + +P0: `content = '\n'.join(lines).rstrip() + '\n'` (and the later `.rstrip()` after collapsing blanks) will still remove trailing spaces from the last line of the archive, so the output is not a fully faithful copy even after removing per-line trimming. Prefer trimming only trailing newlines (or leaving content unchanged) so end-of-file hard line breaks aren’t lost. + +### Thread 24: docs/pr-discussions/PR-0357-tools-pr-preservation-minimal-archive-otto-207-session-backf.md:86 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T14:55:41Z): + +P0: This archive contains a whitespace-only line (line 85) which will trip markdownlint MD009 (no trailing spaces) because `docs/pr-discussions/**` is not in the markdownlint ignore list. Either normalize whitespace-only lines to empty during archive generation (while still preserving meaningful inline trailing spaces), or add this directory to markdownlint ignores to keep CI green. +```suggestion + +``` + +### Thread 25: docs/pr-discussions/PR-0354-tools-backlog-split-phase-1a-generator-schema-example-row-aa.md:84 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T14:55:42Z): + +P0: This archive includes a whitespace-only line (line 83) which will fail markdownlint MD009 (no trailing spaces). If the intent is for `docs/pr-discussions/**` to be lint-clean, the archiver needs to normalize whitespace-only lines (or the directory should be markdownlint-ignored like other verbatim archives). +```suggestion + +``` + +### Thread 26: docs/pr-discussions/PR-0350-docs-frontier-rename-pass-2-hindu-ff7-egyptian-greek-norse-s.md:63 (unresolved) + +**@copilot-pull-request-reviewer** (2026-04-24T14:55:42Z): + +P0: Whitespace-only line at 62 will violate markdownlint MD009 (no trailing spaces). This appears to be a systematic artifact of archiving Codex connector comments; consider normalizing whitespace-only lines during archive generation or excluding `docs/pr-discussions/**` from markdownlint if verbatim preservation is required. +```suggestion + +``` + +## General comments + +### @chatgpt-codex-connector (2026-04-24T14:50:43Z) + +You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage). diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index 6d5e5b39..a4a600b6 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -30,8 +30,8 @@ # 2 PR fetch failed (auth / network / GraphQL errors / not found) # # Review-thread drain Otto-226 fixes (PR #357): -# - Pagination + truncation warning for threads (>100) and -# per-thread comments (>100). +# - Pagination for threads (>100) and per-thread comments +# (>100). # - Top-level `errors` and `pullRequest: null` detection # before dereferencing (both Python and shell paths). # - Dynamic owner/name from `gh repo view` (works from @@ -60,6 +60,20 @@ # switched `.strip()` to `.rstrip('\n')` for review / # thread-comment / general-comment bodies so indented # code blocks and bullets survive the round-trip. +# Review-thread drain Otto-236 fixes (PR #357, fourth pass): +# - Header "truncation warning" claim removed — the +# implementation paginates, never emits a warning, so the +# comment now describes actual behaviour. +# - PR-description body now uses `.rstrip('\n')` instead of +# `.rstrip()` so a hard-line-break on the last line of +# the description is preserved. +# - End-of-file content join uses `.rstrip('\n')` instead +# of `.rstrip()` so end-of-file hard-line-breaks survive. +# - Whitespace-only lines (e.g. indented connector output +# with only spaces/tabs, and no preceding text) are +# normalized to empty so markdownlint MD009 stays clean. +# Lines with any non-whitespace character keep trailing +# whitespace intact (two-space hard-line-breaks). set -euo pipefail @@ -381,7 +395,10 @@ body = pr.get('body') or '' if body.strip(): lines.append('## PR description') lines.append('') - lines.append(body.rstrip()) + # Preserve trailing whitespace on the last line (markdown two-space + # hard-line-break); only strip trailing newlines so the next + # section header is spaced correctly. + lines.append(body.rstrip('\n')) lines.append('') reviews = (pr.get('reviews') or {}).get('nodes', []) @@ -437,24 +454,37 @@ if comments: lines.append(body_text) lines.append('') -content = '\n'.join(lines).rstrip() + '\n' # Preserve trailing whitespace on user-authored text: # markdown uses two trailing spaces as a hard-line-break, # and this tool's purpose is a faithful audit copy. +# Only strip trailing newlines at end-of-file (not spaces), +# so an end-of-file hard-line-break survives. +content = '\n'.join(lines).rstrip('\n') + '\n' # We DO collapse runs of 3+ consecutive blank lines down # to 2 so markdownlint MD012 stays green on the generated # archives (the source content sometimes has 3+ blank # lines around
blocks). +# We ALSO normalize whitespace-only lines (e.g. ' ' from +# Codex connector comments) to empty so they do not trip +# markdownlint MD009. A line containing only spaces/tabs +# has no meaningful trailing whitespace to preserve — it +# cannot be a hard-line-break since there is no preceding +# text on the same line. Lines with any non-whitespace +# character keep their trailing whitespace intact. collapsed = [] blank_run = 0 for raw_line in content.split('\n'): + # Normalize whitespace-only lines to empty without touching + # inline trailing whitespace on lines that contain text. + if raw_line and raw_line.strip() == '': + raw_line = '' if raw_line == '': blank_run += 1 else: blank_run = 0 if blank_run <= 2: collapsed.append(raw_line) -content = '\n'.join(collapsed).rstrip() + '\n' +content = '\n'.join(collapsed).rstrip('\n') + '\n' with open(path, 'w', encoding='utf-8') as f: f.write(content) print(f'wrote {path} ({len(content)} bytes, {len(threads)} threads, {len(reviews)} reviews, {len(comments)} comments)') From 3c4c9b84097f4a551341eae770f1b368e1a58aef Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 11:59:37 -0400 Subject: [PATCH 08/10] =?UTF-8?q?fix(#357):=20Codex=20P1=20audit-fidelity?= =?UTF-8?q?=20carve-out=20=E2=80=94=20skip=20blank-line=20collapse=20insid?= =?UTF-8?q?e=20fenced=20code=20blocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review thread on PR #357 (line 486, P1, unresolved after 4 prior drain rounds): the formatter globally collapses every run of blank lines to at most 2 after assembling the archive, which silently rewrites user-authored bodies. In PR comments / reviews that include fenced code blocks, logs, or templates where 3+ consecutive blank lines are intentional, this changes the preserved content and breaks the script's stated audit-fidelity goal. Narrow fix: toggle code-fence state while scanning (``` / ~~~ at the start of a line, ignoring leading whitespace), and SKIP both the blank-line-run collapse and the whitespace-only normalization inside fenced regions. Outside fences, MD012 / MD009 hygiene still applies to tool-generated scaffolding so archives stay lint-clean. Rationale: markdownlint MD012 already exempts fenced code from the "no multiple consecutive blank lines" rule by design, so this fix aligns with the linter's own semantics. Fenced regions in PR review text are exactly where audit fidelity must win over scaffolding hygiene — that is where logs, templates, and preformatted payloads live. Smoke-tested against PR #357 itself: re-running archive-pr.sh 357 produces a 107-line diff of recovered content (mostly inside the
fenced blocks from Codex / Copilot connector payloads that the prior collapse was truncating). Archive-file churn reverted on this branch — archive regeneration belongs in a separate PR, not here. Gates: `bash -n` clean + `shellcheck` clean. Co-Authored-By: Claude Opus 4.7 --- tools/pr-preservation/archive-pr.sh | 42 +++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index a4a600b6..bb3d0027 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -74,6 +74,16 @@ # normalized to empty so markdownlint MD009 stays clean. # Lines with any non-whitespace character keep trailing # whitespace intact (two-space hard-line-breaks). +# Review-thread drain Otto-241 fix (PR #357, fifth pass): +# - Blank-line-run collapse + whitespace-only normalization +# are now SKIPPED inside fenced code blocks (``` / ~~~). +# Fenced blocks are where user-authored 3+ blank-line +# runs and leading whitespace are intentional (logs, +# templates, preformatted output); markdownlint MD012 +# already exempts fenced code from the "no multiple +# consecutive blank lines" rule, so audit fidelity wins +# inside fences. Outside fences, MD012/MD009 hygiene +# still applies to tool-generated scaffolding. set -euo pipefail @@ -471,11 +481,39 @@ content = '\n'.join(lines).rstrip('\n') + '\n' # cannot be a hard-line-break since there is no preceding # text on the same line. Lines with any non-whitespace # character keep their trailing whitespace intact. +# +# Codex P1 audit-fidelity carve-out (PR #357 thread on +# blank-line collapse): inside fenced code blocks (```), +# user-authored content must survive verbatim — fenced +# blocks are where 3+ consecutive blank lines are +# intentional (logs, templates, preformatted output), +# and markdownlint MD012 already exempts fenced code +# from the "no multiple consecutive blank lines" rule +# by design. So we toggle code-fence state as we scan +# and skip the collapse + whitespace-only normalization +# inside fenced regions. Outside fences, MD012/MD009 +# hygiene still applies to tool-generated scaffolding. collapsed = [] blank_run = 0 +in_fence = False for raw_line in content.split('\n'): - # Normalize whitespace-only lines to empty without touching - # inline trailing whitespace on lines that contain text. + # Detect fenced-code-block boundaries (``` or ~~~ at + # the start of a line, ignoring leading whitespace). + stripped = raw_line.lstrip() + if stripped.startswith('```') or stripped.startswith('~~~'): + in_fence = not in_fence + blank_run = 0 + collapsed.append(raw_line) + continue + if in_fence: + # Inside a fenced block: preserve verbatim. + # No whitespace-only normalization, no blank-run + # collapse. This is the audit-fidelity path. + collapsed.append(raw_line) + continue + # Outside fences: normalize whitespace-only lines to empty + # without touching inline trailing whitespace on lines + # that contain text. if raw_line and raw_line.strip() == '': raw_line = '' if raw_line == '': From 828fefa55e694f1c4e7cf7a946fbb2f1428f42fa Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 22:26:03 -0400 Subject: [PATCH 09/10] drain(#357): fence-marker type-match + gh --jq consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex/Copilot threads on #357's archive-pr.sh: 1. **Codex P2 — fence detector conflates ``` and ~~~.** CommonMark requires the closing fence to use the SAME marker character as the opener (backticks close backticks; tildes close tildes). The previous `in_fence = not in_fence` on any fence-shaped line would prematurely close a backtick fence when a tilde line appeared inside it (and vice versa). Fix: track fence_marker on open, only flip back to False when the marker matches. Different-marker fence lines inside an open fence fall through to the verbatim branch so they're preserved as content. 2. **Copilot — `gh repo view -q` → `--jq` for consistency.** Other repo scripts (e.g. tools/hygiene/check-github-settings-drift.sh) use `--jq`. Switching to the long form matches the rest of the factory's gh invocations and avoids any `-q` ambiguity across gh versions. Bash -n syntax check passes. Co-Authored-By: Claude Opus 4.7 --- tools/pr-preservation/archive-pr.sh | 35 ++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index bb3d0027..d82b45bb 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -108,7 +108,7 @@ REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" # with exit 1 rather than silently defaulting to a baked-in # NWO (better to fail loud than archive to the wrong repo # path on a fork). -REPO_NWO="$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null || true)" +REPO_NWO="$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null || true)" if [ -z "${REPO_NWO}" ]; then echo "error: could not detect repo via 'gh repo view'. Is gh authenticated and this a GitHub repo?" >&2 exit 1 @@ -496,15 +496,38 @@ content = '\n'.join(lines).rstrip('\n') + '\n' collapsed = [] blank_run = 0 in_fence = False +fence_marker = None # '`' or '~' — opener type must match closer for raw_line in content.split('\n'): # Detect fenced-code-block boundaries (``` or ~~~ at # the start of a line, ignoring leading whitespace). + # Per CommonMark, the closing fence must use the SAME + # marker character as the opener (backticks close + # backticks; tildes close tildes). Previously the + # detector flipped on any fence-shaped line, which + # would prematurely close a backtick fence on a tilde + # line (and vice versa). stripped = raw_line.lstrip() - if stripped.startswith('```') or stripped.startswith('~~~'): - in_fence = not in_fence - blank_run = 0 - collapsed.append(raw_line) - continue + marker = None + if stripped.startswith('```'): + marker = '`' + elif stripped.startswith('~~~'): + marker = '~' + if marker is not None: + if not in_fence: + in_fence = True + fence_marker = marker + blank_run = 0 + collapsed.append(raw_line) + continue + if marker == fence_marker: + in_fence = False + fence_marker = None + blank_run = 0 + collapsed.append(raw_line) + continue + # Different-marker fence line inside an open fence: + # fall through to the in_fence verbatim branch so + # the line is preserved without flipping state. if in_fence: # Inside a fenced block: preserve verbatim. # No whitespace-only normalization, no blank-run From 737192b479cae3cf3e4aa7d6f1b1581c7059cfbb Mon Sep 17 00:00:00 2001 From: Aaron Stainback Date: Fri, 24 Apr 2026 22:41:26 -0400 Subject: [PATCH 10/10] drain(#357): REPO_ROOT git-tree guard + mktemp template + fence-length tracking + README cross-ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five Copilot + Codex threads: 1. **REPO_ROOT bogus-path risk.** `git rev-parse --show-toplevel || pwd` falls back to pwd outside a git checkout, but `gh repo view` can succeed via `gh repo set-default`, so the script could write docs/pr-discussions/ into a bogus REPO_ROOT directory. Hard-fail when not inside a git working tree. 2. **mktemp portability.** Plain `mktemp` with no template works on GNU coreutils (Linux) but fails on BSD mktemp (macOS). README advertises macOS support, so added `-t zeta-archive-pr.XXXXXX` template that works on both. 3. **Fence-length tracking (Codex P2 + Copilot).** Prior fix tracked marker TYPE (backtick vs tilde) but not fence LENGTH. Per CommonMark §4.5, the closing fence must be at least as long as the opener — a 4-backtick opener contains a 3-backtick line as content, not a closer. Now tracks both marker + length on open; closer must match BOTH. 4. **README cross-ref correction.** Canonical source for "agents, not bots" terminology is GOVERNANCE.md §3 ("Contributors are agents, not bots"). CLAUDE.md carries a session-bootstrap pointer at the same rule. Reworded to name GOVERNANCE as canonical with CLAUDE.md as the pointer. Co-Authored-By: Claude Opus 4.7 --- tools/pr-preservation/README.md | 7 ++-- tools/pr-preservation/archive-pr.sh | 51 ++++++++++++++++++++++------- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/tools/pr-preservation/README.md b/tools/pr-preservation/README.md index a075b66c..c2e8ef58 100644 --- a/tools/pr-preservation/README.md +++ b/tools/pr-preservation/README.md @@ -104,9 +104,10 @@ Tracked on the PR-preservation BACKLOG row. Phases: (Copilot, Codex, Claude Code personas, github-actions) archive verbatim; human-reviewer comments get a privacy-pass step. Scope open. Terminology per - GOVERNANCE §3 + CLAUDE.md "agents, not bots" — Copilot - and Codex are agents with agency and accountability, - not bots. + `GOVERNANCE.md` §3 ("Contributors are agents, not + bots") — Copilot and Codex are agents with agency + and accountability, not bots. `CLAUDE.md` carries + a session-bootstrap pointer at the same rule. ## Dependencies diff --git a/tools/pr-preservation/archive-pr.sh b/tools/pr-preservation/archive-pr.sh index d82b45bb..f447efdd 100755 --- a/tools/pr-preservation/archive-pr.sh +++ b/tools/pr-preservation/archive-pr.sh @@ -101,7 +101,15 @@ if ! [[ "$PR" =~ ^[0-9]+$ ]]; then echo "usage: $0 " >&2 exit 1 fi -REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" +# Require an actual git checkout. `gh repo view` can succeed +# outside a checkout (via `gh repo set-default` / an auth'd +# session pointing at a remote), which would cause the +# archive to be written to a bogus REPO_ROOT derived from +# `pwd`. Hard-fail if we aren't inside a git working tree. +if ! REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)"; then + echo "error: not inside a git working tree. Archive-pr.sh must run from a Zeta checkout so the docs/pr-discussions/ output lives in the right repo." >&2 + exit 1 +fi # Dynamic owner / name — works from forks or after rename. # Requires `gh repo view` to succeed; the script hard-fails @@ -120,7 +128,10 @@ export REPO_ROOT PR REPO_OWNER REPO_NAME OUT_DIR="${REPO_ROOT}/docs/pr-discussions" mkdir -p "$OUT_DIR" -TMP="$(mktemp)" +# Use an explicit template so mktemp works on both GNU +# coreutils (Linux) and BSD mktemp (macOS) — plain `mktemp` +# with no argument fails on BSD. +TMP="$(mktemp -t zeta-archive-pr.XXXXXX)" trap 'rm -f "$TMP"' EXIT # Paginated fetch: drive it from Python (single interpreter, @@ -497,37 +508,53 @@ collapsed = [] blank_run = 0 in_fence = False fence_marker = None # '`' or '~' — opener type must match closer +fence_length = 0 # opener run length — closer must be >= this for raw_line in content.split('\n'): # Detect fenced-code-block boundaries (``` or ~~~ at # the start of a line, ignoring leading whitespace). - # Per CommonMark, the closing fence must use the SAME - # marker character as the opener (backticks close - # backticks; tildes close tildes). Previously the - # detector flipped on any fence-shaped line, which - # would prematurely close a backtick fence on a tilde - # line (and vice versa). + # Per CommonMark §4.5: + # - Closing fence must use the SAME marker character + # as the opener (backticks close backticks; + # tildes close tildes). + # - Closing fence must be AT LEAST AS LONG as the + # opener. An opener of 4 backticks needs a + # closer of 4+ backticks; 3 backticks inside a + # 4-backtick fence is content, not a closer. + # This is how CommonMark lets you nest fences — + # a longer opener contains shorter fence-shaped + # lines as literal content. stripped = raw_line.lstrip() marker = None + marker_len = 0 if stripped.startswith('```'): marker = '`' + marker_len = len(stripped) - len(stripped.lstrip('`')) elif stripped.startswith('~~~'): marker = '~' + marker_len = len(stripped) - len(stripped.lstrip('~')) if marker is not None: if not in_fence: + # Opening fence: record marker + length so the + # closer can be matched strictly. in_fence = True fence_marker = marker + fence_length = marker_len blank_run = 0 collapsed.append(raw_line) continue - if marker == fence_marker: + if marker == fence_marker and marker_len >= fence_length: + # Closing fence: same marker, length >= opener. in_fence = False fence_marker = None + fence_length = 0 blank_run = 0 collapsed.append(raw_line) continue - # Different-marker fence line inside an open fence: - # fall through to the in_fence verbatim branch so - # the line is preserved without flipping state. + # Fence-shaped line that isn't a valid closer + # (wrong marker, or shorter than the opener): + # fall through to the in_fence verbatim branch + # so the line is preserved as content without + # flipping state. if in_fence: # Inside a fenced block: preserve verbatim. # No whitespace-only normalization, no blank-run