Skip to content

Commit 4d6af58

Browse files
odgrimclaude
andauthored
fix(GitClone): local checkout produces the same outputs as a clone (#195)
* fix(GitClone): produce the same outputs from a local checkout as from a clone Two ways a local checkout silently came up short of the clone path's outputs, leaving downstream blocks stuck on "Waiting for outputs from: … (repo_owner, org_id, repo_name, repo_id)". Remote resolution only ever looked at `origin` (GitClient.getInfo). A checkout that names its remote something else — a fork whose upstream is the interesting one, a repo re-pointed after `git init` — yielded no remote at all, so there was no repo_owner/repo_name, and no org_id/repo_id either, since those are looked up from the owner and name. inspectLocalRepo now falls back to whichever remote the repo does have. `origin` is still preferred, so remote order decides nothing in the common case. The org/repo ids are resolved once, at confirm time, from the session token. Because selecting a checkout deliberately didn't wait on the linked auth block, confirming before authenticating produced a block permanently missing org_id and repo_id. Confirming now waits for the auth block exactly as cloning does; the path field, the folder picker, and the inline "is this a git repo?" check stay usable while it's pending, since none of those need credentials. Docs updated: the local source no longer claims to need no credentials, and the no-remote caution now names the outputs that go missing with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(GitClone): stop the local-checkout preview from reading as a finished block The "is this a git repo?" check rendered in the same green success styling as the completed state — check icon, root, remote, branch, file count — so a checked directory looked like a block that had run. It hasn't: nothing is registered and no outputs exist until "Use This Repo" is confirmed, and a block that has produced no outputs leaves every consumer reporting each referenced name as missing ("Waiting for outputs from: clone_repo (repo_owner, org_id, repo_name, repo_id)") — which reads as the outputs being unsupported rather than the block being unfinished. The preview is now informational rather than success-styled, green being reserved for the completed state, and says outright that the directory is not in use yet. The no-remote warning names the outputs that go missing with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * uptick to 0.21.1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(GitClone): address review — docs contradictions and interrupt-safe git queries Three findings from the review of #195. The docs said choosing a directory "reads it only" and in the same sentence that it "registers the repo exactly as a clone would" — the contradiction at the heart of the reported bug. Inspection and confirmation are now described as the separate steps they are, including that a block which hasn't been confirmed has produced nothing and leaves its consumers reporting missing outputs. The integration paragraph still said a pull request opens against the checkout's `origin` remote, which contradicts the new fallback two paragraphs below it. It now says "the checkout's remote". firstRemoteUrl spawned git without ever killing the child, so an interrupted fiber would orphan it. Rather than plumbing a Scope through inspectLocalRepo and the handler — which would leave this helper inconsistent with countFiles three lines away, which has the same property and is shared with the clone path — the shared readGitLines helper wraps its drain in Effect.ensuring(kill). Same guarantee, no signature change, no interruption path exists today anyway: on the normal path the process has already exited and the signal is a no-op. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a32ecf8 commit 4d6af58

7 files changed

Lines changed: 144 additions & 25 deletions

File tree

docs/src/content/docs/authoring/blocks/GitClone.mdx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ When paired with a `<GitHubAuth>` block, the GitClone block enables a "Browse Gi
4343

4444
Users often already have the repository cloned — a long-lived `infrastructure-live` checkout, a work in progress branch, a monorepo they never want to re-download. The block's source picker lets them choose **Use local checkout** and select that directory instead of cloning.
4545

46-
Selecting a directory reads it only: the block resolves the repository root (so any subdirectory of the checkout works), reads the `origin` remote and current branch, and registers the repo exactly as a clone would. Nothing is fetched, pulled, or modified, and no credentials are needed — the files are already there.
46+
Choosing a directory only **inspects** it: the block resolves the repository root (so any subdirectory of the checkout works), reads the repository's remote and current branch, and counts its tracked files. Nothing is fetched, pulled, or modified, and nothing is registered yet — the directory is reported on, not adopted.
47+
48+
Confirming with **Use This Repo** is what registers the checkout and produces the block's outputs, exactly as a completed clone does. Until then the block has produced nothing, and any later block referencing its outputs still reports them as missing.
49+
50+
Inspecting a directory needs no credentials, so browsing works while a linked auth block is still pending. Confirming does wait on it, exactly as cloning does — the GitHub `org_id` / `repo_id` outputs are resolved from the session token at that moment, so adopting a checkout before authentication finishes would leave the block without them.
4751

4852
```mdx
4953
<GitClone
@@ -63,10 +67,10 @@ Setting `prefilledRepoDir` starts the block on the local source. Use `source` to
6367
<GitClone id="repo" source="clone" hideSourceSelect prefilledUrl="https://github.com/acme/infra" />
6468
```
6569

66-
Everything downstream behaves the same either way: the same `clone_path`, `repo_owner`, and `repo_name` outputs, the same `$REPO_FILES` variable, the same workspace file tree, and the same [`<GitPullRequest>`](/authoring/blocks/gitpullrequest) integration — a pull request opens against the checkout's `origin` remote and current branch.
70+
Everything downstream behaves the same either way: the same `clone_path`, `repo_owner`, and `repo_name` outputs, the same `$REPO_FILES` variable, the same workspace file tree, and the same [`<GitPullRequest>`](/authoring/blocks/gitpullrequest) integration — a pull request opens against the checkout's remote and current branch.
6771

6872
<Aside type="caution">
69-
A checkout with no `origin` remote can still be selected, but blocks that open a pull request need one. The block says so inline when it finds no remote.
73+
A checkout with no remote at all can still be selected, but it produces no `repo_owner` / `repo_name` (nor the GitHub ids derived from them), and blocks that open a pull request need one. The block says so inline when it finds no remote. `origin` is preferred; if the checkout names its remote something else, that one is used.
7074
</Aside>
7175

7276
### Pre-filled Values
@@ -203,7 +207,7 @@ After a successful clone, the GitClone block produces outputs that can be refere
203207
| `repo_id` | Immutable GitHub numeric ID of the repository (when a GitHub token is available) | `87654321` |
204208
| `repo_url` | The full URL of the cloned repository | `https://github.com/acme-corp/infrastructure-live` |
205209

206-
For a local checkout, `clone_path` is the repository root the user selected, and `repo_owner` / `repo_name` come from its `origin` remote (omitted when the repo has no remote).
210+
For a local checkout, `clone_path` is the repository root the user selected, and `repo_owner` / `repo_name` come from its remote — `origin` when present, otherwise whichever remote the checkout has. They are omitted entirely when the repo has no remote, which also means no `org_id` / `repo_id`, since those are looked up from the owner and name.
207211

208212
Reference outputs in downstream blocks using template variables:
209213

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "runbooks",
3-
"version": "0.21.0",
3+
"version": "0.21.1",
44
"private": true,
55
"description": "Gruntwork Runbooks",
66
"author": {

src/domain/git/local-repo.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,31 @@ describe("inspectLocalRepo", () => {
140140
expect(info.owner).toBeUndefined()
141141
})
142142

143+
it("falls back to a non-origin remote when there is no origin", async () => {
144+
const info = await inspect("/home/me/fork", {
145+
dirs: ["/home/me/fork"],
146+
commands: [
147+
{ command: "git", args: ["remote"], outputLines: ["upstream"], exitCode: 0 },
148+
{
149+
command: "git",
150+
args: ["remote", "get-url", "upstream"],
151+
outputLines: ["git@github.com:acme/infra.git"],
152+
exitCode: 0,
153+
},
154+
lsFiles(["main.tf"]),
155+
],
156+
git: {
157+
getRepoRoot: () => Effect.succeed("/home/me/fork"),
158+
// getInfo only looks at origin, which this checkout doesn't have.
159+
getInfo: () => Effect.succeed({ branch: "main", refType: "branch" as const }),
160+
},
161+
})
162+
163+
expect(info.remoteUrl).toBe("git@github.com:acme/infra.git")
164+
expect(info.owner).toBe("acme")
165+
expect(info.repo).toBe("infra")
166+
})
167+
143168
it("omits owner/repo when the repo has no remote", async () => {
144169
const info = await inspect("/home/me/local-only", {
145170
dirs: ["/home/me/local-only"],

src/domain/git/local-repo.ts

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
* `<DirPicker>` — behave identically either way.
1010
*/
1111
import path from "path"
12-
import { Effect } from "effect"
12+
import { Effect, Stream, Chunk } from "effect"
1313
import { GitClient } from "../../services/GitClient.ts"
1414
import type { GitInfo } from "../../services/GitClient.ts"
1515
import { FileSystem } from "../../services/FileSystem.ts"
1616
import { ProcessSpawner } from "../../services/ProcessSpawner.ts"
1717
import { GitError } from "../../errors/index.ts"
18+
import { gitSpawnEnv } from "./env.ts"
1819
import { countFiles, parseOwnerRepoFromURL } from "./operations.ts"
1920

2021
/** Metadata describing a local checkout selected by the user. */
@@ -118,14 +119,21 @@ export const inspectLocalRepo = (
118119
),
119120
)
120121

122+
// getInfo only knows about `origin`. A checkout can legitimately name its
123+
// remote something else — a fork whose upstream is the interesting one, or
124+
// a repo re-pointed after `git init` — and without a remote there is no
125+
// repo_owner/repo_name (nor the GitHub ids derived from them) for
126+
// downstream blocks to consume. Fall back to whatever remote does exist.
127+
const remoteUrl = info.remoteUrl ?? (yield* firstRemoteUrl(absolutePath))
128+
121129
const fileCount = yield* countFiles(absolutePath)
122-
const parsed = info.remoteUrl ? parseOwnerRepoFromURL(info.remoteUrl) : undefined
130+
const parsed = remoteUrl ? parseOwnerRepoFromURL(remoteUrl) : undefined
123131

124132
return {
125133
absolutePath,
126134
relativePath: relativeToWorkingDir(absolutePath, workingDir),
127135
fileCount,
128-
remoteUrl: info.remoteUrl,
136+
remoteUrl,
129137
branch: info.branch,
130138
refType: info.refType,
131139
commitSha: info.commitSha,
@@ -134,6 +142,42 @@ export const inspectLocalRepo = (
134142
} satisfies LocalRepoInfo
135143
})
136144

145+
/**
146+
* URL of the first remote the repo has, or undefined when it has none. Only
147+
* consulted after `origin` comes up empty, so remote order decides nothing in
148+
* the common case. Best-effort: a repo we can't read remotes from is still a
149+
* usable checkout, it just yields no owner/repo.
150+
*/
151+
const firstRemoteUrl = (repoPath: string) =>
152+
Effect.gen(function* () {
153+
const names = yield* readGitLines(repoPath, ["remote"])
154+
if (names.length === 0) return undefined
155+
const urls = yield* readGitLines(repoPath, ["remote", "get-url", names[0]])
156+
return urls[0]
157+
}).pipe(Effect.catchAll(() => Effect.succeed(undefined)))
158+
159+
/**
160+
* Run a short git query and return its stdout lines, or none if it failed.
161+
*
162+
* `Effect.ensuring` kills the child if this effect is interrupted mid-flight.
163+
* On the normal path the process has already exited and the signal is a no-op,
164+
* so this costs nothing and leaves no orphan behind if a caller ever gains a
165+
* cancellation path.
166+
*/
167+
const readGitLines = (repoPath: string, args: string[]) =>
168+
Effect.gen(function* () {
169+
const spawner = yield* ProcessSpawner
170+
const proc = yield* spawner.spawn("git", args, { cwd: repoPath, env: gitSpawnEnv() })
171+
172+
return yield* Effect.gen(function* () {
173+
const lines = Chunk.toArray(yield* Stream.runCollect(proc.output))
174+
.filter((l) => l.source === "stdout")
175+
.map((l) => l.line.trim())
176+
.filter(Boolean)
177+
return (yield* proc.exitCode) === 0 ? lines : []
178+
}).pipe(Effect.ensuring(proc.kill.pipe(Effect.ignore)))
179+
})
180+
137181
/**
138182
* Display path for a checkout: relative to the working directory when it lives
139183
* inside it, absolute otherwise. A local checkout is usually somewhere else

web/src/components/mdx/GitClone/GitClone.tsx

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,17 @@ function GitCloneInteractive({
382382
const { bg: statusClasses, icon: IconComponent, iconColor: iconClasses } = statusConfig[cloneStatus] ?? statusConfig.pending
383383

384384
const isLocalSource = activeSource === 'local'
385-
// Selecting a checkout that already exists on disk needs no credentials
386-
// only cloning does. Auth still gates the clone form as before.
385+
// Browsing and checking a directory needs no credentials, so the local form
386+
// stays usable while a linked auth block is still pending.
387387
const isFormDisabled =
388388
cloneStatus === 'running' || !hasAllBlockingDependencies || (!isLocalSource && !gitHubAuthMet)
389389
const isCloneDisabled = isFormDisabled || !gitUrl.trim()
390-
const isUseRepoDisabled = isFormDisabled || localPreviewStatus !== 'valid'
390+
// Confirming, though, waits for auth exactly like cloning does. The GitHub
391+
// org/repo ids are resolved once, at confirm time, from the session token —
392+
// adopting a checkout before the auth block finishes would silently produce a
393+
// block missing org_id/repo_id, which is precisely the parity with clone that
394+
// this source is supposed to keep.
395+
const isUseRepoDisabled = isFormDisabled || !gitHubAuthMet || localPreviewStatus !== 'valid'
391396

392397
// Early return for validation errors (e.g. missing id prop)
393398
if (validationError) {
@@ -430,14 +435,15 @@ function GitCloneInteractive({
430435
/>
431436
)}
432437

433-
{/* Blocked state: waiting for the referenced auth block (cloning only) */}
434-
{hasAllBlockingDependencies && !gitHubAuthMet && !isLocalSource && (
438+
{/* Blocked state: waiting for the referenced auth block */}
439+
{hasAllBlockingDependencies && !gitHubAuthMet && (
435440
<div className="mb-4 p-3 bg-warning-muted border border-warning/30 rounded-md flex items-start gap-2">
436441
<AlertTriangle className="size-4 text-warning mt-0.5 shrink-0" />
437442
<div>
438443
<p className="text-sm font-medium text-warning-foreground m-0">Waiting for git authentication</p>
439444
<p className="text-xs text-warning-foreground m-0 mt-0.5">
440-
Complete the &apos;{githubAuthId ?? gitAuthId}&apos; authentication block above before cloning.
445+
Complete the &apos;{githubAuthId ?? gitAuthId}&apos; authentication block above
446+
before {isLocalSource ? 'selecting a repository' : 'cloning'}.
441447
</p>
442448
</div>
443449
</div>

web/src/components/mdx/GitClone/__tests__/GitClone.local.test.tsx

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,24 @@ describe("GitClone — repository source picker", () => {
9898
expect(repoDirInput()).toBeInTheDocument()
9999
})
100100

101-
it("does not wait on an auth block to select a local checkout", () => {
101+
it("keeps the directory form usable while a linked auth block is pending", () => {
102102
renderGitClone({ source: "local", gitAuthId: "git-auth" })
103-
expect(screen.queryByText(/Waiting for git authentication/i)).not.toBeInTheDocument()
103+
// Browsing and checking a directory needs no credentials...
104104
expect(repoDirInput()).not.toBeDisabled()
105+
expect(screen.getByRole("button", { name: /Browse/i })).not.toBeDisabled()
106+
})
107+
108+
it("waits for a linked auth block before a checkout can be confirmed", async () => {
109+
renderGitClone({ source: "local", gitAuthId: "git-auth", prefilledRepoDir: "/home/me/infra" })
110+
111+
// ...but confirming does wait, because the GitHub org/repo ids are resolved
112+
// from the session token at confirm time and can't be filled in later.
113+
expect(screen.getByText(/Waiting for git authentication/i)).toBeInTheDocument()
114+
await waitFor(
115+
() => expect(screen.getByText(/Git repository found/i)).toBeInTheDocument(),
116+
{ timeout: 2000 },
117+
)
118+
expect(screen.getByRole("button", { name: /Use This Repo/i })).toBeDisabled()
105119
})
106120
})
107121

@@ -110,7 +124,7 @@ describe("GitClone — local checkout", () => {
110124
renderGitClone({ source: "local", prefilledRepoDir: "/home/me/infra" })
111125

112126
await waitFor(
113-
() => expect(screen.getByText(/Git repository/i)).toBeInTheDocument(),
127+
() => expect(screen.getByText(/Git repository found/i)).toBeInTheDocument(),
114128
{ timeout: 2000 },
115129
)
116130

@@ -161,6 +175,22 @@ describe("GitClone — local checkout", () => {
161175
expect(repoDirInput()).toHaveValue("/home/me/infra")
162176
})
163177

178+
it("tells the user the checked directory is not in use until confirmed", async () => {
179+
renderGitClone({ source: "local", prefilledRepoDir: "/home/me/infra" })
180+
181+
await waitFor(
182+
() => expect(screen.getByText(/Git repository found/i)).toBeInTheDocument(),
183+
{ timeout: 2000 },
184+
)
185+
186+
// The preview must not read as a finished block: nothing is registered
187+
// until "Use This Repo", and a block with no outputs leaves every consumer
188+
// waiting on outputs that were never produced.
189+
expect(screen.getByText(/Not in use yet/i)).toBeInTheDocument()
190+
expect(screen.queryByText(/Using local checkout/i)).not.toBeInTheDocument()
191+
expect(registerWorkTree).not.toHaveBeenCalled()
192+
})
193+
164194
it("registers the checkout as a worktree and reports success", async () => {
165195
const user = userEvent.setup()
166196
renderGitClone({ source: "local", prefilledRepoDir: "/home/me/infra" })
@@ -211,7 +241,7 @@ describe("GitClone — local checkout", () => {
211241
await waitFor(
212242
() =>
213243
expect(
214-
screen.getByText(/remote blocks that open a pull request need one/i),
244+
screen.getByText(/blocks that open a pull request need one/i),
215245
).toBeInTheDocument(),
216246
{ timeout: 2000 },
217247
)

web/src/components/mdx/GitClone/components/LocalRepoForm.tsx

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CheckCircle, FolderOpen, Loader2, XCircle } from "lucide-react"
1+
import { FolderGit2, FolderOpen, Loader2, XCircle } from "lucide-react"
22
import { Button } from "@/components/ui/button"
33
import { InfoTooltip } from "@/components/mdx/GitPullRequest/components/InfoTooltip"
44
import type { LocalRepoInfo } from "../types"
@@ -76,13 +76,18 @@ export function LocalRepoForm({
7676
</div>
7777
)}
7878

79+
{/* Deliberately NOT success-styled: this only reports what the directory
80+
is, and no outputs exist until the user confirms below. Green is
81+
reserved for the completed state, or a checked directory reads as a
82+
finished block and downstream blocks look broken for want of outputs
83+
nobody produced yet. */}
7984
{previewStatus === 'valid' && preview && (
80-
<div className="p-3 bg-success-muted border border-success/30 rounded-md space-y-1">
81-
<div className="flex items-center gap-2 text-sm font-medium text-success">
82-
<CheckCircle className="size-4 shrink-0" />
83-
Git repository
85+
<div className="p-3 bg-info-muted border border-info/40 rounded-md space-y-1">
86+
<div className="flex items-center gap-2 text-sm font-medium text-info">
87+
<FolderGit2 className="size-4 shrink-0" />
88+
Git repository found
8489
</div>
85-
<div className="text-xs text-success space-y-0.5">
90+
<div className="text-xs text-muted-foreground space-y-0.5">
8691
<div>
8792
Root: <code className="font-mono">{preview.absolutePath}</code>
8893
</div>
@@ -100,9 +105,14 @@ export function LocalRepoForm({
100105
</div>
101106
{!preview.remoteUrl && (
102107
<div className="text-warning-foreground">
103-
No <code className="font-mono">origin</code> remote — blocks that open a pull request need one.
108+
No remote — this repo produces no <code className="font-mono">repo_owner</code> or{' '}
109+
<code className="font-mono">repo_name</code>, and blocks that open a pull request need one.
104110
</div>
105111
)}
112+
<div className="pt-1 text-foreground">
113+
Not in use yet — choose <strong>Use This Repo</strong> to make this repository
114+
and its outputs available to later blocks.
115+
</div>
106116
</div>
107117
</div>
108118
)}

0 commit comments

Comments
 (0)