Skip to content

Fix stale canary checks and repository paths - #8737

Draft
hhyykk wants to merge 6 commits into
mainfrom
codex/review-cleanup-01a1d82f
Draft

hhyykk wants to merge 6 commits into
mainfrom
codex/review-cleanup-01a1d82f

Conversation

@hhyykk

@hhyykk hhyykk commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

Two Canary profiles advertised a deleted worker-bridge script. They now select the maintained bridge smoke, which checks read-only mounts, preflight, privacy and active-user observation. A regression checks direct python and python3 script paths across the Canary catalog profiles.

Correct the broken event-ledger protocol paths. Link retired examples to their exact historical source and retain the original evidence. Remove 25 unused TypeScript imports without changing call signatures or assertions.

Related to the review in #8703; intended base is main.

Author Declaration

  • Written by: model_agent (OpenAI Codex, GPT-6 family).

Implemented against

  • Specification and revision: the review scope in Fix release recovery and runtime compatibility #8703, tested after merging main at 185e0d90df7ef0d6a38a743958fe0ea9d695267f; the newer main 3cf2f45552dc2fdad0a160e3e736c1b7c9be21ee has no overlapping files and its projected merge contains exactly the same 21 changed files.
Criterion Disposition Symbol / path Test or command
Selected Canary checks must exist and exercise the current bridge contract implemented loopx/canary/planner.py tests/canary/test_quality_surface_catalog.py, both changed smokes
Preserve historical evidence and fix invalid paths implemented docs/concepts/interaction-pattern-catalog.md, course 04 verify pinned Git objects and current paths
Audit unused declarations without altering behavior implemented within scope control-plane imports normal and additional no-unused TypeScript checks
  • Self-check: compared callers, retained exports, reviewed the diff and tested against current source. No dependencies or production data migration were added.

Scope And Continuation

Validation

  • Windows reader compatibility follow-up: trying the PowerShell parser first and falling back to POSIX is unsafe. A legacy POSIX-rendered value containing two consecutive U+201A quotes was accepted by that parser but decoded to one quote. A revised local candidate distinguished the tested producer forms using the quoted versus unquoted quota and context-read tokens, retained the existing pagination/identity assertions, and used the existing command_argv helper for a .ps1 launcher. Across the two relevant base revisions it passed 5,000 generated argument vectors per revision in both producer formats and eight six-page synthetic subprocess runs with mixed page formats; changed capture identity and five malformed expressions were rejected. The .ps1 route used a synthetic local pwsh shim, not PowerShell or Windows. Microsoft's pwsh documentation describes the File form. The exact failed CI argv was not logged, the candidate is not committed, and native Windows validation remains required.

  • Main 859a23e preflight: GitHub pull metadata reported base 3cf2f45; comparison with main found merge base 185e0d9. Complete inventories contain 21 branch paths and 272 main paths, with one overlap. Its in-memory git merge-file result had no text conflict and matched the local-authority source hash used by the earlier projected checks. All six PR commits have Signed-off-by trailers. This is a file-level comparison, not a full-tree merge or CI result.

  • Subsequent behavior follow-up at main 859a23e and this PR head: Node 26.12.1 ran the existing tests with project ESM source supplied by an in-memory projection. Both the projected PR and local candidate passed all 232 targeted tests, plus four governance cases selected by name. Test names and statuses matched between versions. A module-load audit verified the expected source hashes for all six changed files in each version; the loaded module path sets also matched. The runs used synthetic data in a network-isolated sandbox that allowed writes only in a temporary directory. This is not a real checkout, a complete project suite, or native CI; the candidate remains uncommitted.

  • Main 859a23e candidate follow-up: the existing six-file local-only patch applies to the projected PR tree in memory with exact context. With TypeScript 6.0.3, the ordinary typecheck reports zero diagnostics both before and after the patch; the additional no-unused checks report 16 before and 7 after, with nine diagnostic occurrences removed and none added. The comparison counts duplicate messages separately. The first local counting script deduplicated those messages and failed its own count assertion; the corrected multiset comparison passed. These are in-memory compiler results, not a committed change, fresh-checkout tsc, current-main behavior tests, or CI. The 236 targeted behavior tests below ran against the earlier baaf657 projection.

  • Current-main compatibility note: PR #8703 TypeScript core shard 2 exposed a duplicate SHA-256 matcher already present in its main parent. The old main base used by this PR did not contain the new inbox module. A virtual-source scan of main 97fb254 reproduces the guard failure; this PR has not been tested as a full merge against that main.

  • PR #8703 dashboard shard 3 also failed at 184 of 200 items. The same scenario passed on PR #8733 with 200 items and a 771 ms uninstrumented expansion. The scenario, key fixture and work-board sources and lockfiles match, and the jobs used Node 22.22.3 and Chrome Headless Shell 151.0.7922.34. The product trees and runners differ, so this is not a controlled attribution. Separately, a local static synthetic DOM using the unchanged digest produced five groups of 40 and visible counts 15, 52, 89, 126, 163 and 200 as disclosures opened. That limited Chrome 145 check did not run the application or reproduce the CI result; inspect actual group, disclosure and timing state before changing the test or UI.

  • Windows parser compatibility investigation: before commit 2ecfc26, the capture reader emitted POSIX shlex-joined commands; the later Windows renderer emits PowerShell literals. A local PowerShell-only parser prototype rejected a historical POSIX command containing an apostrophe, while POSIX shlex changed an apostrophe in a modern PowerShell path even when the command had no ampersand prefix. A separate in-memory prototype matched 1,500 generated argument vectors and exercised six pages through a real synthetic child process; it rejected a changed capture identity. These are local diagnostics, not a committed fix, native Windows validation or proof of the CI runner argv. A repair must preserve both historical and current command forms.

  • CI queue snapshot (2026-11-14 around 09:36 UTC): each of the six Python test shards was pending behind 23 entries in its repository-wide concurrency group, including one running shard from this main run. The GitHub concurrency API was queried for each job, including shard 1. Queue positions can change and are not an estimated completion time. This snapshot does not establish the result of other checks.

  • The checks aggregate failed because dashboard-acceptance was skipped after dashboard-browser shard 3 failed. It is a downstream gate result, not a third independent failure. In the failing scenario, the expansion loop samples the visible count until a five-second deadline and then samples it again for the assertion. The reported 184 alone does not establish whether the deadline was reached or visibility changed between samples; the prior local base/merge comparison did not reproduce 184. Investigate with timing and disclosure-state evidence while retaining the existing 200-item and two-second acceptance limits.

  • A separate Windows job on PR #8703 also failed in the base CLI differential with WinError 2. Its actual test merge and this PR use identical fixture helper bytes, but their product sources differ. A repair needs validation against both revisions; neither log records the missing argv.

  • Updated TypeScript audit: with TypeScript 6.0.3 and the actual source files from main bd2718a485c86fe4f0ad4d1356fe9aeeccfed1ba plus the clean projected PR changes, an in-memory compiler host reports 41 no-unused diagnostics on main and 16 on the PR projection. It reports no ordinary typecheck diagnostics in either tree. The host was calibrated against the earlier real tsc results at main baaf657: 40 on main and 15 on the PR projection, with no ordinary diagnostics. The one new main diagnostic is the unused researchIdentifier import in loopx/control_plane/capabilities/explore_research_execution.ts; the PR adds no no-unused diagnostic relative to this main. This is an in-memory source projection, not a real-checkout tsc run. The local-only nine-diagnostic candidate below is not included in the 16.

  • Windows differential follow-up: the probe copies the candidate checkout’s tests/control_plane/cli_output_fixture.py into the shared fixture directory for both the base and candidate arms, while each arm loads its own product source. A fix to that helper can therefore reach the failed base measurement without replacing the baseline or skipping the comparison. In a separate parse-only check, the actual renderer from base 3cf2f455 was round-tripped through PowerShell 7.7 on Linux for an executable and capture path containing spaces, ASCII apostrophes and all four escaped smart single quotes; an appended unquoted --select also survived. The existing POSIX shlex.split put & in executable position for that case. A bare executable case matched both parsers. These are synthetic parser checks, not a native Windows run or proof of the exact CI argv.

  • Latest-main follow-up (2026-11-14): main at f17ce83544f8b0420b6cb13dfdd086ca23cf6646 changed 80 paths since baaf657e5df6b779be1401d143af6aa5970454ef; none overlaps the 21 files currently in this PR. A whole-tree merge and typecheck against the newer main have not run. The current CI test merge has parent 3cf2f45552dc2fdad0a160e3e736c1b7c9be21ee; its failures are described below.

  • Dashboard browser shard 3 failed on goal-workspace-m2: 184 of 200 tasks were visible in its uninstrumented qualification pass. The previous packaged shard at this same PR head passed.

  • Follow-up local Linux comparison used the exact CI base 3cf2f455 and synthetic merge tree d1072ce8, Node 22.22.3 and Chromium 151, with the existing development and coverage scenario and unchanged limits. Two alternating runs per tree each yielded one failure at a different 1-second lens-switch budget and one complete pass; every run that reached the count saw all 200 tasks. Both complete runs reported slower coverage-only diagnostics, which the existing test does not use as its qualification gate. The CI 184-item symptom was not reproduced; local results do not prove its cause or establish performance parity on the CI host.

  • Follow-up at main baaf657e5df6b779be1401d143af6aa5970454ef: the extra TypeScript checks report 40 diagnostics on main and 15 on the clean projected merge, with 25 removed and none added. Both used TypeScript 6.0.3.

  • A separate local-only candidate removes nine more diagnostics without removing assertions or awaited calls. The normal typecheck and 236 targeted tests passed on both the unchanged projected merge and the local candidate. The candidate is not committed or included in this PR.

  • Windows CI failed while measuring the base, before the candidate: read_pinned_cli_body raised FileNotFoundError (WinError 2). The relevant producer and reader are byte-identical between that base and this PR. A local reproduction using those source versions and Windows command rendering makes shlex.split place the PowerShell & operator in executable position. The CI log did not record argv, so this mechanism is supported but not confirmed as that runner's exact missing executable. Removing only & is insufficient: the same local reproduction shows POSIX splitting also changes a PowerShell-quoted path containing an apostrophe.

  • Tested revision: 4aee125ac805161b3b6136a6f1a39ca9470bd121 for targeted checks after merging current main. The full TypeScript run below was on 07413af94058695c39116a6d096c8b30f5a6334f.

  • Run state: finished (the full local Python suite was interrupted).

  • Input classes: synthetic, public_fixture.

Check kind Result Public-safe evidence / limitation
regression_parity passed The added Canary existence check failed before the fix for both missing script references and passed after it.
real_entrypoint passed python3 examples/canary/catalog-planner-smoke.py, python3 examples/cli-worker-bridge-command-modularization-smoke.py, and python3 examples/worker-bridge-active-user-feed-smoke.py.
unit passed 62 targeted Python tests on the merged head; the Canary catalog suite includes the new regression.
unit passed 243 targeted TypeScript tests across the affected control-plane, lease, authority and Goal modules.
static passed npm run typecheck:control-plane, project-scoped Ruff, mypy and CLI output-budget smoke passed on the merged head.
static failed Extra --noUnusedLocals --noUnusedParameters: 40 diagnostics before, 15 after; no new diagnostic. This extra check is not enabled by the normal project typecheck.
unit failed Full control-plane run on 07413af94058695c39116a6d096c8b30f5a6334f: 6,436 passed, 85 skipped, 2 failed. Host heartbeat failed to start within the test's two-second window; the exact test later passed three times on the candidate and once on an isolated main source copy. The SQLite rehearsal hit its 5 GiB free-disk reserve; the same test passed after task-owned temporary files were cleaned, with the threshold unchanged.
unit failed Full local Python suite: 38,887 tests collected; interrupted after the 10-minute local resource window with 1,256 passed, 778 skipped and 2 failed. Both failures were the same synthetic backpressure assertion; the exact File and SQLite tests also failed with that assertion on an isolated copy of main 185e0d90df7ef0d6a38a743958fe0ea9d695267f. The previous-base run was interrupted when main advanced (734 passed, 232 skipped).
integration passed loopx canary premerge --from-git-diff --git-diff-base origin/main: 10 catalog, 8 risk-profile and 1 public-boundary checks passed on the merged head. The first run on the prior base timed out the vocabulary smoke while other suites were running; the timeout was unchanged.
static failed On the prior base, the full boundary check with the local shared registry found two unrelated Goal source-routing errors. The same source scan with an isolated empty registry passed; 49 credential-shaped nonliteral references remained warnings.
  • Coverage and gaps: local checks use temporary fixtures and are not live production or release validation. Full-run failures and unfinished checks are recorded above; CI will independently run on the PR head.

Frontend / Visual Evidence

  • UI impact: none.
  • Before / after / first viewport / states / attention and craft review: N/A; no UI changed.
  • Source data: none.

Type of Change

  • Bug fix
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Public docs or presentation surface (README, protocols, dashboard)

Technical Direction

  • N/A: maintenance of existing checks and documentation paths.

Shared-authority RFC fixture impact

  • N/A: no authority schema, routing, promotion, migration or fixture semantics changed.

Boundary Checklist

  • No private state, credentials, raw traces or local machine paths are included.
  • No maintainer-owned benchmark work was duplicated.
  • Changes stay within the review scope.
  • No paired RFC changed.
  • UI impact is marked none.
  • Every commit has a DCO Signed-off-by trailer.

Dashboard-browser local reproduction (2026-11-14)

The PR #8703 dashboard shard 3 job reported 184 visible work items instead of 200. In one local run of the original goal-workspace-m2 scenario, the 200-item expansion assertion passed in 953 ms. That run then failed a different existing performance gate: one median of three loaded-lens transitions was 1,028 ms against the 1,000 ms limit; the maximum individual sample was 1,081 ms. The uninstrumented qualification failed, so the original test did not run its coverage pass. No acceptance threshold was changed.

The run used Chrome Headless Shell 151.0.7922.34, matching CI, on OpenCloudOS with Node 26.12.1. It used the original scenario and fixture, a local Vite development server, and an in-memory overlay of the ten PR dashboard source files; those sources match the failed CI merge 2fe7226cb2a83d0ab3dae224618670437cb7129c. The remaining files came from an older local checkout, and the host differs from CI. This single run does not establish the cause of the 184-item CI result or its relationship to the PR.

Real PowerShell reader validation (2026-11-14)

The failed Windows runner image lists PowerShell 7.6.9. A local Linux test used that PowerShell version from the official release, verified against the release SHA-256 manifest.

For each of the two affected base revisions, 1,002 PowerShell-rendered argument vectors matched the real PowerShell parser and the candidate reader; 1,002 legacy POSIX-rendered vectors also round-tripped. The cases include apostrophes, smart quotes, Unicode, spaces and shell metacharacters. Four synthetic .ps1 paging runs completed six pages each, and identity tampering and five unsafe commands were rejected.

A separate test used the actual LoopX loopx.ps1 and loopx_entry.py scripts from an older checkout; both scripts match base 87a736bd. The original reader reproduced a missing executable for the PowerShell call operator. The candidate used real PowerShell to read a 1,193-byte synthetic capture through five real LoopX pages, reproducing the original bytes without creating a registry. When the capture was changed after page one, the next read failed without returning partial content or a continuation command. The test restored its temporary capture.

These runs were isolated to loopback-only networking and temporary writes. They do not prove which executable was missing in CI: its argv was not logged. The candidate is not committed, the checkout is not the full CI merge, and native Windows CI remains required.

Current-main overlap audit (2026-11-14)

A read-only tree comparison used main 8d7d234 and each PR's actual base and merge base. It checked every path changed on both sides with an in-memory three-way merge. This is not a full checkout, build or CI run.

For #8737, 21 PR paths and 281 main paths changed from merge base 185e0d9; the single overlapping path merged cleanly in memory. GitHub reports actual PR base 3cf2f45, which differs from that merge base; a direct base-to-head tree comparison has 31 differing paths and must not be treated as 31 PR changes.

The uncommitted two-line portion of the Windows reader candidate merged cleanly with #8266's fixture changes but conflicted with #5677. This is only a textual check of those two lines, not a merge or test of a complete fix. #8758 already owns the separate digest repair.

Python lint audit (2026-11-14)

On main 4604f93, Ruff 0.15.22 checked all 3,911 tracked Python files individually via stdin. The 2,071 files in the previously used project scope had zero diagnostics. The additional all-tracked scan reported 132 diagnostics outside that scope. All source blobs were checked against their Git hashes, and known F401/F821 inputs verified that stdin and direct-file checks agree. This is a per-file source audit, not a full-checkout Ruff run.

The four Python files changed by #8737 reported zero Ruff diagnostics at this head. Their merge-base versions match main.

Two of the extra findings are duplicate imports in loopx/status.py. An in-memory candidate removes only the second occurrence of each binding, keeps both public aliases, and reduces Ruff F811 from two to zero. The original and candidate passed the same 18 existing pytest cases, and each alias remained identical to its owner. The checked status and test files match main; other dependencies came from an older checkout. The candidate is not committed, and full CI is still required.

The candidate also merged cleanly in memory with the status.py changes in #8647. This is a textual merge check, not an integrated test. Ruff also marked two pytest fixture imports in benchmark/tests/test_checkpoint_storage.py as unused; pytest fixture discovery confirmed that the Docker-gated test uses both, so they must not be removed automatically. Docker was not invoked.

Current TypeScript unused audit (2026-11-14)

On main 4604f93, a hash-verified in-memory TypeScript 6.0.3 compiler host checked 528 control-plane source files. Normal type checking reported zero diagnostics for main, the projected PR, and the existing nine-diagnostic candidate. Extra noUnused checks reported 41, 16, and 7 diagnostics respectively.

One additional local candidate removes the unused researchIdentifier named import from explore_research_execution.ts while retaining the other imports from the same module. The three existing Explore research test files passed 24/24 both before and after, with the same test names and 14 hash-audited loaded modules. Applying that removal on top of the earlier nine-diagnostic candidate left zero normal type errors and six unused diagnostics. Neither candidate is committed.

The six remaining diagnostics concern name, root, identity, request, expectedRequestDigest, and goalId. They have not been removed automatically. In particular, the lifecycle receipt digest is checked later under the mutation lock; any cleanup there must preserve the existing validation and failure order.

These checks used source overlays and the installed node_modules, not a clean checkout or CI. The earlier 236-test qualification for the nine-diagnostic candidate used main 859a23e; the 24-test Explore check used the current main snapshot.

Continuation-note parameter audit (2026-11-14)

On main 4604f93, the unused name parameter in the private boundedString function has 15 call sites. A local candidate removes that parameter and the corresponding string-literal arguments. The function is not exported, and the AST check found no other references.

The original and candidate each passed all 37 existing continuation tests. A 156-case comparison of valid, invalid and boundary inputs produced identical full results and error messages. The target and test files match main; other dependencies came from an older checkout. An initial isolated run had 11 failures in both variants and in an unmodified control. Those failures disappeared after enabling loopback inside the isolated network namespace, which the Python-to-TypeScript runtime requires.

Combining this candidate with the previously described nine-declaration and one-import candidates produced zero normal TypeScript diagnostics and five extra noUnused diagnostics across 528 files. This used an in-memory compiler host; it is not a clean-checkout typecheck or CI. None of the candidates is committed.

Private parameter audit (2026-11-14)

On main 4604f93, the private functions reductionWithTurnOutcome and successEnvelope each receive one unused parameter at their single call site. An AST check verified both arguments are simple identifiers and are unused in the corresponding function bodies. Identity and request validation elsewhere remain in place.

Using the main sources and this PR's removal of the unused task-lease import, the original and local candidate each passed all 68 existing settlement, identity, task-lease and CLI tests, with no skipped tests. The source loader checked the hashes of 109 loaded modules. Runs used isolated temporary storage and installed dependencies. This is not a complete PR checkout or CI run.

Combined with the earlier local candidates, TypeScript 6.0.3 reported zero ordinary diagnostics and three additional noUnused diagnostics across 528 source files. The three concern root, expectedRequestDigest, and goalId; they remain unchanged. The earlier local candidates and these two parameter edits are uncommitted.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant