From 351f44ff753899e3e7fc0135857cf5e2edcdfdd3 Mon Sep 17 00:00:00 2001 From: JUN Date: Thu, 10 Sep 2026 08:42:43 +0900 Subject: [PATCH] docs(devlog): record #4147, #3859, and the #4141 unblock Three more rows are settled. #4153 merged as 2ce5f381f and closed #4147, as the contributor's own commit so authorship reaches his graph. #4160 merged as 8a5cfd366 and closed #3859. And PR #4152 landed as 9ba04b64d, which frees #4141 to start. Two operational facts are written down because they were easy to get wrong. A fork pull request does not start repository CI by itself, so the thin check list on #4153 was action_required rather than a passing PR. And the force-push that unstacked #4160 left an earlier run cancelled at the same SHA, whose aggregate job then reported failure; that is the third cancelled run this round that could have been read as a verdict. Also records the one real defect the Lane B audit found. The free-only filter counts the group header from the unfiltered rows, so the header claims more models than the list shows. The empty state gets it right; the header does not. Assigned to Lane B. NOT RUN: local test suite, typecheck, build, lint. Remote CI is the gate. --- .../110_delivery_record.md | 55 +++++++++++++++++-- .../_research/_audit_wp3.md | 54 ++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 devlog/_plan/260910_post249_round2/_research/_audit_wp3.md diff --git a/devlog/_plan/260910_post249_round2/110_delivery_record.md b/devlog/_plan/260910_post249_round2/110_delivery_record.md index 54c6eafa2a..81c81f3bb2 100644 --- a/devlog/_plan/260910_post249_round2/110_delivery_record.md +++ b/devlog/_plan/260910_post249_round2/110_delivery_record.md @@ -19,13 +19,58 @@ Append-only. One row per deliverable, filled when it actually lands on `dev`. | roadmap | — | #4155 | `3b9fab90e` | green | `a7509fe00` | — | | A1 | #4129 | #4157 | `421aea87a` | green | `4498fb910` | yes | | A2 | #4148 | #4161 | `799330bcf` | green | `5b8f1fcfa` | yes | -| A3 | #4141 | — | | | | held on #4152 | -| B1 | #3666 | #4156 | `3ff57ce49` | blocked | | | -| B2 | #4075 | #4158 | `3dc7bd19b` | blocked | | | -| B3 | #3859 | #4160 | | green | | parent blocked | +| A3 | #4141 | — | | | | in progress, #4152 landed | +| B1 | #3666 | #4156 | `3ff57ce49` | held | | | +| B2 | #4075 | #4158 | `3dc7bd19b` | held | | | +| B3 | #3859 | #4160 | `c8a734cd0` | green | `8a5cfd366` | yes | | B4 | #1711 | — | | | | awaiting decision | | B5 | #4038 | — | | | | awaiting decision | -| — | #4147 | #4153 | `abf35fa94` | green | | | +| — | #4147 | #4153 | `abf35fa94` | green | `2ce5f381f` | yes | + +### #4147 landed as the contributor's own commit + +#4153 merged unmodified, so authorship stays with @richardfeiliu-a11y and reaches +his contribution graph. Nothing was reimplemented or carried, which is why no +`Co-authored-by` trailer was needed. + +Two things are worth carrying forward from it. First, the review took one pass +because the contributor read a live `~/.zcode/v2/config.json` and the shipped +parser in `ZCode.app` instead of choosing between the two contradictory schemas in +the issue text — and this tree independently agrees with what he found, since +`src/integrations/ownership-policy.ts` already treats `models.*.reasoning` as +`enabled`/`variants`. Second, a fork pull request does not start repository CI on +its own: Cross-platform CI and React Doctor sat at `action_required` until +approved, which is why the check list looked thin for a while and would have been +easy to mistake for a passing PR. + +### #3859 was unstacked rather than left to wait + +#4160 was published on top of #4075 and #3666, both of which are held by the +screenshot gate. It depends on neither, so it was rebased straight onto `dev`, +retargeted, and merged on its own. + +That force-push produced the round's third cancelled run. An earlier +Cross-platform CI run at the same SHA was cancelled by the concurrency group, and +its aggregate `ci` job reported failure as a consequence. The verdict is run +`34417147997`, which actually concluded. This is the third time this round that a +cancelled run looked like a failure or a pass; the rule that only a real +conclusion counts has earned its place. + +### #4141 is unblocked + +PR #4152 landed as `9ba04b64d`. Lane A was told to adopt the `runLaunchctl` seam +that PR established rather than invent a second one, to re-verify every anchor in +`040_4141_launchctl_bootout.md` first because `src/service.ts` moved underneath +it, and to prove the behaviour with stderr fixtures — running `launchctl` remains +forbidden while a live proxy is up. + +### Open finding on #4156 + +The Lane B audit (`_research/_audit_wp3.md`) passed all three diffs but found one +real defect: `gui/src/pages/Models.tsx:1405` and `:1475` still count the group +header and `activeCount` from the unfiltered rows, so with the free-only filter on +the header claims more models than the list shows. The empty state at `:1681` does +it correctly. Assigned to Lane B. A1 and A2 were audited again **after** they landed, against `origin/dev` rather than against the lane's own report. Both match the fix the plan chose, both diff --git a/devlog/_plan/260910_post249_round2/_research/_audit_wp3.md b/devlog/_plan/260910_post249_round2/_research/_audit_wp3.md new file mode 100644 index 0000000000..8217a9f954 --- /dev/null +++ b/devlog/_plan/260910_post249_round2/_research/_audit_wp3.md @@ -0,0 +1,54 @@ +Read-only audit of #4156, #4158, and #4160 against `devlog/_plan/260910_post249_round2/`. Local product tests were not run. + +## #4156 — #3666 free-only filter + +Chosen fix matches the plan. `discoveredPricingStatus` reads provider `pricing.prompt|input` and `pricing.completion|output`, classifies only a complete non-negative numeric pair, and treats everything else as unknown. A `:free` suffix is ignored. `modelCosts` is still only a `manualPricing` marker and never feeds the classifier. + +Unknown is omitted from `CatalogModel` (`pricingStatus?: "free" | "paid"`) instead of stored as `"unknown"`. Callers treat absence as not-free, which keeps the fail-closed rule and the existing deep-equal hint tests. + +Filter order is correct on both surfaces: free-only runs, then search/sort, then `PAGE` / `CHIP_RENDER_CAP`. + +**SHOULD-FIX** `gui/src/pages/Models.tsx:1405` and `:1475` — group header still counts `rows` / `activeCount` from the unfiltered set. Plan 050 said counts and empty states read the filtered set. Empty state does (`:1681`); the header does not. + +**NOTE** `tests/gui/models-free-filter.test.ts:26` — pins the shared predicate, not `renderGroup` / `ProviderModelInventory` slice order. A later change that filtered after `slice(0, PAGE)` would still pass this file. Source order is correct today (`Models.tsx:1427-1435`, `ProviderModels.tsx:85-87,267`). + +**NOTE** `src/codex/catalog/parsing.ts:158` — plan wrote `pricingStatus: "free" | "paid" | "unknown"`; the field is omitted when unknown. Behavior matches fail-closed. + +Named classifier cases are present and would be red without the change: zero strings free (`catalog-free-pricing-status.test.ts:24-28`), priced pair paid (`:35-38`), Ollama unknown/absent (`:47-52`), one-sided/negative/non-numeric (`:55-70`), `:free` suffix not evidence (`:79-86`). Wire coverage landed here via `listManagementModelRows` rather than `tests/server/model-costs-management-api.test.ts`. No existing “must stay green” assertion was deleted. + +## #4158 — #4075 discovery-dependency hint + +Chosen fix matches. A failed group with rows now renders `DiscoveryDependencyHint` (`Models.tsx:1672`). The copy interpolates `pws.liveModels` (`models-provider-hints.tsx:54`) and navigates with `navigateHash("providers")` (`:55`). `models.discoveryFailedDependency` exists in all nine locale catalogs. Hint children are element-wrapped, so the header-child rule in `gui/tests/models-provider-head.test.ts` is not broken. + +The named regression is in `gui/tests/models-discovery-failed-hint.test.tsx:123-139` (badge + new sentence + control name + settings link on a failed group with a row). Those assertions would be red without the hint. Healthy / missing discovery cases stay silent (`:141-151`). Plan asked for `tests/gui/` plus layout.json; the file sits next to the existing happy-dom harness under `gui/tests/` instead, which is the right place for a page mount. + +**NOTE** this PR’s GitHub diff also includes `tests/codex-integration/catalog-free-pricing-status.test.ts` because #4158 is still based on `f4fae62ef` (first #4156 commit), while #4156 HEAD is `3ff57ce49`. That is stack drift of the #3666 wire-pin follow-up, not a #4075 plan miss. No #4075 “must stay green” test was weakened. + +## #4160 — #3859 email mask toggle + +Chosen fix matches, and the privacy default is not widened. `privacy.maskEmails` omitted/`true`/malformed stays masked; only literal `false` unmasks (`privacy.ts:22-23`, `config.ts:1151`). `getLoginStatus` takes a boolean and defaults to masked, with no config I/O (`oauth/index.ts:1816`). `oauth-account-routes` passes `emailMaskingEnabled(config)` and does not remask (`oauth-account-routes.ts:240,276`). + +Existing masked assertions still assert the masked form: + +- `tests/codex-integration/codex-auth-api.test.ts:1281-1305` +- `tests/oauth/oauth-status-privacy.test.ts:55-70` +- `tests/oauth/oauth-accounts-api.test.ts:260-269` (untouched) +- `tests/oauth/oauth-login-summary.test.ts:17` (untouched) +- `tests/cli/cli-status-oauth-health.test.ts:113` (untouched) +- `tests/gui/provider-workspace-auth.test.ts:51` (untouched) + +Opt-out is new cases, not a rewrite of those (`oauth-status-privacy.test.ts:81-100`, `codex-auth-api.test.ts:1313-1358`). + +The one rewritten test is the source-contains pin the plan already called brittle (`codex-auth-api.test.ts:5825-5834`). It now requires `projectEmail(..., maskFlowEmails)` at both login-status boundaries and forbids spreading raw `st.email`. That preserves the guarantee instead of deleting it. + +**NOTE** `src/codex/auth-api.ts:2748` still calls `getLoginStatus("chatgpt")` with the default mask. That poll only reads `done` / `loggedIn`, so it is not a disclosure path. + +No BLOCKER on this privacy surface: default remains masked, malformed values fail closed, tokens stay redacted on the unmask path. + +## Lane A disjointness + +None of the three diffs touch `src/server/responses/core.ts`, `src/claude/inbound.ts`, or `src/service.ts`. + +#4156 PASS +#4158 PASS +#4160 PASS