enhancement: preflight workspace update roots

[codeforester]

Summary

• preflight manifest repository roots before pulling
• report dirty, non-default, detached, non-tracking, non-Git, and linked-worktree roots with recovery guidance
• preserve the no-implicit-mutation boundary and expose machine-readable preflight reasons in JSON
• document the behavior in workspace help and manifest documentation

Review follow-up

• resolve a repository's default branch from the remote with git ls-remote --symref instead of requiring a local remote-HEAD ref
• keep preflight skips for optional manifest repositories non-fatal
• run the same read-only preflight during --dry-run, so unsafe roots are shown as skipped rather than planned
• share the Git subprocess environment setup between pull and preflight probes

Validation

• 23 passed, 9 subtests in cli/python/base_projects/tests/test_workspace_update.py and the published schema test
• 15/15 workspace BATS tests
• git diff --check
• targeted Pylint checks passed for workspace_update.py
• the full Python gate reached 1670 passed with one unrelated existing failure in base_config lifecycle log creation (test_show_config_uses_base_cli_lifecycle)

The full bin/base-test gate reached all 1001 BATS cases but has unrelated existing failures in branch-stale, inspection JSON, repo installer/update, version/prompt, and base-test environment fixtures. No workspace-update test failed.

Fixes #2336

[codeforester]

Correctness: default-branch detection relies on a local ref that is often unset, causing false "unknown_default_branch" skips

workspace_update_remote_default_branch() resolves the manifest's default branch via git symbolic-ref --quiet --short refs/remotes/origin/HEAD when repos[].default_branch isn't configured (the new field, so no existing manifest has it set yet). That local ref is frequently absent — repos checked out via actions/checkout, mirrors, manual git init && git remote add && git fetch, or clones predating a default-branch rename never get it populated. In that case unknown_default_branch" is added to issues, and preflight_workspace_update_targetskips the checkout as fatal — even though the checkout is clean and correctly tracking its upstream andgit pull --ff-only` would have worked fine before this PR.

The codebase already has a more robust pattern for this in cli/python/base_release/release_readiness.py:187 (remote_default_branch), which asks the remote directly via git ls-remote --symref origin HEAD instead of depending on local ref-tracking state. Reusing/adapting that approach here would avoid the false-positive skip.

Failure scenario: a manifest repo cloned via CI checkout tooling (no local origin/HEAD), with no default_branch set in the manifest (the common case immediately after this PR ships), clean working tree, valid upstream tracking — workspace update now reports it skipped/fatal with unknown_default_branch instead of pulling it, a regression from pre-PR behavior.

[codeforester]

Correctness: preflight failures are always fatal=True, ignoring target.required

target = replace(target, action="skip", fatal=True) hardcodes fatal=True for any preflight issue (dirty, non-default-branch, detached HEAD, linked worktree, etc.), regardless of whether the repo is required. Compare to workspace_update_manifest_target (line ~306), which correctly sets fatal=repo.required when a repo is simply missing.

The effect: an optional (required: false) manifest repo that happens to be dirty or checked out on a feature branch — a very plausible state for a repo someone is actively working in — now makes the entire workspace update invocation report overall failure (counts.failed incremented, non-zero exit code), even though the exact same repo being entirely absent would correctly be treated as non-fatal.

Suggested fix: fatal=target.required here, matching the missing-repo precedent, so preflight-skips are only fatal for required repos.

Failure scenario: manifest lists an optional scratch/experimental repo the developer is mid-edit in (dirty tree). basectl workspace update (e.g. run from CI or a pre-flight script) now exits non-zero solely because of that optional repo's local dirtiness, even though every required repo updated cleanly.

[codeforester]

Correctness/docs: --dry-run never runs preflight, so the plan can misreport repos that would actually be skipped

preflight_workspace_update_target(target) is only called in the non-dry-run else branch below. When dry_run is true, every present repo is unconditionally reported as WorkspaceUpdateResult("planned"), with no dirty/branch/upstream/worktree check at all.

This means basectl workspace update --dry-run will show PLANNED for a repo that is actually dirty, on a non-default branch, or a linked worktree — and a real (non-dry-run) run immediately afterward will instead skip/fail that same repo. The updated docs (docs/workspace-manifest.md) say preflight happens "before pulling" but don't call out that dry-run bypasses it, so the dry-run preview is not a reliable preview of the real run's outcome. None of the existing dry-run tests (e.g. test_workspace_update_dry_run_preserves_order_and_includes_active_base, the JSON dry-run test, or tests/contracts/test_workspace_update_schema.py) exercise a repo with a preflight issue, so this gap is untested.

Failure scenario: a repo has uncommitted local changes. workspace update --dry-run reports it planned (implying it will update fine), but the follow-up real workspace update skips it as dirty — the dry-run gave a false all-clear.

[codeforester]

Reuse/simplification: git-subprocess env setup is duplicated

run_workspace_git_probe rebuilds env = os.environ.copy(); env["GIT_TERMINAL_PROMPT"] = "0"; env["LC_ALL"] = "C" — the exact same three lines already exist in execute_workspace_update_target (lines ~364-366). Worth factoring into a single workspace_update_git_env()" helper (or having execute_workspace_update_targetcallrun_workspace_git_probe`-style plumbing) so the two subprocess call sites stay in sync if this env ever needs a third variable added.

[codeforester]

Follow-up on the previous review comments

Verified the fix commit against the actual test suite (21 passed, 3 subtests, 0 failed). Three of the four findings are solidly fixed and regression-tested:

• Preflight-skip now respects target.required - confirmed by test_workspace_update_optional_preflight_skip_is_nonfatal, which asserts an optional dirty repo does not fail the overall run.
• --dry-run now runs preflight first - confirmed by test_workspace_update_dry_run_reports_preflight_diagnostics, which explicitly asserts planned=0 for a dirty repo and that git pull is never invoked.
• The duplicated env-setup block is consolidated into workspace_update_git_environment().

The default-branch detection fix (switching to git ls-remote --symref <remote> HEAD) is correct and wired up properly, but its test coverage is lighter than the other three fixes:

• run_workspace_git_probe is fully mocked in the existing test (test_workspace_update_preflight_allows_clean_default_branch_tracking), so nothing actually exercises a real repo lacking a local refs/remotes/origin/HEAD - the exact real-world scenario this fix targets.
• There's no standalone unit test for parse_workspace_remote_default_branch covering edge cases (empty output, ambiguous refs, malformed lines). Its sibling function parse_remote_default_branch in cli/python/base_release/release_readiness.py has exactly this kind of dedicated test (test_remote_default_branch_rejects_ambiguous_or_incomplete_responses in cli/python/base_release/tests/test_engine.py) - worth mirroring here for consistency.

Not a blocker - the logic is correct - but a good follow-up to close the gap.

Posted via Claude Code

[codeforester]

Follow-up complete on commit c565a72. The new remote-default-branch tests are isolated in their own test class so the repository-wide Pylint public-method limit remains satisfied. Local validation: 23 focused tests passed, 9 subtests passed, full Pylint passed, and git diff --check passed.