name: core-internals description: Provides guidance and best practices on concurrent synchronization, IPC, filesystem atomicity, git subprocess wrapping, XML manifests, and CLI argument parsing for git-repo.

Git Repo Core Internals Engineering Guide

Executive Summary

Welcome to the authoritative engineering guide for Git Repository Management within the git-repo codebase. This repository of folk knowledge exists to safeguard the intricate orchestration of our multi-repository synchronization toolchain. Historically, complex operations such as parallelized network fetches, layered manifest overrides, and interactive terminal rendering have been vulnerable to subtle concurrency races, deadlocks, and fragile system states. This guide captures these critical failure modes—ranging from IPC serialization bottlenecks and filesystem lock contentions to non-hermetic test pollution—and establishes rigid engineering constraints to prevent their regression.

To maintain system stability, this guide enforces strict architectural boundaries across the tooling ecosystem. It mandates stateless execution for multiprocessing pools, guaranteed atomicity for worktree layout modifications, and deterministic error translation for all standard Git subprocesses. Furthermore, it defines the standard operating procedures for canonical manifest object deduplication, the modernization of our hermetic testing frameworks, and the delivery of consistent, machine-readable CLI interfaces. By adhering to these paradigms, incoming engineers will ensure the reliability, performance, and extensibility of git-repo's core repository management infrastructure.

Summary

Chapter Theme / Title	Scope & Objective
Concurrent Synchronization & IPC	This domain governs the stable
: : orchestration of parallel network :
: : fetches, local checkouts, and :
: : interleaved subprocess routines. It :
: : strictly enforces safe :
: : multiprocessing IPC, deterministic :
: : Git locking via exponential backoff, :
: : and state synchronization across :
: : concurrent pool workers. :
**Filesystem Atomicity & Worktree	This domain governs the deterministic
: Layout** : creation, migration, and cleanup of :
: : internal repository structures and :
: : Git worktrees. It relies on ephemeral :
: : temporary directories, atomic rename :
: : operations, and robust error recovery :
: : to prevent corrupted states during :
: : unexpected interruptions. :
**Subprocess Git Integration & Error	This chapter defines the constraints
: Translation** : for wrapping, executing, and :
: : translating standard Git subprocesses :
: : within the Repo tooling ecosystem. It :
: : mandates the use of centralized :
: : command abstractions, strict version :
: : gating, and deterministic stream :
: : handling to guarantee reliable :
: : repository state management and :
: : actionable error reporting. :
**Manifest Object Model &	This chapter governs the parsing,
: Deduplication** : validation, and canonicalization of :
: : XML manifest components. It strictly :
: : enforces semantic immutability via :
: : NamedTuple implementations, defensive :
: : copying for hierarchical override :
: : scoping, and deterministic :
: : JSON-backed file tracking across the :
: : subsystem. :
**Hermetic Testing & Test	This domain governs the migration of
: Modernization** : legacy unittest suites to modern :
: : pytest functional paradigms and the :
: : establishment of hermetic session :
: : fixtures. It enforces strict :
: : environment isolation to prevent :
: : global state pollution (e.g., :
: : developer .gitconfig bleeding) while :
: : safely intercepting standard streams :
: : and filesystem paths. :
**CLI Argument Parsing & UX	This chapter governs the lifecycle,
: Consistency** : validation, and execution of :
: : command-line arguments, enforcing :
: : strict standardization for :
: : machine-readable serialization, :
: : unified logging, and deterministic, :
: : thread-safe terminal interactions. :
Repo Hooks Framework	The Repo Hooks Framework governs the
: : execution, parameter validation, and :
: : lifecycle management of user-defined :
: : scripts within the repository :
: : ecosystem. It ensures seamless :
: : integration of extensions like :
: : post-sync or pre-upload while :
: : strictly isolating their execution :
: : failures from core operational :
: : workflows. :

Chapter: Concurrent Synchronization & IPC

Context: This domain governs the stable orchestration of parallel network fetches, local checkouts, and interleaved subprocess routines. It strictly enforces safe multiprocessing IPC, deterministic Git locking via exponential backoff, and state synchronization across concurrent pool workers.

Summary

Rule ID	Principle / Constraint	Priority	Primary Symptom / Trap
T1-01	Stateless Class Methods	High	Passing bound instance
: : for Parallel Pool Workers : : methods to a parallel :
: : : : executor, dragging :
: : : : unnecessary state into :
: : : : the multiprocessing :
: : : : serialization pipeline. :
T1-02	Buffered Serialization of	High	Executing terminal print
: : Concurrent Standard : : calls directly from :
: : Output : : within a parallelized :
: : : : worker routine. :
T1-03	Interleaved Sync Path	High	Dispatching parallel
: : Validation : : checkout jobs across all :
: : : : project variants without :
: : : : verifying layout types. :
T1-04	Guaranteed	Critical	Placing save operations
: : Synchronization State : : at the very end of an :
: : Persistence : : execution path without :
: : : : exception guards. :
T1-05	Jittered Exponential	Critical	Firing `git submodule
: : Backoff for Git : : init` concurrently :
: : Configuration Locks : : without retry logic for :
: : : : `config.lock` failures. :
T1-06	Exponential Backoff for	High	Failing a git command
: : Concurrent Git Config : : immediately without :
: : Locks : : evaluating standard :
: : : : streams for transient :
: : : : lock errors. :
T1-07	Hard Termination on	Critical	Logging an error state
: : Interleaved Sync Stalls : : inside a `while` loop but :
: : : : allowing the next loop :
: : : : iteration to execute. :
T1-08	Resource Management via	High	Initializing proxy
: : Context Handlers for IPC : : connections or :
: : : : multiprocessing managers :
: : : : as raw variable :
: : : : assignments without a :
: : : : guaranteed teardown :
: : : : phase. :
T1-09	Dual-Channel Error	High	Iterating over aggregated
: : Handling in Parallel : : data lists to decide if a :
: : Processing : : parallel orchestration :
: : : : should abort. :
T1-10	Minimum Process Pool Job	Critical	Directly using
: : Count Safeguard : : `min(target, len(items))` :
: : : : to determine pool size, :
: : : : which breaks if the item :
: : : : list is unexpectedly :
: : : : empty. :
T1-11	Safeguarding Object State	Critical	Truncating object
: : Across Parallel Execution : : payloads to scalar :
: : Boundaries : : indices during IPC :
: : : : context setup without :
: : : : updating the receiver :
: : : : logic to rehydrate the :
: : : : objects. :
T1-12	Explicit Context	High	Assuming child workers
: : Initialization for : : inherit updated class :
: : Multiprocessing Pools : : variables inherently :
: : : : without explicit :
: : : : initialization. :
T1-13	Dynamic Task Chunk Sizing	Medium	Passing a hardcoded batch
: : in Parallel Execution : : integer to the :
: : : : `chunksize` parameter. :
T1-14	Deferred Worktree	High	Returning early from the
: : Operations in Sync Local : : sync phase without :
: : Half : : applying required file :
: : : : operations. :

Rules

T1-01: Stateless Class Methods for Parallel Pool Workers

Rule: Always implement multiprocessing pool targets as stateless class methods or standalone functions to bypass process serialization constraints.
What: Worker execution targets within multiprocessing pools must be constructed as fully decoupled class methods or standalone functions to bypass process serialization constraints.
Applies To: Multiprocessing process pools, concurrent task scheduling, and ExecuteInParallel implementations.
Why: Command execution objects were heavily bound to system state. Attempting to pass instance methods (self.method) to a multiprocessing pool frequently caused PicklingError crashes, as the underlying Python serialization mechanism cannot cleanly isolate bound object graphs. Failing to adhere to this typically results in IPC Serialization Error.

Trap 1: Passing bound instance methods to a parallel executor, dragging unnecessary state into the multiprocessing serialization pipeline.

Don't:

class InfoCommand:
    def _worker_logic(self, project):
        pass

    def run(self):
        self.ExecuteInParallel(jobs, self._worker_logic, projects)

Do:

class InfoCommand:
    @classmethod
    def _worker_logic(cls, project_idx):
        project = cls.get_parallel_context()["projects"][project_idx]
        pass

    def run(self):
        self.ExecuteInParallel(jobs, self._worker_logic, range(len(projects)))

Exceptions: Threading-based execution models where memory is shared and pickling is not strictly enforced.

T1-02: Buffered Serialization of Concurrent Standard Output

Rule: Must capture standard output generated by concurrently executing tasks into isolated memory buffers for sequential display in the parent process.
What: Data emitted by concurrently executing tasks must be captured into isolated memory buffers and returned to the parent process for sequential display.
Applies To: Parallel process execution layers generating human-readable CLI output.
Why: When command execution was parallelized, worker processes wrote directly to standard output. This created severe race conditions resulting in garbled, interleaved text output on the user's terminal. Failing to adhere to this typically results in Interleaved Terminal Output.

Trap 1: Executing terminal print calls directly from within a parallelized worker routine.

Don't:

@classmethod
def _DiffHelper(cls, project):
    print(f"Project: {project.name}")
    print(f"Revision: {project.rev}")

Do:

@classmethod
def _DiffHelper(cls, project):
    buf = io.StringIO()
    buf.write(f"Project: {project.name}\n")
    buf.write(f"Revision: {project.rev}\n")
    return buf.getvalue()

# In main process:
for output in results:
    print(output, end="")

T1-03: Interleaved Sync Path Validation

Rule: Always validate the presence of a worktree when executing interleaved sync processes to prevent layout evaluation crashes.
What: When executing interleaved sync processes (parallelized network fetches and checkouts), the operation must explicitly validate the presence of a worktree, as not all Git repository types (e.g., mirrors) maintain local file checkouts.
Applies To: Concurrency logic within sync.py.
Why: Changing the default mode of repo sync to interleaved parallelized checkout tasks indiscriminately. This immediately broke AOSP mirror syncing because mirrors lack local checkout paths, resulting in a TypeError when evaluating NoneType paths. Failing to adhere to this typically results in TypeError / Sync Failure.

Trap 1: Dispatching parallel checkout jobs across all project variants without verifying layout types.

Don't:

Assuming project.worktree always contains an os.PathLike object during interleaved syncing.

Do:

Adding guard clauses to verify project.worktree is not None and handling --mirror modes explicitly before adding checkout tasks to the thread pool.

T1-04: Guaranteed Synchronization State Persistence

Rule: Must execute synchronization telemetry saving operations within try...finally blocks to guarantee data retention across execution interrupts.
What: Core sync operations must wrap network and filesystem operations in try...finally blocks, ensuring that synchronization metadata (_fetch_times and _local_sync_state) is persisted even if the sync operation fails or is interrupted.
Applies To: subcmds/sync.py, particularly the _Fetch logic and multiprocessing worker loops.
Why: If a synchronization process encountered an error (like a fetch failure), the operation exited early, dropping valuable telemetry and optimization data (fetch times) for projects that successfully synced prior to the crash. Failing to adhere to this typically results in Telemetry Loss / Sync State Inconsistency.

Trap 1: Placing save operations at the very end of an execution path without exception guards.

Don't:

result = self._Fetch(to_fetch, opt, err_event)
if not result.success:
    raise SyncError("failed")

# BAD: Never reached if an error is raised
self._fetch_times.Save()

Do:

try:
    result = self._Fetch(to_fetch, opt, err_event)
    if not result.success:
        raise SyncError("failed")
finally:
    # GOOD: Always saves telemetry state
    self._fetch_times.Save()

T1-05: Jittered Exponential Backoff for Git Configuration Locks

Rule: Always wrap concurrent Git mutations to shared configurations in an exponential backoff routine with jitter to mitigate transient filesystem locks.
What: When initializing submodules concurrently (e.g., git submodule init), the system must employ an exponential backoff mechanism with randomized jitter to handle transient filesystem lock errors on .git/config.
Applies To: Submodule initialization and any highly parallel Git subprocesses mutating shared configuration state.
Why: Running repo sync -j<N> caused multiple child processes to simultaneously attempt git submodule init, which led to lock contention on the parent project's config file and caused the entire sync operation to fail. Failing to adhere to this typically results in File Lock Contention / Sync Failure.

Trap 1: Firing git submodule init concurrently without retry logic for config.lock failures.

Don't:

# BAD: Fails immediately if .git/config.lock exists
subprocess.run(["git", "submodule", "init", "--", path], check=True)

Do:

# GOOD: Catch lock errors and retry with jitter
for attempt in range(MAX_RETRIES):
    p = subprocess.run(["git", "submodule", "init", "--", path], stderr=subprocess.PIPE)
    if p.returncode == 0:
        break
    if "could not lock config file" in p.stderr:
        time.sleep(base_delay * (2 ** attempt) + random.uniform(0, jitter))

Exceptions: Non-lock related Git errors should still fail immediately without retrying.

T1-06: Exponential Backoff for Concurrent Git Config Locks

Rule: Must analyze captured Git error streams to detect config lock contention and orchestrate retries via structured logging instead of immediate process failure.
What: Concurrent git operations modifying .git/config (such as submodule initialization) must utilize an exponential retry mechanism with jitter to handle transient filesystem lock contention. Standard output and error streams must be captured and parsed to detect these locks.
Applies To: project.py, specifically concurrent git submodule init operations or any parallel processes altering local repository configuration.
Why: When synchronizing with high job values (-j), multiple parallel processes modifying .git/config triggered race conditions, throwing ‘could not lock config file’ errors and aborting the sync. Failing to adhere to this typically results in Transient Sync Failure / Lock Contention.

Trap 1: Failing a git command immediately without evaluating standard streams for transient lock errors.

Don't:

# BAD: Fails immediately on lock contention
if GitCommand(self, cmd).Wait() != 0:
    raise GitError(f"{self.name} submodule init failed")

Do:

# GOOD: Captures stdout/stderr and retries on 'lock' matches
git_cmd = GitCommand(None, cmd, capture_stdout=True, capture_stderr=True)
if git_cmd.Wait() != 0:
    error = git_cmd.stderr or git_cmd.stdout
    if "lock" in error:
        # apply exponential backoff + jitter logic
    else:
        git_cmd.VerifyCommand()  # Propagate real error

Trap 2: Using raw print statements to log retry attempts, which breaks structured output formats.

Don't:

# BAD: Unstructured print to stdout
print(f"Attempt {attempt+1}: git {' '.join(cmd)} failed. Sleeping...")

Do:

# GOOD: Forwarded to centralized logger
logger.warning("Attempt %d/%d: git %s failed. Error: %s. Sleeping %.2fs before retrying.", attempt+1, max_retries, cmd, error, delay)

Exceptions: Non-lock related Git errors should break the retry loop and be propagated immediately via git_cmd.VerifyCommand().

T1-07: Hard Termination on Interleaved Sync Stalls

Rule: Must trigger a definitive execution break and flag error signals whenever an unresolvable stall is detected in an interleaved control loop.
What: When an unresolvable stall is detected within an interleaved or parallel execution loop, the system must trigger a definitive state break (e.g., setting global events and breaking the loop) to prevent infinite deadlocks.
Applies To: subcmds/sync.py, specifically _SyncInterleaved or any looping logic monitoring pending sets.
Why: A bug existed where a stall was logged but the loop terminator (break) and error trigger (err_event.set()) were accidentally omitted. This allowed the sync routine to lock up infinitely when interdependent projects failed to checkout. Failing to adhere to this typically results in Infinite Loop / Deadlock.

Trap 1: Logging an error state inside a while loop but allowing the next loop iteration to execute.

Don't:

# BAD: Omitted termination control flow
if prev_pending == pending_relpaths:
    logger.error("Stall detected")
prev_pending = pending_relpaths

Do:

# GOOD: Set event signals and immediately break
if prev_pending == pending_relpaths:
    logger.error("Stall detected")
    err_event.set()
    break

T1-08: Resource Management via Context Handlers for IPC

Rule: Always initialize multiprocessing components and background proxies within with statement context blocks to assure teardown semantics.
What: Multiprocessing managers and SSH proxies initialized for parallel workflows must be wrapped in with block context managers to guarantee cleanup of background processes and multiplexing sockets.
Applies To: Concurrent network operations and parallelized repository synchronization (subcmds/sync.py).
Why: When adding an interleaved fetch/checkout feature, parallel operations required shared synchronization dictionaries and SSH connection multiplexing. Without context managers, exceptions or interrupts (e.g., KeyboardInterrupt) would leave orphaned background processes and abandoned sockets. Failing to adhere to this typically results in Resource Leaks / Orphaned Sockets.

Trap 1: Initializing proxy connections or multiprocessing managers as raw variable assignments without a guaranteed teardown phase.

Don't:

# BAD: No guaranteed cleanup on exception
manager = multiprocessing.Manager()
ssh_proxy = ssh.ProxyManager(manager)
process_tasks(ssh_proxy)

Do:

# GOOD: Context managers assure RAII semantics
with multiprocessing.Manager() as manager:
    with ssh.ProxyManager(manager) as ssh_proxy:
        process_tasks(ssh_proxy)

T1-09: Dual-Channel Error Handling in Parallel Processing

Rule: Must decouple fast-fail signaling events from aggregated batch data processing logic to ensure immediate worker termination across processes.
What: In concurrent execution environments, inter-process signaling (for immediate control flow) must be strictly separated from data aggregation (for end-of-batch error reporting).
Applies To: Multi-process synchronization pools (_SyncInterleaved, _SyncPhased) and worker callback processors.
Why: Reviewers questioned why a separate event flag was used when an array of errors was already being populated. Using only an aggregated list prevented workers from rapidly terminating each other via fail-fast mechanisms. Failing to adhere to this typically results in Delayed Worker Termination.

Trap 1: Iterating over aggregated data lists to decide if a parallel orchestration should abort.

Don't:

# BAD: Waiting to process the whole batch before aborting
errors.append(result.error)
if errors and opt.fail_fast:
    pool.close()

Do:

# GOOD: Using an IPC Event for immediate cross-process signaling
err_event.set()
errors.append(result.error)
if not ret and opt.fail_fast:
    pool.close()

T1-10: Minimum Process Pool Job Count Safeguard

Rule: Must bind dynamically computed multiprocessing job allocations to a minimum floor of 1 to prevent initialization crashes.
What: When determining the number of worker processes for parallel execution based on a variable target length (e.g., an array of projects), the resulting worker count must be explicitly bound to a minimum of 1.
Applies To: Network fetch logic and multiprocessing pool initializations (e.g., _Fetch in sync.py).
Why: A regression occurred where passing an empty project list evaluated the worker count to 0. This crashed the multiprocessing pool initialization with a ValueError. Returning early was rejected as a fix because secondary side-effects (like state saves) within the function still needed to execute. Failing to adhere to this typically results in ValueError / Process Pool Crash.

Trap 1: Directly using min(target, len(items)) to determine pool size, which breaks if the item list is unexpectedly empty.

Don't:

jobs = min(opt.jobs_network, len(projects_list))

Do:

jobs = max(1, min(opt.jobs_network, len(projects_list)))

T1-11: Safeguarding Object State Across Parallel Execution Boundaries

Rule: Always transmit deeply serialized objects directly across parallel boundaries, or strictly reconstruct object relationships within the downstream worker.
What: When offloading command execution to parallel processes, workers must receive properly serialized complex object data, not dissociated identifiers, unless the downstream API explicitly reconstructs the object state.
Applies To: Parallel execution contexts (ParallelContext, ExecuteInParallel) within multi-project commands like repo forall and repo upload.
Why: A refactoring attempt to optimize IPC by passing integer project indices instead of full project objects to parallel workers caused critical breakage. Worker processes subsequently attempted to access attributes (like .manifest) on integer types, leading to unhandled exceptions. Failing to adhere to this typically results in AttributeError / Early Exit Regression.

Trap 1: Truncating object payloads to scalar indices during IPC context setup without updating the receiver logic to rehydrate the objects.

Don't:

# Worker receives an integer index instead of a Project
manifests = {
    project.manifest.topdir: project.manifest
}

Do:

# Worker receives fully serialized Project objects (or properly reconstructs them)
manifests = {
    project.manifest.topdir: project.manifest
}

T1-12: Explicit Context Initialization for Multiprocessing Pools

Rule: Must dictate shared states through an explicit initializer argument injected into the multiprocessing pool rather than implicitly relying on execution fork behavior.
What: Shared parallel context must be managed using an explicit context manager and passed via initializer and initargs in multiprocessing.Pool, rather than relying on unvalidated class attributes or fork memory semantics.
Applies To: Any command or module utilizing multiprocessing.Pool (e.g., ExecuteInParallel).
Why: Relying on fork memory semantics for sharing context works on some Linux systems but fails deterministically on environments (like macOS/Windows) where memory is not automatically shared or when the multiprocessing start method defaults to spawn. Failing to adhere to this typically results in State Leakage / Uninitialized Context.

Trap 1: Assuming child workers inherit updated class variables inherently without explicit initialization.

Don't:

# BAD: Implicit state sharing
cls.parallel_context = data
with multiprocessing.Pool(jobs) as pool:
    pool.imap_unordered(func, inputs)

Do:

# GOOD: Explicit initializer passed to workers
with multiprocessing.Pool(
    jobs,
    initializer=cls._SetParallelContext,
    initargs=(cls._parallel_context,)
) as pool:
    pool.imap_unordered(func, inputs)

Trap 2: A context manager directly yielding an internal dictionary, allowing unvalidated and lingering usage.

Don't:

# BAD: Direct yield of state without lifecycle validation
@contextlib.contextmanager
def ParallelContext(cls):
    yield cls._parallel_context

Do:

# GOOD: Lifecycle management and defensive assertions
@contextlib.contextmanager
def ParallelContext(cls):
    assert cls._parallel_context is None
    cls._parallel_context = {}
    try:
        yield
    finally:
        cls._parallel_context = None

T1-13: Dynamic Task Chunk Sizing in Parallel Execution

Rule: Always formulate the chunksize of mapping workers dynamically based on the dataset to job ratio to avoid workload serialization blocks.
What: When utilizing multiprocessing.Pool.imap_unordered, the chunksize must be dynamically calculated based on the ratio of inputs to worker jobs, rather than statically hardcoded.
Applies To: Parallel task dispatch mechanisms (ExecuteInParallel, synchronization loops).
Why: Using a statically hardcoded chunk size (e.g., WORKER_BATCH_SIZE = 32) forced all tasks onto a single worker thread when the total number of projects in the manifest was fewer than 32, completely negating the benefit of parallelism for smaller workloads. Failing to adhere to this typically results in Thread Serialization / Under-utilization.

Trap 1: Passing a hardcoded batch integer to the chunksize parameter.

Don't:

# BAD: Hardcoded chunksize leads to serialization for small input lists
submit(func, inputs, chunksize=WORKER_BATCH_SIZE)

Do:

# GOOD: Dynamic chunking balances load across workers
calc_chunk = min(max(1, len(inputs) // jobs), WORKER_BATCH_SIZE)
submit(func, inputs, chunksize=calc_chunk)

Exceptions: When evaluating specifically within tests that forcefully isolate to a single job execution (jobs=1).

T1-14: Deferred Worktree Operations in Sync Local Half

Rule: Must enqueue manifest copy and link actions into the synchronization buffer explicitly prior to issuing early return signals for up-to-date repositories.
What: File link (linkfile) and copy (copyfile) directives must be scheduled using the synchronization buffer (syncbuf.later1) within the Sync_LocalHalf execution phase. They must not be bypassed if the repository exits its sync early due to being up-to-date.
Applies To: Local synchronization logic of projects (project.py -> Sync_LocalHalf).
Why: A bug existed where if a project had published commits in Gerrit (meaning its local state was merged), the sync process would return early and entirely skip applying the manifest‘s linkfile and copyfile operations, leading to missing or stale files in the developer’s worktree. Failing to adhere to this typically results in Worktree Desync / Missing Files.

Trap 1: Returning early from the sync phase without applying required file operations.

Don't:

# BAD: Skipped file modifications when commits are published
if pub == head:
    return

Do:

# GOOD: Schedule file operations via syncbuf before returning
if pub == head:
    syncbuf.later1(self, _doff, not verbose)
    return

Cross-Domain Dependencies

Upstream: T3 | Subprocess Git Integration & Error Translation - Standardizes Git command stream capturing and exception typing, enabling reliable lock contention detection.
Downstream: T2 | Filesystem Atomicity & Worktree Layout - Relies on parallel execution guard clauses to ensure worktrees exist before checkout operations are dispatched.
Downstream: T6 | CLI Argument Parsing & UX Consistency - Requires buffered standard outputs and structured logger directives from parallel tasks to maintain coherent terminal formatting.

Chapter: Filesystem Atomicity & Worktree Layout

Context: This domain governs the deterministic creation, migration, and cleanup of internal repository structures and Git worktrees. It relies on ephemeral temporary directories, atomic rename operations, and robust error recovery to prevent corrupted states during unexpected interruptions.

Summary

Rules

T2-01: Unique Subproject Keys in Worktree Environments

Rule: Always use repository-relative paths instead of git directory names as unique deduplication keys to prevent collisions in worktree environments.
What: Repository-relative paths must be used instead of git directory names as unique deduplication keys to prevent collisions in environments utilizing git worktrees.
Applies To: Subproject iteration and deduplication logic, primarily within command.py or project discovery utilities.
Why: When tracking derived subprojects, the system originally used gitdir as a unique identifier. In git worktree setups, multiple distinct project instances can share the same underlying gitdir, leading to silent data collisions and missed projects during sync operations. Failing to adhere to this typically results in Data Collision / Omission.

Trap 1: Using the git directory or repository name as a unique hash key for deduplicating project objects.

Don't:

derived_projects.update(
    (p.gitdir, p) for p in project.GetDerivedSubprojects()
)

Do:

derived_projects.update(
    (p.RelPath(local=False), p) for p in project.GetDerivedSubprojects()
)

T2-02: Non-Destructive Bottom-Up Directory Pruning

Rule: Always utilize non-destructive, bottom-up directory iteration that exclusively targets empty directories and symlinks during legacy path cleanup.
What: Legacy path cleanup must utilize non-destructive, bottom-up directory iteration that specifically targets empty directories and symlinks, avoiding recursive tree destruction.
Applies To: Workspace migration, obsolete symlink cleanup, and platform_utils file deletion wrappers.
Why: When a subproject's destination directory changed, the synchronization tool utilized aggressive recursive deletion (rmtree) to remove the previous location. This routinely endangered untracked developer code or extraneous files housed inside the obsolete tree. Failing to adhere to this typically results in Accidental Data Deletion.

Trap 1: Using aggressive recursive delete calls to wipe an old link destination without verifying its internal contents.

Don't:

if platform_utils.isdir(absDest):
    platform_utils.rmtree(absDest)

Do:

# Use a safe helper that only removes empty dirs/symlinks bottom-up
platform_utils.removedirs(absDest)

Trap 2: Attempting directory deletion without explicitly pruning stale manifest files first.

Don't:

platform_utils.removedirs(need_remove_path)
# Misses files entirely as removedirs stops at regular files

Do:

if os.path.isfile(need_remove_path) or os.path.islink(need_remove_path):
    platform_utils.remove(need_remove_path)
platform_utils.removedirs(os.path.dirname(need_remove_path))

Exceptions: Explicitly defined cases where a directory is formally tracked and deleted from the manifest metadata, explicitly commanding removal.

T2-03: Guaranteed Cleanup of Temporary Git Artifacts

Rule: Must wrap any operation mutating the local .git repository with temporary states inside a try/finally block to guarantee restoration.
What: Operations mutating the local .git repository with temporary states (e.g., tracking alt refs during fetch) must wrap the operation in a try/finally block to guarantee state restoration.
Applies To: Subprocess network operations and filesystem interactions affecting the .git database.
Why: If a network operation threw an exception (like a GitAuthError prompting for credentials), the cleanup logic was bypassed. This left orphaned references in the object database and corrupted future repository syncs. Failing to adhere to this typically results in Stale Repository State.

Trap 1: Running destructive or state-modifying operations without wrapping the cleanup routine in a finally block.

Don't:

setup_temporary_refs()
run_git_fetch() # Might throw GitAuthError
cleanup_temporary_refs()

Do:

setup_temporary_refs()
try:
    run_git_fetch()
finally:
    cleanup_temporary_refs()

T2-04: Atomic Directory Initialization via Temporary Worktrees

Rule: Always perform directory initialization via temporary directories and an atomic rename to prevent fragmented repository states.
What: Directory initialization processes must use temporary directories and atomic renames to prevent partial or corrupted states upon unexpected interruption.
Applies To: Filesystem operations involving git repository setup, specifically superproject initialization and dot-git dir creation.
Why: Historically, initializing git directories directly in their final path left behind broken, partially-initialized structures if the user or system interrupted the process. This caused subsequent workflow executions to fail permanently. Failing to adhere to this typically results in Corrupt State / Initialization Failure.

Trap 1: Initializing a complex directory structure directly at its final target path without atomicity guarantees.

Don't:

os.makedirs(final_git_dir, exist_ok=True)
subprocess.run(['git', 'init', final_git_dir])

Do:

temp_dir = tempfile.mkdtemp(dir=target_base)
try:
    subprocess.run(['git', 'init', temp_dir])
    platform_utils.rename(temp_dir, final_git_dir)
finally:
    platform_utils.rmtree(temp_dir, ignore_errors=True)

T2-05: Pathlib Adoption over os.path

Rule: Avoid legacy os.path APIs; prefer modern pathlib.Path structures for traversing, joining, and creating files.
What: Use modern Python pathlib.Path paradigms for traversing, joining, and creating files instead of the legacy os.path APIs.
Applies To: Filesystem and worktree directory manipulation.
Why: Chaining os.path.join and calling os.mkdir repeatedly bloated the codebase, was less readable, and invited platform separator path bugs. Failing to adhere to this typically results in Pathing Errors / High Complexity.

Trap 1: Relying on heavily nested string-based os.path constructions.

Don't:

manifest_dir = os.path.join(self.repodir, 'manifests')
os.mkdir(manifest_dir)
with open(os.path.join(manifest_dir, 'config'), 'w') as fp:
    fp.write(data)

Do:

manifest_dir = self.repodir / 'manifests'
manifest_dir.mkdir(parents=True, exist_ok=True)
(manifest_dir / 'config').write_text(data)

Exceptions: Legacy modules pending modernization where intermingling os.path strings and Path objects would break typing contracts.

T2-06: Atomic Directory Initialization via Temporary Renames

Rule: Never construct complex internal .git directories in-situ; execute initialization inside an ephemeral directory and atomically rename it upon completion.
What: Complex directory structures (like internal .git directories) must be constructed inside an ephemeral temporary directory on the same filesystem volume, and only moved to their final path via an atomic rename once fully validated.
Applies To: Filesystem creation (os.makedirs) and repository initialization (_InitGitDir).
Why: Interrupted or failed operations left partially initialized .git directories in-situ. Follow-up operations encountered corrupt directory trees, which the legacy recovery logic could not reliably clean up. Failing to adhere to this typically results in Corrupt Repository State.

Trap 1: Creating and mutating a system directory at its final, visible destination.

Don't:

os.makedirs(self.gitdir)
self._ReferenceGitDir(self.objdir, self.gitdir)

Do:

tmp_gitdir = create_tmp_dir(os.path.dirname(self.gitdir))
os.makedirs(tmp_gitdir)
self._ReferenceGitDir(self.objdir, tmp_gitdir)
platform_utils.rename(tmp_gitdir, self.gitdir)

T2-07: Encapsulation of Transient Configuration State

Rule: Never reassign class-level instance attributes to intermediate paths during object setup; use local variables for temporary state tracking.
What: Never temporarily reassign class-level instance attributes (e.g., self.config) to point to intermediate or ephemeral paths. Use local variables for temporary objects during setup, assigning the final result to the class instance only upon success.
Applies To: Object initialization workflows within project.py.
Why: During atomic .git directory initialization, self.config was temporarily overwritten to point to a temporary working directory. If the operation crashed, the object was left in an invalid state, pointing to a deleted transient path. Failing to adhere to this typically results in Corrupt Instance State.

Trap 1: Swapping out instance configuration variables back and forth during setup.

Don't:

self.config = GitConfig.ForRepository(gitdir=tmp_gitdir)
# initialization steps
platform_utils.rename(tmp_gitdir, self.gitdir)
self.config = GitConfig.ForRepository(gitdir=self.gitdir)

Do:

tmp_config = GitConfig.ForRepository(gitdir=tmp_gitdir)
# apply setup using tmp_config
platform_utils.rename(tmp_gitdir, self.gitdir)
self.config = GitConfig.ForRepository(gitdir=self.gitdir)

T2-08: Explicit Temporary Resource Ownership and Release

Rule: Always nullify references to a temporary directory immediately after successfully renaming it to its permanent destination.
What: When executing atomic directory initializations via temporary paths, the reference to the temporary directory must be explicitly nullified immediately after successfully renaming it to its final destination.
Applies To: Filesystem operations, specifically in project.py atomic Git directory initializations (_InitGitDir).
Why: If a temporary directory reference was maintained after being atomic-renamed, the subsequent error-handling or finally cleanup block could mistakenly delete the directory. In parallel environments, another concurrent job could validly create a new temporary directory at that exact freed path, causing the current process to inadvertently nuke an active resource belonging to another job. Failing to adhere to this typically results in Data Loss / Race Condition.

Trap 1: Keeping a stale variable reference to a temporary directory after it has been renamed, allowing a deferred cleanup routine to operate on an unowned path.

Don't:

try:
    tmp_gitdir = tempfile.mkdtemp()
    # ... initialize ...
    platform_utils.rename(tmp_gitdir, self.gitdir)
    # BAD: tmp_gitdir still holds the path string
finally:
    if tmp_gitdir and os.path.exists(tmp_gitdir):
        platform_utils.rmtree(tmp_gitdir) # Risk of race condition

Do:

try:
    tmp_gitdir = tempfile.mkdtemp()
    # ... initialize ...
    platform_utils.rename(tmp_gitdir, self.gitdir)
    tmp_gitdir = None  # GOOD: explicitly release ownership of the path
finally:
    if tmp_gitdir and os.path.exists(tmp_gitdir):
        platform_utils.rmtree(tmp_gitdir)

T2-09: Configuration Object Invalidation Post-Rename

Rule: Must discard intermediate configuration instances bound to temporary paths once the underlying directory is atomic-renamed.
What: Configuration instances (e.g., GitConfig) bound to a temporary filesystem path must be abandoned after the path is atomic-renamed. A new instance tracking the permanent directory path must be used for subsequent settings.
Applies To: Git config lifecycle management across atomic filesystem boundary events.
Why: Applying configuration changes like gc.pruneExpire using an object instantiated against a temporary path resulted in IO errors or lost state if the operations occurred after the directory was moved to its final, permanent location. Failing to adhere to this typically results in Lost Configuration / IO Error.

Trap 1: Using a temporary config object to set flags after the underlying directory has been renamed.

Don't:

curr_config = GitConfig.ForRepository(gitdir=tmp_gitdir)
platform_utils.rename(tmp_gitdir, self.gitdir)
curr_config.SetString("gc.pruneExpire", "never")  # BAD: Path no longer valid

Do:

curr_config = GitConfig.ForRepository(gitdir=tmp_gitdir)
platform_utils.rename(tmp_gitdir, self.gitdir)
self.config.SetString("gc.pruneExpire", "never")  # GOOD: Using permanent config tracking self.gitdir

T2-10: Granular Safety Checks for Destructive Worktree Operations

Rule: Must evaluate explicit dependency subsets and prompt overrides before wiping shared object directories.
What: Destructive operations (like wiping directories) must separately evaluate and prompt overrides for distinct risk categories (uncommitted local changes vs. shared object directories), and only delete a shared object directory if all dependent projects are successfully wiped.
Applies To: Commands modifying or deleting project directories and .repo/project-objects.
Why: Deleting shared .repo state indiscriminately could leave other referencing projects broken if they relied on the same object directory, requiring strict subset checks before rmtree execution. Failing to adhere to this typically results in Repository Corruption / Data Loss.

Trap 1: Deleting a shared object directory immediately upon deleting a single project that uses it.

Don't:

# BAD: Deletes shared objdir, breaking other projects
project.DeleteWorktree(force=True)
if os.path.exists(project.objdir):
    platform_utils.rmtree(project.objdir)

Do:

# GOOD: Verify all users of the objdir are deleted first
project.DeleteWorktree(force=True)
successful_wipes.add(project.relpath)

# Later...
if users.issubset(successful_wipes):
    platform_utils.rmtree(objdir)

T2-11: Explicit Warnings for Legacy Garbage Collection Fallbacks

Rule: Must emit a visible warning to sys.stderr when disabling Git garbage collection via legacy fallback configurations.
What: When disabling Git garbage collection for shared network repositories using legacy mechanisms (gc.pruneExpire=never), the tooling must emit a visible sys.stderr warning due to the unreliability of older Git clients.
Applies To: Git configuration manipulation and local filesystem management (_GCProjects).
Why: Older Git clients (pre 2.7.0) do not natively support the extensions.preciousObjects flag. To prevent garbage collection from corrupting shared objects, repo forces gc.pruneExpire to never. Because this is imperfect, it can lead to unreliable repository states, requiring explicit user notification. Failing to adhere to this typically results in Data Loss / Corruption.

Trap 1: Silently executing a legacy fallback for repository safety configurations without informing the user of the potential unreliability.

Don't:

# BAD: Silent fallback to legacy GC directive
if git_require((2, 7, 0)):
    project.config.SetString('extensions.preciousObjects', 'true')
else:
    project.config.SetString('gc.pruneExpire', 'never')

Do:

# GOOD: Alerting the user to the legacy system risk
if git_require((2, 7, 0)):
    project.config.SetString('extensions.preciousObjects', 'true')
else:
    print('WARNING: shared projects are unreliable when using old versions of git...', file=sys.stderr)
    project.config.SetString('gc.pruneExpire', 'never')

T2-12: Atomic Commits for Filesystem Restructuring

Rule: Always strictly isolate structural filesystem modifications and migrations into single atomic commits to maintain bisectability.
What: Refactoring critical filesystem paths (like migrating internal .git submodule structures) must be strictly isolated into atomic commits to preserve bisectability.
Applies To: Worktree layout updates, .git metadata migrations, and submodule structure changes.
Why: A monolithic commit attempted to restructure how submodules were stored (from subprojects/ to modules/), update initialization logic, and alter fetching. This density made code review difficult and created a high risk of regressions that would be impossible to bisect cleanly. Failing to adhere to this typically results in Unbisectable Regressions.

Trap 1: Bundling filesystem path migrations, structural re-designs, and initialization logic changes into a single Pull Request/Patchset.

Don't:

Submit 1 patchset: [Refactor] Move submodules to modules/, update git config flags, and modify sync --init behaviors.

Do:

Submit 3 sequential patchsets: 1) Isolate init flag changes. 2) Migrate filesystem directories from subprojects to modules. 3) Enable Git recurse submodules integration.

T2-13: Precise Symlink Target Replacement During Submodule Migration

Rule: Always use explicit path prefixes instead of generic substring replacements to update symlink targets during legacy layout migrations.
What: Symlink updates during filesystem layout migrations must use exact path prefixes rather than global substring replacements to update relative targets safely.
Applies To: Repository migration logic (project.py); specifically _MigrateSubprojectLinks or similar symlink target modifications.
Why: During the migration of legacy submodule directories, using generic string replacement (target.replace) on symlink paths unintentionally corrupted paths if a user's directory structure legitimately contained the target string (e.g., subproject-objects) in a higher-level parent directory name. Failing to adhere to this typically results in Broken Symlinks / Corruption.

Trap 1: Using a generic string .replace() to update a portion of a file path.

Don't:

if "subproject-objects" in target:
    new_target = target.replace(
        "subproject-objects", "module-objects"
    )

Do:

if target.startswith("../../subproject-objects/"):
    new_target = target.replace(
        "../../subproject-objects/", "../../module-objects/"
    )

T2-14: Isolating Filesystem Migrations from Network-Only Operations

Rule: Never execute internal repository layout migrations as part of network fetch iterations.
What: Internal repository layout migrations must only execute during local worktree initialization phases, never during network fetch iterations.
Applies To: Repository sync operations (subcmds/sync.py) and worktree checkout logic (project.py).
Why: Binding irreversible filesystem layout migrations to the sync command's network-fetch loop risked partially migrating the repository if the process was interrupted, or if a user requested a network-only sync, leaving the local workspace out of sync with internal references. Failing to adhere to this typically results in Repository Corruption.

Trap 1: Executing internal directory migrations globally prior to downloading network changes.

Don't:

# In subcmds/sync.py during network fetching
for p in all_projects:
    p._MigrateOldSubprojectDirs()
    # ... fetch logic

Do:

# Inside project.py during _InitWorkTree() or checkout
def _InitWorkTree(self):
    self._MigrateOldSubmoduleDirs()
    # ... symlink creation and checkout logic

T2-15: Idempotent Cross-Platform File Cleanup

Rule: Must employ platform_utils.remove() for file cleanup to safely absorb OS access violations and gracefully swallow ENOENT.
What: File cleanup operations must use the internal platform_utils.remove() instead of the standard os.remove() to handle Windows-specific OS limitations. Furthermore, cleanup must be idempotent by explicitly catching and swallowing errno.ENOENT errors.
Applies To: Local worktree operations, internal state file cleanup (e.g., deleting obsolete copyfiles or linkfiles).
Why: Sync operations would abruptly crash on Windows due to unhandled EACCES issues on read-only files/symlinks with native os.remove(). Furthermore, if a tracking file was already deleted manually by the user or a prior interrupted run, unhandled ENOENT exceptions would needlessly fail the operation. Failing to adhere to this typically results in Crash on Missing/Locked File.

Trap 1: Relying on standard library file removal and failing to handle the scenario where the file no longer exists.

Don't:

if path:
  try:
    os.remove(path)
  except OSError as error:
    print(f'error: remove {path} failed.')
    return 1

Do:

try:
  platform_utils.remove(path)
except OSError as e:
  if e.errno == errno.ENOENT:
    # File does not exist, safe to ignore
    pass

T2-16: Self-Healing Binary JSON Deserialization

Rule: Always read internal JSON state files in binary mode (rb) and implement explicit error catching to seamlessly discard corrupted states.
What: Internal JSON tracking files must be read in binary mode (rb) to allow the JSON parser to detect text encoding safely. Additionally, parsing must be wrapped in a try/except block to automatically delete the file and recover if the JSON is corrupted.
Applies To: Manifest metadata tracking files, internal cache parsing (e.g., copy-link-files.json).
Why: When the operating system‘s default text encoding did not match the file’s encoding, a UnicodeDecodeError could occur. Additionally, if the JSON state file was corrupted (e.g., due to an abrupt power loss or killed process), repo sync would permanently fail until the user manually discovered and deleted the corrupted state file. Failing to adhere to this typically results in Corrupt State Deadlock.

Trap 1: Opening JSON state files in text mode without validating structure, allowing exceptions to bubble up and break the CLI.

Don't:

with open(copylinkfiles_path, 'r') as fd:
  old_copylinkfiles_path = json.load(fd)

Do:

with open(copylinkfiles_path, 'rb') as fp:
  try:
    old_copylinkfiles_path = json.load(fp)
  except:
    print('error: %s is not a json formatted file.' % copylinkfiles_path, file=sys.stderr)
    platform_utils.remove(copylinkfiles_path)
    return False

T2-17: Atomic File Writes for Repository Configurations

Rule: Must utilize atomic file writes rather than standard contexts when generating core .git configuration files.
What: System state modifications, specifically writing the .git pointer files for submodules, must be performed using an atomic write pattern (e.g., writing to a lockfile and renaming) rather than standard file descriptors.
Applies To: All file I/O operations directly modifying .git contents, worktrees, or manifest definitions.
Why: Using standard open() and writing to .git files directly leaves the repository vulnerable to corruption if the process is interrupted (e.g., via Ctrl+C or SIGTERM) during the write loop, resulting in a partially written pointer. Failing to adhere to this typically results in Corrupted Git Worktree.

Trap 1: Using the standard python open() context manager and print() to mutate core git directories.

Don't:

with open(dotgit, "w") as fp:
    print(f"gitdir: {rel_path}", file=fp)

Do:

_lwrite(dotgit, f"gitdir: {rel_path}\n")

T2-18: File Descriptor Lock Scope Minimization

Rule: Minimize file descriptor lock retention by executing string and state logic outside of standard reading contexts.
What: Data processing and variable manipulation must be extracted out of file descriptor lock contexts (with open(...)) to minimize resource retention.
Applies To: Any block parsing local text or configuration files.
Why: Holding file descriptors open while executing CPU-bound string manipulations increases lock contention probability and expands the attack surface for race conditions when multiple concurrent processes read/write repository states. Failing to adhere to this typically results in Resource Lock Contention.

Trap 1: Processing strings and building directory paths inside the file reading scope.

Don't:

with open(dotgit) as fp:
    setting = fp.read()
    gitdir = setting.split(":")[1].strip()
    dotgit_path = os.path.normpath(os.path.join(self.worktree, gitdir))

Do:

with open(dotgit) as fp:
    setting = fp.read()

# Lock released, safe to process
gitdir = setting.split(":")[1].strip()
dotgit_path = os.path.normpath(os.path.join(self.worktree, gitdir))

T2-19: Two-Phase Atomic Deletion for Worktrees

Rule: Must enact permanent .git and worktree removal via a two-phase destruction pattern utilizing a temporary trash path.
What: When permanently removing a .git directory or worktree, the system must first perform an atomic rename to a temporary trash path before initiating the recursive deletion.
Applies To: Garbage collection (gc.py), worktree cleanup, and submodule removal logic.
Why: Recursive deletion is not instantaneous. If a user forcefully interrupted (Ctrl+C) the rmtree operation, the directory was left in a ‘wedged’ (partially deleted but still registered) state, permanently breaking future syncs for that repository. Failing to adhere to this typically results in Wedged / Orphaned Worktree.

Trap 1: Directly triggering rmtree on a live repository path.

Don't:

for path in to_delete:
    platform_utils.rmtree(path)

Do:

for path in to_delete:
    temp_path = rename_to_trash(path)
    platform_utils.rmtree(temp_path)

T2-20: Conditional Absolute Path Resolution for Git Worktrees

Rule: Always independently verify if a tracked worktree path is absolute before initiating internal path recreation procedures.
What: Updates to .git directory reference files (e.g., gitdir:) must verify whether the stored worktree path is absolute before attempting to recreate it as a relative path to avoid path corruption and permissions errors.
Applies To: Internal Git directory layout management, specifically cross-platform Git worktree initialization (project.py).
Why: On certain platforms like Windows, modifying the internal dotgit file in situ fails due to file permissions. Code was added to delete and recreate the file using relative paths. However, depending on the Git version, the initial path could already be relative, making conversion unsafe without an isabs() check. Failing to adhere to this typically results in Permission Denied / Path Corruption.

Trap 1: Blindly deleting and rewriting the gitdir file using os.path.relpath without checking if the source path is absolute first.

Don't:

platform_utils.remove(dotgit)
with open(dotgit, "w", newline="\n") as fp:
    print("gitdir:", os.path.relpath(git_worktree_path, self.worktree), file=fp)

Do:

if os.path.isabs(git_worktree_path):
    platform_utils.remove(dotgit)
    with open(dotgit, "w", newline="\n") as fp:
        print("gitdir:", os.path.relpath(git_worktree_path, self.worktree), file=fp)

T2-21: Independent Manifest Repository Shallow Cloning

Rule: Never propagate the primary manifest --depth flag to govern the core manifest repository's clone boundaries.
What: The depth configuration for cloning the central manifest repository must be explicitly decoupled from the depth configuration applied to the child projects it governs, and its default must not regress existing multi-stage init workflows.
Applies To: Repo initialization commands (repo init, subcmds/init.py).
Why: Using the global --depth flag inadvertently forced shallow clones on all child projects. Introducing a dedicated --manifest-depth option fixed this, but defaulting it to 1 caused a regression during ‘double repo init’ workflows (where subsequent checkouts failed against truncated manifest histories). Failing to adhere to this typically results in Workflow Regression / History Truncation.

Trap 1: Defaulting to a shallow clone for configuration repositories, causing subsequent initialization/branch switching commands to fail due to missing objects.

Don't:

group.add_option('--manifest-depth', type='int', default=1, metavar='DEPTH',
                 help='create a shallow clone of the manifest repo')

Do:

group.add_option('--manifest-depth', type='int', default=0, metavar='DEPTH',
                 help='create a shallow clone of the manifest repo')

Cross-Domain Dependencies

Upstream: T3 | Subprocess Git Integration & Error Translation - Network operations and git process invocations require safe staging paths and catchable exceptions to trigger filesystem cleanups.
Downstream: T1 | Concurrent Synchronization & IPC - Parallel checkout operations depend heavily on atomic path modifications and lock-free directory states to prevent race conditions.
Downstream: T4 | Manifest Object Model & Deduplication - Filesystem migrations and subproject tree building directly parse and reflect canonicalized XML manifest declarations.

Chapter: Subprocess Git Integration & Error Translation

Context: This chapter defines the constraints for wrapping, executing, and translating standard Git subprocesses within the Repo tooling ecosystem. It mandates the use of centralized command abstractions, strict version gating, and deterministic stream handling to guarantee reliable repository state management and actionable error reporting.

Summary

Rule ID	Principle / Constraint	Priority	Primary Symptom / Trap
T3-01	Unborn Branch Resolution	High	Catching a `rev-parse`
: : via Symbolic-Ref : : GitError and immediately :
: : : : falling back to manual :
: : : : file system reads or :
: : : : emitting warnings. :
T3-02	Centralized GitCommand	Medium	Wrapping `subprocess.run`
: : Abstraction Usage : : manually inside a class :
: : : : to invoke Git. :
T3-03	NUL-Byte Delimiters for	High	Formatting git fields
: : Git Output Parsing : : with tabs and defending :
: : : : against length :
: : : : variations. :
T3-04	Subprocess Execution	Medium	Stringifying Path
: : Context and Native Path : : objects, using :
: : Passing : : Git-specific directory :
: : : : shifts, and suppressing :
: : : : piped output. :
T3-05	Runtime Git Version	High	Invoking a modern git CLI
: : Constraints for Promisor : : flag without ensuring :
: : Packs : : compatibility first. :
T3-06	Centralized Dependency	Medium	Catching missing binary
: : Availability Caching : : exceptions to terminate :
: : : : the process instead of :
: : : : returning control to the :
: : : : caller. :
T3-07	Subprocess Return Code	High	Calling a non-existent
: : Validation : : subprocess execution :
: : : : method on an already :
: : : : completed process object. :
T3-08	Strict Git Version	High	Injecting convenient,
: : Compatibility : : modern CLI flags into :
: : : : subprocess wrappers. :
T3-09	Rebase Override for	High	Throwing a
: : Published Local Branches : : `LocalSyncFail` when an :
: : : : upstream gain exists, :
: : : : completely ignoring the :
: : : : `force_rebase` argument. :
T3-10	SSO Authentication	High	Treating all exit code
: : Failure Translation : : 128 errors equally :
: : : : without sniffing stdout :
: : : : for authentication prompt :
: : : : aborts. :
T3-11	Direct Read Fallback for	High	Raising a fatal internal
: : Unborn Git HEAD : : exception immediately :
: : : : upon a `GitError` when :
: : : : checking for the HEAD :
: : : : reference. :
T3-12	Safe Git Subprocess	Critical	Assuming subprocess
: : Output Handling : : success and attempting to :
: : : : return an output variable :
: : : : that hasn't been :
: : : : initialized. :
T3-13	Shallow Clone SHA-1 Fetch	Critical	Assuming a fetch of an
: : Fallback : : arbitrary SHA-1 will :
: : : : unconditionally succeed :
: : : : if `--depth` is used. :
T3-14	Validating Gerrit Push	High	Constructing push options
: : Option Syntax : : with incorrect, :
: : : : unverified key strings. :
T3-15	Submodule Initialization	Critical	Blindly initializing
: : Guard Under Git Worktrees : : submodules without :
: : : : verifying if the :
: : : : underlying Git worktree :
: : : : feature is active. :
T3-16	XDG Compliant Git	High	Hardcoding the user's
: : Configuration Resolution : : home directory as the :
: : : : sole location for :
: : : : `.gitconfig`. :
T3-17	Strongly-Typed Subprocess	High	Catching a subprocess
: : Error Contexting : : failure and re-raising it :
: : : : as a generic git error :
: : : : with a concatenated :
: : : : string message. :
T3-18	Line-Based Log Extraction	Medium	Truncating stdout to a
: : for Subprocess : : fixed number of :
: : Diagnostics : : characters and ignoring :
: : : : stderr entirely. :

Rules

T3-01: Unborn Branch Resolution via Symbolic-Ref

Rule: Always attempt git symbolic-ref before falling back to manual file parsing to correctly resolve unborn branches.
What: When querying local Git branch structures, git symbolic-ref must be attempted as a fallback before file parsing to correctly identify unborn branches where rev-parse fails.
Applies To: Subprocess Git integrations resolving HEAD or local refs.
Why: Freshly initialized repositories and orphan branches present ‘unborn’ states where standard rev-parse commands returned errors, triggering false-positive warnings or crashing the client. Failing to adhere to this typically results in Incorrect Branch Detection.

Trap 1: Catching a rev-parse GitError and immediately falling back to manual file system reads or emitting warnings.

Don't:

try:
    return rev_parse(HEAD)
except GitError:
    logger.warning("Unparseable HEAD")
    return parse_file(HEAD)

Do:

try:
    return rev_parse(HEAD)
except GitError:
    try:
        return run_git_command("symbolic-ref", "-q", HEAD)
    except GitError:
        pass
    return parse_file(HEAD)

T3-02: Centralized GitCommand Abstraction Usage

Rule: Strictly use the internal GitCommand abstraction for local Git operations instead of invoking raw subprocess.run.
What: Use the internal GitCommand abstraction for executing Git operations rather than raw subprocess.run to guarantee consistent tracing, environment handling, and error translations.
Applies To: Any module interacting with local git binaries, such as git_refs.py.
Why: Developers bypassing the centralized wrapper led to duplicated subprocess execution logic, loss of global tracing capabilities (e.g., REPO_TRACE), and fragmented error handling across the codebase. Failing to adhere to this typically results in Missing Tracing / Environment Bugs.

Trap 1: Wrapping subprocess.run manually inside a class to invoke Git.

Don't:

def _Run(self, *cmd):
    return subprocess.run(
        ['git', f'--git-dir={self._gitdir}', *cmd],
        stdout=subprocess.PIPE,
        stderr=subprocess.DEVNULL
    )

Do:

from git_command import GitCommand

def _Run(self, cmd):
    p = GitCommand(
        None,
        cmd,
        capture_stdout=True,
        capture_stderr=True,
        bare=True,
        gitdir=self._gitdir,
    )
    p.Wait()
    return p

Exceptions: Low-level testing utilities isolated from the core application state, where bypassing GitCommand is necessary to bootstrap a test repository.

T3-03: NUL-Byte Delimiters for Git Output Parsing

Rule: Always specify NUL-byte delimiters (%00) when parsing Git command output to prevent splitting errors.
What: When parsing Git formatted output, utilize the NUL byte (%00) instead of tabs or spaces, and leverage precise tuple unpacking without length validation.
Applies To: Parsing output from Git commands like git for-each-ref or git status.
Why: Using whitespace or tabs to split field data exposed the parser to failures if reference names or other entities inherently included those characters. NUL delimiters guarantee absolute safety and allow removal of defensive parsing loops. Failing to adhere to this typically results in Data Parsing Failure.

Trap 1: Formatting git fields with tabs and defending against length variations.

Don't:

# Using tab delimiter and checking len
subprocess.run(['git', 'for-each-ref', '--format=%(objectname)\t%(refname)'])
for line in output.splitlines():
    fields = line.split('\t')
    if len(fields) < 2:
        continue
    ref_id, name = fields[:2]

Do:

# Using NUL delimiter and direct unpacking
GitCommand(..., ['for-each-ref', '--format=%(objectname)%00%(refname)%00%(symref)'])
for line in output.splitlines():
    ref_id, name, symref = line.split('\0')

T3-04: Subprocess Execution Context and Native Path Passing

Rule: Pass native pathlib.Path objects via the cwd argument rather than stringifying them or utilizing Git's -C flag.
What: Execute subprocesses with correct internal API paradigms: omit Git's -C in favor of the cwd= parameter, pass pathlib.Path objects inherently, and drop -q when actively capturing output.
Applies To: Subprocess command building and execution in test scaffolding and Git interactions.
Why: Passing -C combined with forced string casting of paths created redundant logic. Additionally, suppressing underlying git output via -q completely masked the root cause of errors captured within the internal stream buffers. Failing to adhere to this typically results in Obscured Diagnostics.

Trap 1: Stringifying Path objects, using Git-specific directory shifts, and suppressing piped output.

Don't:

subprocess.check_call(
    ['git', '-C', str(git_dir), 'init', '-q'],
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE
)

Do:

subprocess.check_call(
    ['git', 'init'],
    cwd=git_dir,
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE
)

T3-05: Runtime Git Version Constraints for Promisor Packs

Rule: Explicitly verify local Git version compatibility before injecting modern CLI flags into subprocess calls.
What: Operations utilizing modern Git arguments like --missing=allow-promisor must be explicitly gated by a minimal supported version check against the local executable.
Applies To: Subprocess handlers for git rev-list and repacking mechanisms on partial clones.
Why: Git 2.17.0 introduced advanced promisor arguments. Hardcoding these arguments in the execution engine crashed older repository clients on machines where Git had been downgraded or not updated. Failing to adhere to this typically results in Subprocess Crash.

Trap 1: Invoking a modern git CLI flag without ensuring compatibility first.

Don't:

if opt.repack:
    return self.repack_projects(projects, opt)

Do:

if opt.repack:
    git_require((2, 17, 0), fail=True, msg="--repack")
    return self.repack_projects(projects, opt)

T3-06: Centralized Dependency Availability Caching

Rule: Centralize and cache external dependency checks to fail gracefully without abruptly terminating the calling process.
What: External dependency checks (e.g., verifying SSH installation) must be centralized, cached internally, and fail gracefully without forcibly exiting the calling process.
Applies To: System API wrappers and subprocess managers (e.g., ProxyManager, ssh.py).
Why: Deep utility functions checking for external binaries previously called sys.exit(1) upon FileNotFoundError. This abruptly terminated the execution for users in environments missing the dependency, even if their specific workflow did not actually require it. Failing to adhere to this typically results in Unexpected Process Termination.

Trap 1: Catching missing binary exceptions to terminate the process instead of returning control to the caller.

Don't:

def version():
    try:
        return _parse_ssh_version()
    except FileNotFoundError:
        print("fatal: ssh not installed", file=sys.stderr)
        sys.exit(1)

Do:

def version():
    try:
        return _parse_ssh_version()
    except FileNotFoundError as e:
        print("warn: ssh not installed", file=sys.stderr)
        raise e

Trap 2: Duplicating inline try-except blocks for missing binaries across multiple execution methods rather than centralizing state.

Don't:

def run_command1(self):
    try:
        subprocess.Popen(['ssh'])
    except FileNotFoundError:
        return False

def run_command2(self):
    try:
        subprocess.Popen(['ssh'])
    except FileNotFoundError:
        return False

Do:

def __enter__(self):
    try:
        version()
        self._ssh_installed = True
    except FileNotFoundError:
        self._ssh_installed = False
    return self

def run_command(self):
    if not self._ssh_installed:
        return False
    subprocess.Popen(['ssh'])

T3-07: Subprocess Return Code Validation

Rule: Validate subprocess success strictly using check_returncode() on the completed process object.
What: When utilizing the standard Python subprocess library, the correct method to validate execution success on a CompletedProcess object is check_returncode(). Do not call check_call() on the returned object.
Applies To: Any Python code wrapping git commands and handling subprocess.CompletedProcess results.
Why: Legacy code inadvertently called check_call() on the result of run_command, which triggered a runtime AttributeError instead of raising the expected subprocess.CalledProcessError on failure. Failing to adhere to this typically results in AttributeError.

Trap 1: Calling a non-existent subprocess execution method on an already completed process object.

Don't:

ret = subprocess.run(cmd, capture_output=True)
ret.check_call()  # Raises AttributeError

Do:

ret = subprocess.run(cmd, capture_output=True)
# This will raise subprocess.CalledProcessError for us
ret.check_returncode()

T3-08: Strict Git Version Compatibility

Rule: Never pass Git flags that are unsupported by the minimum required Git version listed in the project's manifest.
What: Do not pass modern CLI flags to subprocess Git operations if those flags are not supported by the minimum required Git version listed in the project's dependency manifest.
Applies To: Git subprocess string assembly in project.py.
Why: The --no-auto-gc flag was appended to clone commands to prevent deadlocks, but this flag did not exist in Git 1.7.9, immediately breaking checkout operations for environments restricted to the minimum supported baseline. Failing to adhere to this typically results in Unknown Option Error.

Trap 1: Injecting convenient, modern CLI flags into subprocess wrappers.

Don't:

cmd.append("--no-auto-gc")

Do:

Disabling auto-gc via configuration setters (self.config.SetInt("gc.auto", 0)) that are safely interpreted across legacy Git versions.

T3-09: Rebase Override for Published Local Branches

Rule: Allow rewriting of published local commits by respecting the force_rebase argument during sync operations.
What: When a local branch is published but not yet merged upstream, repo sync --rebase must respect the force_rebase argument to allow rewriting published commits, rather than unconditionally throwing a sync failure.
Applies To: project.py, specifically _Rebase and _FastForward path execution during sync.
Why: The tool previously threw an error instructing the user to run with --rebase, but didn't actually execute the rebase when the flag was provided, forcing developers to manually fix the state. Failing to adhere to this typically results in Broken UX / Unhandled Rebase State.

Trap 1: Throwing a LocalSyncFail when an upstream gain exists, completely ignoring the force_rebase argument.

Don't:

if upstream_gain and not force_rebase:
    # Fails even if force_rebase is TRUE due to logic gap
    fail(LocalSyncFail("branch is published... rerun with the --rebase option"))

Do:

if upstream_gain:
    if force_rebase:
        syncbuf.later1(self, _dorebase, not verbose)
    else:
        fail(LocalSyncFail("branch is published... rerun with the --rebase option"))

T3-10: SSO Authentication Failure Translation

Rule: Intercept Git exit code 128 and translate it to a typed GitAuthError when stdout indicates an SSO session abort.
What: Git commands that fail with exit code 128 due to an SSO session abort must be explicitly intercepted and translated into a typed GitAuthError, preventing them from being masked as a missing reference.
Applies To: project.py, Git Subprocess Wrappers evaluating gitcmd.stdout and return codes (specifically fetch/sync operations interacting with SSO remote helpers).
Why: In non-interactive environments, an expired SSO token caused Git to fail with a generic exit code 128. The tool misinterpreted this as a missing upstream reference, leading to confusing error logs that obscured the underlying authentication requirement. Failing to adhere to this typically results in Ambiguous Auth Failure.

Trap 1: Treating all exit code 128 errors equally without sniffing stdout for authentication prompt aborts.

Don't:

# BAD: Ambiguous handling of 128
elif current_branch_only and is_sha1 and ret == 128:
    # Assuming missing upstream reference
    raise GitError("Couldn't find the ref you asked for")

Do:

# GOOD: Explicit trap for SSO helpers
elif ret == 128 and gitcmd.stdout and "remote helper 'sso' aborted session" in gitcmd.stdout:
    raise GitAuthError(gitcmd.stdout)

Exceptions: Repositories containing multiple remotes where an auth failure on a secondary remote might be non-fatal. (Currently flagged for future review but translation stands).

T3-11: Direct Read Fallback for Unborn Git HEAD

Rule: Implement a direct file system read of .git/HEAD as a fallback when standard sub-commands fail to resolve the reference.
What: When standard Git commands fail to resolve the current HEAD (e.g., throwing a GitError), the system must fall back to directly reading the internal .git/HEAD file from the filesystem before propagating a fatal error.
Applies To: project.py, GetHead() implementations traversing repository state.
Why: Empty repositories or states featuring unborn branches caused git rev-parse HEAD to fail outright. Without a fallback, this cascaded into a NoManifestException and aborted the entire synchronization process. Falling back to the raw .git/HEAD text bypassed the strict ref checks. Failing to adhere to this typically results in Hard Crash / NoManifestException.

Trap 1: Raising a fatal internal exception immediately upon a GitError when checking for the HEAD reference.

Don't:

# BAD: Strict reliance on git sub-commands
try:
    return self.rev_parse(HEAD)
except GitError as e:
    path = self.GetDotgitPath(subpath=HEAD)
    raise NoManifestException(path, str(e))

Do:

# GOOD: Fallback text parsing mechanism
try:
    return self.rev_parse(HEAD)
except GitError as e:
    path = self.GetDotgitPath(subpath=HEAD)
    try:
        with open(path) as fd:
            # Read branch manually
    except OSError:
        raise NoManifestException(path, str(e))

Exceptions: If the physical .git/HEAD file raises an OSError (meaning it truly does not exist on disk), a NoManifestException is necessary and valid.

T3-12: Safe Git Subprocess Output Handling

Rule: Always validate the return code of a Git subprocess before accessing its standard output properties.
What: Git subprocess output wrappers must explicitly handle non-zero exit codes to prevent accessing uninitialized variables, and stream properties must be spell-checked against the underlying class definitions.
Applies To: Git subprocess wrappers (git_superproject.py, GitCommand) and any execution context capturing stdout/stderr.
Why: When a Git command (like git rev-parse) failed, the code attempted to return a variable (data) that was only initialized on success, causing an UnboundLocalError. Furthermore, error logging attempts crashed due to a typo accessing p.stdwerr instead of p.stderr. Failing to adhere to this typically results in Runtime Crash / UnboundLocalError.

Trap 1: Assuming subprocess success and attempting to return an output variable that hasn't been initialized.

Don't:

# BAD: data is unbound if retval != 0
retval = p.Wait()
if retval == 0:
    data = p.stdout
else:
    log_error("Failed")
return data

Do:

# GOOD: Explicit error handling with early return
retval = p.Wait()
if retval != 0:
    log_error(f"Failed: {p.stderr}")
    return None
return p.stdout

Trap 2: Typo in the attribute name for capturing standard error from the GitCommand class.

Don't:

# BAD: Invalid attribute triggers AttributeError
self._LogWarning("Error: {}", p.stdwerr)

Do:

# GOOD: Correct attribute reference
self._LogWarning("Error: {}", p.stderr)

T3-13: Shallow Clone SHA-1 Fetch Fallback

Rule: Provide a full-sync fallback when shallow fetch requests for specific SHA-1s are rejected by server policies.
What: Git fetch requests targeting specific SHA-1s with --depth enabled must handle graceful fallbacks, as server-side policies often restrict unadvertised object fetching.
Applies To: Git fetch routines in project.py, specifically during incremental or optimized shallow clones.
Why: When optimizing syncs by directly fetching specific SHA-1s with a depth limitation, operations failed silently or returned generic errors because some Git servers do not advertise all objects, explicitly rejecting such requests. Failing to adhere to this typically results in Sync Failure.

Trap 1: Assuming a fetch of an arbitrary SHA-1 will unconditionally succeed if --depth is used.

Don't:

# BAD: Missing fallback for unadvertised objects
ret = fetch_sha1(depth=1)
if ret < 0:
    break

Do:

# GOOD: Explicit fallback to a full sync (depth-less) if the specific SHA-1 is rejected
ret = fetch_sha1(depth=1)
if depth and is_sha1 and ret == 1:
    # Server may not allow fetching unadvertised refs, fallback to full sync
    break

Exceptions: Servers specifically configured to allow uploadpack.allowReachableSHA1InWant or similar parameters, though the client cannot assume this.

T3-14: Validating Gerrit Push Option Syntax

Rule: Pass custom Gerrit push options strictly using the documented custom-keyed-value parameter syntax.
What: When passing custom metadata to Gerrit via Git push options, strictly adhere to the custom-keyed-value syntax as defined by the remote server documentation.
Applies To: upload.py operations mapping superproject state to git push -o.
Why: The client attempted to attach superproject metadata to push operations using custom-key-value, which was ignored by the Gerrit server because the documented parameter was custom-keyed-value. Failing to adhere to this typically results in Metadata Loss / Server Rejection.

Trap 1: Constructing push options with incorrect, unverified key strings.

Don't:

# BAD: Incorrect key format
push_options.append(f"custom-key-value=rootRepo:{host}/{sp_name}")

Do:

# GOOD: Matches external documentation
push_options.append(f"custom-keyed-value=rootRepo:{host}/{sp_name}")

T3-15: Submodule Initialization Guard Under Git Worktrees

Rule: Skip Git submodule initialization sequences if the repository is actively utilizing Git worktrees.
What: Submodule initialization operations (git submodule init) must be explicitly skipped if the repository is configured to use Git worktrees.
Applies To: Git command wrappers, worktree initialization sequences, and submodule lifecycle operations in project.py.
Why: Upstream Git has known bugs and incomplete feature support regarding the use of submodules within worktrees. Attempting to initialize submodules under these conditions leads to undefined behavior and broken repository states. Failing to adhere to this typically results in Upstream Git Bug / Inconsistency.

Trap 1: Blindly initializing submodules without verifying if the underlying Git worktree feature is active.

Don't:

self._InitWorkTree(force_sync=force_sync, submodules=submodules)
if self.has_subprojects:
    self._InitSubmodules()

Do:

self._InitWorkTree(force_sync=force_sync, submodules=submodules)
# Avoid doing a submodule init when using worktrees as the support for
# submodules is incomplete.
if self.has_subprojects and not self.use_git_worktrees:
    self._InitSubmodules()

T3-16: XDG Compliant Git Configuration Resolution

Rule: Resolve user Git configuration files natively honoring the XDG_CONFIG_HOME environment variable before defaulting to legacy paths.
What: Git configuration lookups must natively respect the XDG_CONFIG_HOME environment variable, falling back to ~/.config/git/config before finally querying the legacy ~/.gitconfig location.
Applies To: Git configuration parsing, environment setup, and hermetic tests.
Why: Users who migrated their dotfiles to the XDG Base Directory Specification experienced issues because internal tooling rigidly expected ~/.gitconfig to exist, bypassing user intentions and leading to unauthenticated or unconfigured repository actions. Failing to adhere to this typically results in Orphaned Configuration State.

Trap 1: Hardcoding the user's home directory as the sole location for .gitconfig.

Don't:

def _getUserConfig():
    return os.path.expanduser("~/.gitconfig")

Do:

def _getUserConfig():
    """This matches git: https://git-scm.com/docs/git-config#FILES"""
    xdg_config_home = os.getenv("XDG_CONFIG_HOME", os.path.expanduser("~/.config"))
    xdg_config_file = os.path.join(xdg_config_home, "git", "config")
    if os.path.exists(xdg_config_file):
        return xdg_config_file
    return os.path.expanduser("~/.gitconfig")

T3-17: Strongly-Typed Subprocess Error Contexting

Rule: Raise strongly-typed GitCommandError exceptions with granular state context instead of generic Git errors.
What: Git commands that yield non-zero exit codes must raise a specialized GitCommandError containing granular execution context, rather than a generic GitError.
Applies To: Subprocess execution wrappers within git_command.py and downstream consumers handling Git process states.
Why: When commands failed silently or raised generic errors, downstream handlers could not differentiate between configuration errors and actual subprocess failures, leading to loss of arguments and return codes. Failing to adhere to this typically results in Lost Diagnostic Context.

Trap 1: Catching a subprocess failure and re-raising it as a generic git error with a concatenated string message.

Don't:

if self.rc != 0:
    raise GitError("%s: %s" % (command[1], e))

Do:

if self.rc != 0:
    raise GitCommandError(
        message="git command failure",
        project=project.name,
        command_args=self.cmdv,
        git_rc=self.rc,
        git_stdout=stdout,
    )

T3-18: Line-Based Log Extraction for Subprocess Diagnostics

Rule: Extract and log the first meaningful line of both standard output and error streams rather than enforcing arbitrary truncation limits.
What: When capturing subprocess logs for error reporting, extract the first meaningful line of stdout and stderr instead of relying on arbitrary character limits.
Applies To: Error truncation logic for Git subprocesses in Wait() blocks.
Why: Truncating output by an arbitrary number of characters (e.g., 80 chars) occasionally cut off the actual error string. Furthermore, ignoring stderr hid the root cause of the subprocess failure. Failing to adhere to this typically results in Obfuscated Error Logs.

Trap 1: Truncating stdout to a fixed number of characters and ignoring stderr entirely.

Don't:

stdout = self.stdout[:80] if self.stdout else None

Do:

# Prefer extracting the first full line and including stderr
stdout_line = self.stdout.splitlines()[0] if self.stdout else None
stderr_line = self.stderr.splitlines()[0] if self.stderr else None

Cross-Domain Dependencies

Upstream: T2 | Filesystem Atomicity & Worktree Layout - Subprocess executions must coordinate with filesystem atomicity rules, particularly regarding submodule paths and worktree usage to prevent upstream Git bugs.
Downstream: T5 | Hermetic Testing & Test Modernization - Testing frameworks depend on hermetic GitCommand wrappers and mocked stream capture to validate subprocess behaviors safely.

Chapter: Manifest Object Model & Deduplication

Context: This chapter governs the parsing, validation, and canonicalization of XML manifest components. It strictly enforces semantic immutability via NamedTuple implementations, defensive copying for hierarchical override scoping, and deterministic JSON-backed file tracking across the subsystem.

Summary

Rule ID	Principle / Constraint	Priority	Primary Symptom / Trap
T4-01	Strict Traversal Prefix	High	Using broad substring
: : Matching for Relative : : matching to detect :
: : URLs : : relative traversal :
: : : : sequences. :
T4-02	Extend-Project	High	Omitting group
: : Inheritance Scoping : : inheritance constraints :
: : : : for dynamically :
: : : : overridden node elements. :
T4-03	Tuple Immutability in	Medium	Instantiating transient
: : Membership Checks : : list objects inside loop :
: : : : conditions. :
T4-04	Defensive Copying of	High	Assigning a shared
: : Shared Collections Prior : : collection reference and :
: : to In-Place Mutation : : immediately using an :
: : : : in-place operator, :
: : : : unknowingly mutating the :
: : : : original source. :
T4-05	Immutable NamedTuples for	High	Overriding `__hash__` and
: : Hashable Object : : `__eq__` on a standard :
: : Deduplication : : mutable Python class to :
: : : : allow set insertion. :
T4-06	Strict Empty Set	Critical	Falling back to the empty
: : Initialization Over : : bracket literal when a :
: : Dictionary Literals : : null set is encountered :
: : : : prior to a union :
: : : : operation. :
T4-07	Immutable Manifest	High	Implementing custom
: : Elements for Reliable : : `__eq__` and `__hash__` :
: : Deduplication : : on mutable classes and :
: : : : using `list.append()` :
: : : : which allows duplicates :
: : : : to accrue. :
T4-08	Mutually Exclusive XML	Medium	Independent `if`
: : Node Parsing : : conditions checking :
: : : : `nodeName` sequentially :
: : : : in a loop. :
T4-09	JSON Serialization for	High	Dumping file paths
: : File Path State Tracking : : sequentially into a flat :
: : : : text file using newlines :
: : : : as the only delimiter. :
T4-10	Set Operations for File	Medium	Using manual for-loops
: : Deltas : : and list appends to :
: : : : calculate the difference :
: : : : between two collections. :
T4-11	Upstream Base Revision	Medium	Overriding a project
: : Validation in Layered : : revision without :
: : Manifests : : asserting the previous :
: : : : expected state. :

Rules

T4-01: Strict Traversal Prefix Matching for Relative URLs

Rule: Always explicitly evaluate exact path-traversal prefixes (./ or ../) when resolving relative URLs. Never rely on generic substring matching.
What: Relative URL path detection must explicitly evaluate exact path-traversal prefixes to avoid inadvertently capturing hidden files or custom naming structures.
Applies To: URL string manipulation and Git submodule path resolution.
Why: The path parser attempted to identify relative submodule paths by checking if the URL started with a dot (.). This excessively broad evaluation failed on identically named hidden repositories or custom sub-namespaces (e.g., .foo), improperly joining them to the base URL. Failing to adhere to this typically results in Invalid Path Resolution.

Trap 1: Using broad substring matching to detect relative traversal sequences.

Don't:

if url.startswith("."):
    url = urllib.parse.urljoin("%s/" % self.remote.url, url)

Do:

if url.startswith("./") or url.startswith("../"):
    url = urllib.parse.urljoin("%s/" % self.remote.url, url)

T4-02: Extend-Project Inheritance Scoping

Rule: Must explicitly restrict extend-project XML nodes from automatically inheriting contextually local execution scopes (local: groups) from root parents.
What: When managing XML manifestation directives, extend-project nodes must be actively restricted from inheriting contextually local execution scopes (local: groups).
Applies To: XML manifest engine mappings and override properties.
Why: Without explicit protection, extended project entities unintentionally inherited top-level project local groups, confusing the inclusion and exclusion scoping directives that drive synchronization boundaries. Failing to adhere to this typically results in Scope Contamination.

Trap 1: Omitting group inheritance constraints for dynamically overridden node elements.

Don't:

Allow XML logic to process extend-project the exact same way it processes raw project roots without negative test coverage.

Do:

Implement testing and parsing guards proving that elements processed under extend-project actively strip local: group attributes.

T4-03: Tuple Immutability in Membership Checks

Rule: Always use immutable tuples instead of list literals for membership tests against constant sequences within hot loops.
What: Utilize immutable tuples () instead of list literals [] when performing membership tests (the in operator) against constant sequences to minimize reallocation overhead and adhere to idiomatic Python.
Applies To: XML manifest parsing logic (manifest_xml.py) and hot loops across the codebase.
Why: During recursive node parsing, list literals were instantiated on every iteration solely to check if a node name matched fixed schema elements. Failing to adhere to this typically results in Performance Degradation.

Trap 1: Instantiating transient list objects inside loop conditions.

Don't:

if node.nodeName in ["include", "project"]:

Do:

if node.nodeName in ("include", "project"):

T4-04: Defensive Copying of Shared Collections Prior to In-Place Mutation

Rule: Must defensively copy shared parent collections before applying in-place update operators (|=), strictly handling null-safety during the process.
What: When propagating inherited collections (e.g., manifest group sets) to child nodes, shared parent sets must be defensively copied prior to applying in-place union or update operators (|=). Null-safety must be handled during the copy.
Applies To: XML Manifest Parsing (manifest_xml.py); hierarchical state propagation where child elements inherit and augment parent states.
Why: Assigning a parent‘s group set directly to a child and then using the in-place union operator (|=) modified the original parent set in memory. This caused the child’s specific groups to ‘leak’ into unrelated sibling nodes processed subsequently. Failing to adhere to this typically results in State Corruption / Group Leakage.

Trap 1: Assigning a shared collection reference and immediately using an in-place operator, unknowingly mutating the original source.

Don't:

# BAD: In-place modification mutates the shared parent_groups object
nodeGroups = parent_groups
if node.hasAttribute("groups"):
    nodeGroups |= self._ParseSet(node.getAttribute("groups"))

Do:

# GOOD: Create a localized copy before mutation
nodeGroups = parent_groups.copy() if parent_groups else set()
if node.hasAttribute("groups"):
    nodeGroups |= self._ParseSet(node.getAttribute("groups"))

Trap 2: Falling back to an empty set instantiation after a copy call without guarding against a null reference.

Don't:

# BAD: Raises AttributeError if parent_groups is None
include_groups = parent_groups.copy() or set()

Do:

# GOOD: Null-safe default evaluation prior to the copy invocation
include_groups = (parent_groups or set()).copy()

T4-05: Immutable NamedTuples for Hashable Object Deduplication

Rule: Always subclass NamedTuple to define custom objects stored in hash-based collections. Never implement manual __hash__ and __eq__ functions on mutable classes.
What: Custom objects stored in hash-based collections (like set or used as dict keys) for deduplication must guarantee immutability. Avoid manually implementing __eq__ and __hash__ on mutable classes; instead, subclass NamedTuple.
Applies To: Manifest deduplication logic (project.py); specifically elements like _CopyFile and _LinkFile stored in sets.
Why: Manually implementing __hash__ on a standard class based on its mutable __dict__ attributes created a landmine. If the object's properties changed after insertion into a set, the hash mismatched, rendering the object unreachable and breaking deduplication. Failing to adhere to this typically results in Memory Leak / Deduplication Failure.

Trap 1: Overriding __hash__ and __eq__ on a standard mutable Python class to allow set insertion.

Don't:

# BAD: Properties can mutate, altering the hash post-insertion
class _CopyFile:
    def __init__(self, src, dest):
        self.src = src
        self.dest = dest
    def __hash__(self):
        return hash(repr(sorted(self.__dict__.items())))
    def __eq__(self, other):
        return self.__dict__ == other.__dict__

Do:

# GOOD: Use typing.NamedTuple to guarantee immutability
from typing import NamedTuple

class _CopyFile(NamedTuple):
    src: str
    dest: str

T4-06: Strict Empty Set Initialization Over Dictionary Literals

Rule: Must explicitly invoke set() when initializing empty collections meant for set operations. Never fall back to the empty bracket literal {}.
What: When initializing or providing a fallback empty collection meant for set operations (such as union | or |=), explicitly instantiate a set() rather than using the empty literal {}.
Applies To: Python collection management globally; prominently surfaced during list-to-set manifest group migrations.
Why: During a migration from lists to sets for group parsing, an empty default was incorrectly represented as {}. Since {} evaluates to a dict in Python, attempting a set union between a valid set and the empty dict threw an immediate, fatal TypeError. Failing to adhere to this typically results in Runtime Crash / TypeError.

Trap 1: Falling back to the empty bracket literal when a null set is encountered prior to a union operation.

Don't:

# BAD: '{}' creates an empty dictionary, throwing a TypeError upon union
expanded_project_groups = {"all"} | (self.groups or {})

Do:

# GOOD: Explicitly initialize an empty set
expanded_project_groups = {"all"} | (self.groups or set())

T4-07: Immutable Manifest Elements for Reliable Deduplication

Rule: Always represent manifest file operations as inherently immutable structures to guarantee safe, order-preserving deduplication.
What: Manifest elements representing file operations must be implemented as immutable structures (e.g., NamedTuple) to ensure safe deduplication and deterministic order preservation, avoiding manual string-based hashing.
Applies To: Manifest parsing models, specifically <copyfile> and <linkfile> abstractions.
Why: Previously, manifest objects were stored in mutable lists, relying on manual string representations (repr) and sorted dictionary items for hashing. This risked state corruption and failed to cleanly deduplicate repeated entries while maintaining the relative declaration sequence. Failing to adhere to this typically results in Silent Data Corruption / Redundant File Operations.

Trap 1: Implementing custom __eq__ and __hash__ on mutable classes and using list.append() which allows duplicates to accrue.

Don't:

class _CopyFile:
    def __hash__(self):
        # BAD: Manual serialization of mutable dictionary
        return hash(repr(sorted(self.__dict__.items())))

self.copyfiles.append(_CopyFile(src, dest))

Do:

class _CopyFile(NamedTuple):
    # GOOD: Inherently immutable and hashable
    src: str
    dest: str

# GOOD: Using a dict to deduplicate while preserving insertion order
self.copyfiles[_CopyFile(src, dest)] = True

T4-08: Mutually Exclusive XML Node Parsing

Rule: Always chain mutually exclusive XML node type checks using an if-elif structure rather than independent sequential statements.
What: When parsing mutually exclusive XML node types, use an if-elif structure to avoid redundant condition evaluations.
Applies To: XML parsing loops (e.g., iterating over node.childNodes in manifest_xml.py).
Why: A sequence of independent if statements forced unnecessary string comparisons even after a matching node name was found. Failing to adhere to this typically results in CPU Overhead / Inefficient Parsing.

Trap 1: Independent if conditions checking nodeName sequentially in a loop.

Don't:

if n.nodeName == "copyfile":
    self._ParseCopyFile(p, n)
if n.nodeName == "linkfile":
    self._ParseLinkFile(p, n)

Do:

if n.nodeName == "copyfile":
    self._ParseCopyFile(p, n)
elif n.nodeName == "linkfile":
    self._ParseLinkFile(p, n)

T4-09: JSON Serialization for File Path State Tracking

Rule: Must persist dynamically generated manifest file outputs using structured JSON serialization. Never use newline-delimited plaintext for path lists.
What: Persistent tracking mechanisms for manifest-generated file operations must use structured JSON serialization instead of newline-delimited plaintext to support special characters in paths.
Applies To: Manifest state management (subcmds/sync.py), internal .repo tracking files (e.g., copy-link-files.json).
Why: Tracking <copyfile> and <linkfile> operations via flat, newline-delimited text files broke serialization and cleanup when file paths legally contained newline (\n) characters. Structured JSON protects against these bounds issues. Failing to adhere to this typically results in Parsing Errors / Orphaned Files.

Trap 1: Dumping file paths sequentially into a flat text file using newlines as the only delimiter.

Don't:

with open('linkfile.list', 'w') as fd:
    for path in new_linkfiles:
        fd.write(path + '\n')

Do:

with open('copy-link-files.json', 'w') as fd:
    json.dump({'linkfile': new_linkfiles, 'copyfile': new_copyfiles}, fd)

T4-10: Set Operations for File Deltas

Rule: Always compute the delta between unordered lists of file paths using native Python set operations (-).
What: When computing the delta between lists of file paths (e.g., figuring out which obsolete files to remove), utilize Python set operations rather than manual iterations with truthiness and membership checks.
Applies To: Deduplicating objects, comparing manifest states, computing file differences.
Why: Calculating the difference between historical tracking data and new target paths was done using O(N*M) nested iterations with redundant truthiness checks, leading to verbose and less performant code. Failing to adhere to this typically results in Algorithmic Inefficiency.

Trap 1: Using manual for-loops and list appends to calculate the difference between two collections.

Don't:

need_remove_files = []
for old_path in old_dict.get('linkfile', []):
  if old_path and old_path not in new_paths:
    need_remove_files.append(old_path)

Do:

need_remove_files = []
need_remove_files.extend(
    set(old_dict.get('linkfile', [])) -
    set(new_paths)
)

Exceptions: Ordered constraints where file evaluation sequence matters (sets are unordered).

T4-11: Upstream Base Revision Validation in Layered Manifests

Rule: Must assert multi-layer manifest overrides (extend-project) against the parent project's expected state by utilizing a base-rev attribute.
What: When overriding (extend-project) or removing (remove-project) projects in a layered manifest structure, changes should be validated against the expected state of the parent project via a base-rev attribute.
Applies To: XML Manifest parsing logic (manifest_xml.py) during multi-manifest ingestion.
Why: Downstream projects using layered manifests would unknowingly apply overrides (like custom revisions) to upstream components that had drastically changed their underlying state or branches, resulting in ‘undead patches’ that silently masked critical upstream changes. Failing to adhere to this typically results in Silent Configuration Divergence.

Trap 1: Overriding a project revision without asserting the previous expected state.

Don't:

<!-- BAD: Blind override leaves system vulnerable to upstream changes -->
<extend-project name="my-project" revision="refs/changes/123" />

Do:

<!-- GOOD: Base-rev acts as a guardrail -->
<extend-project name="my-project" base-rev="upstream_hash" revision="refs/changes/123" />

Trap 2: Checking string-based manifest attributes using implicit truthiness.

Don't:

# BAD: Implicit truthiness skips validation if revisionExpr is unexpectedly empty
if base and p.revisionExpr and p.revisionExpr != base:
    raise Error()

Do:

# GOOD: Strict comparison catches empty/None states if a base is declared
if base and p.revisionExpr != base:
    raise Error()

Exceptions: The base-rev attribute is optional to maintain backward compatibility for users migrating to the new manifest schema.

Cross-Domain Dependencies

Upstream: T2 | Filesystem Atomicity & Worktree Layout - File path extraction and submodule path resolution mapped here strictly govern worktree creation and legacy migration algorithms downstream.
Downstream: T6 | CLI Argument Parsing & UX Consistency - Structured JSON serialization outputted from manifest paths sets the direct format constraint for CLI UX and tooling integrations.

Chapter: Hermetic Testing & Test Modernization

Context: This domain governs the migration of legacy unittest suites to modern pytest functional paradigms and the establishment of hermetic session fixtures. It enforces strict environment isolation to prevent global state pollution (e.g., developer .gitconfig bleeding) while safely intercepting standard streams and filesystem paths.

Summary

Rules

T5-01: Idiomatic Truthiness Validation in Tests

Rule: Always use implicit truthiness or falsiness evaluations in test assertions unless strict type matching is explicitly required.
What: Test suites must use implicit truthy/falsy validations rather than asserting strict identity against boolean singletons unless exact type enforcement is necessary.
Applies To: Python assertions in test frameworks (pytest, unittest).
Why: Tests relying on strict boolean identity (is True) became brittle when refactoring configurations or flag implementations that returned alternative truthy values. Failing to adhere to this typically results in Brittle Assertions.

Trap 1: Asserting strict identity against Python boolean singletons.

Don't:

assert opts.include_summary is True

Do:

assert opts.include_summary

Exceptions: Situations where distinguishing between None and False is structurally critical to the application logic.

T5-02: Pytest Over Legacy Unittest Framework

Rule: Must implement new test suites using modular pytest definitions instead of propagating unittest.TestCase boilerplate.
What: New test modules must be implemented using standalone pytest functional constructs rather than inheriting from unittest.TestCase to avoid heavy class-based boilerplate.
Applies To: Test module architecture (tests/test_*.py).
Why: The project previously utilized deeply nested unittest.TestCase structures with complex setUp inheritance, which obfuscated test initialization and hindered code reusability. Failing to adhere to this typically results in High Boilerplate / Lock-In.

Trap 1: Subclassing unittest.TestCase and configuring state inside a setUp method for new test files.

Don't:

class InfoCommand(unittest.TestCase):
    def setUp(self):
        self.cmd = build_cmd()

Do:

def _get_cmd():
    return build_cmd()

def test_command_defaults():
    cmd = _get_cmd()
    assert cmd.is_valid()

T5-03: Dedicated Test Utilities for API Helpers

Rule: Never overload conftest.py with generic API helpers; always place reusable test queries in dedicated utility modules.
What: Shared test APIs and environment-probing logic (e.g., checking Git feature support) must be placed in dedicated utility modules (like utils_for_test.py) rather than duplicated or shoved into conftest.py.
Applies To: Test suite architecture and environment scaffolding.
Why: Developers were duplicating identical environment checks across different test files or misusing conftest.py (which should only contain fixtures) to store functional APIs. Failing to adhere to this typically results in Test Code Duplication.

Trap 1: Copy-pasting utility functions across test modules or placing standard functions into pytest conftest files.

Don't:

# Inside test_project.py AND test_git_refs.py
def _SupportsReftable():
    return check_git_support()

Do:

# Inside tests/utils_for_test.py
def supports_reftable():
    return check_git_support()

# Inside test_project.py
import utils_for_test
@unittest.skipUnless(utils_for_test.supports_reftable(), "...")

Exceptions: Test fixtures (such as initialized directories or mock generators) which belong in conftest.py.

T5-04: Test Framework Modernization to Pytest

Rule: Avoid global module-level environment mutations; consistently leverage pytest global configuration fixtures.
What: Legacy unittest.TestCase structures must be migrated to functional pytest paradigms, leveraging global configuration fixtures over class-based setUp and tearDown methods.
Applies To: Entire Python test suite and all new unit tests.
Why: Heavy inheritance and isolated setup code within unittest.TestCase resulted in repeated environment mutation logic, obscured dependencies, and an inability to easily share global setup mechanisms like dummy user identities or tracing control. Failing to adhere to this typically results in Global State Pollution.

Trap 1: Modifying environment variables globally at module load to support tests.

Don't:

import os
os.environ['REPO_TRACE'] = '0'

class GitRefsTest(unittest.TestCase):
    def setUp(self):
        self.tempdir = tempfile.TemporaryDirectory()

Do:

# Rely on conftest.py fixtures automatically handling environments.
def test_git_refs(tmp_path, disable_repo_trace, setup_user_identity):
    gitdir = tmp_path / '.git'
    # Run assertions using standard assert statements
    assert head == refs.get('HEAD')

T5-05: Standard Output Capture via Context Managers

Rule: Must utilize contextlib.redirect_stdout to capture standard output streams rather than injecting raw stream mocks.
What: Use the standard library contextlib.redirect_stdout for capturing command output in test scopes, instantiated directly as a one-liner, rather than utilizing the unittest.mock framework to patch system streams.
Applies To: Python test suites (Pytest/Unittest) intercepting CLI stdout payloads.
Why: Tests were previously relying on @mock.patch for sys.stdout or utilizing multi-line context manager instantiations, which unnecessarily coupled output capture to the mocking engine and added boilerplate. Failing to adhere to this typically results in Unnecessary Mocking / Test Bloat.

Trap 1: Using the mock framework to intercept a core language stream.

Don't:

@mock.patch("sys.stdout", new_callable=io.StringIO)
def test_something(self, mock_stdout):
    # test logic

Do:

import contextlib
import io

def test_something():
    with contextlib.redirect_stdout(io.StringIO()) as stdout:
        # test logic

Trap 2: Creating the string buffer variable outside the context manager scope unnecessarily.

Don't:

stdout = io.StringIO()
with contextlib.redirect_stdout(stdout):
    _run_status(manifest, ["-o"])

Do:

with contextlib.redirect_stdout(io.StringIO()) as stdout:
    _run_status(manifest, ["-o"])

T5-06: Direct Helper Functions Over Fixture Factories

Rule: Avoid defining dummy pytest fixtures solely to return nested function callbacks; use straightforward module-level helpers instead.
What: Do not abuse the Pytest fixture system to generate and return local functions. If a utility does not benefit from the fixture lifecycle management (setup/teardown), it should be a standard module-level or private helper function.
Applies To: Pytest files requiring test data initialization or repetitive setup procedures.
Why: Test frameworks became overly complex when developers wrapped simple initialization logic in @pytest.fixture annotations just to yield a callable function to the test case. Failing to adhere to this typically results in Fixture Graph Bloat.

Trap 1: Defining a pytest fixture that yields a nested function to bypass global state limitations.

Don't:

@pytest.fixture
def init_temp_git_tree():
    def _init_temp_git_tree(git_dir: Path) -> None:
        # init logic
    return _init_temp_git_tree

Do:

def _init_temp_git_tree(git_dir: Path) -> None:
    # init logic

T5-07: Elimination of Redundant Path Assertions

Rule: Never prepend path existence assertions prior to invoking permissions or loading assertions that implicitly validate existence.
What: Do not assert the existence of a file prior to asserting its specific permissions or state. Subsequent OS-level operations implicitly validate path existence.
Applies To: Python unittests validating filesystem layouts, binary configurations, or hook executable permissions.
Why: Tests contained boilerplate blocks invoking os.path.isfile() purely to offer a custom string format failure, even though the immediate next line invoking os.access() would fail correctly with an implicit lack of existence. Failing to adhere to this typically results in Test Boilerplate.

Trap 1: Chaining explicit existence checks before access checks.

Don't:

self.assertTrue(os.path.isfile(repo_path), f"{repo_path} does not exist")
self.assertTrue(os.access(repo_path, os.X_OK), f"{repo_path} is not executable")

Do:

self.assertTrue(os.access(repo_path, os.X_OK), f"{repo_path} is not executable")

T5-08: Pathlib Utility Constants for Test Path Resolution

Rule: Always resolve test filesystem boundaries using centralized pathlib objects instead of string-concatenating legacy os.path operations.
What: Path constructions in test files should utilize modern pathlib objects via centralized utility constants (e.g., utils_for_test.THIS_DIR) instead of verbose os.path operations.
Applies To: Python test suites interacting with the file system.
Why: Manual path constructions using nested os.path.dirname(__file__) calls created verbose, hard-to-read boilerplate that cluttered the test logic. Failing to adhere to this typically results in Verbose Code Base.

Trap 1: Using nested os.path.dirname and os.path.join to traverse back to the parent directory.

Don't:

repo_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "repo")

Do:

repo_path = utils_for_test.THIS_DIR.parent / "repo"

T5-09: Pytest Adoption for Test Suite Definition

Rule: Must author all file-level test assertions utilizing pytest syntax constructs over legacy unittest equivalents.
What: Test files must be defined using the modern pytest framework, avoiding class-based unittest.TestCase inheritance and relying on standard assert semantics.
Applies To: The entire Python testing suite.
Why: The legacy unittest structure required extensive, verbose boilerplate for state setup, test case classes, and specific assertion methods that restricted readability. Failing to adhere to this typically results in Test Verbosity.

Trap 1: Creating test case classes that inherit from unittest.TestCase and asserting via self.assertEqual.

Don't:

import unittest
class RepoTests(unittest.TestCase):
    def test_example(self):
        self.assertEqual(a, b)

Do:

# Standalone pytest functions
def test_example():
    assert a == b

T5-10: Minimal Mocking Isolation for Test Reliability

Rule: Avoid deeply nested or exhaustively mocked test wrappers; isolate and invoke inner target logic primitives instead.
What: Mock usage must be limited strictly to necessary external intercepts. Exhaustive mock environments must not be created just to drive a small piece of inner logic; instead, structural refactoring must isolate the logical boundary.
Applies To: Test suites involving deep API structures and side-effects.
Why: Relying on heavily nested, brittle ‘house of cards’ mock configurations meant tests frequently failed when unrelated code modifications broke the strict internal mock assumptions, turning tests into throwaway artifacts. Failing to adhere to this typically results in Fragile Mock Collapse.

Trap 1: Using stacked patch decorators for non-relevant sub-routines (e.g., standard error outputs or colorizers) just to trigger an embedded autocorrection flow.

Don't:

with mock.patch("main.RepoClient", return_value=mock_client):
    with mock.patch("main.SetDefaultColoring"):
        repo._RunLong("tart", gopts, argv, log)

Do:

Isolate the logic into _autocorrect_command_name() and test it by passing minimal data primitives: res = self.repo._autocorrect_command_name("tart", mock_config).

Exceptions: Testing hard, native OS-level boundary bindings.

T5-11: Canonical Resolution for System Library Mocks

Rule: Always patch standard libraries at their canonical origin namespace rather than targeting the local module import resolution boundary.
What: Standard library functions must be targeted for testing patches at their canonical origin namespace rather than the local import footprint of the target module.
Applies To: Test configuration defining mock.patch paths.
Why: Patching against the local module import representation strictly coupled tests to a given file's exact inclusion syntax. Minor refactors altering module import aliases completely disabled testing verification. Failing to adhere to this typically results in Brittle Patch Routing.

Trap 1: Patching time.sleep by referencing the specific host application file where it was locally imported.

Don't:

@mock.patch("main.time.sleep")

Do:

@mock.patch("time.sleep")

T5-12: Memoized Function Cache Clearing in Setup

Rule: Always flush the lru_cache of mocked globally-accessible memoized functions within the test initialization sequence.
What: When mocking globally accessible functions decorated with @functools.lru_cache, the cache must be explicitly cleared in the test suite's setUp routine.
Applies To: Unit test suites interacting with memoized functions.
Why: Failing to clear the lru_cache of globally executed functions during testing caused subsequent tests in the runner to receive cached mock results injected by prior tests, leading to non-deterministic, cascading failures. Failing to adhere to this typically results in State Contamination / Flaky Tests.

Trap 1: Mocking the underlying behavior of a cached function but leaving the LRU cache intact across tests.

Don't:

class SshTests(unittest.TestCase):
    def test_version(self):
        with mock.patch("ssh._run_ssh_version", return_value="OpenSSH_1.2\n"):
            self.assertEqual(ssh.version(), (1, 2))

Do:

class SshTests(unittest.TestCase):
    def setUp(self):
        ssh.version.cache_clear()

    def test_version(self):
        with mock.patch("ssh._run_ssh_version", return_value="OpenSSH_1.2\n"):
            self.assertEqual(ssh.version(), (1, 2))

T5-13: Idiomatic Standard Stream Redirection

Rule: Always employ contextlib.redirect_stderr directly instead of overwriting standard stream objects globally using patch frameworks.
What: To intercept stderr or stdout in testing, utilize the standard library's context managers (contextlib.redirect_stderr) instead of overriding the stream objects globally using mock.patch.
Applies To: Unit testing standard output logs and error streams.
Why: Using generic mocks to replace sys.stderr creates brittle tests with broader side effects. The standard library provides dedicated tools strictly designed for capturing stream text safely. Failing to adhere to this typically results in Non-Idiomatic Tooling / Brittle Mocks.

Trap 1: Using mock.patch to manually inject a StringIO object over sys.stderr.

Don't:

with mock.patch("sys.stderr", new=io.StringIO()) as mock_stderr:
    run_command()

Do:

f = io.StringIO()
with contextlib.redirect_stderr(f):
    run_command()

T5-14: Context Manager Nesting Consolidation

Rule: Consolidate contiguous mock or context manager deployments into single-line definitions to minimize indentation footprints.
What: Contiguous, deeply nested context managers (with blocks) should be consolidated into a single with statement to reduce horizontal indentation and structural complexity.
Applies To: Complex test fixtures or code paths requiring multiple synchronized states or file handles.
Why: Individual context managers per line resulted in excessive code indentation (pyramid of doom), harming readability and consuming unnecessary screen space. Failing to adhere to this typically results in Excessive Indentation / Pyramid of Doom.

Trap 1: Adding a new level of indentation for every context manager invoked.

Don't:

with mock.patch("sys.stderr") as mock_stderr:
    with mock.patch("socket.socket") as ms:
        run_test()

Do:

with mock.patch("sys.stderr") as mock_stderr, mock.patch("socket.socket") as ms:
    run_test()

T5-15: Anti-Change Detector Tests

Rule: Avoid exact full-string assertions when evaluating formatted logs or stringified error conditions to prevent brittle side-effect maintenance.
What: When verifying error logging or outputs, tests must assert on flexible core substrings rather than exact line matches, and exceptions configured as mock side effects should use class types rather than instantiated string objects.
Applies To: Unit testing string outputs, logs, and simulated mock failures.
Why: Tests acting as ‘change detectors’ failed unnecessarily when minor formatting, capitalization, or prefix strings (e.g., ‘WARNING’ vs ‘warning’) were updated in the underlying code, despite the core logic remaining functional. Failing to adhere to this typically results in Brittle Tests / Extraneous Boilerplate.

Trap 1: Asserting exact equality of string outputs including standardized prefixes.

Don't:

self.assertEqual(stderr.getvalue(), "repo: warning: git trace2 logging failed: Mock error")

Do:

self.assertIn("git trace2 logging failed", stderr.getvalue())

Trap 2: Instantiating an exception with a dummy string for a simple side-effect mock.

Don't:

mock_socket.connect.side_effect = OSError("Mock error")

Do:

mock.patch("socket.socket", side_effect=OSError)

Exceptions: Tests explicitly verifying exact formatting serialization (e.g., JSON schema adherence).

T5-16: Infrastructure Prebuilts via vpython

Rule: Must execute continuous integration environments using vpython wrappers instead of relying on legacy source-compiling orchestration frameworks like pyenv.
What: Leverage vpython for multi-version Python testing rather than tox combined with pyenv to utilize prebuilt binaries and minimize environment setup overhead.
Applies To: Hermetic testing setups, CI pipelines, and multi-version coverage scripts.
Why: Using pyenv and tox forced CI and local test runs to download and compile Python versions from source on the fly, adding minutes of overhead to every test run. Shifting to vpython leveraged prebuilt infrastructure runtimes. Failing to adhere to this typically results in Slow CI Builds.

Trap 1: Relying on tools that dynamically build Python from source to test across multiple versions.

Don't:

Using tox.ini with pyenv to orchestrate matrix testing across Python versions, incurring source-compilation penalties.

Do:

Invoking tests via vpython, which provisions hermetic virtual environments rapidly using precompiled infrastructure binaries.

T5-17: Modernized Filesystem Paths via Pathlib

Rule: Always employ pathlib objects for mock file I/O operations rather than manual string handling and context managers.
What: Test case file system operations should use the modern pathlib library rather than legacy os.path and manual file handles.
Applies To: Unit tests and file system operations across the codebase.
Why: Legacy tests relied heavily on os.path.join and with open(...) which led to verbose, repetitive boilerplate when constructing mock repository states. Failing to adhere to this typically results in Verbose Boilerplate / Maintainability Overhead.

Trap 1: Using os.path.join and with open(..., 'w') for basic text file creation in tests.

Don't:

# BAD: Verbose manual file writing
root_m = os.path.join(self.manifest_dir, "root.xml")
with open(root_m, "w") as fp:
    fp.write("<manifest>...")

Do:

# GOOD: Pathlib usage for concise writes
root_m = self.manifest_dir / "root.xml"
root_m.write_text("<manifest>...")

T5-18: Standardized Module-Level Mock Imports

Rule: Import external test dependencies and internal domain targets as complete module spaces rather than cherry-picking precise classes.
What: Test files must import mock as a module (from unittest import mock) and internal targets as a module (from subcmds import wipe), rather than importing specific classes directly.
Applies To: All unit tests.
Why: Direct class imports caused namespace pollution and deviated from the Google Python Style Guide, making it harder to track the origin of dependencies. Failing to adhere to this typically results in Style Guide Violation / Namespace Pollution.

Trap 1: Importing specific classes or exceptions directly from test targets or standard libraries.

Don't:

from unittest.mock import MagicMock
from subcmds.wipe import Wipe, WipeError

mock_obj = MagicMock()

Do:

from unittest import mock
from subcmds import wipe

mock_obj = mock.MagicMock()

T5-19: Pytest Adoption for New Test Modules

Rule: Must restrict the creation of new unittest.TestCase files strictly unless explicitly bypassing native pytest infrastructure constraints.
What: New test modules must be written using the pytest framework rather than the legacy unittest structure.
Applies To: All newly added test files.
Why: The project was modernizing its testing infrastructure, aiming to eliminate unittest boilerplate and leverage native pytest fixtures for global state management. Failing to adhere to this typically results in Technical Debt / Legacy Test Framework Propagation.

Trap 1: Creating subclasses of unittest.TestCase and using self.assertEqual.

Don't:

class WipeUnitTest(unittest.TestCase):
    def test_wipe(self):
        self.assertTrue(os.path.exists(path))

Do:

def test_wipe(tmp_path):
    assert os.path.exists(path)

Exceptions: Unless a compelling reason exists requiring native unittest mechanics.

T5-20: High-Fidelity Test Targets over Permissive Mocks

Rule: Avoid stubbing robust architectural models like Project or Manifest via permissive MagicMock allocations; instantiate genuine domain classes instead.
What: Tests should instantiate and utilize real domain objects (e.g., the Project class) rather than relying heavily on generic MagicMock instances, to ensure test fidelity.
Applies To: Unit testing, particularly for core models like Project or Manifest.
Why: Overuse of MagicMock led to tests passing despite broken implementations because the mocks lacked side-effects and realistic attribute validation. Failing to adhere to this typically results in False Positive Test Results.

Trap 1: Using MagicMock to bypass instantiating core models.

Don't:

# BAD: Mocks won't fail if the API changes
proj = mock.MagicMock()
proj.name = "test"
proj.DeleteWorktree.side_effect = lambda: True

Do:

# GOOD: Using actual domain objects for accurate side-effects
proj = Project(manifest=mock_manifest, name="test", ...)

Exceptions: External network calls or complex, deeply decoupled subsystems where instantiating the real object is prohibitively difficult.

T5-21: Session-Scoped Hermetic Git Identity Injection

Rule: Must inject pseudo Git commit identities as session-scoped environment variables rather than manually rewriting local Git configurations per test.
What: Automated test suites must globally stub Git's author and committer identities using session-scoped environment variables to ensure execution in clean or unconfigured environments.
Applies To: Pytest configuration (tests/conftest.py) and all tests executing raw Git subprocess commands.
Why: Tests interacting with local Git repositories crashed with GitCommandError on CI runners or clean developer machines lacking a global user.name and user.email. Patching this locally in tests via git config calls introduced I/O overhead and redundant code. Failing to adhere to this typically results in GitCommandError / Test Flakiness.

Trap 1: Executing git config inside individual test setup blocks to fake a user identity.

Don't:

def test_commit(self):
    subprocess.run(["git", "config", "user.name", "Foo Bar"])
    subprocess.run(["git", "config", "user.email", "foo@bar.com"])
    # Run test logic

Do:

@pytest.fixture(autouse=True, scope="session")
def setup_user_identity(monkeysession):
    """Set env variables for author and committer name and email."""
    monkeysession.setenv("GIT_AUTHOR_NAME", "Foo Bar")
    monkeysession.setenv("GIT_COMMITTER_NAME", "Foo Bar")
    monkeysession.setenv("GIT_AUTHOR_EMAIL", "foo@bar.baz")
    monkeysession.setenv("GIT_COMMITTER_EMAIL", "foo@bar.baz")

T5-22: Automated Lifecycle Management for Test Filesystem Artifacts

Rule: Must utilize automated TemporaryDirectory constructs integrated securely into test teardown sequences instead of performing unmanaged manual disk sweeps.
What: Test fixtures must use state-managed temporary directory constructs (e.g., tempfile.TemporaryDirectory) whose lifecycle is explicitly bound to standard unit test teardown phases, avoiding manual unmanaged path allocation and raw removal.
Applies To: All test suites performing filesystem mock operations or Git repository initialization (tests/test_subcmds_forall.py).
Why: Tests were manually allocating paths via tempfile.mkdtemp and issuing raw shutil.rmtree calls. This pattern proved fragile, resulting in leaked directories across runs or skipped cleanup phases if a test failed early. Failing to adhere to this typically results in Orphaned Artifacts / Resource Leaks.

Trap 1: Spawning detached temporary directories and manually maintaining cleanup blocks.

Don't:

def setUp(self):
    self.tempdir = tempfile.mkdtemp()

def tearDown(self):
    shutil.rmtree(self.tempdir, ignore_errors=True)

Do:

def setUp(self):
    self.tempdirobj = tempfile.TemporaryDirectory(prefix="forall_tests")
    self.tempdir = self.tempdirobj.name

def tearDown(self):
    self.tempdirobj.cleanup()

T5-23: Idiomatic String Evaluation in Mocked Test Streams

Rule: Avoid manually slicing and measuring length properties for string evaluations; consistently evaluate standard streams using built-in truthiness routines.
What: Evaluating outputs from captured stdout or stderr in tests must utilize Pythonic abstractions like splitlines() and implicit truthiness evaluation, rather than raw newline splitting and explicit length checks.
Applies To: Hermetic Test suites executing mock command line calls and inspecting sys.stdout.
Why: Legacy test code relied on manual string manipulation like split("\n") and boolean evaluations like len(line) > 0. This approach was unidiomatic, fragile against cross-platform line endings, and bloated test suite verbosity. Failing to adhere to this typically results in Fragile Tests / Unidiomatic Logic.

Trap 1: Iterating over raw string splits and manually validating line length to count valid output strings.

Don't:

line_count = 0
for line in mock_stdout.getvalue().split("\n"):
    if len(line) > 0:
        line_count += 1

Do:

line_count = sum(1 if x else 0 for x in mock_stdout.getvalue().splitlines())

T5-24: Hermetic Isolation of Git Configuration Directories in Tests

Rule: Always isolate Git initialization by injecting temporary dummy directories via environment overrides (HOME / USERPROFILE) to restrict test interference.
What: Tests utilizing local git operations must hermetically isolate environment variables like HOME and USERPROFILE to prevent developers' global configurations from leaking into test executions.
Applies To: Global test initialization routines (conftest.py, Pytest fixtures, class-level test setup).
Why: Historically, if a developer's global .gitconfig required GPG signing (commit.gpgsign=true), a git subprocess spawned during test setup would unexpectedly open an interactive prompt (e.g., vim) to solicit a signing key, causing the test suite to deadlock permanently. Failing to adhere to this typically results in Test Deadlock / State Bleed.

Trap 1: Relying on default system paths for the home directory during test execution.

Don't:

# BAD: Allows test subprocesses to read ~/.gitconfig
subprocess.run(['git', 'tag', '--annotate'])

Do:

# GOOD: Override environment variables to an isolated temp directory
@pytest.fixture(autouse=True, scope="session")
def alt_home(tmp_path_factory, monkeysession):
    var = "USERPROFILE" if platform_utils.isWindows() else "HOME"
    monkeysession.setenv(var, str(tmp_path_factory.mktemp("home")))

Exceptions: Tests that explicitly exist to validate the parsing logic of a user's real, existing .gitconfig (which is highly discouraged).

T5-25: Scope Hierarchy Enforcement in Pytest Fixtures

Rule: Never propagate lower-scoped test fixtures (e.g., function-scoped states) into global session-scoped initialization procedures.
What: A session-scoped Pytest fixture cannot depend on lower-scoped fixtures (such as a function-scoped tmp_path). To create temporary paths at the session level, tmp_path_factory.mktemp must be utilized.
Applies To: Pytest fixture definitions, specifically within conftest.py.
Why: A session-scoped setup mechanism attempted to use the function-scoped tmp_path fixture. This caused pytest initialization failures. Furthermore, reusing function-scoped directories for session-level state caused unintended disk thrashing as the directory was continually wiped at the teardown of every function. Failing to adhere to this typically results in Scope Mismatch / State Masking.

Trap 1: Injecting tmp_path into a fixture decorated with scope="session".

Don't:

# BAD: Session fixture depending on function fixture
@pytest.fixture(scope="session")
def session_tmp_homedir(tmp_path):
    return tmp_path

Do:

# GOOD: Using the factory to generate a session-level path
@pytest.fixture(scope="session")
def session_tmp_homedir(tmp_path_factory):
    return tmp_path_factory.mktemp("home")

T5-26: Strict Python Version Compatibility for Third-Party Plugins

Rule: Must strictly align test dependency injection configurations against the underlying project's absolute minimum supported Python versions.
What: Test tooling dependencies must strictly align with the project's minimum supported Python version. External plugin implementation logic should be inlined with proper attribution if the plugin requires a newer Python version than the project supports.
Applies To: Dependency management (tox.ini), conftest.py, and CI environments.
Why: A developer introduced a new Pytest plugin to manage isolated home directories. However, the plugin required Python 3.8+, while the project maintained compatibility down to Python 3.6+. This mismatch caused post-submit CI pipeline checks to crash upon test environment installation. Failing to adhere to this typically results in CI Installation Failure.

Trap 1: Adding modern third-party test dependencies to legacy-supported projects without validating the minimum required Python constraints.

Don't:

# BAD: tox.ini adds dependency unsupported by CI target
deps =
    pytest
    pytest-home >= 0.4

Do:

# GOOD: Inline the logic and remove the external dependency
# adapted from pytest-home 0.5.1
@pytest.fixture(autouse=True, scope="session")
def session_tmp_homedir(tmp_path_factory, monkeysession):
    # implementation logic here

Exceptions: When the project‘s minimum supported Python version is officially bumped to match the plugin’s requirement.

T5-27: Hermetic Isolation of Terminal UI Color Configurations

Rule: Must aggressively lock default terminal coloring states when generating UI tests to isolate regressions from variable environment pipelines.
What: Unit tests that assert on UI or logging text containing ANSI color codes must explicitly force the coloring state to prevent ambient git configurations or CI terminal limitations from altering test outcomes.
Applies To: Unit tests interacting with the color or repo_logging modules.
Why: Tests validating color outputs failed non-deterministically based on the ambient environment (e.g., CI runners lacking a TTY, or developers having color.ui=false in their global .gitconfig). Failing to adhere to this typically results in Flaky Tests / Non-Deterministic Failures.

Trap 1: Relying on the default state of the Coloring class without forcing it during setup, implicitly depending on environmental variables.

Don't:

def setUp(self):
    config_fixture = fixture("test.gitconfig")
    self.config = git_config.GitConfig(config_fixture)
    self.color = color.Coloring(self.config, "status")

Do:

def setUp(self):
    config_fixture = fixture("test.gitconfig")
    self.config = git_config.GitConfig(config_fixture)
    color.SetDefaultColoring("always")
    self.color = color.Coloring(self.config, "status")

T5-28: Abstract Intent Documentation in Unit Tests

Rule: Never embed exact literal assertions or explicit structural values directly inside unit test docstring commentary.
What: Test docstrings should describe the behavioral intent or structural coverage of the test rather than hardcoding the exact assertion values or low-level implementation details.
Applies To: Python unittest module docstrings.
Why: Docstrings that hardcoded exact ANSI escape sequences or config values became stale and misleading whenever the underlying mappings or formatting codes were updated. Failing to adhere to this typically results in Documentation Rot / Maintenance Overhead.

Trap 1: Documenting exact data values being asserted directly in the test docstring.

Don't:

def test_Color_Parse_first_parameter_none(self):
    """fg is black(30), bg is red(31+10=41), attr is ul(4)"""
    val = self.color._parse(None, 'black', 'red', 'ul')
    self.assertEqual('\x1b[4;30;41m', val)

Do:

def test_Color_Parse_first_parameter_none(self):
    """check fg & bg & attr"""
    val = self.color._parse(None, "black", "red", "ul")
    self.assertEqual("\x1b[4;30;41m", val)

Cross-Domain Dependencies

Upstream: T2 | Filesystem Atomicity & Worktree Layout - Managing safe creation and atomic removal of temporary worktrees and .git directories supports reliable test state fixtures.
Downstream: T3 | Subprocess Git Integration & Error Translation - Hermetic test constraints directly dictate how standard git streams, configuration injections, and subprocess executions are mocked or intercepted.
Downstream: T6 | CLI Argument Parsing & UX Consistency - Strict terminal environment control in testing enforces deterministic verification of CLI autocorrect flows and standardized UI colorizations.

Chapter: CLI Argument Parsing & UX Consistency

Context: This chapter governs the lifecycle, validation, and execution of command-line arguments, enforcing strict standardization for machine-readable serialization, unified logging, and deterministic, thread-safe terminal interactions.

Summary

Rules

T6-01: Deterministic Machine-Readable Output Formats

Rule: Always expose strict, structured serialization options (e.g., JSON) for CLI tools used in automated pipelines to prevent reliance on brittle text parsing.
What: CLI tools leveraged for inspection must expose strict, structured serialization (e.g., JSON) to negate reliance on unstable text parsing in CI/CD environments.
Applies To: Command-line interface design, specifically subcommands utilized by build pipelines or external orchestrators.
Why: Downstream automated pipelines were forced to retrieve configuration data by executing tools like repo info and using string matching (grep) against human-readable formatting, creating a brittle dependency susceptible to UI modifications. Failing to adhere to this typically results in Pipeline Parsing Failure.

Trap 1: Providing only aesthetically formatted terminal text for diagnostic commands.

Don't:

Command executes and prints human-readable strings. Consumers must run repo info | grep "Manifest revision" to extract variables.

Do:

Integrate a --format=json command argument that bypasses human-readable display logic and cleanly serializes underlying data models directly to standard output.

Exceptions: Interactive-only terminal prompts that are heavily stateful and fundamentally incompatible with automation.

T6-02: Elimination of Niche Short Options

Rule: Never assign single-character short options for niche or rarely used flags to preserve namespace availability.
What: Avoid assigning single-character short options for niche or rarely used command-line flags to prevent namespace bloat.
Applies To: CLI argument parsing (e.g., optparse or argparse definitions).
Why: Short options were being added arbitrarily for specific data summaries, leading to a cluttered short-option namespace and hindering future expansion. Failing to adhere to this typically results in Cluttered CLI UX.

Trap 1: Assigning a short option to a feature that isn't commonly used by developers.

Don't:

p.add_option("-s", "--summary", action="store_true")

Do:

p.add_option("--include-summary", action="store_true")

T6-03: Extensible Boolean Toggle Flags

Rule: Always implement paired positive/negative toggle flags for granular outputs rather than relying on rigid, single-purpose boolean switches.
What: Conditional output sections must be controlled by paired positive/negative toggle flags (e.g., --include-x and --no-include-x) rather than single-purpose boolean flags.
Applies To: CLI design and option definitions for output manipulation.
Why: Rigid, single-purpose flags for rendering specific summaries limited extensibility and prevented developers from precisely tailoring outputs for scripts or automation. Failing to adhere to this typically results in Rigid Output Rendering.

Trap 1: Creating rigid, single-purpose boolean switches for granular output components.

Don't:

p.add_option("--summary", action="store_true", help="show only manifest summary")

Do:

p.add_option("--include-summary", action="store_true", default=True)
p.add_option("--no-include-summary", dest="include_summary", action="store_false")

T6-04: Context-Aware CLI Output Separators

Rule: Must conditionally verify that succeeding content blocks will render before printing visual formatting elements like horizontal separators.
What: Structural visual elements, such as horizontal separators or newlines, must only be printed if the succeeding content block is actually verified to render.
Applies To: CLI output rendering logic.
Why: Output formatting logic printed hardcoded line separators after summaries, resulting in dangling visual artifacts when subsequent project details were dynamically excluded by user flags. Failing to adhere to this typically results in Redundant Output Noise.

Trap 1: Hardcoding visual formatting elements directly after a primary block instead of conditionally checking the next block.

Don't:

print_summary()
print_separator()
if include_projects:
    print_projects()

Do:

print_summary()
if not include_projects:
    return
print_separator()
print_projects()

T6-05: Output Format Standardization via Enums

Rule: Must manage and standardize target output formats using an explicit Python Enum class rather than brittle string matching.
What: Command-line output formats (like TEXT or JSON) must be managed using an explicit Python Enum class and standardize formatting behaviors (e.g., disabling pagers for JSON, enforcing sort keys).
Applies To: CLI data serialization and OutputFormat dispatching.
Why: Unstandardized JSON serialization methods across subcommands resulted in inconsistent output spacing, failure to suppress interactive pagers, and brittle string-based format matching. Failing to adhere to this typically results in Broken Machine Automation.

Trap 1: Handling output formatting via raw string matching and print() statements without suppressing the interactive pager.

Don't:

if opt.format == "json":
    print(json.dumps(data))

Do:

class OutputFormat(enum.Enum):
    TEXT = enum.auto()
    JSON = enum.auto()

if output_format == OutputFormat.JSON:
    sys.stdout.write(json.dumps(data, indent=2, sort_keys=True) + "\n")

T6-06: Symmetric Execution Dispatching

Rule: Must symmetrically separate procedural logic into format-specific handlers rather than mixing inline logic with format conditional statements.
What: Complex commands handling multiple output formats must keep the main execution function clean by symmetrically dispatching logic to format-specific handlers (e.g., _ExecuteText and _ExecuteJson).
Applies To: Command entry points (e.g., the Execute method in subcommand classes).
Why: Inline output rendering mixed with conditional format dispatching created monolithic execution methods that were difficult to maintain and test. Failing to adhere to this typically results in Spaghetti Logic.

Trap 1: Mixing one output format's dispatch call with the inline procedural logic of the fallback format.

Don't:

def Execute(self, opt, args):
    if opt.format == 'json':
        return self._ExecuteJson(opt, args)
    # 50 lines of inline text rendering logic...

Do:

def Execute(self, opt, args):
    if opt.format == OutputFormat.JSON:
        self._ExecuteJson(opt, args)
    else:
        self._ExecuteText(opt, args)

T6-07: CLI Argument Simulation over Internal State Mutation

Rule: Always inject simulated execution parameters into the command-line argv arguments list rather than manually overwriting parsed attribute states during tests.
What: When executing a command programmatically within a test, operational parameters (e.g., job counts) must be injected into the simulated argv string list rather than manually mutating the parsed options object.
Applies To: Command testing and execution harnesses where parameters alter system behavior (e.g., multiprocessing threads).
Why: Test cases were bypassing the CLI parser and manually setting opts.jobs = 1 to force single-threaded execution. This prevented the test from exercising the command's full option-parsing and validation lifecycle. Failing to adhere to this typically results in Test Invalidation / False Positives.

Trap 1: Overriding a configuration parameter directly on the parsed namespace object.

Don't:

opts, args = cmd.OptionParser.parse_args(argv)
cmd.CommonValidateOptions(opts, args)
opts.jobs = 1
cmd.Execute(opts, args)

Do:

opts, args = cmd.OptionParser.parse_args(argv + ["--jobs=1"])
cmd.CommonValidateOptions(opts, args)
cmd.Execute(opts, args)

T6-08: Deterministic Sorting of Parallel Execution Results

Rule: Must explicitly sort accumulated payload data from asynchronous thread loops before broadcasting standard outputs or logs.
What: Elements collected asynchronously from a multiprocessing pool must be explicitly sorted before being emitted to standard output or logging pipelines.
Applies To: Command-line output rendering and any loop iterating over aggregated parallel results.
Why: Warning messages regarding bloated project directories were printed based on the order that asynchronous threads returned their payload, leading to flaky UX and non-deterministic command outputs. Failing to adhere to this typically results in Non-Deterministic Console Output.

Trap 1: Iterating over a list of accumulated multi-process results without sorting.

Don't:

for project_name in self._bloated_projects:
    logger.warning(f'warning: Project {project_name} is bloated.')

Do:

for project_name in sorted(self._bloated_projects):
    logger.warning(f'warning: Project {project_name} is bloated.')

T6-09: Strict Adherence to Feature Requests

Rule: Never implement speculative bypass flags, preemptive CLI overrides, or configurations without a documented and verifiable user request.
What: Avoid preemptively implementing fallback features, command-line overrides, or configurations without a documented user request.
Applies To: CLI argument parsers and manifest parsing engines.
Why: Speculative feature additions, like bypass flags for manifest limits, caused feature creep and bloated the CLI toolset with unused, undocumented edge cases. Failing to adhere to this typically results in Feature Creep.

Trap 1: Adding a --force-jobs CLI bypass flag simultaneously alongside the new sync-j-max manifest attribute.

Don't:

Implementing an override flag in main.py just in case users want to bypass the newly added manifest restriction.

Do:

Deploying the manifest constraint directly without a CLI override, and documenting it in docs/manifest-format.md. Wait for a bug tracker request for an override flag.

T6-10: Minimal Logic in Exception Handlers

Rule: Must restrict try...except blocks entirely to targeted error handling and never bury structural flow control or terminal loops within them.
What: Exception handling blocks (try...except) must only contain the specific error-handling logic and never encapsulate complex business logic, terminal loops, or major execution routing.
Applies To: Command dispatcher routing and system control flows.
Why: Burying significant CLI routing tasks—like spelling suggestions, autocorrection countdowns, and subprocess recursion—inside a KeyError handler obscured program logic and suppressed nested tracebacks. Failing to adhere to this typically results in Opaque Routing State.

Trap 1: Placing the entire typo-correction logic, user prompts, and command reloading inside except KeyError:.

Don't:

try:
    cmd = self.commands[name]()
except KeyError:
    # 50 lines of complex autocorrection logic
    return self._RunLong(...)

Do:

if name not in self.commands:
    name = self._autocorrect_command_name(name)
    if not name:
        return 1
# Execute minimal command logic outside exception
cmd = self.commands[name]()

T6-11: Unified Logging Protocol via Standard Logger

Rule: Dispatch all console messages using the standard centralized logging framework. Never rely on raw print statements mapped directly to terminal error streams.
What: All console outputs must be dispatched via the system's centralized RepoLogger mechanism, completely replacing raw terminal standard-error prints.
Applies To: CLI presentation logic and application-wide console communications.
Why: The use of raw print(file=sys.stderr) bypassed the system's unified logging configuration, resulting in unformatted outputs and broken terminal interactivity. Failing to adhere to this typically results in Unmanaged Output Spillage.

Trap 1: Outputting warnings directly to the sys.stderr stream using the built-in print function.

Don't:

print(f"WARNING: You called a command named '{name}'", file=sys.stderr)

Do:

logger.warning("You called a repo command named '%s'", name)

Exceptions: Prompts that require strict, synchronously unformatted interaction via input().

T6-12: Strict Translation of Configuration Booleans

Rule: Always cast string-based Git configurations to lowercase and map them strictly against Git's recognized array of boolean equivalents.
What: Parsers querying Git configuration properties must implement strict hardcoding to safely capture Git's expansive list of non-standard boolean string representations (e.g., ‘yes’, ‘on’, ‘true’, ‘off’).
Applies To: Configuration parsers mapping user git config state to internal state variables.
Why: Using standard integer casts on retrieved git configuration strings caused runtime failures when users specified string-based boolean values sanctioned by the official Git system. Failing to adhere to this typically results in Configuration Casting Panic.

Trap 1: Wrapping string-typed configuration variables inside int() and catching value exceptions to deduce defaults.

Don't:

try:
    autocorrect = int(autocorrect)
except ValueError:
    autocorrect = 0

Do:

autocorrect = str(autocorrect).lower()
if autocorrect in ("0", "false", "off", "no", "show"):
    autocorrect = 0
elif autocorrect in ("1", "true", "on", "yes", "immediate"):
    autocorrect = -1

T6-13: Interruptibility of Interactive Runtime Delays

Rule: Must wrap artificially injected execution delays in handlers that correctly detect and gracefully back out of operations upon a keyboard interrupt signal.
What: Any thread-blocking delay generated by the system (e.g., autocorrection warning timers) must actively handle thread interrupt signals so users can gracefully cancel impending state mutations.
Applies To: Thread blocks, specifically interactive or automated execution delays.
Why: Forced sequential time sleeps preceding an automated command execution trapped users. If a user detected an erroneous correction during the sleep interval, initiating a keyboard interrupt would fail to abort execution, resulting in destructive command routing. Failing to adhere to this typically results in Irrevocable Execution.

Trap 1: Placing raw time suspension code before a return statement without wrapping it in a trap for cancellation overrides.

Don't:

time.sleep(delay)
return assumed_command

Do:

try:
    time.sleep(delay)
except KeyboardInterrupt:
    return None
return assumed_command

T6-14: Strict Pre-Access Initialization of CLI Options

Rule: Never retrieve attributes from a parsed options namespace until the shared subcommand option validation layer has fully completed processing.
What: Command-line option attributes (e.g., copts.verbose) must strictly be accessed only after the options object has fully passed through the common subcommand validation layer.
Applies To: CLI entry points (main.py) and option routing blocks.
Why: Accessing attributes on the configuration options object prior to invoking CommonValidateOptions (which populates global defaults) resulted in AttributeError crashes, completely breaking primary application functions like repo sync. Failing to adhere to this typically results in AttributeError / Process Crash.

Trap 1: Consuming CLI option flags before the shared initialization pipeline populates default properties.

Don't:

git_trace2_event_log.verbose = copts.verbose
cmd.CommonValidateOptions(copts, cargs)
cmd.ValidateOptions(copts, cargs)

Do:

cmd.CommonValidateOptions(copts, cargs)
cmd.ValidateOptions(copts, cargs)
git_trace2_event_log.verbose = copts.verbose

T6-15: Omission of Redundant Optparse Destinations

Rule: Avoid manually injecting dest= bindings in argument declarations if the implicit destination perfectly mirrors the target variable name.
What: Do not explicitly pass dest= arguments in CLI option definitions if the value identically matches optparse's implicit fallback behavior (which strips leading dashes and replaces inner dashes with underscores).
Applies To: Command-line argument definitions using the optparse library within subcmds/.
Why: Developers repeatedly specified dest= overrides that exactly mirrored the auto-generated variable name, adding unnecessary boilerplate and triggering automated regression test failures. Failing to adhere to this typically results in Code Bloat / Test Failure.

Trap 1: Redundantly configuring destination variables in CLI arguments.

Don't:

p.add_option(
    "--no-interleaved",
    dest="no_interleaved",
    action="store_false",
)

Do:

p.add_option(
    "--no-interleaved",
    action="store_false",
)

T6-16: Subcommand Scoping in Commit Messages

Rule: Always prefix your commit messages with the explicit CLI subcommand or modular subsystem the patch directly targets.
What: Git commit message subject lines must explicitly scope changes to the CLI subcommand or module they affect (e.g., sync: or project: ).
Applies To: Commit message authoring.
Why: Vague commit messages such as ‘Default to interleaved mode’ obfuscated history. Scoping was mandated to ensure maintainers could quickly trace changes within the mono-repo. Failing to adhere to this typically results in Unclear Git History.

Trap 1: Writing global-sounding commit messages for isolated subcommand patches.

Don't:

Default to interleaved mode

Do:

sync: Default to interleaved mode

Exceptions: Changes that structurally alter the core framework or affect multiple commands concurrently.

T6-17: Module-Scoped Exception Naming

Rule: Declare isolated module exceptions natively as Error instead of prefixing them with the resident module's name.
What: Custom exceptions specific to a module (especially subcommands) should be named Error rather than prefixing them with the module name.
Applies To: Module-level exception definitions, specifically within subcommand implementations.
Why: Developers inconsistently named module exceptions (e.g., WipeError in wipe.py), violating the style guide and polluting the namespace upon import. Failing to adhere to this typically results in Style Guide Violation / API Inconsistency.

Trap 1: Redundantly prefixing the exception name with the module name inside the module definition.

Don't:

# inside wipe.py
class WipeError(RepoExitError):
    pass

Do:

# inside wipe.py
class Error(RepoExitError):
    pass

T6-18: Explicit Terminology in Destructive CLI Commands

Rule: Must rigorously define nuanced Git terminology natively within the terminal help output if a command has destructive capabilities.
What: Destructive commands must clearly define ambiguous terms (like ‘worktree’) in their help documentation and provide concrete usage examples.
Applies To: CLI help text, specifically helpDescription properties in Command subclasses executing file deletion.
Why: Users misunderstood destructive operations because terminology overlapped with internal Git jargon, increasing the risk of accidental repository deletions. Failing to adhere to this typically results in Accidental Data Loss.

Trap 1: Using terms like ‘worktree’ in help text without explicit definition or examples.

Don't:

helpDescription = “The repo wipe command removes the specified projects from the worktree.”

Do:

helpDescription = “The repo wipe command removes the specified projects from the worktree (the checked out source code)... \n\nExamples:\n %prog ...”

T6-19: Fail-Fast Argument Validation in Subcommands

Rule: Must perform strict threshold checks for required argument counts at the validation layer before the command is permitted to execute.
What: Validation checks for mandatory command-line arguments must occur in the ValidateOptions hook rather than within the core Execute block.
Applies To: All Command subclasses.
Why: Performing validation in the execution body delayed error feedback, triggering unnecessary setup logic and violating the command lifecycle architecture. Failing to adhere to this typically results in Delayed Execution Errors / Architectural Inconsistency.

Trap 1: Checking for missing positional arguments directly inside Execute().

Don't:

def Execute(self, opt, args):
    if not args:
        raise UsageError("no projects specified")
    # ... execution logic

Do:

def ValidateOptions(self, opt, args):
    if not args:
        self.OptionParser.error("no projects specified")

def Execute(self, opt, args):
    # ... execution logic safely assumes valid args

T6-20: Respect Global Verbosity Flags

Rule: Always wrap non-critical progress prints or logging updates behind global verbosity checks.
What: Commands must remain silent by default. Output must be guarded by checking the global opt.verbose flag, preventing log noise during standard execution.
Applies To: All Command subclasses executing file operations or progress updates.
Why: Commands were unconditionally printing internal states (e.g., directory deletions), cluttering terminal output for users expecting standard silent behavior. Failing to adhere to this typically results in Terminal Spam / UX Degradation.

Trap 1: Unconditionally printing progress messages to stderr or stdout.

Don't:

print(f"Deleting objects directory: {objdir}", file=sys.stderr)

Do:

if opt.verbose:
    print(f"Deleting objects directory: {objdir}", file=sys.stderr)

T6-21: Thread-Safe Terminal Querying during IO Capture

Rule: Must query terminal geometry using a safely cached baseline file descriptor to prevent crash failures when the standard error stream is redirected to memory buffers.
What: Background threads that query terminal properties (like width) must reference a persistently cached file descriptor of the original sys.stderr, as the global stream can be temporarily redirected to in-memory objects lacking file descriptors.
Applies To: progress.py, UI monitoring loops, and subprocesses executed during stderr/stdout redirection.
Why: When running parallel processes that captured output into an io.StringIO() buffer, a background progress thread attempting to dynamically measure the terminal width via sys.stderr.fileno() triggered an io.UnsupportedOperation: fileno exception, crashing the entire UI loop. Failing to adhere to this typically results in Thread Crash / io.UnsupportedOperation.

Trap 1: Directly querying sys.stderr.fileno() inside long-running concurrent loops.

Don't:

# BAD: Fails if stderr is mocked to memory
col = os.get_terminal_size(sys.stderr.fileno()).columns

Do:

# GOOD: Querying a module-level cached reference mapped at boot
col = os.get_terminal_size(_STDERR.fileno()).columns

T6-22: Global State via Persistent Configuration over Environment Variables

Rule: Never propagate novel OS-level environment variables to modify executable behavior, strictly enforcing the existing .repoconfig/config framework instead.
What: Global user or CI settings (e.g., Git-LFS defaults) must be managed using the explicit .repoconfig/config framework. Do not introduce novel OS-level environment variables to modify executable behavior.
Applies To: Configuration parsers, command-line argument mapping, and option handlers across the CLI.
Why: A contributor attempted to add a REPO_GIT_LFS environment variable to ease CI automation. Maintainers rejected the patch, citing that the proliferation of disjointed environment variables builds technical debt compared to maintaining a single, consistent file-based configuration layer. Failing to adhere to this typically results in Configuration Fragmentation / Tech Debt.

Trap 1: Adding environment variable mappings into command-line parsers to support ‘headless’ configurations.

Don't:

Read configuration defaults from os.environ.get("REPO_GIT_LFS") alongside CLI flags.

Do:

Instruct users to provision a ~/.repoconfig/config file in automated setups instead of changing process environment logic.

Exceptions: Legacy environment variables (e.g., REPO_URL) remain supported for backward compatibility but are explicitly designated as technical debt.

T6-23: CLI Command Parity with Git Semantics

Rule: Tightly bind local CLI utility semantics and argument definitions to match analogous Git operations explicitly.
What: Internal CLI command responsibilities and user-facing terminology must align with established Git semantics and project-wide conventions.
Applies To: Command-line interface modules (e.g., subcmds/status.py, subcmds/info.py).
Why: Global metadata (like superproject commit IDs) was mistakenly added to repo status, which cluttered the local working-tree status. Additionally, the term ‘hash’ was used instead of the standard ‘revision’. Failing to adhere to this typically results in UX Inconsistency / Clutter.

Trap 1: Adding project-wide or environmental metadata queries to commands designed for local working-tree analysis.

Don't:

Injecting global repository state (e.g., --superproject-hash) into the status command, which breaks parity with git status.

Do:

Relocating environmental and global metadata display to the info command, integrating it cleanly into the default summary header.

Trap 2: Using non-standard terminology for Git object identifiers in user-facing CLI flags.

Don't:

# BAD: Using 'hash' which is inconsistent with existing repo terminology
p.add_option("--superproject-hash", help="print the superproject hash")

Do:

# GOOD: Using 'revision'
p.add_option("--superproject-rev", help="print the superproject revision")

T6-24: Context-Aware Sync Operation Termination

Rule: Must catch and defer local sync iteration failures gracefully rather than forcing immediate unrecoverable exits across all active streams.
What: Non-critical update failures during local sync iterations must set an error flag but must only force a hard exit if the user explicitly provided the --fail-fast CLI argument.
Applies To: All local project iteration workflows within subcmds/sync.py.
Why: A single project failing an isolated sub-task (like a symlink update) would immediately trigger sys.exit(1), aborting the entire pipeline for all other projects and ignoring standard error aggregation strategies. Failing to adhere to this typically results in Premature Pipeline Termination.

Trap 1: Triggering an immediate system exit on an update failure.

Don't:

if self.UpdateCopyLinkfileList():
  print('\nerror: Local update copyfile or linkfile failed.', file=sys.stderr)
  sys.exit(1)

Do:

if not self.UpdateCopyLinkfileList():
  err_event.set()
  err_update_linkfiles = True
  if opt.fail_fast:
    print('\nerror: Local update copyfile or linkfile failed.', file=sys.stderr)
    sys.exit(1)

T6-25: Explicit Exit Codes on CLI Cancellation

Rule: Always force the process to yield a non-zero exit code if an interactive confirmation prompt is declined or cancelled.
What: Interactive CLI workflows that are cancelled or declined by the user must immediately return a non-zero exit code to signal termination to parent processes.
Applies To: All interactive subcommands (gc, sync with prompts, etc.).
Why: If a tool cleanly exited (returning None / 0) after a user denied a destructive action prompt, shell automation wrappers and CI systems falsely assumed the destructive cleanup successfully completed. Failing to adhere to this typically results in Silent Automation Failure.

Trap 1: Using an empty return to back out of a declined CLI confirmation prompt.

Don't:

ask = input("Proceed? [y/N] ")
if ask.lower() != "y":
    return

Do:

ask = input("Proceed? [y/N] ")
if ask.lower() != "y":
    return 1

T6-26: Non-Interactive Safeties for Destructive Commands

Rule: Must implement explicit automation bypass flags for any command capable of irreversibly destroying system state.
What: Any command capable of destroying state (e.g., garbage collection, wiping local commits) must provide both a --dry-run and a --yes (or --force) flag to support safe CI automation.
Applies To: Command-line definitions for potentially destructive operations.
Why: Tools that rely exclusively on input() prompts block automated scripts indefinitely. By explicitly enforcing non-interactive modes, scripts can safely trigger automated cleanups or log dry-runs without freezing. Failing to adhere to this typically results in Automation Hang / CI Pipeline Freeze.

Trap 1: Hardcoding interactive prompts without checking configuration flags for automation modes.

Don't:

Prompting the user with input('Proceed?') unconditionally before executing a destructive delete.

Do:

Checking if opt.dry_run: to skip execution, or if opt.yes: to skip the confirmation prompt.

T6-27: Strict Interdependent Argument Validation

Rule: Always enforce strict prerequisites checks when parsing command-line flags intended to modify or inherit context from other flags.
What: Command-line options that exist solely to modify or qualify another specific command-line option must be explicitly validated to raise an error if their prerequisite option is absent.
Applies To: CLI argument parsing routines (subcmds/init.py, ValidateOptions).
Why: A new --manifest-upstream-branch argument was added to qualify a provided commit hash. However, without strict validation, users could supply it independently, leading to ambiguous or unhandled configuration states. Failing to adhere to this typically results in CLI Ambiguity / Unhandled Option State.

Trap 1: Accepting dependent modifier options unconditionally without checking prerequisites.

Don't:

Accepting --manifest-upstream-branch in the CLI parser and directly assigning it to the internal state regardless of the presence of --manifest-branch.

Do:

Explicitly checking the parsed options and raising an OptionParser.error if the modifier is present but the target --manifest-branch is null.

T6-28: Actionable CLI Error Messages for Synchronization Failures

Rule: Embed direct, explicit commands and flag references within terminal error payloads to rescue users from failing execution states.
What: When a terminal failure occurs during CLI operations, the error payload must provide specific, actionable flags or manual steps for the user to resolve the failing state.
Applies To: Sync command error reporting (subcmds/sync.py, project.py).
Why: When a local branch had published commits but was behind upstream, the sync failed with a generic ‘punt’ message, leaving users unaware that passing --rebase could force the sync. Failing to adhere to this typically results in User Friction / Dead End.

Trap 1: Failing a sync operation with a descriptive but non-actionable error message.

Don't:

fail(LocalSyncFail(
    "branch %s is published (but not merged) and is now %d commits behind"
    % (branch.name, len(upstream_gain)),
    project=self.name,
))

Do:

fail(LocalSyncFail(
    "branch %s is published (but not merged) and is now %d commits behind. "
    "Fix this manually or rerun with the --rebase option to force a rebase."
    % (branch.name, len(upstream_gain)),
    project=self.name,
))

T6-29: Launcher Script Version Upgrades for Important Changes

Rule: Increment the VERSION tuple within the standalone repo launcher script when making important or significant functional changes.
What: If you modify the standalone repo launcher file with an important or large change (e.g., updates to Python version checks, significant logic fixes, or major argument parsing updates), you must increment the VERSION = (X, Y) tuple.
Applies To: Standalone repo launcher script, release engineering workflows.
Why: The repo source code entry point (main.py) compares the runner wrapper version to the copy of the launcher in the source tree to prompt users to upgrade. If an important launcher modification is submitted without a corresponding version bump, users executing older wrappers won't receive the required update prompts. Failing to adhere to this typically results in Launcher Version Skew / Missing Update Warnings.
Exceptions: Minor or cosmetic changes (like fixing typos in comments or minor docstrings) do not require a version bump.

Trap 1: Submitting a major or important patch to the repo launcher script without updating its VERSION definition.

Don't:

# Making significant changes to Python version handling logic
# but leaving VERSION unchanged:
VERSION = (2, 65)

Do:

# Incrementing the minor version tuple to signal the update:
VERSION = (2, 66)

Cross-Domain Dependencies

Upstream: T1 | Concurrent Synchronization & IPC - CLI rendering relies on sorted, deterministic results emitted from asynchronous multiprocessing pools.
Downstream: T3 | Subprocess Git Integration & Error Translation - Command-line terminology and state configurations must tightly align with wrapped Git subprocess semantics.

Chapter: Repo Hooks Framework

Context: The Repo Hooks Framework governs the execution, parameter validation, and lifecycle management of user-defined scripts within the repository ecosystem. It ensures seamless integration of extensions like post-sync or pre-upload while strictly isolating their execution failures from core operational workflows.

Summary

Rules

T7-01: Explicit Registration of Hook Parameters in API Contract

Rule: Always register new metadata or arguments intended for user-defined lifecycle scripts within the internal _API_ARGS validation dictionary.
What: Any new metadata or argument passed into user-defined lifecycle scripts must be explicitly defined in the hook's internal validation dictionary (_API_ARGS).
Applies To: Repo Hooks Framework (hooks.py and Hook Runners).
Why: New arguments sent to the post-sync execution context triggered internal exceptions because the core application strictly validates **kwargs against a registered list of supported arguments. Failing to adhere to this typically results in Runtime Hook Failure.

Trap 1: Passing a new variable to hook.Run() without whitelisting it in the _API_ARGS map.

Don't:

# In subcmds/sync.py
hook.Run(repo_topdir=self.client.topdir, sync_duration_seconds=dur)

# In hooks.py (Missing registration)
_API_ARGS = {
    'post-sync': {'repo_topdir'}
}

Do:

# In hooks.py
_API_ARGS = {
    'post-sync': {'repo_topdir', 'sync_duration_seconds'}
}

T7-02: Floating-Point Precision for Hook Telemetry Metrics

Rule: Must pass system telemetry and operational duration metrics to downstream extension hooks as precise floating-point values.
What: System telemetry, specifically operational duration metrics, must be passed to downstream extension hooks as granular floating-point values. The executing system must not truncate or cast telemetry to integers.
Applies To: Performance telemetry generation and hook contexts.
Why: Internal performance timers originally cast their delta outputs to integers before emitting the context to the hook system. This caused a loss of sub-second precision, limiting telemetry usefulness. Failing to adhere to this typically results in Telemetry Data Loss.

Trap 1: Casting float calculations directly to integers before passing to downstream systems.

Don't:

sync_duration_seconds = time.time() - start_time
self._RunPostSyncHook(opt, sync_duration_seconds=int(sync_duration_seconds))

Do:

sync_duration_seconds = time.time() - start_time
self._RunPostSyncHook(opt, sync_duration_seconds=sync_duration_seconds)

T7-03: Non-Blocking Post-Sync Hook Integration

Rule: Never allow post-operation custom hooks to block or alter the success state of the core operation.
What: Custom hooks executed strictly after an operation finishes (like post-sync) must run via the encapsulated RepoHook framework and be entirely non-blocking, so their failure does not negatively affect the core operation's recorded success state.
Applies To: subcmds/sync.py and RepoHook integrations executing after repository data modifications.
Why: Teams needed a reliable way to provision local Git commit-msg hooks even if developers bypassed repo upload with manual git push. A post-sync hook was added, but the rigid requirement was that if the user script failed, the actual project synchronization must still report as a success to avoid corrupting UX. Failing to adhere to this typically results in Blocked Sync Operations.

Trap 1: Using raw subprocess calls to run custom scripts and tying the command‘s exit code to the main tool’s exit code.

Don't:

# BAD: Post-action failure corrupts the sync outcome
if subprocess.run(["run_hooks.sh"]).returncode != 0:
    raise SyncError("Hook failed")

Do:

# GOOD: Managed execution safely wrapping and suppressing exceptions
hook = RepoHook("post-sync")
hook.Run() # Designed strictly to warn on failure

T7-04: Dynamic CLI Option Injection for Repo Hooks

Rule: Always utilize the RepoHook option group injection framework for command-line integration rather than synthesizing dummy configuration objects.
What: Command-line integration for new hooks must utilize the RepoHook option group injection framework rather than synthesizing dummy configuration objects.
Applies To: Repo Hooks Framework; specifically subcommand definitions involving hook invocations (e.g., subcmds/sync.py).
Why: When integrating the post-sync hook, a developer initially created a minimal ‘DummyOpt’ object to satisfy parameter checks because the sync command lacked native hook flags. This diverged from standard hook architecture and could cause attribute errors. Failing to adhere to this typically results in AttributeError / Option Divergence.

Trap 1: Bypassing standard option parsers by passing a mock object with hardcoded values into hook factory methods.

Don't:

# BAD: Using a dummy object to satisfy missing CLI arguments
class DummyOpt:
    bypass_hooks = False
    allow_all_hooks = True
    ignore_hooks = True

hook = RepoHook.FromSubcmd(opt=DummyOpt(), ...)

Do:

# GOOD: Injecting the standard hook option group into the parser
RepoHook.AddOptionGroup(parser, 'post-sync')

# Later during execution, pass the actual resolved options:
hook = RepoHook.FromSubcmd(opt=actual_opt, ...)

T7-05: Boolean Evaluation for Repo Hook Execution Status

Rule: Always evaluate hook execution success using the boolean return value of Run(), never by wrapping the call in external exception handlers.
What: Execution wrappers for Repo Hooks must rely on the boolean return value of Run() to evaluate success, rather than wrapping the call in broad try/except Exception blocks.
Applies To: Repo Hooks Framework; specifically the invocation layer for any hook type (post-sync, pre-upload).
Why: A developer wrapped the hook execution in a broad Exception catch block. The hook framework inherently traps and standardizes exceptions internally; wrapping it externally duplicates logic, masks context, and leads to silent failure processing. Failing to adhere to this typically results in Swallowed Exceptions / Logic Duplication.

Trap 1: Wrapping a framework execution method in a generic exception handler.

Don't:

# BAD: Redundant outer try/except block swallowing state
try:
    hook.Run()
except Exception as e:
    print(f"Warning: hook failed: {e}")

Do:

# GOOD: Relying on the established boolean return contract
success = hook.Run()
if not success:
    print("Warning: hook reported failure.")

T7-06: Universal Trigger Specification for Execution Lifecycle

Rule: Must document the exact, literal programmatic triggers for hooks rather than relying on idealized use-cases like full checkouts.
What: Hook documentation must strictly delineate the exact success states that trigger execution, explicitly noting conditions where operational state (like a local checkout) might be bypassed or skipped.
Applies To: Repo Hooks API contracts and documentation (docs/repo-hooks.md).
Why: Users of the new post-sync hook might assume it only runs after a complete workspace checkout. However, it effectively fires on any zero-exit-code sync operation, including network-only fetches where the local filesystem state remains completely untouched. Failing to adhere to this typically results in State Assumption Bugs.

Trap 1: Documenting hook triggers based on an idealized use-case (a full checkout) rather than the literal programmatic trigger.

Don't:

Documenting that the hook is to install post-processing tasks specifically for successful full checkouts.

Do:

Documenting that the hook runs when repo sync completes without errors, explicitly warning that this does not guarantee all projects were synced or checked out (e.g., repo sync -n performs network fetches only).

T7-07: Forward-Compatible kwargs for Repo Hooks Signatures

Rule: Always design hook script entrypoints to accept variable keyword arguments (**kwargs) to ensure forward compatibility with future framework expansions.
What: All repository hook script entrypoints (main functions) must accept variable keyword arguments (**kwargs) to maintain forward compatibility with future API augmentations.
Applies To: Repo Hooks scripts and internal examples (e.g., post-sync.py, pre-upload.py).
Why: Historically, if the repo tool evolved to pass new environmental context (like repo_topdir), user-defined scripts with strict function signatures would crash with a TypeError. Failing to adhere to this typically results in TypeError / API Breakage.

Trap 1: Defining a hook entrypoint with a strict argument list that cannot gracefully handle new inputs.

Don't:

# BAD: Strict signature that will crash on API update
def main(repo_topdir=None):
    print("Running tasks...")

Do:

# GOOD: Accepts forward-compatible **kwargs
def main(repo_topdir=None, **kwargs):
    """
    Args:
      kwargs: Leave this here for forward-compatibility.
    """
    print("Running tasks...")

Cross-Domain Dependencies

Upstream: T6 | CLI Argument Parsing & UX Consistency - The dynamic hook injection framework fundamentally relies on standardized option parsers to map CLI flags cleanly into hook runner contexts.