Replies: 1 comment
|
Adding a live-use confirmation from another affected setup: the thread-header Push action fails consistently when the repository's pre-push build/CI gate runs longer than 30 seconds. This matches the current I’m preparing a focused fix that exempts explicit |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Before submitting
Area
apps/server
Problem or use case
git pushfrom T3 Code is capped at 30 seconds, so any repository whosepre-pushhook does real work cannot push from the app.Up front, so it is weighted correctly: this is derived from reading the source at
d3037064, not from a live reproduction in the app. Repro steps are below and I would be happy to confirm them if that is useful. Everything about the git invocation path below is quoted from the tree.The good news first, because it is what makes this worth fixing rather than working around: T3 Code spawns the real
gitbinary, so it inherits hooks correctly.No
isomorphic-git/simple-git/nodegit/dugitein anypackage.json, and zero occurrences of--no-verify,noVerify, orhooksPathacrossapps/server/src,packages/shared/src,apps/web/src,apps/mobile/src. That is the right design and it is why the app is usable in hook-gated repos at all.The problem is the timeout applied on top:
pushCurrentBranchcallsrunGiton all three of its paths with notimeoutMsoverride, so every push inherits the 30 s default:git pushdoes not return untilpre-pushexits. So a hook that runs anything substantial is killed at 30 s.There is no escape hatch today.
timeoutMsis internal plumbing with hardcoded call-site values (5_000,20_000) and the30_000default. It is not reachable from user settings, and the only git-related env vars read anywhere inapps/server/srcareGIT_ASKPASS,GIT_SSH, andGIT_TERMINAL_PROMPT.Two things make the failure worse than a plain error:
git, so the UI reports a failed push. Nothing indicates that the push itself was fine and a hook was interrupted, so users will reasonably suspect their remote, credentials, or network.gitdoes not necessarily reap the hook's own children. A hook that spawns a build or test runner can be left running, detached, holding CPU and locks. In our case that process is a ~16-minute merge gate.How common is this? Any repo using
husky,lefthook, or a hand-installedpre-pushthat runs more than a lint. Typical examples that exceed 30 s: a test suite, a type-check on a large monorepo, a build, or a CI-style gate. This is not an exotic configuration.Prior art in this repo: #4296 is the same class of defect — a hardcoded 5 s fetch timeout, whose overrun filled disks with orphaned
tmp_pack_*files. Hardcoded git timeouts appear to be a recurring source of trouble.Repro
pre-pushhook containingsleep 45; exit 0.sleep 600 &a child, then check whether the child survives the timeout.Proposed solution
In rough order of how much they solve, though any one of them unblocks the case:
1. Do not apply a wall-clock timeout to hook-running git commands.
push,commit,merge,rebaseandamall run user hooks of unbounded duration. A fixed timeout on them is a category error: the app cannot know how long a user's hook legitimately takes. Read-only queries (status,rev-parse,log) are where short timeouts genuinely belong, and they already have their own tighter values.2. If a bound is wanted, use an inactivity timeout, not a total one. Reset the timer on stdout/stderr output rather than capping total duration. That still catches a genuinely wedged process while letting a 10-minute hook that is printing progress run to completion. This is strictly better than any fixed number, because it does not require guessing.
3. Make it configurable, with a generous default for hook-running commands. A per-repo or global setting —
git.commandTimeoutMs, or something narrower likegit.hookTimeoutMs— would be enough. Even a plain env override (T3_GIT_TIMEOUT_MS) would unblock every affected user immediately, and the codebase already readsT3_-prefixed env vars elsewhere.4. Regardless of the above: surface the cause, and clean up children. When a git command is killed by the timeout, say so explicitly — "git push exceeded the 30s limit; a pre-push hook may still be running" beats a generic push failure. And kill the process group, so an interrupted hook does not leave detached children behind (the #4296 lesson).
Why this matters
gitis the hard part and it is done. This is a timeout policy on top of a correct foundation.Smallest useful scope
Exempt
pushfromDEFAULT_TIMEOUT_MS— either no timeout, or a much larger one — leaving every other command exactly as it is today.That is a change to the three
runGitcall sites inpushCurrentBranch(GitVcsDriverCore.ts~1883 / ~1949 / ~1986), or a single change to the default resolution at:712. It fixes the reported problem with essentially no blast radius, andcommit/merge/rebasecan follow later if anyone hits them.An even smaller stopgap, if a policy discussion is needed first: honour a
T3_GIT_TIMEOUT_MSenv override at:712. One line, no UI, unblocks affected users immediately.I am happy to open a PR for whichever shape you prefer — say which and I will follow the repo's conventions.
Alternatives considered
--no-verifyfrom the app. Rejected outright, and I would argue against it as a feature. It converts a safety mechanism into an opt-out, and the whole reason the hook exists is that unverified pushes are expensive to undo.Risks or tradeoffs
GIT_TERMINAL_PROMPT=0is already set in this codebase, and an inactivity timeout (proposal 2) bounds a silent process without capping a productive one.All reactions