Skip to content

Commit 4a8652f

Browse files
committed
fix(git-push-setup): presence probes fail closed + drop orphan ParseGitHost export — final Codex pass
- A transport error on the .git presence read (configured pairs) now refuses instead of proceeding: fail-open left needsReconstruct=false, so the self-heal could mint a marker-only repo masking the reconstruction the pair needed, and the tokenless recall could report already-configured on wiring it couldn't verify. Both sites pinned by TestGitPushSetupContainer_PresenceProbeError_FailsClosed. - ops.ParseGitHost export deleted: F3's final gate is ops.IsGitHubRemote; the exported wrapper had no callers left. Stale comment/spec/plan references corrected (GLC-3 row now names the fail-closed gate). - Live-verification ledger corrected (Codex catch): the standard self-deploy e2e commits before deploying, so it never proved dirty-tree delivery — replaced with the dedicated dirty-tree probe results: no-git cold start → deploy → build ACTIVE → count=1/'zcp init'/identity survived -g round-trip/tree honestly dirty/content served.
1 parent e5c7fc6 commit 4a8652f

6 files changed

Lines changed: 113 additions & 19 deletions

File tree

docs/spec-workflows.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1121,7 +1121,7 @@ Managed runtime services carry a `/var/www/.git/` that direct `zerops_deploy` tr
11211121
|----|-----------|
11221122
| GLC-1 | Every runtime service added to the project via bootstrap or adopt has `/var/www/.git/` initialized **container-side** (via `ops.SSHDeployer.ExecSSH`, never SFTP MKDIR), owned by `zerops:zerops`, with identity filled set-if-absent (default `user.email = agent@zerops.io`, `user.name = Zerops Agent`) and a reachable HEAD. Enforced by `autoMountTargets` post-mount hook: after `ops.MountService` succeeds it calls `ops.InitServiceGit`. The SSH-exec path matters because zembed's SFTP MKDIR regression creates root-owned directories, which would corrupt `.git/objects/` and break subsequent git operations. Errors are logged but do not mark the mount FAILED — GLC-2 is the safety net. |
11231123
| GLC-2 | `deploy_ssh.go::buildSSHCommand` must tolerate a missing `.git/` as the migration/recovery fallback. The init guard stays inside the OR branch (`test -d .git || git init -q -b main`); identity is set-if-absent OUTSIDE it (never stomping a user-set value — the B13 fix's actual requirement was "identity exists", not "identity is ZCP's"); a HEAD guarantee follows so zcli's archiver always has a commit to diff against. Direct deploy mints NO other commit — the dirty tree ships via zcli's ephemeral stash-archive. Same identity-ensure shape in `BuildGitOriginSyncCommand` (GAP4-1). Pinned by `ops/deploy_git_test.go` + `ops/git_identity_test.go`. |
1124-
| GLC-3 | `ops.DeployGitIdentity` is the single source of the default identity value, consumed only through the single-owner fragment builders in `ops/git_identity.go`. No code path writes identity unconditionally or persists the HEAD-guarantee marker commit's identity into repo config. **Human attribution (F3)**: at git-push-setup, `ops.DeriveGitHubIdentity` derives name/email from the PAT for github.com remotes only (`ops.ParseGitHost` is the gate); the result seeds repo-local config IFF the current value is absent or EXACTLY equals the robot identity (`ops.BuildGitIdentitySeedCommand`) — a genuinely custom value is preserved and reported, never overwritten. This migration fires ONCE per value: a later PAT rotation to a different GitHub account does NOT re-seed, because the now-human identity no longer exactly-matches the robot default (identity is user-owned once set). A buildFromGit clone carrying a recipe-baked non-robot identity is likewise never auto-migrated (neither absent nor exactly-robot) — same preserved/reported treatment. Reconstruction (`BuildGitReconstructCommand`) takes the derived identity directly when available, landing a rebuilt repo human-attributed from its first init rather than robot-then-migrate; the tokenless recall path has no PAT to derive from, so it only detects a still-exactly-robot identity and prompts a one-time re-run with `gitToken` — it never fabricates one. Release tags, export commits, and flatten commits carry no inline identity of their own (`git tag -a` / plain `git commit`) — they read ambient repo config, so they inherit the seeded human identity automatically once F3 has run. |
1124+
| GLC-3 | `ops.DeployGitIdentity` is the single source of the default identity value, consumed only through the single-owner fragment builders in `ops/git_identity.go`. No code path writes identity unconditionally or persists the HEAD-guarantee marker commit's identity into repo config. **Human attribution (F3)**: at git-push-setup, `ops.DeriveGitHubIdentity` derives name/email from the PAT for github.com remotes only (`ops.IsGitHubRemote` — fail-closed exact-host gate — decides); the result seeds repo-local config IFF the current value is absent or EXACTLY equals the robot identity (`ops.BuildGitIdentitySeedCommand`) — a genuinely custom value is preserved and reported, never overwritten. This migration fires ONCE per value: a later PAT rotation to a different GitHub account does NOT re-seed, because the now-human identity no longer exactly-matches the robot default (identity is user-owned once set). A buildFromGit clone carrying a recipe-baked non-robot identity is likewise never auto-migrated (neither absent nor exactly-robot) — same preserved/reported treatment. Reconstruction (`BuildGitReconstructCommand`) takes the derived identity directly when available, landing a rebuilt repo human-attributed from its first init rather than robot-then-migrate; the tokenless recall path has no PAT to derive from, so it only detects a still-exactly-robot identity and prompts a one-time re-run with `gitToken` — it never fabricates one. Release tags, export commits, and flatten commits carry no inline identity of their own (`git tag -a` / plain `git commit`) — they read ambient repo config, so they inherit the seeded human identity automatically once F3 has run. |
11251125
| GLC-4 | The ZCP-host container has no git state. `/var/www` there is the SSHFS mount base, not a code directory; no `.git/` is ever initialized on it, and no `git config --global` is written. `zcp init` in container mode (`init_container.go::containerSteps`) performs only Claude config + optional VS Code setup. Developer-side git workflows (e.g. `zcp sync recipe push-app`) run on developer laptops with the developer's own `~/.gitconfig` and are never expected to pass through a Zerops-deployed ZCP service. |
11261126
| GLC-5 | Mount-side `git init` (from the ZCP-host into a managed service's SSHFS-mounted `/var/www/{hostname}/`) is forbidden agent behavior, covered by `develop-first-deploy-write-app.md` guidance. zembed's SFTP MKDIR would produce root-owned `.git/objects/` which poisons every subsequent deploy. Recovery: `ssh {host} "sudo rm -rf /var/www/.git"` and let GLC-2's safety net re-init. |
11271127
| GLC-6 | Local-env `strategy=git-push` requires a user-owned git repo with ≥1 commit (verified against `zcli@v1.0.61` `handler_archiveGitFiles.go:67-75`). ZCP does **not** auto-init git in the user's working directory — identity, default branch and `.gitignore` conventions are personal. `develop-platform-rules-local.md` instructs the agent to ask the user to run `git init && git add -A && git commit -m '<msg>'` themselves; `handleLocalGitPush` pre-flight catches the case as a hard fallback. The default `zerops_deploy` strategy uses `zcli --no-git` and needs no git state. The container-mode default path (GLC-2) depends on the same zcli floor for its `--workspace-state=all` archiver — no runtime probe, containers ship platform-maintained zcli. |

internal/ops/deploy_git_push.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,3 @@ func parseGitHost(rawURL string) string {
109109

110110
return defaultGitHost
111111
}
112-
113-
// ParseGitHost is the exported form of parseGitHost for callers outside
114-
// this package (e.g. tools deciding whether a remote qualifies for GitHub
115-
// identity derivation — F3). Single owner: host extraction never gets a
116-
// second implementation.
117-
func ParseGitHost(rawURL string) string { return parseGitHost(rawURL) }

internal/ops/github_user.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const githubAPIHost = "api.github.com"
2525

2626
// IsGitHubRemote reports whether remoteURL's host is EXACTLY github.com —
2727
// the strict, fail-CLOSED gate for GitHub PAT derivation (F3, Codex
28-
// diff-review finding 1a). Deliberately NOT ops.ParseGitHost: that helper
28+
// diff-review finding 1a). Deliberately NOT parseGitHost (ops-internal, credential scoping): that helper
2929
// fail-OPENs to "github.com" on any parse failure or malformed host, which
3030
// is safe for its actual owner (url-scoping an already-validated
3131
// credential-helper config key) but would be a PAT-confinement bug here —

internal/tools/workflow_git_push_setup.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ func classifyGitIdentitySeedLine(line, seededTok, preservedTok, writeFailedTok s
714714
// gitPushSetupDeriveAndSeedIdentity implements F3 human attribution:
715715
// derives a git identity from the GitHub PAT (github.com remotes only —
716716
// ops.IsGitHubRemote is the single owner, a strict fail-CLOSED host check
717-
// deliberately distinct from ops.ParseGitHost's fail-open credential-
717+
// deliberately distinct from parseGitHost's fail-open credential-
718718
// scoping default; other hosts skip derivation and keep the robot
719719
// fallback), then — unless reconstruction is about to run,
720720
// which fills identity itself as part of its own init (there is no .git
@@ -812,9 +812,19 @@ func gitPushSetupDeriveAndSeedIdentity(
812812
// call itself fails transport-wise.
813813
func gitPushSetupPreProbeSelfHeal(ctx context.Context, sshDeployer ops.SSHDeployer, pushHost string, meta *workflow.ServiceMeta) (needsReconstruct bool, blocker *mcp.CallToolResult) {
814814
if meta.GitPushState == topology.GitPushConfigured {
815-
if presentOut, presentErr := sshDeployer.ExecSSH(ctx, pushHost, "test -d /var/www/.git && echo present || echo absent"); presentErr == nil {
816-
needsReconstruct = strings.Contains(string(presentOut), "absent")
815+
presentOut, presentErr := sshDeployer.ExecSSH(ctx, pushHost, "test -d /var/www/.git && echo present || echo absent")
816+
if presentErr != nil {
817+
// Fail CLOSED: with the presence unknown, proceeding would let
818+
// the self-heal below create a marker-only repo that masks the
819+
// reconstruction a configured pair may need (the exact defect
820+
// the pre-probe ordering exists to prevent) — refuse instead.
821+
return false, convertError(platform.NewPlatformError(
822+
platform.ErrSSHDeployFailed,
823+
withSSHStderr(fmt.Sprintf("git-push-setup: could not determine whether %q still carries /var/www/.git", pushHost), presentErr),
824+
"Verify SSH to the push source is healthy, then re-call. NO remote ref, secret, origin, or meta state was modified.",
825+
), WithRecoveryStatus())
817826
}
827+
needsReconstruct = strings.Contains(string(presentOut), "absent")
818828
}
819829

820830
if !needsReconstruct {
@@ -990,7 +1000,17 @@ func gitPushReconstruct(ctx context.Context, sshDeployer ops.SSHDeployer, pushHo
9901000
// robot identity — deriving from GitHub needs a PAT (F3 item 4).
9911001
func gitPushConfiguredRecall(ctx context.Context, sshDeployer ops.SSHDeployer, input WorkflowInput, meta *workflow.ServiceMeta) *mcp.CallToolResult {
9921002
presentOut, presentErr := sshDeployer.ExecSSH(ctx, meta.Hostname, "test -d /var/www/.git && echo present || echo absent")
993-
if presentErr == nil && strings.Contains(string(presentOut), "absent") {
1003+
if presentErr != nil {
1004+
// Fail CLOSED: "already-configured" promises working wiring; with
1005+
// the presence read failing we cannot make that claim, and a
1006+
// vanished .git would silently skip its reconstruction.
1007+
return convertError(platform.NewPlatformError(
1008+
platform.ErrSSHDeployFailed,
1009+
withSSHStderr(fmt.Sprintf("git-push-setup: could not verify /var/www/.git still exists on %q", meta.Hostname), presentErr),
1010+
"Verify SSH to the push source is healthy, then re-call. NO remote ref, secret, origin, or meta state was modified.",
1011+
), WithRecoveryStatus())
1012+
}
1013+
if strings.Contains(string(presentOut), "absent") {
9941014
divergence, reconErr := gitPushReconstruct(ctx, sshDeployer, meta.Hostname, meta.RemoteURL, ops.DeployGitIdentity)
9951015
if reconErr != nil {
9961016
return convertError(platform.NewPlatformError(
@@ -1034,7 +1054,7 @@ func gitPushConfiguredRecall(ctx context.Context, sshDeployer ops.SSHDeployer, i
10341054
// anything itself — it can only detect the still-robot state and point at
10351055
// the fix; it never fabricates an identity. Gated to github.com remotes
10361056
// (ops.IsGitHubRemote — the same strict fail-closed check the derivation
1037-
// gate uses, not ops.ParseGitHost's fail-open credential-scoping default):
1057+
// gate uses, not parseGitHost's fail-open credential-scoping default):
10381058
// F3 only derives identity from GitHub, so prompting the same re-run for a
10391059
// GitLab/other/malformed remote would be a false promise. Read failure
10401060
// (SSH down, malformed output) is silent — this is advisory, never a

internal/tools/workflow_git_push_setup_container_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,73 @@ func TestGitPushSetupContainer_RotationWithToken_MissingGitStillReconstructs(t *
527527
}
528528
}
529529

530+
// TestGitPushSetupContainer_PresenceProbeError_FailsClosed pins the final-
531+
// pass fix: when the pre-self-heal `.git` presence read on a CONFIGURED
532+
// pair fails at the transport level, setup must refuse — proceeding would
533+
// let the self-heal mint a marker-only repo that masks the reconstruction
534+
// the pair may need, and (on the tokenless recall) would falsely report
535+
// "already-configured". Both entry shapes are covered: rotation-with-token
536+
// and tokenless recall.
537+
func TestGitPushSetupContainer_PresenceProbeError_FailsClosed(t *testing.T) {
538+
t.Parallel()
539+
for _, tc := range []struct {
540+
name string
541+
gitToken string
542+
}{
543+
{"rotation with token", "ghp_rotated_token"},
544+
{"tokenless recall", ""},
545+
} {
546+
t.Run(tc.name, func(t *testing.T) {
547+
t.Parallel()
548+
stateDir := t.TempDir()
549+
if err := workflow.WriteServiceMeta(stateDir, &workflow.ServiceMeta{
550+
Hostname: "appdev",
551+
Mode: topology.PlanModeStandard,
552+
StageHostname: "appstage",
553+
GitPushState: topology.GitPushConfigured,
554+
RemoteURL: "https://github.com/example/app.git",
555+
BootstrapSession: "test",
556+
BootstrappedAt: "2026-05-23",
557+
}); err != nil {
558+
t.Fatalf("WriteServiceMeta: %v", err)
559+
}
560+
561+
ssh := &containerSSHStub{
562+
dispatch: func(cmd string) ([]byte, error) {
563+
if strings.Contains(cmd, "test -d /var/www/.git") {
564+
return nil, errors.New("ssh transport lost")
565+
}
566+
return []byte("ok"), nil
567+
},
568+
}
569+
client := platform.NewMock().
570+
WithServices([]platform.ServiceStack{{ID: "svc-appdev", Name: "appdev"}})
571+
572+
result, _, _ := handleGitPushSetup(
573+
context.Background(), client, nil, ssh, "test-project",
574+
WorkflowInput{Service: "appdev", RemoteURL: "https://github.com/example/app.git", GitToken: tc.gitToken},
575+
stateDir, runtime.Info{InContainer: true},
576+
)
577+
if !result.IsError {
578+
t.Fatalf("presence-probe transport error must fail closed, got success: %s", extractText(result))
579+
}
580+
body := extractText(result)
581+
if !strings.Contains(body, "NO remote ref, secret, origin, or meta state was modified") {
582+
t.Errorf("fail-closed refusal should carry the no-mutation guarantee; got: %s", body)
583+
}
584+
// Nothing after the failed probe may have mutated the repo: no
585+
// self-heal chain, no reconstruction, no origin sync.
586+
for _, cmd := range ssh.commands {
587+
for _, forbidden := range []string{"git init", "commit-tree", "git remote add origin", "set-url origin"} {
588+
if strings.Contains(cmd, forbidden) {
589+
t.Errorf("command ran despite unknown presence (%q): %s", forbidden, cmd)
590+
}
591+
}
592+
}
593+
})
594+
}
595+
}
596+
530597
// TestGitPushSetupContainer_TokenNeverEchoed pins that the token value
531598
// never appears in any response, regardless of probe outcome. Sentinel
532599
// scan against the full JSON body.

plans/git-contract-2026-07-12.md

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,11 @@ lint-local`.
199199
Codex-verified: works for fine-grained PATs without extra permissions).
200200
HTTP via the EXISTING `ops.HTTPDoer` seam already threaded through
201201
`RegisterWorkflow` (Codex finding 5) — no new package-global client, no
202-
parallel test hook. ONLY for remotes whose host is `github.com`
203-
(`parseGitHost` is the owner); other hosts skip derivation (P1 robot
202+
parallel test hook. ONLY for remotes whose host is `github.com` — via
203+
the fail-closed `ops.IsGitHubRemote` (final-pass correction: the
204+
original `parseGitHost` reuse was fail-open — invalid hosts default to
205+
github.com for credential scoping, which must never authorize sending
206+
the PAT to GitHub's API); other hosts skip derivation (P1 robot
204207
fallback stands).
205208
2. `confirmGitPushSetupContainer` (workflow_git_push_setup.go): after probe
206209
passes, best-effort seed via SSH: write user.name/user.email IFF current
@@ -297,10 +300,20 @@ On eval-zcp (services provisioned ad-hoc, deleted after):
297300
`stash create` + `archive` mechanism carries the dirty file on the real
298301
container; B13 class (history without identity) self-heals and
299302
`stash create` succeeds.
300-
- Item for P2 delivery end-to-end: `TestE2E_Deploy_SelfDeploy` PASS (123s)
301-
— import → code written via SSH (uncommitted tree) → real
302-
`zerops_deploy` through the new command → build → HTTP 200 → deleted.
303-
Content shipped from a dirty tree with zero ZCP commits.
303+
- P2 delivery end-to-end, TWO live proofs (final-pass correction: the
304+
first ledger draft over-credited `TestE2E_Deploy_SelfDeploy`, whose
305+
fixture commits before deploying — Codex caught it):
306+
- `TestE2E_Deploy_SelfDeploy` PASS (123s) — the standard committed-tree
307+
path through the new command: build → HTTP 200 → deleted.
308+
- Throwaway `TestE2E_GitDirtyTreeSelfDeploy` PASS (214s; file deleted
309+
after run) — the P2 crown jewel AND the item-5 `-g` round-trip in one:
310+
service imported directly (bootstrap never ran ⇒ NO `.git` at all),
311+
app written via SSH, ZERO git commands issued by the test, real
312+
`ops.DeploySSH` → safety-net built repo+identity+HEAD cold →
313+
`ops.PollBuild` → ACTIVE. Post-deploy on the replaced container:
314+
`count=1`, `msg=zcp init`, `email=agent@zerops.io` (identity survived
315+
the `-g` artifact round-trip), `dirty=2` (user's work honestly
316+
uncommitted), `served=yes` (the uncommitted content IS the artifact).
304317
- `TestE2E_InitServiceGit` PASS against the live container (new semantics).
305318
- Item 6 (P4 GitHub round-trip: real PAT → GET /user → seeded identity →
306319
push attribution visible on GitHub) NOT live-verified — requires a PAT +

0 commit comments

Comments
 (0)