Skip to content

Commit 92c9d6d

Browse files
eduralphclaude
andcommitted
fix(publish): own-repo base remote + stash the dirty target tree (#83)
Two publish preconditions were unmet on a normal own-repo cycle, so it failed until the operator did manual git surgery (hit contributing into Wyrd). 1. Hardcoded `upstream` remote. The new-PR path branched off a remote literally named `upstream` (`fetch upstream`, `checkout -B … upstream/<base>`). A fork has upstream=canonical / origin=fork, but contribution_model=own-repo (#75) has only origin, so publish died on the fetch. Now `[publisher].base_remote` (default upstream; rendered `origin` for own-repo, `upstream` for fork) drives both, and _check_repo requires only the remotes the path actually uses (base + push), so own-repo needs no second remote. The stack path requires only the brief's remote. 2. Clean-tree requirement vs. Do/Check dirtying it. publish re-applies the fix from patch.diff onto a fresh branch (it doesn't use the working tree), but `checkout -B` + the old _check_repo clean-guard aborted on a dirty tree — and Do edits the target in place + repo-scoped gates run against the working tree, so the tree is always dirty at publish, with nothing reverting it. publish now STASHES the tree (incl. untracked) before checkout/apply and RESTORES it after / on failure (both the new-PR and stack paths), so edit-in-place and a clean publish checkout coexist. Tests: base_remote configurable (own-repo branches off origin, no upstream assumed); a real-git publish over a DIRTY checkout succeeds and restores the operator's edits + untracked file on the original branch, with the fix branch pushed. Suite green (145). Closes #83. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Eduard Ralph <15236434+eduralph@users.noreply.github.com>
1 parent 508e7b2 commit 92c9d6d

4 files changed

Lines changed: 142 additions & 30 deletions

File tree

template/pdca.toml.jinja

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ issue_trailer = "Fixes #{id}"
6565
[publisher]
6666
fix_branch_pattern = "fix/{id}-{slug}"
6767
feature_branch_pattern = "enhancement/{id}-{slug}"
68+
# The remote the new-PR path branches the fix off of (issue #83). A fork branches off
69+
# `upstream` (canonical) and pushes to `origin` (the fork); an own-repo target has only
70+
# `origin`. Defaulted from contribution_model; change it if your remotes differ.
71+
base_remote = "{{ 'upstream' if contribution_model == 'fork' else 'origin' }}"
6872

6973
[publisher.checkouts]
7074
# "<org/repo>" = "../<checkout-dir>" # only the exceptions to the sibling default

template/src/pdca_harness/config.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ class Config:
8282
# Branch patterns are .format(id=, slug=) strings; issue_trailer is .format(id=).
8383
fix_branch_pattern: str = "fix/{id}-{slug}"
8484
feature_branch_pattern: str = "enhancement/{id}-{slug}"
85+
# The remote the new-PR path branches the fix off of (issue #83). A fork branches off
86+
# ``upstream`` (the canonical repo); an own-repo target has only ``origin``. The
87+
# rendered pdca.toml sets it per contribution_model; default ``upstream`` preserves the
88+
# prior fork behavior for a config lacking the key.
89+
base_remote: str = "upstream"
8590
issue_trailer: str = "Fixes #{id}" # commit/PR trailer; "" → none enforced
8691
repo_checkouts: dict[str, str] = field(default_factory=dict) # repo_spec → local path
8792
gates_checks: list[dict] = field(default_factory=list)
@@ -199,6 +204,7 @@ def leaf(name: str) -> LeafConfig:
199204
notes_cmd=tracker.get("notes_cmd", ""),
200205
fix_branch_pattern=publisher_cfg.get("fix_branch_pattern", "fix/{id}-{slug}"),
201206
feature_branch_pattern=publisher_cfg.get("feature_branch_pattern", "enhancement/{id}-{slug}"),
207+
base_remote=publisher_cfg.get("base_remote", "upstream"),
202208
issue_trailer=tracker.get("issue_trailer", "Fixes #{id}"),
203209
repo_checkouts=dict(publisher_cfg.get("checkouts", {})),
204210
gate_target_default=gates.get("target_default", ""),

template/src/pdca_harness/publish.py

Lines changed: 79 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,10 @@ def publish(
129129
repo = _checkout_path(cfg, repo_spec)
130130

131131
git = lambda *a: ["git", "-C", str(repo), *a]
132+
base_remote = cfg.base_remote
132133
steps = [
133-
git("fetch", "upstream"),
134-
git("checkout", "-B", branch, f"upstream/{base}"),
134+
git("fetch", base_remote),
135+
git("checkout", "-B", branch, f"{base_remote}/{base}"),
135136
git("apply", str((d / "patch.diff").resolve())),
136137
# `commit -a` stages only modified-tracked files and would silently drop the
137138
# patch's NEW files (the regression test — the most important file in a fix
@@ -155,22 +156,31 @@ def publish(
155156

156157
if dry_run:
157158
print(f"publish --dry-run — {d.name} → draft PR on {repo_spec} ({branch}{base}):")
159+
print(f" # stash the target working tree (Do/Check leave it dirty), restore it after")
158160
for c in steps + ([pr_cmd] if open_pr else []):
159161
print(" " + " ".join(shlex.quote(x) for x in c))
160162
return 0
161163

162-
# Real run: mutate the local checkout — guard it is present and clean first.
163-
rc = _check_repo(repo, repo_spec)
164+
# Real run: the checkout must exist with the base + push remotes. Do/Check edit the
165+
# target in place, so the tree is normally dirty — stash it (publish re-applies the
166+
# fix from patch.diff onto a fresh branch, it doesn't use the working tree) and
167+
# restore it afterward, so edit-in-place and a clean publish checkout coexist (#83).
168+
rc = _check_repo(repo, repo_spec, required_remotes={base_remote, "origin"})
164169
if rc != 0:
165170
return rc
166171

167-
for c in steps:
168-
print("→ " + " ".join(c[3:])) # drop the `git -C <repo>` prefix in the echo
169-
if subprocess.run(c).returncode != 0:
170-
hint = " (patch may not apply against upstream/%s — rebase the fix)" % base \
171-
if c[3] == "apply" else ""
172-
print(f"publish: step failed: {' '.join(c)}{hint}", file=sys.stderr)
173-
return 1
172+
orig_ref = _current_ref(repo)
173+
stashed = _stash_worktree(repo)
174+
try:
175+
for c in steps:
176+
print("→ " + " ".join(c[3:])) # drop the `git -C <repo>` prefix in the echo
177+
if subprocess.run(c).returncode != 0:
178+
hint = " (patch may not apply against %s/%s — rebase the fix)" % (base_remote, base) \
179+
if c[3] == "apply" else ""
180+
print(f"publish: step failed: {' '.join(c)}{hint}", file=sys.stderr)
181+
return 1
182+
finally:
183+
_restore_worktree(repo, orig_ref, stashed)
174184

175185
pr_url = ""
176186
pr_failed = False
@@ -250,13 +260,14 @@ def _publish_stacked(
250260
if dry_run:
251261
print(f"publish --dry-run — {d.name} → commit stacked onto {repo_spec} "
252262
f"PR branch {branch} (base {base_ref}):")
263+
print(f" # stash the target working tree (Do/Check leave it dirty), restore it after")
253264
for c in steps:
254265
print(" " + " ".join(shlex.quote(x) for x in c))
255266
print(" " + " ".join(shlex.quote(x) for x in pr_list)
256267
+ " # resolve the existing open PR (no new PR is created)")
257268
return 0
258269

259-
rc = _check_repo(repo, repo_spec)
270+
rc = _check_repo(repo, repo_spec, required_remotes={remote})
260271
if rc != 0:
261272
return rc
262273

@@ -268,15 +279,21 @@ def _publish_stacked(
268279
"'Onto branch' brief field to use the default new-PR flow.", file=sys.stderr)
269280
return 1
270281

271-
for c in steps:
272-
print("→ " + " ".join(c[3:])) # drop the `git -C <repo>` prefix in the echo
273-
if subprocess.run(c).returncode != 0:
274-
hint = ""
275-
if c[3:5] == ["apply", "--check"]:
276-
hint = (f" — the patch no longer applies to {base_ref} (it advanced since "
277-
"the fix was built and tested; rebuild/re-Check against the PR branch)")
278-
print(f"publish: step failed: {' '.join(c)}{hint}", file=sys.stderr)
279-
return 1
282+
# Stash the (Do/Check-dirtied) tree so checkout -B + apply run clean; restore after (#83).
283+
orig_ref = _current_ref(repo)
284+
stashed = _stash_worktree(repo)
285+
try:
286+
for c in steps:
287+
print("→ " + " ".join(c[3:])) # drop the `git -C <repo>` prefix in the echo
288+
if subprocess.run(c).returncode != 0:
289+
hint = ""
290+
if c[3:5] == ["apply", "--check"]:
291+
hint = (f" — the patch no longer applies to {base_ref} (it advanced since "
292+
"the fix was built and tested; rebuild/re-Check against the PR branch)")
293+
print(f"publish: step failed: {' '.join(c)}{hint}", file=sys.stderr)
294+
return 1
295+
finally:
296+
_restore_worktree(repo, orig_ref, stashed)
280297

281298
(d / "publish.json").write_text(json.dumps({
282299
"mode": "stacked",
@@ -396,28 +413,60 @@ def _t4_passes(cfg: Config, d: Path) -> bool:
396413
return True
397414

398415

399-
def _check_repo(repo: Path, repo_spec: str) -> int:
400-
"""The local checkout must exist, be clean, and have upstream + origin remotes."""
416+
def _check_repo(repo: Path, repo_spec: str, required_remotes=("upstream", "origin")) -> int:
417+
"""The local checkout must exist and have the remotes this publish path needs.
418+
419+
A dirty tree is NOT a failure (issue #83): Do/Check edit the target in place, so the
420+
tree is normally dirty at publish time — :func:`_stash_worktree` cleans it for the
421+
checkout and :func:`_restore_worktree` puts it back. ``required_remotes`` is the set
422+
this path actually uses (base + push), so own-repo (no ``upstream``) is accepted.
423+
"""
401424
hint = (f"create/clone the checkout for '{repo_spec}' at {repo} "
402425
"(or set [publisher.checkouts] in pdca.toml if it lives elsewhere)")
403426
if not (repo / ".git").exists():
404427
print(f"publish: checkout not found: {repo}{hint}", file=sys.stderr)
405428
return 1
406-
porcelain = subprocess.run(["git", "-C", str(repo), "status", "--porcelain"],
407-
capture_output=True, text=True).stdout.strip()
408-
if porcelain:
409-
print(f"publish: {repo} has uncommitted changes — clean it first:\n{porcelain}",
410-
file=sys.stderr)
411-
return 1
412429
remotes = subprocess.run(["git", "-C", str(repo), "remote"],
413430
capture_output=True, text=True).stdout.split()
414-
for r in ("upstream", "origin"):
431+
for r in required_remotes:
415432
if r not in remotes:
416433
print(f"publish: {repo} has no '{r}' remote — {hint}", file=sys.stderr)
417434
return 1
418435
return 0
419436

420437

438+
def _current_ref(repo: Path) -> str:
439+
"""The checkout's current branch (or commit SHA if detached) — to return to after publish."""
440+
r = subprocess.run(["git", "-C", str(repo), "symbolic-ref", "--quiet", "--short", "HEAD"],
441+
capture_output=True, text=True)
442+
ref = r.stdout.strip()
443+
if ref:
444+
return ref
445+
return subprocess.run(["git", "-C", str(repo), "rev-parse", "HEAD"],
446+
capture_output=True, text=True).stdout.strip()
447+
448+
449+
def _stash_worktree(repo: Path) -> bool:
450+
"""Stash the target's dirty tree (incl. untracked) so ``checkout -B`` + ``apply`` run on
451+
a clean base; return True iff something was stashed (the caller restores it). Publish
452+
re-applies the fix from ``patch.diff``, so the working-tree edits are not needed here."""
453+
dirty = bool(subprocess.run(["git", "-C", str(repo), "status", "--porcelain"],
454+
capture_output=True, text=True).stdout.strip())
455+
if dirty:
456+
subprocess.run(["git", "-C", str(repo), "stash", "push", "--include-untracked",
457+
"-m", "pdca-publish"], capture_output=True, text=True)
458+
return dirty
459+
460+
461+
def _restore_worktree(repo: Path, orig_ref: str, stashed: bool) -> None:
462+
"""Return the checkout to where publish found it: back on ``orig_ref`` with the stashed
463+
edits popped — so Do/Check's edit-in-place survives a publish. Best-effort."""
464+
subprocess.run(["git", "-C", str(repo), "checkout", "--quiet", orig_ref],
465+
capture_output=True, text=True)
466+
if stashed:
467+
subprocess.run(["git", "-C", str(repo), "stash", "pop"], capture_output=True, text=True)
468+
469+
421470
_BY_RE = re.compile(r"^- By / date:\s*(.+?)\s*/", re.MULTILINE)
422471

423472

template/tests/test_publish_slice.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,59 @@ def test_commit_is_signed_off_both_paths(self) -> None:
155155
self.assertIn("commit -s -F", out, f"{iid}: commit not signed off")
156156
self.assertNotIn("commit -F", out) # the unsigned form is gone
157157

158+
def test_base_remote_is_configurable(self) -> None:
159+
# Own-repo (#83): branch the fix off `origin` (no `upstream` remote needed).
160+
self.cfg.base_remote = "origin"
161+
_bundle(self.cfg, "OWN", brief_body=_FIX_BRIEF, accepted=True)
162+
buf = io.StringIO()
163+
with redirect_stdout(buf):
164+
publish.publish(self.cfg, "OWN", dry_run=True)
165+
out = buf.getvalue()
166+
self.assertIn("fetch origin", out)
167+
self.assertIn("checkout -B fix/OWN-my-fix origin/main", out)
168+
self.assertNotIn("upstream", out) # no upstream remote assumed
169+
170+
def test_publish_succeeds_with_dirty_target_tree(self) -> None:
171+
# Own-repo, dirty tree (#83): Do/Check edit the target in place, so publish must
172+
# stash → publish off a clean checkout → restore, not abort on the dirty tree.
173+
import subprocess as sp
174+
repo = self.tmp / "checkout"
175+
origin = self.tmp / "origin.git"
176+
sp.run(["git", "init", "-q", "--bare", str(origin)], check=True)
177+
sp.run(["git", "clone", "-q", str(origin), str(repo)], check=True)
178+
run = lambda *a: sp.run(["git", "-C", str(repo), *a], check=True, capture_output=True)
179+
run("config", "user.email", "t@example.com")
180+
run("config", "user.name", "T")
181+
run("config", "commit.gpgsign", "false")
182+
(repo / "file.txt").write_text("base\n", encoding="utf-8")
183+
run("add", "-A"); run("commit", "-q", "-m", "base")
184+
run("branch", "-M", "main"); run("push", "-q", "-u", "origin", "main")
185+
# The builder edits in place + leaves an untracked file (the dirty cycle state).
186+
(repo / "file.txt").write_text("base\nbuilder edit\n", encoding="utf-8")
187+
(repo / "untracked.txt").write_text("u\n", encoding="utf-8")
188+
189+
self.cfg.base_remote = "origin"
190+
self.cfg.repo_checkouts = {"example-org/example-repo": str(repo)}
191+
d = _bundle(self.cfg, "DIRTY", brief_body=_FIX_BRIEF, accepted=True)
192+
(d / "patch.diff").write_text(
193+
"diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n"
194+
"@@ -1 +1,2 @@\n base\n+fix line\n", encoding="utf-8")
195+
196+
buf = io.StringIO()
197+
with redirect_stdout(buf), redirect_stderr(buf):
198+
rc = publish.publish(self.cfg, "DIRTY", open_pr=False, by="T", today="2026-06-05")
199+
self.assertEqual(rc, 0, buf.getvalue())
200+
# The operator's dirty edits are restored — edit-in-place survives publish.
201+
self.assertEqual((repo / "file.txt").read_text(encoding="utf-8"), "base\nbuilder edit\n")
202+
self.assertTrue((repo / "untracked.txt").exists())
203+
cur = sp.run(["git", "-C", str(repo), "branch", "--show-current"],
204+
capture_output=True, text=True).stdout.strip()
205+
self.assertEqual(cur, "main") # back on the original branch
206+
# The fix branch was pushed to origin.
207+
refs = sp.run(["git", "-C", str(repo), "ls-remote", "--heads", "origin"],
208+
capture_output=True, text=True).stdout
209+
self.assertIn("fix/DIRTY-my-fix", refs)
210+
158211
def test_pr_head_is_fork_owner_qualified(self) -> None:
159212
"""Regression (#23b): a fork-based PR's --head must be OWNER:BRANCH, else gh
160213
resolves the branch against the base repo and fails 'Head ref must be a

0 commit comments

Comments
 (0)