Skip to content

fix: unblock main CI — duplicate instanceId getter + pr-ci-sweeper lookback time-bomb#111362

Closed
steipete wants to merge 1 commit into
mainfrom
claude/fix-gateway-duplicate-instanceid
Closed

fix: unblock main CI — duplicate instanceId getter + pr-ci-sweeper lookback time-bomb#111362
steipete wants to merge 1 commit into
mainfrom
claude/fix-gateway-duplicate-instanceid

Conversation

@steipete

@steipete steipete commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

main CI is fully red. ui/src/api/gateway.ts declares the instanceId getter twice (TS2300: Duplicate identifier), failing check-prod-types, check-test-types, check-lint, check-additional-boundaries-a, QA Smoke CI, and the compact shards — i.e. every downstream job that typechecks/builds the UI. This blocks all PR landings.

Why This Change Was Made

Two independently-merged PRs each added an identical get instanceId() to GatewayApi:

Git merged them without a textual conflict (the getters sit in different spots), but the result declares the same class member twice. Both bodies are identical (return this.opts.instanceId), so this is a pure duplicate — remove one. The earlier declaration carries the explicit : string | undefined return type, so that one is kept.

User Impact

None functional — restores green CI so PRs can land again. No behavior change; the getter returns the same value.

Evidence

Second breakage (same red main): pr-ci-sweeper lookback time-bomb

After the duplicate-getter fix, checks-node-compact-large-6 still failed on test/scripts/pr-ci-sweeper.test.ts — independent and also pre-existing. runPrCiSweeper hardcoded Date.now() for its 24h lookback, but the tests build PR fixtures at NOW - 2h with NOW pinned to 2026-07-18T12:00:00Z. Once real wall-clock crossed ~24h past that pin (today, 2026-07-19), the fixtures aged out of the lookback window and two assertions flipped from refire to skip / outside-lookback.

Fix: thread an injectable now (default Date.now()) into runPrCiSweeper — exactly the seam classifyPrForSweep already has — and pin the two runPrCiSweeper tests to NOW. Behavior in production is unchanged (defaults to real time). test/scripts/pr-ci-sweeper.test.ts now 14/14 green (node scripts/run-vitest.mjs).

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS maintainer Maintainer-authored PR labels Jul 19, 2026
@steipete steipete self-assigned this Jul 19, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 19, 2026
@Yigtwxx Yigtwxx mentioned this pull request Jul 19, 2026
Merged
@clawsweeper clawsweeper Bot added the P1 High-priority user-facing bug, regression, or broken workflow. label Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 6:17 AM ET / 10:17 UTC.

Summary
The PR deletes the untyped duplicate instanceId getter from GatewayBrowserClient, retaining the earlier typed accessor.

PR surface: Source -4. Total -4 across 1 file.

Reproducibility: yes. source-reproducible: the current-main class has two accessors with the same instanceId name, and the supplied main CI error identifies the resulting TS2300 failure in ui/src/api/gateway.ts.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add a redacted current-head check-prod-types or equivalent typecheck transcript showing the duplicate-identifier error is gone.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The body describes CI evidence, but it does not include directly inspectable after-fix live typecheck output; add a redacted terminal transcript or linked run output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Next step before merge

  • [P1] The maintainer label and assignment require the responsible maintainer to decide whether to land the otherwise narrow correction; it should not enter an automated repair or closure lane.

Maintainer decision needed

  • Question: Should the assigned maintainer land this narrowly scoped CI-unblocking deletion after confirming the after-fix typecheck output?
  • Rationale: The patch is minimal and appears correct, but the protected maintainer label requires an explicit human handling decision rather than an automated close or repair action.
  • Likely owner: steipete — They are assigned to the PR and authored the related merged changes whose overlap the patch corrects.
  • Options:
    • Confirm proof and merge (recommended): Verify a current-head typecheck transcript or equivalent real after-fix output, then merge the accessor deletion to restore main CI.
    • Hold for broader merge audit: Pause only if maintainers want to audit the two recent UI/gateway merges together for another overlapping class-member declaration.

Security
Cleared: The diff only removes a redundant class accessor and introduces no dependency, workflow, permission, secret, or supply-chain surface.

Review details

Best possible solution:

Retain one typed instanceId accessor in GatewayBrowserClient, attach or confirm a redacted after-fix typecheck transcript, and land this narrow correction through the assigned maintainer path.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: the current-main class has two accessors with the same instanceId name, and the supplied main CI error identifies the resulting TS2300 failure in ui/src/api/gateway.ts.

Is this the best way to solve the issue?

Yes: retaining the typed getter and deleting the behaviorally identical duplicate is the narrowest maintainable fix, with no new API or compatibility surface.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8178b3cc140d.

Label changes

Label changes:

  • add P1: The duplicate declaration blocks UI typecheck-dependent CI jobs and therefore prevents normal pull-request landings.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The body describes CI evidence, but it does not include directly inspectable after-fix live typecheck output; add a redacted terminal transcript or linked run output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The duplicate declaration blocks UI typecheck-dependent CI jobs and therefore prevents normal pull-request landings.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The body describes CI evidence, but it does not include directly inspectable after-fix live typecheck output; add a redacted terminal transcript or linked run output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source -4. Total -4 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 0 4 -4
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 0 4 -4

What I checked:

  • Current-main duplicate: The PR identifies two instanceId accessors in the current-main GatewayBrowserClient; its patch removes only the second, untyped accessor while retaining the typed getter. (ui/src/api/gateway.ts:379, 8178b3cc140d)
  • Narrow proposed correction: The PR head deletes the four-line duplicate accessor at the changed location and does not alter getter behavior, configuration, protocol, or dependency surfaces. (ui/src/api/gateway.ts:379, 3d3b74f4fea9)
  • Feature-history provenance: The related merged changes landed at commits 5c9916950e63ca0fbfb52abf7e9ecb2e43a4d43d and 8178b3cc140d82fa1e1101bf39c4647c15cc0f2c; the PR context ties each to one identical accessor, making this a merge-composition regression rather than a new behavior change. (ui/src/api/gateway.ts:364, 5c9916950e63)
  • Observed branch validation: The supplied PR check state reports successful check-prod-types, check-test-types, check-lint, checks-ui, and multiple CI shards on the proposed head; remaining in-progress checks are not treated as a correctness finding. (3d3b74f4fea9)
  • Release boundary: The latest supplied release is v2026.7.1 at 2d2ddc43d0dcf71f31283d780f9fe9ff4cc04fe4, which predates the current-main merges implicated by this regression; this open repair is not yet part of a shipped release. (2d2ddc43d0dc)

Likely related people:

  • steipete: Authored the two merged feature changes identified in the PR context as introducing the identical accessors, and is assigned to this focused correction. (role: recent area contributor; confidence: high; commits: 5c9916950e63, 8178b3cc140d; files: ui/src/api/gateway.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jul 19, 2026
@steipete steipete changed the title fix(ui): remove duplicate instanceId getter breaking main CI fix: unblock main CI — duplicate instanceId getter + pr-ci-sweeper lookback time-bomb Jul 19, 2026
@steipete
test(scripts): make pr-ci-sweeper lookback deterministic via injectab…
a9ddd3a 
…le now

runPrCiSweeper hardcoded `Date.now()`, but its tests build PR fixtures at
NOW-2h with NOW pinned to 2026-07-18. Once real wall-clock crossed ~24h
past that, the fixtures aged out of the 24h lookback and two tests flipped
from refire to skip/outside-lookback, reddening main. Thread an injectable
`now` (default Date.now()) like classifyPrForSweep already has, and pin the
two runPrCiSweeper tests to NOW.
@steipete
steipete force-pushed the claude/fix-gateway-duplicate-instanceid branch from 879f162 to a9ddd3a Compare July 19, 2026 10:28
@openclaw-barnacle openclaw-barnacle Bot removed the app: web-ui App: web-ui label Jul 19, 2026
@steipete

steipete commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by direct commits already on main that fix both breakages this PR targeted:

main HEAD (b9c914003) now has check-prod-types, check-test-types, and check-lint green again. Closing as redundant.

@steipete steipete closed this Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

maintainer Maintainer-authored PR P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete