Skip to content

fix(miner): add blocked_own_open_pr to AttemptCliResult union - #9362

Closed
nghetien wants to merge 1 commit into
JSONbored:mainfrom
nghetien:fix/issue-9331-a1-ams-attempt-cli-s
Closed

fix(miner): add blocked_own_open_pr to AttemptCliResult union#9362
nghetien wants to merge 1 commit into
JSONbored:mainfrom
nghetien:fix/issue-9331-a1-ams-attempt-cli-s

Conversation

@nghetien

Copy link
Copy Markdown
Contributor

The own-open-PR idempotency guard added in #8808 constructs and emits a blocked_own_open_pr outcome (packages/loopover-miner/lib/attempt-cli.ts, around the duplicateResult object), but AttemptCliResult never declared that member — a real runtime outcome missing from its own exported type, so any onResult consumer typed against the union silently lost the shape (including existingPullRequestNumber).

This adds the missing union member with the exact shape the call site already constructs (reason: string, existingPullRequestNumber: number, plus the common fields). I checked whether the existing as AttemptCliResult cast at that call site could now be dropped, per the issue's second requirement — it can't: TypeScript widens the plain object literal's outcome field to string, so the cast is still needed to narrow it back to the literal type. This matches every other sibling call site in the file (e.g. blocked_rejection_signaled, blocked_worktree_preparation_failed), which keep the same cast even though their outcomes are already declared in the union, so I left it as-is rather than removing it.

Pure type-definition change, no runtime behavior touched — confirmed by git diff --stat (1 line, 1 file). No new tests added per the issue's own guidance (type-only change, no runtime modification).

Closes #9331

Validation

  • npx turbo run build --filter=@loopover/engine — pass
  • npx turbo run build --filter=@loopover/mcp — pass
  • npx turbo run build:tsc build:verify --filter=@loopover/miner — pass
  • npx vitest run --changed=upstream/main --passWithNoTests — 186/186 pass (incl. all 107 in test/unit/miner-attempt-cli.test.ts)
  • npm test (full suite) — 6 pre-existing failures unrelated to this change (test/unit/check-ui-kit-package.test.ts, test/unit/salvageability.test.ts, test/unit/selfhost-metrics.test.ts, test/unit/selfhost-pg-retention.test.ts ×2, test/unit/worker-entry-boundary.test.ts), verified to fail identically with this commit stashed out

The attempt-cli own-open-PR idempotency guard has constructed and
emitted this outcome since JSONbored#8808, but AttemptCliResult never declared
it, so onResult consumers typed against the union silently lost the
shape (existingPullRequestNumber included).
@nghetien
nghetien requested a review from JSONbored as a code owner July 27, 2026 14:13
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.50%. Comparing base (c2ac612) to head (94f8b6c).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9362      +/-   ##
==========================================
+ Coverage   75.38%   75.50%   +0.12%     
==========================================
  Files         275      276       +1     
  Lines       58023    58313     +290     
  Branches     6181     6277      +96     
==========================================
+ Hits        43739    44029     +290     
  Misses      14014    14014              
  Partials      270      270              
Flag Coverage Δ
backend 100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-miner/lib/attempt-cli.ts 100.00% <ø> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 27, 2026
@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-27 14:20:08 UTC

1 file · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This is a correct, minimal type-only fix: it adds the missing `blocked_own_open_pr` member to the `AttemptCliResult` union with the exact shape the call site already constructs (visible in the full file, `duplicateResult`-equivalent path uses `reason: string` and `existingPullRequestNumber: number` alongside the common fields). Before this change, any `onResult` consumer typed against the union (e.g. the miner-ui's attempt route/client) would have had this real runtime outcome's shape silently erased by the `as AttemptCliResult` cast, exactly as the description claims. The change is a single line, matches sibling union members' shape, and the PR's own validation (build, tsc, and the existing 107 attempt-cli tests) already exercises the surrounding file without needing new tests since no runtime behavior changed.

Nits — 5 non-blocking
  • No test explicitly asserts the new union member's shape (e.g., a type-level test with `satisfies` or a runtime check that `blocked_own_open_pr` results match it) — acceptable for a type-only fix but worth a one-line type-level assertion for regression safety.
  • The two FAILED CI checks (validate-code, validate) have no detail provided; given this branch is 3 commits behind the current default branch, that's the more likely cause per the base-branch note rather than a defect in this single-line diff.
  • Consider adding a compile-time check (e.g., a `const _typeCheck: AttemptCliResult = duplicateResult` assertion in a test file) so a future drift between the call site's constructed object and the union stays caught without relying on manual review.
  • Rebase onto the latest default branch to rule out the undetailed validate/validate-code failures being caused by post-divergence changes.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

CI checks failing

  • validate
  • validate-code

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9331
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 10 registered-repo PR(s), 4 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nghetien; Gittensor profile; 10 PR(s), 0 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: minor
Linked issue satisfaction

Addressed
The diff adds the missing blocked_own_open_pr member to AttemptCliResult's union with the fields the issue specifies (reason, existingPullRequestNumber) beyond the common fields, and the PR description explains a reasoned justification for why the accompanying cast could not be safely removed (TS widens outcome to string in the object literal), consistent with sibling cases in the file.

Review context
  • Author: nghetien
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Dart, JavaScript, Python, CSS, C#, C++, TypeScript
  • Official Gittensor activity: 10 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-code)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ams: attempt-cli's blocked_own_open_pr outcome is missing from the exported AttemptCliResult type

1 participant