feat(daemon): persist env workspace to deco/env/<name> branch#1185
feat(daemon): persist env workspace to deco/env/<name> branch#1185hugo-ccabral wants to merge 1 commit into
Conversation
Adds a git-backed persistence path for env pods so admin can drop the EBS PVC that today stores the cloned repo + WIP across restarts. When DECO_ENV_NAME is set: - ensureGit targets `deco/env/<DECO_ENV_NAME>`; ls-remote-probes first and falls back to cloning the upstream branch + `checkout -b` when the env branch does not exist yet. Sets upstream to origin/<upstream> so assertRebased rebases env state onto main on startup. - flushEnvBranch() commits + pushes the working tree to the env branch. Write-locked, skips on a clean tree, refreshes the GitHub App token, never throws (so a flaky network on shutdown can't crash the pod). - main.ts wires flushEnvBranch on a 2-min throttle from the file watcher and into a SIGTERM/SIGINT handler capped at 60s so it cannot run past terminationGracePeriodSeconds. Production pods (no DECO_ENV_NAME) are unaffected: the new code paths are gated on ENV_BRANCH being set, and the existing SOURCE_ASSET_PATH tarball path continues to operate for post-publish snapshots. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces environment-specific Git branch support. When ChangesEnvironment Branch Feature
Sequence DiagramsequenceDiagram
participant Git as Git Operations
participant Init as ensureGit
participant Remote as Remote (GitHub)
participant Watch as FileWatcher
participant Flush as flushEnvBranch
participant Push as origin/refs/heads/ENV_BRANCH
Init->>Remote: Probe ENV_BRANCH existence
alt ENV_BRANCH exists
Init->>Git: Clone ENV_BRANCH
else
Init->>Git: Clone upstream tracking branch
end
Init->>Git: Create local env branch + set upstream
Init->>Git: Fetch origin/upstreamBranch
loop Runtime
Watch->>Watch: Detect file changes
Watch->>Flush: Trigger flushEnvState (throttled)
Flush->>Git: Check for working tree changes
alt Changes exist
Flush->>Git: Commit snapshot
Flush->>Remote: Refresh netrc (GitHub App token)
Flush->>Push: Push to origin/refs/heads/ENV_BRANCH
end
end
Note over Flush: On SIGTERM (60s timeout)
Flush->>Git: Final commit & push
Flush->>Flush: Exit gracefully
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Tagging OptionsShould a new tag be published when this PR is merged?
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
daemon/main.ts (1)
167-197: 💤 Low valueShutdown handler unconditionally
Deno.exit(0)short-circuits the rest of the process.A few small concerns worth a quick pass:
Deno.exit(0)runs as soon as the flush settles (success, failure, or 60s timeout), so other in-flight work — outstanding HTTP requests onDeno.serve, the worker subprocess, file watchers, AI sessions — is hard-killed mid-flight. For env pods this is probably acceptable given k8sterminationGracePeriodSeconds, but it's worth confirming the worker subprocess gets its own SIGTERM from k8s (process groups) rather than being orphaned here.- The exit code is always
0, even when the flush actually fails. If you want shutdown-flush failures to be visible in pod restart metrics / k8s lastTerminationState, preferDeno.exit(err ? 1 : 0).- Once
flushing = true, a second SIGTERM is a no-op (if (flushing) return;). That means a stuck push past the 60s deadline can only be escaped by SIGKILL from k8s when the grace period elapses. That's the expected k8s contract, just noting it for confirmation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@daemon/main.ts` around lines 167 - 197, The shutdown handler currently always calls Deno.exit(0) after attempting the flush in onShutdown, which prematurely hard-kills other in-flight work and hides flush failures; update onShutdown to (1) exit with a non-zero code when the flush rejects or times out (use Deno.exit(1) on error/timeout, Deno.exit(0) only on successful flush), (2) treat the timeout/rejection from the Promise.race as a failure so the exit code reflects that, and (3) allow a second signal to force an immediate hard exit (e.g., track a forceExit flag checked in the signal listeners that bypasses the if (flushing) return; path) so stuck flushes can be escaped; refer to onShutdown, flushing, SHUTDOWN_FLUSH_TIMEOUT_MS, and flushEnvBranch to implement these changes.daemon/git.ts (1)
920-952: ⚡ Quick win
flushEnvBranchholds the write lock across the network push.
lockerGitAPI.lock.wlock()is acquired for the whole function — includinggit.pushto GitHub, which can take seconds-to-tens-of-seconds on a slow network or while waiting onsetupGitHubAppNetrc. While it's held,publish,rebase,discard,checkoutBranch,checkoutExistingBranch,gitRaw, andstatusare all blocked (seeapp.use(lockerGitAPI.wlock)increateGitAPIS). With the file-watch throttle calling this every 2 min on a busy editor session, you'll see admin UI requests intermittently stalling on the daemon for the duration of a push.Consider doing the staging/commit under the lock, then releasing the lock before the network push (or scoping the lock around
git.status()+git.commit()only and pushing without holding it). At minimum, since the design assumes a single writer per env branch, the lock here is mostly serving to keep the index consistent — not to serialize pushes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@daemon/git.ts` around lines 920 - 952, flushEnvBranch currently holds the write lock (lockerGitAPI.lock.wlock()) across network operations; change it to acquire the lock only for local index ops (call git.status(), then git.add() and git.commit() while the lock is held) and then release the lock (dispose the using _) before calling setupGithubTokenNetrc() and git.push(). Keep the try/catch but split the workflow into a locked section that stages/commits and an unlocked section that performs the network setup/push (referencing flushEnvBranch, lockerGitAPI.lock.wlock(), git.status(), git.add(), commit(), setupGithubTokenNetrc(), and git.push()) so pushes don’t block other writers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@daemon/git.ts`:
- Around line 813-870: The clone+fetch flow unintentionally uses single-branch
shallow cloning when ENV_BRANCH is set (because --depth implies
--single-branch), so origin/<upstreamBranch> never becomes a remote-tracking ref
and branch --set-upstream-to fails; fix by making the git.clone call (the
git.clone(...) call that builds args with ENV_BRANCH and branchToClone) include
"--no-single-branch" when ENV_BRANCH is present so the wildcard refspec is
preserved, or alternatively change the subsequent git.fetch(...) (the
fetch(["origin", upstreamBranch, "--depth","1"]) call) to fetch an explicit
refspec that writes refs/remotes/origin/<upstreamBranch> (e.g.
refs/heads/<upstreamBranch>:refs/remotes/origin/<upstreamBranch>) so
branch([`--set-upstream-to=origin/${upstreamBranch}`, ENV_BRANCH]) and
assertRebased/status.tracking work correctly.
- Around line 942-948: The push to refs/heads/${ENV_BRANCH} can fail as
non-fast-forward after a rebase; update the git.push call in the flush/commit
flow (the call that currently invokes git.push(["origin",
`HEAD:refs/heads/${ENV_BRANCH}`])) to use force-with-lease so rebased local HEAD
can be applied while still protecting against concurrent remote writes (i.e.,
add the "--force-with-lease" refspec/option to the git.push invocation).
---
Nitpick comments:
In `@daemon/git.ts`:
- Around line 920-952: flushEnvBranch currently holds the write lock
(lockerGitAPI.lock.wlock()) across network operations; change it to acquire the
lock only for local index ops (call git.status(), then git.add() and
git.commit() while the lock is held) and then release the lock (dispose the
using _) before calling setupGithubTokenNetrc() and git.push(). Keep the
try/catch but split the workflow into a locked section that stages/commits and
an unlocked section that performs the network setup/push (referencing
flushEnvBranch, lockerGitAPI.lock.wlock(), git.status(), git.add(), commit(),
setupGithubTokenNetrc(), and git.push()) so pushes don’t block other writers.
In `@daemon/main.ts`:
- Around line 167-197: The shutdown handler currently always calls Deno.exit(0)
after attempting the flush in onShutdown, which prematurely hard-kills other
in-flight work and hides flush failures; update onShutdown to (1) exit with a
non-zero code when the flush rejects or times out (use Deno.exit(1) on
error/timeout, Deno.exit(0) only on successful flush), (2) treat the
timeout/rejection from the Promise.race as a failure so the exit code reflects
that, and (3) allow a second signal to force an immediate hard exit (e.g., track
a forceExit flag checked in the signal listeners that bypasses the if (flushing)
return; path) so stuck flushes can be escaped; refer to onShutdown, flushing,
SHUTDOWN_FLUSH_TIMEOUT_MS, and flushEnvBranch to implement these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7a9ada4-522e-4d2b-a934-fceaff47e37b
📒 Files selected for processing (2)
daemon/git.tsdaemon/main.ts
| if (ENV_BRANCH) { | ||
| try { | ||
| const remoteRefs = await git.listRemote([ | ||
| "--heads", | ||
| cloneUrl, | ||
| ENV_BRANCH, | ||
| ]); | ||
| envBranchExists = remoteRefs.trim().length > 0; | ||
| } catch (err) { | ||
| console.warn( | ||
| `[ensureGit] ls-remote failed for ${ENV_BRANCH}, falling back to ${upstreamBranch}:`, | ||
| err, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const branchToClone = envBranchExists ? ENV_BRANCH! : upstreamBranch; | ||
|
|
||
| await git | ||
| .clone(cloneUrl, ".", [ | ||
| "--depth", | ||
| "1", | ||
| "--single-branch", | ||
| // In env mode we also want refs for upstreamBranch so assertRebased | ||
| // can rebase onto it; --single-branch would restrict refspec to the | ||
| // checked-out branch only. | ||
| ...(ENV_BRANCH ? [] : ["--single-branch"]), | ||
| "--branch", | ||
| branch ?? DEFAULT_TRACKING_BRANCH, | ||
| branchToClone, | ||
| ]) | ||
| .submoduleInit() | ||
| .submoduleUpdate(["--depth", "1"]); | ||
|
|
||
| if (ENV_BRANCH) { | ||
| if (!envBranchExists) { | ||
| // Cloned upstream — create the env branch locally. First push will | ||
| // create it on origin via flushEnvBranch. | ||
| await git.checkoutLocalBranch(ENV_BRANCH); | ||
| } | ||
| // Make sure origin/<upstream> is fetched so assertRebased has a target. | ||
| await git | ||
| .fetch(["origin", upstreamBranch, "--depth", "1"]) | ||
| .catch((err) => { | ||
| console.warn( | ||
| `[ensureGit] fetch origin ${upstreamBranch} failed:`, | ||
| err, | ||
| ); | ||
| }); | ||
| // Point the env branch's upstream at origin/<upstream> so status.tracking | ||
| // resolves there and assertRebased rebases env state onto upstream. | ||
| await git | ||
| .branch([`--set-upstream-to=origin/${upstreamBranch}`, ENV_BRANCH]) | ||
| .catch((err) => { | ||
| console.warn( | ||
| `[ensureGit] set-upstream-to origin/${upstreamBranch} for ${ENV_BRANCH} failed:`, | ||
| err, | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
git clone --depth 1 --branch X behavior of remote.origin.fetch refspec narrowing
💡 Result:
When you run git clone --depth 1 --branch X <repo>, Git creates a shallow clone that is also effectively a single-branch clone (unless you explicitly disable it). That single-branch behavior narrows the remote fetch refspec (remote.origin.fetch) so that future git fetch operations only update the remote-tracking ref for branch X instead of all branch heads. Evidence-backed behavior: 1) --depth implies --single-branch by default, and the man page explicitly states that further fetches “will only update the remote-tracking branch for the branch this option was used for” [1][2]. 2) In practice, shallow+single-branch results in remote.origin.fetch being set to a branch-specific refspec like +refs/heads/<branch>:refs/remotes/origin/<branch> rather than the default wildcard refspec +refs/heads/*:refs/remotes/origin/* [3]. 3) If you want to fetch other branches from the same shallow clone, you must widen the refspec back to the wildcard form and then fetch again, or do a targeted fetch with an explicit refspec [3]. Example of what you should expect in .git/config after such a clone: - Single-branch/shallow style (narrowed): fetch = +refs/heads/X:refs/remotes/origin/X [3] - Default full fetch style (not narrowed): fetch = +refs/heads/*:refs/remotes/origin/* [3] How to “fix”/override the narrowing: - Widen the refspec, then fetch: git config remote.origin.fetch '+refs/heads/*:refs/remotes/origin/*' [3] git fetch [3] - Or fetch just the missing branch once with an explicit refspec (depth-limited if desired): git fetch --depth=1 origin Y:Y or git fetch --depth=1 origin Y:refs/remotes/origin/Y [4] So, the key point about “refspec narrowing” is: --depth 1 triggers single-branch mode, and single-branch mode narrows the origin fetch refspec to only the chosen branch X, which then controls what git fetch updates going forward [1][2][3].
Citations:
- 1: https://web.mit.edu/git/git-doc/git-clone.html
- 2: https://git-scm.com/docs/git-clone
- 3: https://stackoverflow.com/questions/46809742/how-do-fetch-remote-branch-after-i-cloned-repo-with-git-clone-depth-1
- 4: https://stackoverflow.com/questions/24107485/how-to-checkout-remote-branch-with-git-clone-depth-1
🏁 Script executed:
# First, let's understand the structure and locate the relevant code
git ls-files daemon/ | head -20Repository: deco-cx/deco
Length of output: 415
🏁 Script executed:
# Look at the git.ts file to understand the git wrapper methods
wc -l daemon/git.tsRepository: deco-cx/deco
Length of output: 75
🏁 Script executed:
# Examine the exact code in question with context
sed -n '813,870p' daemon/git.ts | cat -nRepository: deco-cx/deco
Length of output: 2359
🏁 Script executed:
# Find the git method implementations (clone, fetch, branch, etc.)
rg -A 10 "\.clone\(" daemon/git.ts | head -40Repository: deco-cx/deco
Length of output: 435
🏁 Script executed:
# Look for assertRebased implementation
rg -n "assertRebased" daemon/git.ts
rg -n "DEFAULT_TRACKING_BRANCH" daemon/git.tsRepository: deco-cx/deco
Length of output: 726
🏁 Script executed:
# Check the git wrapper class to understand how fetch and branch work
rg -B 5 -A 15 "fetch\(" daemon/git.ts | head -60Repository: deco-cx/deco
Length of output: 1409
🏁 Script executed:
# Read the assertRebased function implementation
sed -n '638,700p' daemon/git.ts | cat -nRepository: deco-cx/deco
Length of output: 2231
🏁 Script executed:
# Read more context around git class/methods
sed -n '1,100p' daemon/git.ts | cat -nRepository: deco-cx/deco
Length of output: 3940
🏁 Script executed:
# Look for the git command wrapper - likely a class or factory
rg -B 2 -A 20 "class.*Git|function.*git|const git =" daemon/git.ts | head -80Repository: deco-cx/deco
Length of output: 720
🏁 Script executed:
# Check how fetch is actually implemented - what command is constructed
rg -B 5 -A 20 "fetch.*function|fetch.*=" daemon/git.ts | head -100Repository: deco-cx/deco
Length of output: 613
🏁 Script executed:
# Look at git status and how tracking is read
sed -n '667,680p' daemon/git.ts | cat -nRepository: deco-cx/deco
Length of output: 518
🏁 Script executed:
# Check how simple-git is used and what it exports
rg -A 5 "from \"simple-git\"" daemon/git.tsRepository: deco-cx/deco
Length of output: 333
🏁 Script executed:
# Look at the exact git.clone call to understand how simple-git constructs the command
sed -n '830,875p' daemon/git.ts | cat -nRepository: deco-cx/deco
Length of output: 2008
🏁 Script executed:
# Check if there are any integration tests or logs showing actual git behavior
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "clone\|ENV_BRANCH" 2>/dev/null | head -5Repository: deco-cx/deco
Length of output: 82
🏁 Script executed:
# Search for any documentation or comments about simple-git behavior
rg -B 3 -A 3 "simple-git" daemon/Repository: deco-cx/deco
Length of output: 1044
🏁 Script executed:
# Check the exact git raw command usage in assertRebased to see what tracking value would be used
sed -n '638,680p' daemon/git.ts | cat -nRepository: deco-cx/deco
Length of output: 1577
Pass --no-single-branch in env mode to preserve multi-branch refspec in shallow clones.
The comment claiming omitted --single-branch keeps a wildcard refspec is incorrect: git's --depth=<n> implicitly enables --single-branch unless --no-single-branch is explicitly passed. The current code (lines 830–841) omits both flags in env mode, so the clone uses only --depth 1 with single-branch enabled by default, resulting in a narrowed .git/config refspec (+refs/heads/<branchToClone>:refs/remotes/origin/<branchToClone>) rather than the intended wildcard.
Consequences:
- The
git fetch origin upstreamBranch --depth 1on line 853 does not createrefs/remotes/origin/<upstreamBranch>; it only updatesFETCH_HEAD - The
git branch --set-upstream-to=origin/<upstreamBranch>on line 863 fails silently (caught by.catch) because the remote-tracking ref doesn't exist assertRebasedreadsstatus.trackingas empty on line 667, falls back toDEFAULT_TRACKING_BRANCH("main"), and the subsequentgit rev-list --count <base>..mainfails or produces wrong results
Fix: Add --no-single-branch when ENV_BRANCH is set. Alternatively, use an explicit refspec on the fetch to force creation of the remote-tracking ref.
Suggested fix
await git
.clone(cloneUrl, ".", [
"--depth",
"1",
- // In env mode we also want refs for upstreamBranch so assertRebased
- // can rebase onto it; --single-branch would restrict refspec to the
- // checked-out branch only.
- ...(ENV_BRANCH ? [] : ["--single-branch"]),
+ // In env mode we also want refs for upstreamBranch so assertRebased
+ // can rebase onto it. Note: --depth implies --single-branch unless
+ // --no-single-branch is given, so we must set it explicitly.
+ ...(ENV_BRANCH ? ["--no-single-branch"] : ["--single-branch"]),
"--branch",
branchToClone,
])Or keep single-branch behavior and use explicit refspec on fetch:
await git
- .fetch(["origin", upstreamBranch, "--depth", "1"])
+ .fetch([
+ "origin",
+ `+refs/heads/${upstreamBranch}:refs/remotes/origin/${upstreamBranch}`,
+ "--depth",
+ "1",
+ ])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (ENV_BRANCH) { | |
| try { | |
| const remoteRefs = await git.listRemote([ | |
| "--heads", | |
| cloneUrl, | |
| ENV_BRANCH, | |
| ]); | |
| envBranchExists = remoteRefs.trim().length > 0; | |
| } catch (err) { | |
| console.warn( | |
| `[ensureGit] ls-remote failed for ${ENV_BRANCH}, falling back to ${upstreamBranch}:`, | |
| err, | |
| ); | |
| } | |
| } | |
| const branchToClone = envBranchExists ? ENV_BRANCH! : upstreamBranch; | |
| await git | |
| .clone(cloneUrl, ".", [ | |
| "--depth", | |
| "1", | |
| "--single-branch", | |
| // In env mode we also want refs for upstreamBranch so assertRebased | |
| // can rebase onto it; --single-branch would restrict refspec to the | |
| // checked-out branch only. | |
| ...(ENV_BRANCH ? [] : ["--single-branch"]), | |
| "--branch", | |
| branch ?? DEFAULT_TRACKING_BRANCH, | |
| branchToClone, | |
| ]) | |
| .submoduleInit() | |
| .submoduleUpdate(["--depth", "1"]); | |
| if (ENV_BRANCH) { | |
| if (!envBranchExists) { | |
| // Cloned upstream — create the env branch locally. First push will | |
| // create it on origin via flushEnvBranch. | |
| await git.checkoutLocalBranch(ENV_BRANCH); | |
| } | |
| // Make sure origin/<upstream> is fetched so assertRebased has a target. | |
| await git | |
| .fetch(["origin", upstreamBranch, "--depth", "1"]) | |
| .catch((err) => { | |
| console.warn( | |
| `[ensureGit] fetch origin ${upstreamBranch} failed:`, | |
| err, | |
| ); | |
| }); | |
| // Point the env branch's upstream at origin/<upstream> so status.tracking | |
| // resolves there and assertRebased rebases env state onto upstream. | |
| await git | |
| .branch([`--set-upstream-to=origin/${upstreamBranch}`, ENV_BRANCH]) | |
| .catch((err) => { | |
| console.warn( | |
| `[ensureGit] set-upstream-to origin/${upstreamBranch} for ${ENV_BRANCH} failed:`, | |
| err, | |
| ); | |
| }); | |
| } | |
| await git | |
| .clone(cloneUrl, ".", [ | |
| "--depth", | |
| "1", | |
| // In env mode we also want refs for upstreamBranch so assertRebased | |
| // can rebase onto it. Note: --depth implies --single-branch unless | |
| // --no-single-branch is given, so we must set it explicitly. | |
| ...(ENV_BRANCH ? ["--no-single-branch"] : ["--single-branch"]), | |
| "--branch", | |
| branchToClone, | |
| ]) | |
| .submoduleInit() | |
| .submoduleUpdate(["--depth", "1"]); | |
| if (ENV_BRANCH) { | |
| if (!envBranchExists) { | |
| // Cloned upstream — create the env branch locally. First push will | |
| // create it on origin via flushEnvBranch. | |
| await git.checkoutLocalBranch(ENV_BRANCH); | |
| } | |
| // Make sure origin/<upstream> is fetched so assertRebased has a target. | |
| await git | |
| .fetch(["origin", upstreamBranch, "--depth", "1"]) | |
| .catch((err) => { | |
| console.warn( | |
| `[ensureGit] fetch origin ${upstreamBranch} failed:`, | |
| err, | |
| ); | |
| }); | |
| // Point the env branch's upstream at origin/<upstream> so status.tracking | |
| // resolves there and assertRebased rebases env state onto upstream. | |
| await git | |
| .branch([`--set-upstream-to=origin/${upstreamBranch}`, ENV_BRANCH]) | |
| .catch((err) => { | |
| console.warn( | |
| `[ensureGit] set-upstream-to origin/${upstreamBranch} for ${ENV_BRANCH} failed:`, | |
| err, | |
| ); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@daemon/git.ts` around lines 813 - 870, The clone+fetch flow unintentionally
uses single-branch shallow cloning when ENV_BRANCH is set (because --depth
implies --single-branch), so origin/<upstreamBranch> never becomes a
remote-tracking ref and branch --set-upstream-to fails; fix by making the
git.clone call (the git.clone(...) call that builds args with ENV_BRANCH and
branchToClone) include "--no-single-branch" when ENV_BRANCH is present so the
wildcard refspec is preserved, or alternatively change the subsequent
git.fetch(...) (the fetch(["origin", upstreamBranch, "--depth","1"]) call) to
fetch an explicit refspec that writes refs/remotes/origin/<upstreamBranch> (e.g.
refs/heads/<upstreamBranch>:refs/remotes/origin/<upstreamBranch>) so
branch([`--set-upstream-to=origin/${upstreamBranch}`, ENV_BRANCH]) and
assertRebased/status.tracking work correctly.
| await git | ||
| .add(["-A"]) | ||
| .commit(message, { "--no-verify": null }); | ||
| // Explicit refspec: the upstream is origin/<DEFAULT_TRACKING_BRANCH> so a | ||
| // bare `git push` would push env state onto upstream. We push to the env | ||
| // branch by name instead. | ||
| await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and understand the context
find . -name "git.ts" -path "*/daemon/*" | head -5Repository: deco-cx/deco
Length of output: 72
🏁 Script executed:
# Get file size to determine how much we can read
wc -l ./daemon/git.tsRepository: deco-cx/deco
Length of output: 77
🏁 Script executed:
# Read the flushEnvBranch function context around lines 942-948
sed -n '920,970p' ./daemon/git.tsRepository: deco-cx/deco
Length of output: 1545
🏁 Script executed:
# Search for ensureGit and assertRebased functions
rg -n "export const ensureGit|const ensureGit|function ensureGit" ./daemon/git.tsRepository: deco-cx/deco
Length of output: 94
🏁 Script executed:
# Read ensureGit function - read a larger range to get the complete function
sed -n '716,820p' ./daemon/git.tsRepository: deco-cx/deco
Length of output: 3565
🏁 Script executed:
# Continue reading ensureGit to find the clone command
sed -n '820,900p' ./daemon/git.tsRepository: deco-cx/deco
Length of output: 2780
🏁 Script executed:
# Search for assertRebased function
rg -n "assertRebased" ./daemon/git.ts | head -20Repository: deco-cx/deco
Length of output: 392
🏁 Script executed:
# Read the assertRebased function
sed -n '638,715p' ./daemon/git.tsRepository: deco-cx/deco
Length of output: 2248
🏁 Script executed:
# Check the catch block and error handling around flushEnvBranch
sed -n '945,955p' ./daemon/git.tsRepository: deco-cx/deco
Length of output: 460
🏁 Script executed:
# Search for ENV_BRANCH setup and any test references to rebase behavior
rg -n "ENV_BRANCH" ./daemon/git.ts | head -20Repository: deco-cx/deco
Length of output: 826
🏁 Script executed:
# Look for any tests or comments about the rebase+push scenario
rg -n "rebase|rebased|rebase-then-flush" ./daemon/git.tsRepository: deco-cx/deco
Length of output: 1766
🏁 Script executed:
# Search for test files related to git functionality
find . -name "*.test.ts" -o -name "*.spec.ts" | grep -i git | head -10Repository: deco-cx/deco
Length of output: 38
🏁 Script executed:
# Search for any force-with-lease or push-related logic
rg -n "force-with-lease|force|--force" ./daemon/git.tsRepository: deco-cx/deco
Length of output: 73
🏁 Script executed:
# Check context around line 389
sed -n '380,400p' ./daemon/git.tsRepository: deco-cx/deco
Length of output: 484
🏁 Script executed:
# Check if git.push is a wrapper that calls validateRawArgs
rg -n "git.push|\.push\(" ./daemon/git.ts | head -20Repository: deco-cx/deco
Length of output: 391
Push to refs/heads/${ENV_BRANCH} will fail as non-fast-forward after a rebase.
When ensureGit clones with --depth 1, the remote-tracking branch origin/ENV_BRANCH points at that shallow commit. When assertRebased rebases the local ENV_BRANCH onto origin/<upstreamBranch>, it creates new commits with a different parent chain. The remote-tracking origin/ENV_BRANCH is never updated by the rebase (only by fetch), so when flushEnvBranch attempts to push, HEAD is not a descendant of origin/ENV_BRANCH—the push fails as non-fast-forward. The error is caught and logged silently at line 949, so env state never persists. This breaks the rebase-then-flush cycle.
Add --force-with-lease to allow the push while still rejecting concurrent writes by other pods:
🛠️ Suggested fix
- await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`]);
+ // Local ENV_BRANCH may have been rebased onto origin/<upstreamBranch>
+ // by assertRebased, so it's not a descendant of origin/ENV_BRANCH.
+ // Use --force-with-lease so we still bail if another pod (mis)wrote to
+ // the same DECO_ENV_NAME branch concurrently.
+ await git.push([
+ "origin",
+ `HEAD:refs/heads/${ENV_BRANCH}`,
+ "--force-with-lease",
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await git | |
| .add(["-A"]) | |
| .commit(message, { "--no-verify": null }); | |
| // Explicit refspec: the upstream is origin/<DEFAULT_TRACKING_BRANCH> so a | |
| // bare `git push` would push env state onto upstream. We push to the env | |
| // branch by name instead. | |
| await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`]); | |
| await git | |
| .add(["-A"]) | |
| .commit(message, { "--no-verify": null }); | |
| // Explicit refspec: the upstream is origin/<DEFAULT_TRACKING_BRANCH> so a | |
| // bare `git push` would push env state onto upstream. We push to the env | |
| // branch by name instead. | |
| // Local ENV_BRANCH may have been rebased onto origin/<upstreamBranch> | |
| // by assertRebased, so it's not a descendant of origin/ENV_BRANCH. | |
| // Use --force-with-lease so we still bail if another pod (mis)wrote to | |
| // the same DECO_ENV_NAME branch concurrently. | |
| await git.push([ | |
| "origin", | |
| `HEAD:refs/heads/${ENV_BRANCH}`, | |
| "--force-with-lease", | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@daemon/git.ts` around lines 942 - 948, The push to refs/heads/${ENV_BRANCH}
can fail as non-fast-forward after a rebase; update the git.push call in the
flush/commit flow (the call that currently invokes git.push(["origin",
`HEAD:refs/heads/${ENV_BRANCH}`])) to use force-with-lease so rebased local HEAD
can be applied while still protecting against concurrent remote writes (i.e.,
add the "--force-with-lease" refspec/option to the git.push invocation).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="daemon/git.ts">
<violation number="1" location="daemon/git.ts:838">
P1: `--depth 1` implicitly enables `--single-branch` unless `--no-single-branch` is explicitly passed (per git-clone docs). Omitting `--single-branch` here has no effect — the clone still narrows the refspec to only `branchToClone`. As a result, the subsequent `fetch origin ${upstreamBranch}` only writes to `FETCH_HEAD` (not `refs/remotes/origin/${upstreamBranch}`), the `--set-upstream-to` on line 863 fails silently because the remote-tracking ref doesn't exist, and `assertRebased` can't resolve its rebase target.
Pass `--no-single-branch` in env mode, or use an explicit refspec on the fetch (`+refs/heads/${upstreamBranch}:refs/remotes/origin/${upstreamBranch}`).</violation>
<violation number="2" location="daemon/git.ts:936">
P2: If push fails after a successful commit, subsequent `flushEnvBranch` calls (including the SIGTERM handler) see a clean working tree and return early — the locally committed snapshot is never pushed. Consider also checking for unpushed commits (e.g., whether HEAD is ahead of the remote ref) or retrying the push separately from the commit.</violation>
<violation number="3" location="daemon/git.ts:948">
P1: After `assertRebased` rebases the env branch onto upstream, the local history diverges from `origin/${ENV_BRANCH}` — a plain push is rejected as non-fast-forward. The error is caught and logged, so env state silently never persists after a rebase cycle.
Use `--force-with-lease` to allow the diverged push while still detecting concurrent writes from another pod.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Explicit refspec: the upstream is origin/<DEFAULT_TRACKING_BRANCH> so a | ||
| // bare `git push` would push env state onto upstream. We push to the env | ||
| // branch by name instead. | ||
| await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`]); |
There was a problem hiding this comment.
P1: After assertRebased rebases the env branch onto upstream, the local history diverges from origin/${ENV_BRANCH} — a plain push is rejected as non-fast-forward. The error is caught and logged, so env state silently never persists after a rebase cycle.
Use --force-with-lease to allow the diverged push while still detecting concurrent writes from another pod.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon/git.ts, line 948:
<comment>After `assertRebased` rebases the env branch onto upstream, the local history diverges from `origin/${ENV_BRANCH}` — a plain push is rejected as non-fast-forward. The error is caught and logged, so env state silently never persists after a rebase cycle.
Use `--force-with-lease` to allow the diverged push while still detecting concurrent writes from another pod.</comment>
<file context>
@@ -842,6 +906,51 @@ export const ensureGit = async ({
+ // Explicit refspec: the upstream is origin/<DEFAULT_TRACKING_BRANCH> so a
+ // bare `git push` would push env state onto upstream. We push to the env
+ // branch by name instead.
+ await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`]);
+ } catch (err) {
+ console.error(`[flushEnvBranch] Failed to flush ${ENV_BRANCH}:`, err);
</file context>
| await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`]); | |
| // After assertRebased, the local env branch is rebased onto upstream and | |
| // is no longer a descendant of origin/ENV_BRANCH. --force-with-lease | |
| // allows the push while still rejecting concurrent writes. | |
| await git.push(["origin", `HEAD:refs/heads/${ENV_BRANCH}`, "--force-with-lease"]); |
| // In env mode we also want refs for upstreamBranch so assertRebased | ||
| // can rebase onto it; --single-branch would restrict refspec to the | ||
| // checked-out branch only. | ||
| ...(ENV_BRANCH ? [] : ["--single-branch"]), |
There was a problem hiding this comment.
P1: --depth 1 implicitly enables --single-branch unless --no-single-branch is explicitly passed (per git-clone docs). Omitting --single-branch here has no effect — the clone still narrows the refspec to only branchToClone. As a result, the subsequent fetch origin ${upstreamBranch} only writes to FETCH_HEAD (not refs/remotes/origin/${upstreamBranch}), the --set-upstream-to on line 863 fails silently because the remote-tracking ref doesn't exist, and assertRebased can't resolve its rebase target.
Pass --no-single-branch in env mode, or use an explicit refspec on the fetch (+refs/heads/${upstreamBranch}:refs/remotes/origin/${upstreamBranch}).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon/git.ts, line 838:
<comment>`--depth 1` implicitly enables `--single-branch` unless `--no-single-branch` is explicitly passed (per git-clone docs). Omitting `--single-branch` here has no effect — the clone still narrows the refspec to only `branchToClone`. As a result, the subsequent `fetch origin ${upstreamBranch}` only writes to `FETCH_HEAD` (not `refs/remotes/origin/${upstreamBranch}`), the `--set-upstream-to` on line 863 fails silently because the remote-tracking ref doesn't exist, and `assertRebased` can't resolve its rebase target.
Pass `--no-single-branch` in env mode, or use an explicit refspec on the fetch (`+refs/heads/${upstreamBranch}:refs/remotes/origin/${upstreamBranch}`).</comment>
<file context>
@@ -794,17 +803,72 @@ export const ensureGit = async ({
+ // In env mode we also want refs for upstreamBranch so assertRebased
+ // can rebase onto it; --single-branch would restrict refspec to the
+ // checked-out branch only.
+ ...(ENV_BRANCH ? [] : ["--single-branch"]),
"--branch",
- branch ?? DEFAULT_TRACKING_BRANCH,
</file context>
| ...(ENV_BRANCH ? [] : ["--single-branch"]), | |
| // --depth implies --single-branch; we must explicitly opt out in env | |
| // mode so the upstream branch remote-tracking ref is available for | |
| // assertRebased. | |
| ...(ENV_BRANCH ? ["--no-single-branch"] : ["--single-branch"]), |
| await setupGithubTokenNetrc(repoOverride); | ||
| } | ||
|
|
||
| const status = await git.status(); |
There was a problem hiding this comment.
P2: If push fails after a successful commit, subsequent flushEnvBranch calls (including the SIGTERM handler) see a clean working tree and return early — the locally committed snapshot is never pushed. Consider also checking for unpushed commits (e.g., whether HEAD is ahead of the remote ref) or retrying the push separately from the commit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At daemon/git.ts, line 936:
<comment>If push fails after a successful commit, subsequent `flushEnvBranch` calls (including the SIGTERM handler) see a clean working tree and return early — the locally committed snapshot is never pushed. Consider also checking for unpushed commits (e.g., whether HEAD is ahead of the remote ref) or retrying the push separately from the commit.</comment>
<file context>
@@ -842,6 +906,51 @@ export const ensureGit = async ({
+ await setupGithubTokenNetrc(repoOverride);
+ }
+
+ const status = await git.status();
+ if (status.files.length === 0) {
+ return;
</file context>
| @@ -794,17 +803,72 @@ export const ensureGit = async ({ | |||
| ? `https://github.com/deco-sites/${site}.git` | |||
| : `git@github.com:deco-sites/${site}.git`); | |||
There was a problem hiding this comment.
Colocar a ORG do github como um parâmetro para não ficar hardcoded
2 participants
Summary
Adds a git-backed persistence path for env pods so admin can drop the EBS PVC that today stores the cloned repo + WIP across restarts. Paired with deco-sites/admin#3160 which removes the PVC from the StatefulSet manifest.
When
DECO_ENV_NAMEis set:ensureGittargetsdeco/env/<DECO_ENV_NAME>. It probes withls-remotefirst; if the branch exists, clones it directly; otherwise clones the upstream branch andcheckout -b's the env branch locally. Upstream is wired toorigin/<upstream>soassertRebasedrebases env state onto main on startup.flushEnvBranch()commits + pushes the working tree to the env branch. Write-locked against publish/rebase/file-watch, skips when the tree is clean, refreshes the GitHub App token, never throws.daemon/main.tswiresflushEnvBranchon a 2-min throttle in the file watcher, and into aSIGTERM/SIGINThandler capped at 60s so it cannot run past the pod'sterminationGracePeriodSeconds.Production pods (no
DECO_ENV_NAME) are unaffected — every new code path is gated onENV_BRANCHbeing set, and the existingSOURCE_ASSET_PATHtarball persistence continues to operate for post-publish snapshots.Why
Env pods today rely on an EBS PVC to keep
.git/+ the user's uncommitted edits across scale-to-zero cycles. EBS attach dominates cold-start (~30–60s), pins the pod to a specific AZ, and complicates pod scheduling. Pushing WIP to a side branch on the user's own repo lets us throw the disk away on shutdown and clone fresh on startup — same persistence outcome, no attach delay, no AZ pinning.Test plan
DECO_ENV_NAME=test-1; verify the branchdeco/env/test-1appears on the fork after edits + restart.[step 1/4]log).mainwhile the env is running, scale env down + up, confirm env branch came back rebased onto the new upstream.kill -9) and verify the next cold start picks up the last successful periodic flush (loss bounded to ~2 min of edits).DECO_ENV_NAME): confirm unchanged behavior — no env branch, no flush, no SIGTERM handler installed.🤖 Generated with Claude Code