-
Notifications
You must be signed in to change notification settings - Fork 1.1k
devlog: record the owner-backlog and bug-PR closeout #2624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+124
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
58 changes: 58 additions & 0 deletions
58
devlog/_plan/260825_owner_backlog_and_bugpr_closeout/130_wp16_late_bug_prs.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # 130 — wp16: bug PRs opened after the 260825 triage | ||
|
|
||
| The original triage snapshot (`000_research_snapshot.md`) enumerated 16 bug-labelled PRs. | ||
| Five more carry the `bug` label and were not in it, either because they were opened later | ||
| or because the label moved after the snapshot was taken: | ||
|
|
||
| | PR | Title | Author | State at triage | | ||
| |---|---|---|---| | ||
| | #2595 | fix(combos): bound preflight retained chunk count | luvs01 | review-ready, closes #2592 | | ||
| | #2583 | fix(kiro): preserve keyword-named composed properties | luvs01 | review-ready, closes #2571 | | ||
| | #2575 | fix(pricing): map Daybreak cost overlays and preserve model identity in logs | riique | draft | | ||
| | #2430 | fix(gui): align the sidebar foot's four rows | olddonkey | review-ready | | ||
| | #2427 | fix(test): pass --parallel so the full suite finishes instead of reading as hung | olddonkey | review-ready | | ||
|
|
||
| ## Why this is its own work-phase | ||
|
|
||
| The DONE criterion is "every bug-labelled PR is terminal", not "the sixteen I happened to | ||
| list yesterday". A snapshot taken at plan time is a starting inventory, not the scope. These | ||
| five are resolved on the same terms as the original sixteen: an independent review that | ||
| re-runs the focused suite against a merge with current `dev`, and a falsification pass that | ||
| reverts the production hunk to prove the regression test is load-bearing. | ||
|
|
||
| ## Verification standard applied | ||
|
|
||
| Each PR was reviewed in its own worktree by a separate reviewer with no knowledge of the | ||
| others' conclusions. The merge into `dev` happened only after: | ||
|
|
||
| 1. `git merge origin/dev` into the PR head resolved cleanly and `bun x tsc --noEmit` stayed at | ||
| exit 0 — a PR green on its own base is not evidence it is green on the current one; | ||
| 2. the focused suite covering the changed subsystem passed on that merge; | ||
| 3. reverting the production hunk made the new test fail, and restoring it made the test pass. | ||
|
|
||
| Step 3 is the one that earns its keep. It has already caught a patch in this unit's history | ||
| (#2488) whose test passed with the fix reverted — the test was pinning behavior that already | ||
| held, so the "fix" was decoration. That patch was dropped and only the test kept. | ||
|
|
||
| #2430 additionally required a rendered check rather than a passing assertion: it is a CSS | ||
| alignment change, and a unit test that reads the stylesheet cannot see what the browser lays | ||
| out. The reviewer built the GUI, served `gui/dist`, drove it with a real browser, and measured | ||
| the four rows — text left edge at x=49, trailing controls ending at x=207, row height 35.5px | ||
| across all four. | ||
|
|
||
| #2427 is the highest-risk of the five because it rewrites the test runner every later | ||
| verification depends on. It was checked against a real `SIGKILL` of the lock owner | ||
| (`RECLAIMED acquired=true`), and the full suite was run through the new runner end to end: | ||
| 14955 pass / 12 skip / 0 fail in 178s across seven lanes. A runner that reports green while | ||
| silently skipping lanes would be worse than the hang it replaces, so the lane totals were | ||
| summed rather than trusting the final line. | ||
|
|
||
| #2575 arrived as a draft with an unticked "resolved all Codex/CodeRabbit findings" box. The | ||
| review found no blockers, so it was marked ready and merged; the unticked box reflected an | ||
| author workflow state, not an outstanding finding. | ||
|
|
||
| ## Outcome | ||
|
|
||
| All five merged into `dev`. Linked issues (#2592, #2571) closed by hand with the merge SHA | ||
| and the falsification result quoted, because GitHub only auto-closes on merges into `main` | ||
| and every PR here targets `dev`. |
66 changes: 66 additions & 0 deletions
66
devlog/_plan/260825_owner_backlog_and_bugpr_closeout/140_closeout.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # 140 — closeout | ||
|
|
||
| ## Where this landed | ||
|
|
||
| Every issue authored by `lidge-jun` is closed. `gh issue list --author lidge-jun --state open` | ||
| returns zero rows. Every `bug`-labelled pull request is terminal except one, and that one is | ||
| open by decision rather than by omission. | ||
|
|
||
| Final `dev` verification at `1a92d6b55`: **15005 pass / 16 skip / 0 fail**, typecheck exit 0. | ||
|
|
||
| ## The one thing deliberately left open | ||
|
|
||
| **#2497** — native main token refresh and replay. It is the credential boundary AGENTS.md | ||
| places under explicit security review, it got one, and three blockers were verified by hand | ||
| rather than taken on a reviewer's word (`120_wp5_2497_security_review.md`): | ||
|
|
||
| 1. `auth.json` publication renames before it links, so a crash between the two strands the file | ||
| and nothing recovers it at startup. | ||
| 2. The same-account fallback adopts a *different* pool refresh grant into native-main. Account-id | ||
| equivalence is not grant ownership — this is the exact hazard `anthropic-routing.ts` fails | ||
| closed on. | ||
| 3. The "exactly one" 401 replay is one *logical* replay. The post-401 send goes through | ||
| `fetchWithTransientRetry`, whose 3x3 ladder means one recovery can be up to nine physical | ||
| sends. Nothing reaches the client twice, but upstream work can commit more than once. | ||
|
|
||
| (2) is a credential-ownership decision, not a defect to pick a side on silently: tightening it | ||
| to grant-only changes what happens to an operator who re-logged in through the pool and expects | ||
| main to follow. Fixing three security blockers inside someone else's 2,600-line credential PR | ||
| and admin-merging it is not a thing to do quietly. | ||
|
|
||
| ## What the run found that nobody asked for | ||
|
|
||
| Three defects that only appeared because the work was verified rather than assumed: | ||
|
|
||
| - **`ocx uninstall` could not remove a config home OpenCodex created itself.** Two of our own | ||
| writers produce files the ownership manifest never claimed. Only the disposable-host acceptance | ||
| could surface this, because only it runs the production uninstall against a home the product | ||
| built. Fixed in #2618. | ||
| - **Three regressions from combining #2463 and #1478**, each green in isolation and red together: | ||
| a `structuredClone` that threw on a non-cloneable provider value, an alias route that swallowed | ||
| the API-key-pool rename endpoint, and a command missing from both documentation sweeps. Fixed | ||
| in #2614. This is the argument for a full-suite gate that a per-PR gate cannot make. | ||
| - **A session lane keyed on the parent thread** would have 503'd every parallel subagent after | ||
| the first. Reproduced before fixing; a lane wants the most specific identity, which is the | ||
| opposite of what account affinity wants. | ||
|
|
||
| ## Recorded, not absorbed | ||
|
|
||
| - **#2622** — the Codex provenance ledger is never written; `updateIntegrationRecord` has no | ||
| production caller. Found while the #1048 rows tried to assert on it. The rows now fail only on | ||
| disagreement, because requiring an entry no production path can produce would have been testing | ||
| an unimplemented writer. | ||
| - **#2568's activation default stays opt-in.** The issue asks for presence-driven activation by | ||
| analogy with a 2-key API pool. An API-key pool spends the operator's own metered credit; | ||
| rotating across subscription accounts spends a second subscription's quota, which is why the | ||
| Anthropic pool shipped opt-in. Turning it on later costs nothing; a default-on rotation that | ||
| surprises someone has already spent the quota. Escalated to the owner with the one-line change | ||
| named. | ||
|
|
||
| ## What earned its keep | ||
|
|
||
| Falsification. Reverting each fix to confirm its test actually fails caught a patch earlier in | ||
| this unit (#2488) whose test passed with the fix removed — it was pinning behavior that already | ||
| held. That patch was dropped and only the test kept. Every fix merged here was checked the same | ||
| way, and three of them (the parent-thread lane, the atomic adoption publication, both uninstall | ||
| hunks) are load-bearing only because that check said so. | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_finThis file records the unit's terminal closeout, but the commit leaves the entire unit under
devlog/_plan, so repository searches for open work will continue to report completed work as outstanding. Move260825_owner_backlog_and_bugpr_closeouttodevlog/_finas part of this closeout commit.AGENTS.md reference: AGENTS.md:L75-L78
Useful? React with 👍 / 👎.