Make df12-build-odw dryRun avoid real worktree creation (#31)

[pandalump]

Summary

A dry run against a repository with surviving roadmap-* branches failed on a
branch collision: runTask called createWorktree unconditionally, and
createWorktree runs git worktree add -b roadmap-... origin/<base> with no
DRY_RUN guard. A dry run should be a safe, read-only validation path, so it
must never mutate git state.

Changes

• Short-circuit DRY_RUN at the head of runTask (in run-task.ts), ahead of
  worktree creation and the write-access preflight, returning a terminal
  dry-run status with stage pre-worktree. The single check precedes the
  addendum/normal branch, covering both lanes.
• Remove the two now-unreachable post-worktree dry-run short-circuits (the
  addendum-lane and the post-design blocks).
• Leave the recovery/continue lane's separate dry-run handling (over
  pre-existing worktrees) untouched.
• Add tests/df12-build-odw-dry-run.test.mjs: drives runTask with
  dryRun: true against a fixture repo whose roadmap-* branches would
  collide, and asserts an unchanged repo-state snapshot (no new branch, no new
  worktree) and that no agent was dispatched.
• Align docs/architecture.md and docs/users-guide.md with the new
  stop-before-worktree semantics.
• Regenerate workflows/df12-build-odw.js from source via make workflow-build.

Follows the CodeRabbit implementation plan (guard at the top of runTask,
Option 2). Deviation: the CodeRabbit result shape included proposals: []; the
plan text omitted only worktree/plan/branch, which are omitted here.

Review-feedback follow-up

Addresses the three review warnings on the initial fix:

• Observability — the DRY_RUN early return emits a structured, parseable
  log() at the decision boundary carrying all four fields: task id, lane
  (derived from task.isAddendum), stage=pre-worktree, and reason=dry-run.
  The returned result object's shape (status, stage, detail, proposals)
  is unchanged. The log sink is injectable in the test harness, so the
  regression suite asserts the emitted fields for both the normal and addendum
  lanes.
• Property/proof coverage — a dedicated fast-check property
  (tests/df12-build-odw-dry-run.property.test.mjs, 100 iterations) sweeps
  arbitrary task ids (fully arbitrary strings, roadmap-shaped dotted ids, and
  branch-colliding ids), both lanes, and pooled repo states, asserting a dry
  run always returns status=dry-run / stage=pre-worktree, dispatches no
  agent, and leaves the durable git-state snapshot byte-for-byte unchanged. The
  runTask harness is extracted into tests/fixtures/load-run-task.mjs and
  shared with the regression suite.
• Developer documentation — docs/developers-guide.md (ODW workflow
  contract) documents the pre-worktree boundary, records that the earlier
  addendum-specific and post-design dry-run exits were removed as unreachable,
  and cross-references the architecture Enforcement boundary table and the
  users' guide Workflow arguments section so all three docs agree. No tracked
  roadmap/execplan work-item exists for Make df12-build-odw dryRun avoid real worktree creation #31, so none required a status update.

Gates pass: make check-fmt, make lint, make typecheck, make test,
make markdownlint, and make nixie are all green.

Closes #31

References

• Lody session: https://lody.ai/leynos/sessions/af7396d9-56f1-453d-8abf-9e274ad85737

[sourcery-ai]

Sorry @pandalump, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Walkthrough

Dry-run handling in runTask (TypeScript) and runTask2 (JS workflow) is refactored so DRY_RUN/DRY_RUN2 short-circuits immediately before worktree creation, replacing prior stage-specific returns. A regression test verifies no git state mutation, and documentation is updated to match.

Changes

Dry-run pre-worktree short-circuit

Layer / File(s)	Summary
Early DRY_RUN exit in runTask src/workflows/df12-build-odw/run-task.ts	A top-of-run DRY_RUN check now returns a dry-run/pre-worktree result before createWorktree runs; the earlier addendum-specific and post-design dry-run returns are removed.
Mirrored DRY_RUN2 exit in runTask2 workflows/df12-build-odw.js	runTask2 gains the equivalent pre-worktree DRY_RUN2 short-circuit; the addendum-only and post-design DRY_RUN2 early returns are deleted.
Regression test for dry-run posture tests/df12-build-odw-dry-run.test.mjs	A new test loads runTask from the workflow source and asserts status: "dry-run", stage: "pre-worktree", no worktree/branch/plan, no agent dispatch, and an unchanged repository snapshot including refs and worktrees.
Dry-run documentation refresh docs/architecture.md, docs/users-guide.md	The enforcement-boundary table and the dryRun argument description are rewritten to state that dry-run halts before worktree creation with no writes and no agent dispatch, while resume/continue keeps separate handling for existing worktrees.

Possibly related PRs

• leynos/df12-build#13: Both PRs modify DRY_RUN/dryRun handling in workflows/df12-build-odw.js's core execution path.

Suggested labels: Issue

Poem

A rabbit dug fast, then stopped at the gate,
No worktree, no branch — a dry run, first-rate.
The docs now confess where the shortcut lies,
No agents dispatched, no premature ties.
🐇 Hop back before mud, and land clean and neat.

🚥 Pre-merge checks | ✅ 17 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Developer Documentation	⚠️ Warning	docs/architecture.md and docs/users-guide.md were updated, but docs/developers-guide.md was not, so the new pre-worktree dry-run boundary remains undocumented there.	Add a developers-guide note for the df12-build-odw dry-run boundary and any related workflow contract changes; then update any tracked roadmap/execplan item if this work is recorded there.
Testing (Property / Proof)	⚠️ Warning	The change introduces a universal no-mutation invariant, but the PR adds only one concrete regression test and no property-based or exhaustive proof coverage.	Add a fast-check property over task IDs and repo states asserting dryRun always returns pre-worktree and never creates refs/worktrees, or justify a rigorous exhaustive proof.
Observability	⚠️ Warning	The new dry-run short-circuit returns silently; no log/metric/tracing at the new decision boundary, so operators only see a final status, not why work stopped.	Emit a structured log on the DRY_RUN early return with task id, lane, stage=pre-worktree, and reason; keep the summary/result fields unchanged.

✅ Passed checks (17 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes short-circuit dry-run before worktree creation, so no worktree or branch is mutated as required by #31.
Out of Scope Changes check	✅ Passed	The test, docs, and regenerated workflow file all support the dry-run worktree fix and stay within scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Testing (Overall)	✅ Passed	Accept it: the new test drives real runTask against a colliding fixture repo, checks status/stage, zero agent calls, and a full pre/post git-state snapshot.
User-Facing Documentation	✅ Passed	PASS: docs/users-guide.md now explains dryRun stops before worktree creation and notes separate recovery/continue handling, matching the behaviour change.
Module-Level Documentation	✅ Passed	All changed code modules open with top-level comments explaining purpose and relationships: run-task.ts, the dry-run test, and the generated workflow file.
Testing (Unit And Behavioural)	✅ Passed	The new test hits the real workflow against a throwaway git repo, checks no agent dispatch, and asserts the before/after repo snapshot stays unchanged.
Testing (Compile-Time / Ui)	✅ Passed	PASS: The new node:test regression uses a stable repo-state snapshot plus explicit branch/worktree assertions, which is the right semantic coverage here.
Unit Architecture	✅ Passed	Dry-run now exits before worktree creation and write preflight; the test verifies no durable git state changes and no agent dispatch.
Domain Architecture	✅ Passed	PASS: The dry-run guard lives in workflow orchestration, not domain code; shared types stay domain-shaped while git worktree concerns remain in adapters.
Security And Privacy	✅ Passed	PASS: The dry-run guard stops before git/worktree mutation; the new test uses only fake df12-test@example.invalid Git identities and no secrets appear in changed files.
Performance And Resource Use	✅ Passed	PASS: The change moves DRY_RUN ahead of worktree creation, removing git fetch/worktree/probe I/O on the hot path; no new unbounded loops, caches, or allocations were added.
Concurrency And State	✅ Passed	PASS: the change only moves an early dry-run return ahead of worktree creation; it adds no shared mutable state, locks, or background tasks, and the test asserts no git mutation.
Architectural Complexity And Maintainability	✅ Passed	PASS: The PR adds a single early DRY_RUN guard and docs/test updates only; no new layers, registries, or indirection were introduced.
Rust Compiler Lint Integrity	✅ Passed	HEAD touches only docs, TypeScript, JavaScript, and a test file; no Rust or Cargo files changed, so no lint-suppression or clone regression is present.
Title check	✅ Passed	The title clearly matches the PR’s dry-run worktree fix and includes the linked issue number.
Description check	✅ Passed	The description directly describes the same dry-run worktree changes, tests, and docs updates in the changeset.

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #31

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-31-dryrun-no-worktree

---

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[leynos]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[leynos]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[leynos]

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope).

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Developer Documentation	⚠️ Warning	docs/architecture.md and docs/users-guide.md were updated, but docs/developers-guide.md was not, so the new pre-worktree dry-run boundary remains undocumented there.	Add a developers-guide note for the df12-build-odw dry-run boundary and any related workflow contract changes; then update any tracked roadmap/execplan item if this work is recorded there.
Testing (Property / Proof)	⚠️ Warning	The change introduces a universal no-mutation invariant, but the PR adds only one concrete regression test and no property-based or exhaustive proof coverage.	Add a fast-check property over task IDs and repo states asserting dryRun always returns pre-worktree and never creates refs/worktrees, or justify a rigorous exhaustive proof.
Observability	⚠️ Warning	The new dry-run short-circuit returns silently; no log/metric/tracing at the new decision boundary, so operators only see a final status, not why work stopped.	Emit a structured log on the DRY_RUN early return with task id, lane, stage=pre-worktree, and reason; keep the summary/result fields unchanged.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 48 --json headRefName,commits -q '.headRefName' 2>/dev/null || true
gh pr diff 48 --name-only 2>/dev/null || true
echo "---developers-guide---"
fd -i developers-guide
echo "---search dry-run in developers-guide---"
rg -n -i 'dry.?run' docs/developers-guide.md || true
echo "---search for fast-check---"
rg -n 'fast-check' -g '*.json' -g '*.mjs' -g '*.ts' || true
echo "---search property test files---"
fd -i 'property' 
echo "---search structured log near DRY_RUN in run-task.ts---"
rg -n 'DRY_RUN' -A5 -B5 src/workflows/df12-build-odw/run-task.ts
echo "---workflows file---"
rg -n 'DRY_RUN2' -A8 -B5 workflows/df12-build-odw.js

Length of output: 2622

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "---developers-guide---"
fd -i developers-guide
echo "---search dry-run in developers-guide---"
rg -n -i 'dry.?run' docs/developers-guide.md || true
echo "---search for fast-check usage---"
rg -n 'fast-check' -g '*.json' -g '*.mjs' -g '*.ts' || true
echo "---search property test files---"
fd -i 'property'
echo "---search structured log near DRY_RUN in run-task.ts---"
rg -n 'DRY_RUN' -A6 -B6 src/workflows/df12-build-odw/run-task.ts
echo "---workflows file---"
rg -n 'DRY_RUN2' -A8 -B5 workflows/df12-build-odw.js
echo "---package.json deps---"
rg -n 'fast-check' package.json || true

Length of output: 2573

---

@leynos, none of the three warnings are resolved yet. Verify against the current diff, since I could not clone the repository (authentication failure), but the file list in the PR's AI-generated summary confirms only these five files changed: docs/architecture.md, docs/users-guide.md, src/workflows/df12-build-odw/run-task.ts, tests/df12-build-odw-dry-run.test.mjs, and workflows/df12-build-odw.js. docs/developers-guide.md is absent, the added test is a single concrete regression case with no property-based coverage, and the new DRY_RUN short-circuit in run-task.ts has no structured logging.

Treat all three as in-scope, per your instruction. No new GitHub issue is warranted since this work belongs in the current PR.

1. Developer Documentation — outstanding

Add a docs/developers-guide.md note describing the new pre-worktree dry-run boundary and update any tracked roadmap/execplan entry.

Update docs/developers-guide.md in leynos/df12-build to document the new dry-run short-circuit in `runTask` (src/workflows/df12-build-odw/run-task.ts). Add a section explaining that `DRY_RUN=true` now returns a terminal `{ status: "dry-run", stage: "pre-worktree" }` result immediately after task-log output, before `createWorktree` or the writable-root preflight execute. State explicitly that no branch, worktree, or probe writes occur, and that the addendum-specific and post-design dry-run exits were removed because they are now unreachable. Cross-reference docs/architecture.md's "Enforcement boundary" table and docs/users-guide.md's "Workflow arguments" section so all three documents describe the same boundary consistently. If the project tracks an execplan or roadmap item for issue `#31`, update its status to reflect this change.

2. Testing (Property / Proof) — outstanding

Add a fast-check property test asserting the no-mutation invariant holds across arbitrary task IDs and repo states.

Add fast-check as a dependency if not already present, then create a property-based test alongside tests/df12-build-odw-dry-run.test.mjs (e.g. tests/df12-build-odw-dry-run.property.test.mjs) in leynos/df12-build. Use the existing `loadRunTask` helper pattern from tests/df12-build-odw-dry-run.test.mjs to load `runTask` from workflows/df12-build-odw.js. Write a fast-check property that generates arbitrary task IDs (strings) and arbitrary minimal repo/lane states (e.g. addendum vs. normal lane, varying branch names), invokes `runTask` with `dryRun: true` for each generated input, and asserts: (a) the returned status is always `"dry-run"`, (b) the stage is always `"pre-worktree"`, (c) no new git refs or worktrees are created (reuse the before/after repo-state snapshot comparison already used in the regression test), and (d) the agent stub is never invoked. Run with a reasonable number of iterations (e.g. 100) to keep CI runtime bounded. If exhaustive proof is preferred over property testing, justify that choice explicitly in a code comment and link to the rationale in the PR description instead.

3. Observability — outstanding

Emit a structured log at the new DRY_RUN early-return in runTask.

In src/workflows/df12-build-odw/run-task.ts (and the regenerated workflows/df12-build-odw.js), add a structured log call immediately before the new `DRY_RUN` early return in `runTask`. Use whatever structured logging utility this codebase already uses elsewhere in run-task.ts (search for existing logger calls, e.g. `console.log`/`logger.info` with structured fields, and match that pattern). Include fields: task id, lane (e.g. addendum vs. normal), stage=`"pre-worktree"`, and reason=`"dry-run"`. Do not alter the returned result object's shape (status, stage, detail, proposals) — only add the log side-effect. After editing run-task.ts, regenerate workflows/df12-build-odw.js per the project's existing build/regeneration process so both files stay in sync. Update tests/df12-build-odw-dry-run.test.mjs to assert the log was emitted with the expected fields, if the logger is mockable/injectable; otherwise add a note explaining why it isn't testable in the current harness.

---

✏️ Learnings added

Learnt from: leynos
URL: https://github.com/leynos/df12-build/pull/48

Timestamp: 2026-07-07T22:10:19.712Z
Learning: For leynos/df12-build PR reviews, three CI-derived warning categories recur on df12-build-odw changes: (1) Developer Documentation — docs/developers-guide.md must be updated alongside docs/architecture.md and docs/users-guide.md whenever workflow contract behaviour changes; (2) Testing (Property/Proof) — the project expects fast-check property-based tests (or an explicit rigorous-proof justification) for universal invariants, not just single regression tests; (3) Observability — new early-return/short-circuit decision boundaries in workflow code (e.g. src/workflows/df12-build-odw/run-task.ts and its regenerated counterpart workflows/df12-build-odw.js) are expected to emit structured logs with task id, lane, stage, and reason fields.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.