docs(preservation): archive recent PR discussions (3046-3051)#3055
Merged
Conversation
Lior automated PR preservation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48ea64ced7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. 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".
There was a problem hiding this comment.
Pull request overview
Adds new git-tracked PR discussion archive markdown files under docs/pr-discussions/ for PRs #3046–#3051, capturing PR metadata, descriptions, and review content as durable substrate.
Changes:
- Add 5 new PR discussion archive files (PRs #3046, #3047, #3048, #3050, #3051) with YAML frontmatter and archived review content.
- Preserve verification/test-plan notes and review threads as part of the archived record.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/pr-discussions/PR-3046-docs-backlog-b-0449-bg-services-slice-5-subscriber-agent-des.md | Adds archived discussion record for PR #3046. |
| docs/pr-discussions/PR-3047-fix-backlog-restore-missing-slice-rows-b-0054-1-b-0055-1.md | Adds archived discussion record for PR #3047. |
| docs/pr-discussions/PR-3048-docs-rules-rule-0-mark-the-3-legacy-hygiene-sh-violations-as.md | Adds archived discussion record for PR #3048. |
| docs/pr-discussions/PR-3050-docs-rules-otto-channels-card-b-0444-shipped-retire-follow-u.md | Adds archived discussion record for PR #3050. |
| docs/pr-discussions/PR-3051-tools-mark-codex-loop-headless-provenance.md | Adds archived discussion record for PR #3051. |
Comments suppressed due to low confidence (3)
docs/pr-discussions/PR-3047-fix-backlog-restore-missing-slice-rows-b-0054-1-b-0055-1.md:36
- P2 (documentation): These table rows start with
||, which renders as an empty first column and makes the archive harder to read. Consider dropping the extra leading|so the table header begins with a single|.
| Signal | Before | After |
|---|---|---|
| Dangling dep refs | 8 | 6 |
| Ready-to-grind candidates | 211 | 217 |
docs/pr-discussions/PR-3048-docs-rules-rule-0-mark-the-3-legacy-hygiene-sh-violations-as.md:27
- P2 (documentation): These table rows start with
||, which renders as an empty first column and makes the archive harder to read. Consider dropping the extra leading|so the table header begins with a single|.
| Was (`.sh`) | Now (`.ts`) |
|---|---|
| `tools/hygiene/audit-lost-files.sh` | [`audit-lost-files.ts`](tools/hygiene/audit-lost-files.ts) |
| `tools/hygiene/audit-trajectories.sh` | [`audit-trajectories.ts`](tools/hygiene/audit-trajectories.ts) |
| `tools/hygiene/audit-backlog-items.sh` | [`audit-backlog-items.ts`](tools/hygiene/audit-backlog-items.ts) |
docs/pr-discussions/PR-3046-docs-backlog-b-0449-bg-services-slice-5-subscriber-agent-des.md:32
- P2 (documentation): These table rows start with
||, which renders as an empty first column and makes the archive harder to read. Consider dropping the extra leading|so the table header begins with a single|.
| Option | Shape | Pro | Con |
|--------|-------|-----|-----|
| A | One subscriber per topic (3 processes) | Simple per-topic logic; failure isolation | 3 processes to monitor; duplicated boilerplate |
| B | Unified subscriber daemon (1 process, dispatches) | One process; reusable polling; cross-topic awareness | Monolith failure mode |
| **C (recommended)** | `subscribeOnce(topic, handler)` library; cron-tick function call | Composes with existing cron; surface-agnostic; failure-isolated; DST-compatible | Subscriber latency = cron tick latency (1min) |
Two review findings addressed: 1. docs/AGENT-BEST-PRACTICES.md: add docs/pr-discussions/** to the no-name-attribution allowlist alongside docs/pr-preservation/** (archive files legitimately carry author attribution as provenance metadata — generated by tools/pr-preservation/archive-pr.ts) 2. PR-3048 archive: rewrite repo-relative links (.claude/rules/..., tools/hygiene/...) to use ../../ prefix so they resolve correctly when viewed from docs/pr-discussions/ on GitHub Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
|
Fixed in 7e6c1b1: P1 threads (5x — attribution allowlist): Added P2 thread (broken relative links): Rewrote repo-relative links in PR-3048 archive file (e.g., P2 thread (table |
AceHack
added a commit
that referenced
this pull request
May 13, 2026
…rged this session Records: PRs #3054 + #3055 (Otto-Desktop's shadow-log + archive) merged. PR #3056 round-1 review surfaced 3 valid Copilot findings (missing eslint-disable, misleading field name, dead ternary); all fixed in 7444a05 + threads resolved. Both my PRs back to wait-ci, threads-clear. Co-Authored-By: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 13, 2026
…p row (#3056) * feat(bg/audit): duplicate-row-id audit tool + B-0451 substrate-cleanup row While resolving the B-0444 ID collision (PR #3053), an inline audit surfaced 12 ADDITIONAL duplicate-ID groups across the backlog directory. Silently-overwriting substrate state is high-severity hygiene risk: a consumer of `id: B-0409` gets one of THREE files depending on load order; every other substrate consumer's implicit primary-key guarantee is broken. Changes: - `tools/bg/audit-duplicate-row-ids.ts` — new audit tool: walks `docs/backlog/**/*.md` via `git ls-files`, extracts each frontmatter `id:` value, reports any ID appearing in >1 file. Exit code 0 = clean; 1 = duplicates found. - `tools/bg/audit-duplicate-row-ids.test.ts` — 14 tests covering id extraction, group sorting, real-world patterns (clean substrate, pair collision, triple collision, missing-id row skip, sub-row IDs, unreadable-file resilience). - `docs/backlog/P1/B-0451-duplicate-row-id-substrate-cleanup-2026-05-13.md` — tracks the cleanup work: lists all 12 collisions, classifies them into two patterns (cross-priority namespace bleed + within-priority concurrent decomposition), defines the per-collision resolution rule (keep the row with external references; renumber the other), and outlines CI-wiring as future work. Empirical findings: - 559 rows scanned - 12 collision groups (1 three-way: B-0409; 11 pairs) - Most pairs are P1-vs-P2 cross-priority bleed (Otto-Desktop vs parallel agents filing in overlapping ranges) - The B-0090.x sub-rows show a within-priority decomposition race (Riven's atomic-children sweep vs earlier B-0090 decomposition, both 2026-05-10/11) Tests: 14/14 pass on `tools/bg/audit-duplicate-row-ids.test.ts`. Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 2317Z — duplicate-ID audit tool surfaces 12 collisions; competing PR closed Records: PR #3053 (B-0444 collision) merged; PR #3051 (Codex provenance) merged; competing PR #3052 closed with substrate-honest comment; new audit tool finds 12 additional ID collisions on main (1 three-way: B-0409). PR #3056 ships the tool + B-0451 row tracking the per-collision cleanup work. Co-Authored-By: Claude <noreply@anthropic.com> * fix(bg/audit): address Copilot round-1 review on PR #3056 Three Copilot findings resolved: 1. Missing `// eslint-disable-next-line sonarjs/no-os-command-from-path` on the `spawnSync("git", ...)` invocation. Added the suffixed- rationale comment matching the form used in `tools/bg/backlog-ready-notifier.ts`. 2. `rowsScanned` was misleadingly named — it counted only rows with an extractable `id:` field, not total files inspected. The tests already asserted the smaller count; the CLI's "X rows scanned" message therefore under-reported. Renamed to `rowsWithId` and updated docstring + CLI message accordingly: "X rows with id field, no duplicate IDs". 3. Dead `idToFiles.size > 0` ternary — `reduce` on an empty iterable already returns 0. Simplified to a plain spread+reduce. Tests: 14/14 pass; the `rowsWithId` rename mechanically updates 3 test assertions. Audit tool still reports 12 collision groups on origin/main (no behavior change beyond the cleaner output). Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 2339Z — Copilot round-1 on PR #3056 addressed; 13 PRs merged this session Records: PRs #3054 + #3055 (Otto-Desktop's shadow-log + archive) merged. PR #3056 round-1 review surfaced 3 valid Copilot findings (missing eslint-disable, misleading field name, dead ternary); all fixed in 7444a05 + threads resolved. Both my PRs back to wait-ci, threads-clear. Co-Authored-By: Claude <noreply@anthropic.com> * fix(bg/audit): surface readFileSync errors instead of silently skipping (Codex P2) Codex P2 on PR #3056: `auditRowFiles` previously caught `readFileSync` failures with a bare `continue;` — silently swallowing the error and moving on. That created a false-negative path: if a backlog file was unreadable (permission, IO error, race with concurrent fs ops), any duplicate ID inside it never got checked, and the CLI could report "no duplicate IDs" with the failure hidden. Fix: - New `ReadError = { file, reason }` type - `AuditResult.readErrors: ReadError[]` accumulates per-file failures (preserves the original "continue scanning" behavior — see ALL problems, not abort on first) - CLI surfaces read errors with a distinct heading + exits non-zero when ANY read error OR duplicate is present - Success message only prints when both counts are 0 Tests updated (15/15 pass): - Renamed "unreadable files are skipped without crashing" → "unreadable files surface as readErrors (Codex P2: don't silently skip)" + assertions on the readErrors[] shape - Added "readErrors is empty when all files readable" to pin the zero-state contract Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 2358Z — Codex P2 round-2 on PR #3056 (surface read errors); rate-limit-failed CI triaged Records: PR #3056 CI failures triaged as GitHub-API-rate-limit exhaust during SARIF upload (not real bugs). Codex P2 round-2 finding addressed: `auditRowFiles` now accumulates `readErrors[]` and CLI fails non-zero on any read error or duplicate. 15/15 tests pass. Thread resolved. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated PR preservation by Lior loop.