Skip to content

fix(git): bound the team-repo remote calls left on plain git (status, promotion, ls-remote) - #1784

Merged
SaulMoro merged 1 commit into
Tencent:mainfrom
PerryLink:fix/git-remaining-remote-timeouts-20261108
Nov 8, 2026
Merged

SaulMoro merged 1 commit into
Tencent:mainfrom
PerryLink:fix/git-remaining-remote-timeouts-20261108

Conversation

@PerryLink

Copy link
Copy Markdown
Contributor

Summary

#1777 bounded the team-repo pull / push helpers and the side-branch worktrees with createGitWithBlockTimeout. Four remote calls were left on plain createGit:

  • getRepoStatus()'s fetch — run by teamai status;
  • fetchPromotionRefs()'s fetch — run by recall promote, under the sync lock;
  • the review-branch push in promotion-publish.ts — run by recall promote --publish;
  • remoteBranchExists()'s ls-remote — run while pruning pending pushes (teamai push).

With an ssh:// origin that opens the channel and then never writes, each one never returns: teamai status stops at Team repo status:, promotion holds the sync lock so later pull/push wait too, and remoteBranchExists's catch can never answer.

This switches those four calls to createGitWithBlockTimeout, with --progress on the transfers so a slow but moving transfer keeps resetting the timeout, as #1777 does. Local commands stay on the plain instance — getRepoStatus's status(), promotion's add/commit — because they print nothing while they work.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Test Plan

  • npx vitest run src/__tests__/git-status-stalled.test.ts — 3 passed (new; reuses the real ssh:// stand-in from git-stalled-remote.test.ts). On the old code the same three cases each time out at 10s.
  • npx tsc --noEmit — passes.
  • npx oxlint --type-aware --deny-warnings on the 4 files — 0 warnings, 0 errors.
  • 212 tests across git.test.ts, git-stalled-remote.test.ts, git-process-tree.test.ts, git-kind-reports.test.ts, maintenance-promote.test.ts, maintenance-prune.test.ts, status-*.test.ts, push-pending-pr.test.ts — all pass.

Real CLI, after npm run build, user-scope config pointing at a clone whose origin is the same stalled ssh:// stand-in (TEAMAI_INIT_PUSH_TIMEOUT_MS=2000):

  • before: timeout 8 node dist/index.js status → exit 124, output stops at Team repo status: (no stalled line);
  • after: node dist/index.js status → exit 0 in ~3.2s, ⚠ Could not check git status: git fetch --progress stalled: no output for 2s, so teamai ended it and the processes it started. …, then the rest of the report;
  • without the stall: exit 0, normal Team repo status.

Related Issues

Related #1749 (closed) — this is the follow-up to #1777, not a reopening.

Notes for Reviewers

  • pushLearningToOrigin() (no callers today) and workflow/cache.ts's withTeamRevision fetch are the only other plain-git remote calls I found; left out to keep this reviewable.
  • remoteBranchExists now answers null after the block timeout instead of hanging; its callers already treat null as "unknown, keep the record".
  • Open PR fix(windows): keep background work alive and hide every child process window #1775 also touches src/utils/git.ts; this branch is a single commit on main (05fe72a).

The status fetch, the promotion fetch and push, and remoteBranchExists's
ls-remote still ran on plain createGit after #1777. A stalled ssh origin
held teamai status, the sync lock around recall promote, and the pending
push prune forever. Bound them with createGitWithBlockTimeout and
--progress; local status/add/commit stay on the plain instance.
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown

No findings in the specified PR diff.

The PR description satisfies the testing requirement: it records a representative real-CLI run after npm run build, including stalled-remote before/after results and a normal-status check.

Review was read-only; I did not run, build, or install anything from the PR.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Approving. I built 6d8b998a and the merge-base (05fe72a2) and ran both against a team remote whose ssh never answers:

  • teamai status: head exits 0 after 30.2 s with the named stall warning. Base hangs at Team repo status: and I killed it at 75 s.
  • teamai recall promote: head exits 0 after 180 s with the promotion-fetch warning. Base printed nothing and I killed it at 260 s.
  • TEAMAI_INIT_PUSH_TIMEOUT_MS=3000 is honoured. After the head runs, none of the stand-in ssh processes are left.

[P2 non-blocking] The publish-step push (promotion-publish.ts:88-90) now goes through the bounded helper, but no test or run covers it. I checked it only by reading the code.

[P2 non-blocking] src/workflow/cache.ts:873 still runs a plain git.fetch(['origin']) on the team remote, as your Notes say, so a stalled origin can still hang there. That fits a follow-up.

[P3 nit]

  • git-status-stalled.test.ts also covers promotion and ls-remote, so its name is narrower than what it tests.
  • The new cases are skipped on Windows, and no CI job runs a stall there.
  • On status and promote, the stall message suggests TEAMAI_INIT_PUSH_TIMEOUT_MS. The advice works, but the variable has an init-scoped name.

The CI review reports no findings, and I agree.

Merge risk: low

  • Alignment: good. It extends #1777's bounded helper to the calls that were left.
  • Impact: small. The same commands, and a stall now ends with a message instead of hanging.
  • Verification: decent. Before and after on the built CLI for status and promote, and green CI. The publish push was checked only by reading.
  • Rollback cost: low.
  • Error detection: good.

@SaulMoro
SaulMoro merged commit be94816 into Tencent:main Nov 8, 2026
18 checks passed
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.

3 participants