Files
Kendall Eberly 868560348d fix(branchsync): correct custody recovery eligibility (#814)
* no-mistakes: apply CI fixes

* no-mistakes(document): Clarify recovery anchor conflict handling
2026-08-21 21:54:34 -07:00

76 KiB

AGENTS.md

This file is for agentic coding tools working in this repo.

This repository is a Go CLI app named no-mistakes. The binary entrypoint is cmd/no-mistakes; implementation code lives under internal/, and the package names there are the layout map (CLI in internal/cli, daemon in internal/daemon, pipeline and steps in internal/pipeline, agent adapters in internal/agent, terminal UI in internal/tui, shared infrastructure in internal/git, internal/ipc, internal/config, internal/db, internal/paths, internal/types). Build, test, and release commands are owned by the Makefile; read it for the full target list instead of relying on a copy here.

Safest local verification sequence after non-trivial changes:

  • gofmt -w .
  • make lint (generated-skill drift check plus go vet)
  • go test -race ./... (the e2e suite is behind the e2e build tag and excluded)
  • make e2e when touching agent integrations, the e2e harness, or recorded fixtures
  • go build -o ./bin/no-mistakes ./cmd/no-mistakes

Fork Routing

  • repos.upstream_url is the parent repository used for PR base routing; repos.fork_url is an optional GitHub fork push target.
  • no-mistakes init --fork-url <url> expects origin to point at the GitHub parent repository and <url> at the contributor fork; plain no-mistakes init preserves an existing fork URL on idempotent refresh.
  • Push and CI auto-fix push code must resolve the push URL via resolvePushURL (internal/pipeline/steps/common_git.go) so configured forks still receive branch updates; the non-fork path recovers the credentialled upstream from the worktree's origin remote at run time because the DB upstream_url is stored redacted (see Credential Redaction below). Repo.PushURL() remains correct only for fork-only callers (e.g. rebase.go), since fork URLs carry no embedded credentials.
  • GitHub PR code must keep --repo pointed at the parent and use --head <fork_owner>:<branch> when fork_url is set; existing-PR lookup must list by the bare branch and filter head-owner fields, never pass <owner>:<branch> to gh pr list --head.
  • Non-GitHub fork MR/PR routing is intentionally out of scope until implemented end to end; if a legacy row has fork_url for another provider, PR creation must skip instead of opening a self PR.
  • Every new run best-effort refreshes registered upstream/fork URLs from the working clone through gate.RefreshRepoURLs: origin is the upstream authority, an existing fork requires one uniquely matching clone remote, both DB fields replace atomically, and every discovery/validation/write failure logs only a bounded reason and continues with the exact old registration. The refresh never rewrites clone or gate remotes; Repo.URLsVerified is run-scoped evidence that trusted fetch/push may use the refreshed DB URL instead of an inherited stale gate origin.

Credential Redaction in Stored URLs and Errors (security)

  • gate.InitWithFork runs the upstream URL through safeurl.Redact before every DB persist (UpdateRepoMetadata*, InsertRepoWithIDAndFork) and the "gate initialized" log line; the bare gate's origin remote still carries the full credentialled URL (via provisionGate) so carved worktrees authenticate. Because the DB copy is redacted, push and CI auto-fix push must recover the credential from the worktree's origin remote at run time (resolvePushURL/resolveUpstreamURL), never from Repo.UpstreamURL/Repo.PushURL().
  • Step-failure errors (executor.go FailStep/log/IPC emit) and the Bitbucket resolve-repo error are redacted via safeurl.RedactText/safeurl.Redact so a credentialled URL wrapped into an error can never reach a step log or runs.error. Reuse internal/safeurl for new redaction sites rather than adding a git-local helper; it is already wired into git.Run/step git-run error formatting.
  • Regressions: TestInitRedactsCredentialURL, TestResolveUpstreamURL_PreservesCredential, TestResolveUpstreamURL_FallsBackToRecordedURL, TestResolvePushURL_ForkWinsOverCredential.

GitLab Backend (internal/scm/gitlab)

  • The backend is pinned against glab v1.5x, whose flag surface drifts between versions: the auth check must be host-scoped (--hostname <host>, falling back to unscoped only when the host is unknown), glab mr list no longer accepts --state opened, and the daemon's detached-HEAD worktree breaks glab ci get, so pipeline jobs are read via the branch-independent glab api .../pipelines/<id>/jobs REST endpoint.
  • The comments in internal/scm/gitlab/gitlab.go own the full rationale for each trap; extend them there when you hit new glab version drift.

Documentation

  • Keep README.md concise and high-level; the bar needs to be extremely high for what shows up there.
  • Most documentation lives in docs/, the published docs site.
  • One owner per fact: docs/src/content/docs/reference/global-config.md and docs/src/content/docs/reference/repo-config.md own configuration keys, docs/src/content/docs/reference/environment.md owns environment variables and the telemetry local/remote split, docs/src/content/docs/concepts/daemon.md owns the daemon lifecycle model, and guides pages explain purpose and link to those owners instead of restating tables and examples.
  • The document.instructions block in .no-mistakes.yaml states this ownership map for the pipeline's document step; update it when ownership moves.

Agent-Guidance Surfaces

  • skills/no-mistakes/SKILL.md is generated: the source of truth is the body constant in internal/skill/skill.go. Edit the body, then make skill; make lint fails CI on drift. Never edit SKILL.md directly. no-mistakes init ships this rendering to agents at user level.
  • Agent-driving guidance is owned by the skill body and the live axi output strings (internal/cli/axi*.go); docs/src/content/docs/guides/agents.md carries only the canonical invariant sentences pinned by internal/cli/axi_guidance_test.go plus a pointer to the skill. When you change driving guidance, change the skill body and the point-of-use axi strings together; that drift test is the sync check.
  • The shared default test-quality rule lives in internal/testguidance; render it only into the task-first skill and pipeline roles that can author, repair, or review tests. Its fake-agent prompt tests are the intentional generated-interface contract, not source-text checks.
  • Review auto-fix is disabled by default (auto_fix.review: 0 in config.go autoFixDefaults), so blocking and ask-user review findings park for an agent decision; keep the skill, the live axi gate note, and docs qualified if you touch review auto-fix.

Unified Agent Tuning (internal/agentcfg)

  • agentcfg is the single owner of the harness-neutral model/effort surface and of the mapping down to each harness's native mechanism (claude/copilot --effort, codex -m + -c model_reasoning_effort, grok --reasoning-effort, pi --thinking, opencode's session-message model/variant, acpx --model for cursor/acp:<target>). Add a harness there, not in an adapter or in eval. rovodev and antigravity are deliberately declared unmappable, so a request for them is a config error rather than a flag that is silently ignored.
  • agent.NewWithOptions is the one funnel: it validates Options.Profile and splices the mapped args after the operator's raw agent_args_override args, so both the pipeline (cfg.AgentProfileFor) and eval replay (Candidate.Profile()) reach every harness by the same path. Never re-derive a model or effort flag at a call site.
  • Precedence is fixed: a raw agent_args_override flag that already pins a knob natively wins and the mapped value is not emitted, which is what keeps every pre-agent_config configuration byte-identical and stops a harness receiving one knob twice. agent_config is global-only for the same reason as agent_args_override.
  • Eval candidates are agent,model=<model>[,effort=<level>] (the previous agent+model spelling is refused with a migration message), effort is part of the persisted candidate identity, and agentNeutralGlobalConfig strips agent, agent_args_override, and agent_config so a replay never inherits the capturing machine's pins.
  • Regressions: internal/agentcfg, internal/agent/profile_test.go, internal/config/config_agent_config_test.go, internal/daemon/pipeline_agent_profile_test.go, TestParseCandidate*, TestReplayPinsCandidateModelAndEffortOnTheHarness, TestCaptureStripsEveryHarnessPinFromThePinnedConfig.

Context, Concurrency, and Processes

  • Thread context.Context through long-running, subprocess, and networked work; prefer exec.CommandContext; use derived contexts and timeouts for cleanup and HTTP calls.
  • Route every long-lived subprocess spawned for a cancellable step or agent invocation through shellenv.ConfigureShellCommand(cmd): it creates a process-tree boundary and installs cmd.Cancel to kill the whole tree, so grandchildren (test workers, build watchers) cannot outlive cancellation and hold the next run's worktree locked.
  • cmd.Cancel covers only cancellation; on clean exit or error the group is not reaped, and leaked grandchildren accumulate until the OS OOM-kills the daemon (surfacing as daemon crashed during execution with no stack trace). Use shellenv.RunShellCommand / OutputShellCommand / CombinedOutputShellCommand for one-shot commands, or StartShellCommand plus TerminateShellCommandGroup when handling pipes manually; the helper doc comments in internal/shellenv own the details. ConfigureShellCommand also installs a 5s cmd.WaitDelay backstop so a grandchild holding an inherited pipe cannot wedge cmd.Wait forever. Regressions: TestCodexAgent_Run_ReapsLeakedGrandchildOnCleanExit, TestRunShellCommandWithEnv_ReapsGrandchildOnCleanExit, TestTerminateShellCommandGroup_*.
  • A process group is a lineage container, not a sandbox: a descendant that calls setsid(2)/setpgid(2) (agent CLIs sandboxing their tool runners, any daemonizing worker script) leaves the group, and after its parent exits nothing lineage-based can name it again - it burns CPU and holds a deleted worktree's cwd forever. internal/procreap is the identity-based backstop: it matches a process by the run worktree its cwd resolves under (deliberately never argv, which a legitimate git worktree remove also carries), never touches pid<=1/itself/its ancestors, spares worktrees whose run is still pending or running, and escalates SIGTERM to SIGKILL only after a grace period. Reach is <NM_HOME>/worktrees by path shape plus exactly the run worktrees a caller names from run records (Options.Worktrees), never a configured worktree root by shape - an operator's own directory is unmatchable unless a run row names it. Every site that removes a run worktree sweeps it first through procreap.SweepRunWorktree(s) (run cleanup and setup failure via RunManager.removeRunWorktree, startup cleanup, eject), scoped and therefore without age floor or run-active check; the unscoped startup sweep in recoverOnStartup keeps the orphanProcessMinAge floor. All best effort. Windows needs none of this - job objects contain the whole tree - so the platform layer reports an empty table. Regressions: internal/procreap, TestSweepOrphanRunProcessesReapsFinishedRunAndSparesActiveOne, TestSweepRunWorktreeProcessesReapsLeakedChildAtRunCleanup, TestTerminateShellCommandGroup_AsksBeforeKilling, TestTerminateShellCommandGroup_EscalatesWhenSIGTERMIsIgnored.
  • On Windows the daemon runs console-less, so route every console child through winproc.Harden(cmd) (no-op elsewhere, idempotent, preserves existing creation flags) or a console window flashes per child (#287). shellenv.ConfigureShellCommand already calls it; one-shot commands built directly must call it themselves. Regressions: TestHarden* in internal/winproc.
  • Protect shared mutable state with the standard sync/atomic tools, and be explicit about ownership and cleanup of goroutines, worktrees, temp dirs, and channels.

Recursive Gate-Execution Containment

  • internal/gatecontext is the single classifier for recursive pipeline control. It combines canonical registered gate common-directory identity with OS-authenticated IPC peer ancestry; NO_MISTAKES_GATE is diagnostic only. CLI preflight, daemon mutation ingress, gate init/eject, branch-sync mutation, and the managed pre-receive hook must all keep using that owner so marker removal, cwd changes, and direct pushes cannot bypass refusal. Read-only AXI status/logs, help, and doctor remain available. Regressions: internal/gatecontext, TestGateStepCannotStartRecursivePipeline.
  • Every pipeline agent prompt receives the phase boundary from internal/gateguidance, and the generated user-level skill reuses the same owner. Step agents return only their assigned phase; the outer executor alone controls other validation, push, PR, and CI phases. Edit internal/skill/skill.go, then run make skill; never edit the generated skill directly.

Filesystem and Paths

  • Use filepath.Join; respect NM_HOME for app state; directories are 0o755 and files 0o644 by convention.
  • On macOS, path comparisons may need symlink resolution (/var vs /private/var); use worktrees.Canonical/worktrees.Contains wherever run worktree paths are compared, so one spelling matches everywhere.
  • Run worktree placement (worktree_roots) is owned by internal/worktrees. Configuration decides it exactly once, at run creation (Layout.Dir in RunManager.startRunWithIntentSource), and the result is persisted in runs.worktree_dir; every later consumer - resume, step diff, startup cleanup, procreap, eject, gatecontext attribution - must read it back through worktrees.RecordedDir and never re-derive it from config, so a mid-flight edit can neither strand a parked run nor point a removal at a directory the run never used. An empty column means the default <NM_HOME>/worktrees/<repoID>/<runID>.
  • worktrees.CheckPlacement is the single policy for an unusable root (inside NM_HOME, inside any registered checkout); config.ValidateWorktreeRoots owns what the config can judge alone. The daemon refuses to start on an unusable placement, so init --worktree-root must refuse exactly the same set or it prints a paste that takes the operator's CLI down, and EVERY init refuses to register a checkout that contains a configured root - the same state reached from the other direction. User-facing semantics live in docs/src/content/docs/reference/global-config.md. Regressions: internal/worktrees, internal/config/config_worktree_roots_test.go, internal/daemon/worktree_roots_test.go, internal/gate/eject_sweep_test.go, internal/cli/init_test.go.

Git on Bare Gate Repos (safe.bareRepository)

  • Agent harnesses and hardened CI inject safe.bareRepository=explicit, which forbids cwd-based discovery of bare repositories. Route every gate git call through git.Run, which detects a bare git dir and prepends --git-dir=<dir>; never shell out to git in a bare gate repo relying on cmd.Dir or -C discovery (issue #362).
  • Startup gate migration is DB-authoritative with a strict validated <id>.git legacy fallback; it must reject non-gates before hook or Git mutation and use git.RunBare so a malformed directory cannot discover an ancestor worktree. Completed migrations carry the content-versioned gate-config stamp and normal restarts must stay filesystem-only for current gates. Regressions: TestMigrateGateConfigsRejectsInvalidDirectoriesAndSkipsCurrentGates, TestColdDetachedStartupProductionGateCardinality.
  • Regressions: TestRunOnBareRepoUnderSafeBareRepositoryExplicit, TestWorktreeAddRemoveOnBareRepoUnderSafeBareRepositoryExplicit, TestInitUnderSafeBareRepositoryExplicit.

gh PR-Targeting From the Bare Gate Repo (internal/scm/github)

  • The daemon runs gh from the detached bare gate repo whose HEAD is the default branch, so every PR-targeting command must name the exact PR explicitly: an empty positional makes gh pr <verb> infer the cwd branch (main) and return no pull requests found for branch main even when the feature PR's checks are green. GetChecks, GetPRState, GetMergeableState, and UpdatePR route through the shared prSelector (number, else URL, else fail closed) — never append a bare pr.Number/pr.URL that can be empty. This is the gh analogue of the git bare-gate-repo trap above.
  • Regressions: TestGetChecksTargetsKnownPRByURLWhenNumberMissing, TestPRTargetingReadsFailClosedWithoutIdentity, TestPRStateAndMergeableTargetKnownPRByURL, TestUpdatePRTargetsKnownPRByURLWhenNumberMissing, TestUpdatePRFailsClosedWithoutIdentity.

Post-Receive Hook Gate Path Resolution (internal/git/hook.go)

  • The hook's --gate value must never come from a bare $(pwd): Git can invoke post-receive from a cwd that collapses to . (issue #269), which the daemon rejects and the pipeline silently never starts. The hook script resolves an absolute gate dir (git first, hook location fallback), and normalizeNotifyGatePath in internal/cli/daemon_cmd.go is an independent second layer that absolutizes whatever an already-installed older hook sends.
  • Regressions: TestPostReceiveHook_ResolvesAbsoluteGateDir, TestPostReceiveHook_FallsBackToHookLocationForGateDir, TestNormalizeNotifyGatePathResolvesLegacyDotGate.

Daemon Singleton Lock (internal/daemon/lock.go)

  • Only one live daemon may own an NM_HOME: an exclusive OS file lock on <NM_HOME>/daemon.lock is acquired as the very first action in RunWithOptions, strictly before stale-run recovery and socket bind, and held for the process lifetime. The kernel releases it on any process death, so a held lock always means a live holder and no staleness heuristic is needed. Without it, a second daemon stole the socket and ran global crash recovery against the live daemon's runs and worktrees.
  • Process launch is not readiness: the PID record is published after the singleton lock and before exclusive recovery, while startup succeeds only after a real IPC health response. The 45s production budget covers cold environment setup and recovery; early exits fail promptly, timeout cleanup reaps detached children before fallback or rollback, and managed plus detached failures retain both causes. Regressions: TestStartDetachedDaemonDetectsChildExitPromptly, TestStartDetachedDaemonTimeoutKillsAndReapsChild, TestStartPreservesManagedAndDetachedFallbackErrors, TestColdDetachedStartupProductionGateCardinality.
  • A successful stop means the daemon process is gone, not merely that IPC health has disappeared, because only process exit releases the singleton lock. Capture the daemon instance before requesting shutdown, and close the shutdown client before waiting because the daemon drains in-flight handlers during exit. See waitForDaemonStop and stopDetachedDaemon; regressions: e2e TestDaemonStopLeavesNoDaemonProcessOwningTheRoot, TestDaemonRestartReplacesTheDaemonWithExactlyOneOwner.
  • Independent layers: internal/ipc listen() dials the socket before unlinking it and refuses to steal a live one; client probes bound the dial with daemon_connect_timeout and fail fast on a dead or wedged socket instead of starting a replacement daemon (EnsureDaemon surfaces the error with a daemon start recovery hint; the health RPC itself is bounded separately by ipc.DefaultDialTimeout).
  • Daemon execution is explicit-only (no-mistakes daemon run --root); never let inherited environment reinterpret probes like --version or status as daemon workers.
  • Startup worktree cleanup is DB-aware: never remove a worktree whose run row is pending or running; startRun inserts the run row before creating the worktree, so a no-row directory is safe to remove immediately. That no-row rule holds only inside <NM_HOME>/worktrees, which is discovered by walking because no-mistakes owns it; a configured worktree root is the operator's directory, so cleanup and eject there act on exactly the recorded run worktrees and never enumerate anything else.
  • The user-facing model lives in docs/src/content/docs/concepts/daemon.md; the lock rationale lives in the internal/daemon/lock.go and daemon.go comments. Regressions: TestAcquireSingletonLock_*, TestRunWithResources_SecondDaemonForSameRootFailsWithoutStealingSocket, TestRunWithOptions_RequiresSingletonLockBeforeRecovery, TestRecoverOnStartup_DoesNotDeleteActiveRunWorktree, TestServe_SecondListenerForLiveSocketDoesNotStealIt, TestDialConnectTimeoutFailsFastAndNamesSocket, TestIsRunningFailsFastWhenSocketAcceptsButDoesNotRespond, TestIsRunningSurfacesExistingDeadSocket, TestDaemonRunRootFromArgs_EnvDoesNotForceDaemonModeForProbes, TestValidateDaemonPIDFallback_RefusesToKillOwnProcess.

Bounded Daemon Logging and Event-Driven AXI Runs

  • internal/logstore owns all daemon-process byte and retention bounds. Lifecycle output uses logs/daemon.log, managed Rovo Dev/OpenCode stdout and stderr use logs/managed-server.log, and service bootstrap/direct crash output uses logs/daemon-bootstrap.log. Rotation snapshots backups and truncates the current inode in place so held service and child descriptors keep writing to the bounded current file. Regressions: internal/logstore/rotate_test.go, TestDetachedDaemonUsesBoundedDedicatedLogSinks, TestManagedServerOutputIsSeparatedFromLifecycleFailureSummary.
  • Successful read-only IPC methods are DEBUG; mutations and stream starts are INFO; every request failure is WARN. AXI run driving is subscribe-first and internal/cli/run_reconciler.go is the sole owner of event reconciliation, reconnect, duplicate-event coalescing, and the slow lost-event heartbeat. Do not reintroduce fixed-interval get_run polling. Regressions: TestSuccessfulReadRequestsDoNotLogAtInfo, TestRequestLoggingKeepsMutationsAndFailuresVisible, TestDriveRun_HealthyWaitStaysWithinRequestBudget, TestRunReconciler_*.

Bounded Loss-Aware Event Subscriptions

  • internal/ipc/events.go (ClassOf) is the single event taxonomy: activity is droppable, state is not, control is broker-generated, and an unrecognized type fails safe to state. Brokers and consumers must read loss tolerance from it rather than re-listing event names.
  • internal/daemon/eventmailbox.go is the single overflow owner: a per-subscriber ring bounded by 64 events and 1 MiB, non-blocking publish (the executor is never stalled), activity as the only evictable class, and everything else folded into one sticky coalescing stream_gap that drains ahead of queued payload. A reserved slot is not enough - it fails at the second simultaneous transition - and producer-side channel receives race the reader, which is why the queue is a ring under a mutex.
  • Every state event and every get_run snapshot carries a monotonic StateRev; runSnapshot samples it before the DB read, which is sound only because every producer writes state and then emits. Consumers apply a delta only when its revision is newer, so a delta queued before a snapshot cannot regress state after it. Every subscription opens gapped, so attach and reconnect always reconcile first.
  • The fix-review working-tree diff is the only gate context that is never persisted, so it is served on demand by ipc.MethodGetStepDiff (RunManager.StepDiff, bounded at 512 KiB) instead of riding the stream: it was the only unbounded payload, and one frame past the 1 MiB transport line limit ends the subscription and hides every later event.
  • Regressions: internal/daemon/eventmailbox_test.go (A1-A13 plus the byte/count ceilings), TestRunSnapshot_*, TestStepDiff_*, TestExecutor_StateEventsAreEmittedAfterTheirDatabaseWrite, TestClassOfUnknownEventFailsSafeToState, TestRunReconciler_StreamGapForcesOneAuthoritativeRead, TestSubscribeOversizedFrameEndsTheStreamAndHidesLaterEvents, internal/tui/overflow_contract_test.go.

Destructive Daemon Lifecycle Guard (internal/lifecycle/guard.go)

  • daemon stop, daemon restart, and update refuse by default while pending/running runs exist (the daemon is machine-wide, so stopping it can fail every active pipeline), list the runs via the shared lifecycle.ActiveRuns/lifecycle.RunList helpers, and require an explicit --force. update -y answers only the different-executable prompt and deliberately does not bypass this guard.
  • Every invocation of the three commands is logged with caller attribution (PID, PPID, parent command line) via logLifecycleInvocation to <NM_HOME>/logs/cli.log; this is the incident forensic trail, do not remove or weaken it.
  • Regressions: TestDaemonStopRefusesWithActiveRunsAndListsThem, TestDaemonStopForceOverridesActiveRunGuard, TestDaemonRestartRefusesWithActiveRuns, TestLifecycleCommandsWriteCallerAttributionToCLILog (internal/cli/daemon_lifecycle_test.go), TestUpdaterRunRefusesWithActiveRunsAndListsThem, TestUpdaterActiveRunGuardAllowsForce (internal/update).

Testing Conventions

  • Prefer e2e tests for behavior that crosses a process or I/O boundary (CLI flags, config loading, git operations, agent spawning, daemon coordination, stdout/stderr, recorded fixtures); unit-test pure helpers where speed and failure localization matter. Prefer creating real git repos in temp dirs over heavy mocking.
  • The e2e suite is behind the e2e build tag; make e2e runs scripts/e2e.sh, which sweeps ./internal/e2e/... and ./internal/pipeline/steps/..., so keep new step-local e2e tests behind the tag too.
  • Temporary e2e daemons (NM_TEST_START_DAEMON=1 / harness) are owned by internal/e2edaemon: exact inventory, concurrency cap (NM_E2E_DAEMON_MAX, default 2), bounded argv checks, and reapers in harness Cleanup, package TestMain, and scripts/e2e.sh EXIT/INT/TERM. A SIGKILL of the wrapper shell does not run its trap; next-run inventory recovery covers that. External sleep-loop keepalives are out of scope. Never point inventory reaping at the shared ~/.no-mistakes service. Regressions: internal/e2edaemon/*_test.go.
  • Packages whose tests shell out to git unset GIT_CONFIG_COUNT in TestMain so ambient GIT_CONFIG_* injection from agent harnesses cannot leak in; a test exercising injected config re-sets it with t.Setenv (see internal/git, internal/gate, internal/daemon, internal/pipeline/steps).
  • Packages whose tests can start a daemon or touch ambient state (cmd/no-mistakes, internal/cli, internal/update) use a package-wide TestMain that points NM_HOME and HOME at fresh temp dirs and disables telemetry/update-check env vars, so a full test run never touches a real ~/.no-mistakes. Follow the same pattern in new such packages.
  • paths.New() refuses the default ~/.no-mistakes root under go test; tests that touch app state must set NM_HOME to a temp dir, and only the production-default path test may opt in with NO_MISTAKES_ALLOW_DEFAULT_ROOT_IN_TESTS=1.
  • Isolate filesystem and environment state with t.TempDir() and t.Setenv().
  • The Windows CI leg is process-spawn bound, not compute bound: git-backed packages cost roughly 10x their Linux time (internal/git 5.7s -> 53s, internal/branchsync 31s -> 415s). The Windows matrix is split into a git-heavy shard and a core remainder so each job's wall stays inside timeout-minutes: 40 and a hang still surfaces as go test -timeout (15m) rather than an evidence-free job cancel. Keep long git-heavy packages off the serial critical path (internal/branchsync runs t.Parallel() for exactly that reason) and keep the Defender scan-exclusion step in ci.yml, whose comment owns the rationale. Regressions: TestCIWorkflow_WindowsTestsRunWithScanExclusions, TestCIWorkflow_WindowsHangSurfacesAsGoTimeoutNotJobCancellation.
  • Go applies an implicit GOOS constraint from a filename suffix, so a test file named *_windows_test.go (or _linux, _darwin) silently compiles only on that platform. Name platform-agnostic tests about Windows something else.
  • On macOS a git-heavy package under -race intermittently reports git <cmd>: signal: segmentation fault. That is not a git or repo bug: ~/Library/Logs/DiagnosticReports/*.ips records the crash as procName: <pkg>.test, parentProc: <pkg>.test, asi: "crashed on child side of fork pre-exec" - the forked child dies before execve. Confirm there before chasing it in Go code; the CI legs are Linux and Windows. The same fork mechanic explains a stray <pkg>.test -test.timeout=... process at high CPU that appears to ignore its own deadline: a pre-exec child inherits the parent's name, argv, and cwd, so it is not a running test binary and no test-side timeout applies to it. internal/procreap reaps those by cwd.

Repo Config Trust Boundary (security)

  • The daemon runs commands.* from .no-mistakes.yaml verbatim via sh -c, and agent selects which process launches with the maintainer's credentials. The code-executing selection fields (commands.{test,lint,format} and agent) are therefore loaded from the trusted default branch at a pinned SHA resolved by a fresh fetch, never from the pushed SHA. The run aborts when the trusted commit or its present config cannot be read and parsed; a readable tree with no config is valid. See internal/daemon/manager.go startRun, loadTrustedRepoConfig, and assertGateTrustedConfigReadable.
  • document.instructions (the repo's documentation placement policy), review.path_instructions (path-scoped review guidance appended to the review prompt), disable_project_settings (the gate-agent project-instruction opt-out), no_ci (positive declaration that the repository intentionally has no CI), and ci.rerun_transient (how many times a transiently failed check may be re-run) are also trusted-only, regardless of allow_repo_commands: a pushed branch must not weaken any of those boundaries, self-declare no-CI to bypass checks, or steer its own review; enabling the commands opt-in must not drop the maintainer's own trusted values; and every re-run ci.rerun_transient authorizes bills another provider-side workflow run to the repository, so a contributor must not be able to raise it (the operator's own global ci.rerun_transient is a separate, non-contributor surface that the trusted repo value still overrides). When the opt-out is enabled, only adapters with verified effective suppression may launch. Other non-executing fields (ignore_patterns, auto_fix, commit, intent, test) are still read from the pushed branch.
  • Selecting which trusted config applies to a run must never depend on a pushed-branch field. review.path_instructions is matched against the COMPLETE changed-file set, never the ignore_patterns-filtered subset, because filtering there lets a contributor suppress a maintainer's rule from their own review by ignoring its glob. reviewablePaths (internal/pipeline/steps/common_diff.go) answers only "does this run have anything to work on".
  • pr.base_branch (the PR, rebase, and CI-merge-conflict-auto-fix integration branch, falling back to Repo.DefaultBranch when unset) is trusted-default-branch-only, but unlike the fields in the bullet above it is the deliberate exception that also honors the allow_repo_commands: true opt-in, since it controls where an already-maintainer-authorized PR lands rather than what executes. Once a PR already exists, its actual forge base branch (read live via scm.PRBaseBranchReader) is authoritative for CI merge-conflict repair and base-branch tip monitoring over a since-changed pr.base_branch, and PR lookup matches the existing PR by branch alone, never filtered by base, so a later config change updates that PR instead of opening a duplicate against the new base. Full semantics are owned by docs/src/content/docs/reference/repo-config.md (pr.base_branch). Regressions: TestEffectiveRepoConfig_PRBaseBranchTrustedOnly, TestEffectiveRepoConfig_PRBaseBranchOptInUsesPushedValue, TestEffectiveRepoConfig_PRBaseBranchOptInWithNoTrustedCopyUsesPushedValue, TestLoadRepoConfig_PRBaseBranchRejectsInvalidBranchName, TestLoadRepoConfig_PRBaseBranchEmptyIsValid, TestPRStep_UsesConfiguredBaseBranch, TestRebaseStep_UsesConfiguredPRBaseBranch, TestCIStep_AutoFixUsesExistingPRBaseAfterConfigChanges, TestPRStep_ExistingPRAgainstDifferentBaseIsUpdatedNotDuplicated.
  • allow_repo_commands is per-repo, read only from the trusted default-branch copy, and defaults false; a contributor cannot self-enable it from a pushed branch. The e2e harness models a trusted single-developer environment and commits allow_repo_commands: true via SetupOpts.AllowRepoCommands; security tests pass false.
  • Regressions: TestLoadTrustedRepoConfig_FailClosedOnFetchFailure, TestLoadTrustedRepoConfig_PinnedSHAReadsFreshDefaultBranch, TestEffectiveRepoConfig_DocumentPolicyTrustedOnly, TestEffectiveRepoConfig_ReviewPathInstructionsTrustedOnly, TestMatchPathInstructions_PushedIgnorePatternsCannotSuppressTrustedRule, TestReviewStep_PushedIgnorePatternsCannotSuppressPathInstructions, TestEffectiveRepoConfig_DisableProjectSettingsTrustedOnly, TestEffectiveRepoConfig_CIRerunTransientTrustedOnly, TestAssertGateTrustedConfigReadable_*, TestNewPipelineAgent_OptOut_*, TestLoadRecoveredConfig_BoundsFetchAndFailsClosed, e2e TestRepoConfigCommandsFromDefaultBranch (incl. pushed_branch_cannot_self_enable), e2e TestReviewPathInstructionsJourney.

CI Monitor Lifecycle

  • ci_timeout is an idle timeout, not an absolute deadline: only timeoutAnchor re-arms when the upstream default-branch tip advances, started stays fixed for poll pacing, and re-arm only ever extends the deadline (fail-safe on transient base-tip failures). Value semantics (0 unset, negative unlimited sentinel, keyword parsing) live in config.go; keep config.DefaultCITimeout and defaultConfigYAML in sync (TestDefaultConfigYAML_MatchesGoDefaults). User-facing semantics are owned by docs/src/content/docs/reference/global-config.md.
  • GitHub readiness is the union of the exact current PR head commit's check rollup and every Actions workflow run returned by the Actions API for that same SHA. A workflow rejected before creating jobs/check-runs is absent from the commit rollup but still present in that API; run discovery errors and unknown run states fail closed instead of certifying a green rollup. Regressions: TestGetChecksIncludesFailedWorkflowRunMissingFromPRRollup, TestGetChecksBindsRollupAcrossABAHeadMovement, TestCIStep_FailedHeadWorkflowRunPreventsChecksPassed.
  • CI readiness never treats an unproven empty forge check list as green. Ready requires observed all-green checks, or trusted default-branch no_ci: true with zero registered checks (internal/pipeline/steps/ci.go decides whether the declaration applies; internal/cimonitor owns the agent-facing log vocabulary and Ready/DeclaredNoCI parse). Delayed registration, pending checks, failures, errors, unknowns, and stale-head evidence stay not-ready; registered checks on a declared no-CI repo are still honored. Regressions: TestChecksPassed_PR607RealLogSequence, TestCIStep_EmptyChecksWithoutNoCIStaysNotReadyPastOldGracePeriod, TestCIStep_EmptyChecksWithTrustedNoCIBecomesReady, TestCIStep_DelayedCheckRegistrationStaysNotReadyUntilGreen, TestCIStep_DeclaredNoCIWithUnexpectedChecksHonorsThem, TestEffectiveRepoConfig_NoCITrustedOnly.
  • Reap an orphaned monitor from outside its worktree with no-mistakes axi abort --run <id>; it needs only NM_HOME and never starts a stopped daemon. A known run succeeds only with durable terminal truth, a recorded nonterminal run fails unconfirmed, and only an unknown id is an idempotent no-op. Bare axi abort stays worktree/branch-scoped.
  • A merged or closed PR observation transactionally completes an active run and its CI step; PR lifecycle state is monotonic, so duplicate or delayed observations cannot reactivate or regress a terminal run. Startup reconciles legacy pending or running rows that already hold terminal PR state before parked-run planning and generic crash recovery. Regressions: TestUpdateRunPRStateFinalizesActiveTerminalOutcomes, TestUpdateRunPRStateIgnoresDuplicateAndDelayedRegressions, TestReconcileTerminalPRRunsFinalizesLegacyActiveRows, TestRecoverOnStartup_FinalizesLegacyTerminalPRRun, e2e TestTerminalPRRunDisappearsFromActiveListing.
  • A provider-reported cancelled check is never a job verdict, so the deterministic rerun runs strictly before any CI fix round: it is the only outcome that earns a rerun, a check cancelled again after its budget parks as ask-user instead of entering the auto_fix.ci loop, and any genuine or unrecognized failure or merge conflict in the same poll suppresses reruns so real failures still escalate on their first observation. The budget is per check name per run and spent on request rather than on success, and a rerun is never issued once the published branch head no longer equals runs.head_sha, because it would certify a commit this run never delivered. Each outstanding rerun records its verified pipeline head and the same-name provider links visible when it was requested; it retires durably when the run head advances or a new conclusive non-cancel link appears. Retirement keeps the spent budget and never changes check buckets. A delayed same-named green sibling can satisfy the link trigger on the same head, matching the default branch's existing name-keyed masking; removing that limitation requires provider truth outside this policy. Classification, the deliberate TIMED_OUT/STALE exclusions, rollup-lag grace, and retirement live in internal/pipeline/steps/ci_transient.go; provider support is the optional scm.CheckRerunner (GitHub only) and user-facing semantics are owned by docs/src/content/docs/reference/repo-config.md. Regressions: TestCIStep_CancelledCheckIsRerunBeforeEscalating, TestCIStep_CancelledCheckStaysUnresolvedAfterItsBudget, TestCIStep_LaggingRerunRollupKeepsWaitingForTheRepublishedCheck, TestCIStep_SameHeadGreenRerunEmitsChecksPassed, TestCIStep_DelayedSameNameCheckRetainsLegacyNameBehavior, TestCIStep_ResolvedRerunDoesNotParkALaterGreenHead, TestRetireResolvedReruns, TestRetireResolvedRerunsRetriesAfterPersistenceFailure, TestCIStep_MovedPublishedHeadTerminatesInsteadOfRerunning, TestCIStep_MovedPublishedHeadClearsCIReadiness, TestClassifyCheckFailure.
  • Terminal is not pending. Readiness must reject every non pass/fail/skip bucket (hasUnresolvedChecks), but only checks that can still finish on their own (hasPendingChecks) may keep the monitor polling. A cancel bucket - GitHub CANCELLED, GitLab canceled, Bitbucket STOPPED, and how GitHub reports a job killed by its own timeout-minutes - is a published conclusion that nothing will replace, so with no rerun outstanding it parks at ciUnresolvedCancelledOutcome (cancelledWithoutRerun) instead of waiting. Conflating the two is the #628 regression that hung real runs for their whole ci_timeout; an unrecognized bucket is deliberately still treated as waiting, because unknown is not evidence of terminal. Regressions: TestCIStep_CancelledCheckAmongPassingChecksEscalatesInsteadOfPollingForever, TestCIStep_ZeroRerunBudgetEscalatesCancelledCheckWithoutMakingItReady, TestCIStep_BitbucketStoppedCheckParksForADecision.
  • CI readiness is read from the provider's live PR head check rollup on every poll, so it always describes the head the forge currently has for that PR; no recorded SHA gates it, and a run whose row still names a pre-advance commit must still recognize green at the head the pipeline last pushed. Regression: TestCIStep_GreenChecksAtAdvancedHeadAreRecognizedWhileRunTracksOlderHead.

Parked / Awaiting-Agent Signal

  • runs.awaiting_agent_since is non-nil iff a step is actually parked at an awaiting_approval/fix_review gate: the executor sets it on gate entry, clears it when waitForApproval returns, and RecoverStaleRuns clears it on crash recovery. It is observability only (rendered as awaiting_agent: parked <duration> in axi status) and never changes gate resolution, auto-resume, or the --yes default.
  • Tests: internal/db/run_test.go, internal/pipeline/executor_approval_test.go, internal/cli/axi_test.go, e2e TestAxiParkedAwaitingAgentSignal.

Review-Loop Agent Sessions (internal/pipeline/sessions.go)

  • Per run, the review loop keeps ONE durable fixer session across review-fix turns, and EVERY review turn (initial review and every full rereview) runs session-free. A rereview certifies fixes implementing the previous review turn's findings, so resuming any review session seats the prescriber as certifier - the mechanism that let one fix round ship wrong code plus the test blessing it with zero findings. Cross-round review context travels only in the explicit sanitized round history; the fixer session is never lent to review turns, no other step uses sessions, and sessions are keyed strictly by run. The rereview prompt reframes fix-round changes as pipeline-authored code under the author-grade adversarial standard (fixRoundProvenanceClause); the same clause is emitted on a later run's initial review when a persisted uncertified range is bound. Prior findings, fix summaries, and same-round tests are claims, not evidence.
  • Fail-safe rules: unsupported adapter runs cold; a failed fixer resume drops the identity and re-runs the same turn in a fresh fixer session, never skipping the turn; a cancelled ctx gets no fallback retry; session_reuse: false forces everything cold. Persistence is minimum metadata only, never prompts or transcripts; SessionRoleReviewer remains only so crash recovery accepts legacy persisted rows, which are never resumed.
  • codex exec resume has a narrower flag surface than codex exec, so an unsupported override fails the resume and falls back; the e2e fakeagent must keep parsing both codex argv shapes (extractCodexPrompt).
  • Regressions: internal/pipeline/sessions_test.go, internal/pipeline/steps/review_session_test.go (incl. TestReviewLoop_RereviewNeverResumesTheSessionThatPrescribedItsFixes), TestReviewStep_RereviewTreatsFixRoundsAsPipelineAuthoredCode, internal/agent/session_test.go.

Recorded Human Decisions on Findings

  • Approve, skip, and abort each record selected_finding_ids = "[]" plus selection_source = user_declined on a gated round with findings (executor.go recordDeclinedRound, db.SetStepRoundDeclined); a round with no findings records no decision. The conditional write must never erase an existing selection. User-facing semantics are owned by docs/src/content/docs/reference/pipeline-steps.md.
  • A decline is stored as the COMPLEMENT of the selection, never as its own list; declinedFindingLines derives it and deliberately excludes auto_fix selections, whose complement is findings still awaiting a decision (rendered under auto_fix_left_unselected, which carries no do-not-re-report instruction).
  • roundHistoryPromptSection (internal/pipeline/steps/round_history.go) now carries three parts: this step's rounds, this run's OTHER steps' decisions, and earlier runs' decisions on this branch (bound per step by pipeline.BindBranchDecisions, unlike review-only BindUncertifiedPipelineRange). Nothing clears branch decisions - a completed review deletes the uncertified range, which is why that channel could not carry a decision forward, but approving a gate IS the decision. The prompt states that a recorded decision SUPERSEDES the user-intent wording.
  • Deliberately ADVISORY and fail-open: no step is blocked and no commit is gated, so an agent may still re-raise a declined finding when the code genuinely changed. There is no reversion detector; assertPipelineHeadContinuity and assertReviewApprovedPushHead remain lineage-only. ci_fix.go and rebase.go build prompts without roundHistoryPromptSection, so they do not receive decisions.
  • Regressions: TestExecutor_GateResolutionsWithoutASelectionRecordTheDecline, TestExecutor_GateResolutionWithNoFindingsRecordsNoDecision, TestExecutor_FixResolutionStillRecordsAUserSelection, internal/db/round_decisions_test.go, TestDeclinedFindingReachesALaterStepInTheSameRun, TestDeclinedFindingReachesALaterRunOnTheSameBranch, TestCompletedReviewDoesNotClearBranchDecisions, TestAutoFixComplementIsNeverPresentedAsAUserDecision.

Uncertified Review Provenance (internal/pipeline/uncertified.go)

  • When a review-step fixer round commits and its re-review does not complete, persist the per-branch uncertified range (from_sha, to_sha). Persist on review-step fixer commits only, not lint or document. On the next run's initial review, bind that range and emit fixRoundProvenanceClause even when Fixing==false, so the replacement reviewer is not cold. Rerun proceeds; there is no refusal or --ack-uncertified-review gate.
  • Missing git objects warn and continue, never block. Clear the range only after a completed review whose approved head equals or is a descendant of to_sha; parked, failed, skipped, and aborted reviews must not clear it. Rebase remaps the persisted SHAs onto the rewritten head so the next review can still bind.
  • Regressions: internal/pipeline/uncertified_test.go, TestCommitAgentFixes_PersistsUncertifiedRangeForReview, TestCommitAgentFixes_LintDoesNotPersistUncertifiedRange, TestCommitAgentFixes_DocumentDoesNotPersistUncertifiedRange, TestFixRoundProvenanceClause_EmitsForUncertifiedRangeWhenNotFixing, TestUncertifiedRange_PersistsThenFeedsNextInitialReview, TestRebaseStep_RemapsUncertifiedRangeWhenHeadRewritten.

Review Fixer Verification Discipline (internal/pipeline/steps/review.go)

  • The review-fix prompt requires all fixes before one focused verification limited to the changed area and forbids the whole repository test/lint suite during the fix round. The dedicated Test and Lint steps are the authoritative gates, although their coverage may be focused when commands are unconfigured. This is a prompt contract, not an enforced sandbox. Regression: TestReviewStep_FixMode_FocusedVerificationContract.

Local Test Is Targeted Validation (internal/pipeline/steps/test.go)

  • Local Test (normal evidence agent and Test-repair agent) validates the requested intent with the smallest relevant checks and end-user-aligned evidence; it is never a repository-wide regression-suite walk. Broad regression belongs to remote CI (go test -race ./... in .github/workflows/ci.yml) and remains mandatory before a PR is ready. commands.test is the same contract when set: targeted baseline, not CI-parity complete-suite configuration; docs owner is docs/src/content/docs/reference/repo-config.md (commands.test), step behavior owner is docs/src/content/docs/reference/pipeline-steps.md (Test). This repository dogfoods an empty commands.test so the agent-driven targeted path is the default; do not reintroduce go test -race ./... as a local Test override. Process-group reaping on clean/error exit (#357) and Unix WaitDelay remain the lifecycle safety net when agents spawn test workers - restoring the agent-driven path must not revive the daemon OOM leak. Those agent turns are bounded by test_agent_timeout (default 30m, global-only): a stalled evidence or repair agent is cancelled and the run fails instead of waiting forever. Native adapters already honor that deadline through CommandContext; the missing piece was the Test step never setting one. Docs owner is docs/src/content/docs/reference/global-config.md. Every other pipeline agent invocation is bounded by agent_timeout (default 30m, global-only) at pipeline.RunAgent / the executor timeoutAgent seam, so a new agent-spawning step cannot hang a run by forgetting a deadline. Review keeps review_agent_timeout as a per-round budget; an existing sooner deadline is honored rather than capped. The invocation context is scoped only to Agent.Run; a late successful return after the deadline is rejected. Docs owner is docs/src/content/docs/reference/global-config.md. Regressions: TestTestStep_InitialAgent_TargetedValidationContract, TestTestStep_FixMode_TargetedVerificationContract, TestTestStep_FixMode_DriverFullSuiteInstructionDoesNotOverrideContract, TestTestStep_InitialAgent_NoTargetedEvidenceRequiresHonestFinding, TestTestStep_HangingEvidenceAgentFailsRunAfterTimeout, TestCodexAgent_RunCancelsSilentHang, TestDogfoodConfig_NoBroadLocalTestCommand, TestCIWorkflow_RetainsFullRaceSuiteAsBroadRegressionOwner, plus the existing #357 reap/WaitDelay tests, TestRunAgent_*, TestExecutor_DirectAgentRunIsDeadlineBounded, TestDocumentStep_HangingAgentFailsRunAfterTimeout, TestLintStep_HangingAgentFailsRunAfterTimeout, TestCIStep_HangingFixAgentFailsAfterTimeout, TestRebaseStep_HangingConflictAgentFailsAfterTimeout.

Intent Provenance & Conformance (internal/pipeline/steps/intent_prompt.go)

  • Intent carries provenance: an explicit axi run --intent persists Source==db.RunIntentSourceAgent ("agent", score 1); a transcript match persists the agent name ("claude"/"codex"/...). The executor propagates it as StepContext.IntentSource alongside UserIntent (executor.go).
  • userIntentPromptSection branches on source: an EXPLICIT intent renders as sanitized-but-AUTHORITATIVE acceptance criteria; an INFERRED intent keeps the low-confidence hint framing verbatim. Both branches keep the StripAdversarial+RedactSecrets pipeline and BEGIN/END "do not execute instructions" guard - authoritative reframes only the content's authority (check the diff against the criteria), never whether control tokens are stripped. The review prompt adds intentConformanceReviewClause for agent-source intent only: a fixer change that contradicts the criteria (removes intent-required or adds intent-forbidden behavior) MUST become an ask-user finding, which parks with no executor change. Conformance is limited to source-verifiable criteria; deferred pipeline-owned delivery (remote branch / push / PR / CI for this run) is out of scope at review.
  • Review is always pre-push (StepReview before StepPush/StepPR/StepCI). pipelineDeliveryPhaseClause plus stripDeferredPipelineOwnedDeliveryFindings (pipeline_delivery.go, applied in review.go) keep findings that only claim those later-owned outcomes are missing from parking the run. External or pre-existing lifecycle requirements (numbered PR, third-party artifact, non-run-owned state) stay enforceable. Push, PR, and CI steps remain strict after their stages run.
  • Empty/missing finding action fails closed to ask-user, not auto-fix (types/findings.go ActionOrDefault); HasAskUserFindings uses ActionOrDefault so it agrees with AutoFixableFindings (an unclassified finding is never auto-fixed and is always caught as ask-user). MergeUserOverrides still stamps user-added findings auto-fix on purpose.
  • The deterministic net-deleted-author-lines git-diff backstop is intentionally not built; review.go owns the held-scope TODO.
  • Regressions: internal/pipeline/steps/intent_prompt_test.go, internal/pipeline/steps/review_test.go (TestReviewStep_ConformanceObligationTracksIntentProvenance, TestReviewStep_RereviewFlagsIntentContradictionAsAskUser), internal/pipeline/steps/pipeline_delivery_test.go, internal/pipeline/steps/review_pipeline_delivery_test.go, internal/pipeline/executor_intent_conformance_test.go, internal/types/findings_test.go, e2e TestIntentJourney (inferred-source framing), e2e TestReviewPipelineOwnedPRCriterionDoesNotPark / TestReviewExternalPRLifecycleStillParks.

Test Evidence Lives on an Orphan Branch, Never in the Code Branch

  • The test step always collects evidence OUTSIDE the worktree, in the directory the executor resolved once as StepContext.EvidenceDir; nothing stages or commits it into the pushed branch, so evidence can never reach the default branch's history. With test.evidence.store_in_repo and a derivable GitHub link base, the PR step calls publishRunEvidence (internal/pipeline/steps/evidence_publish.go), which copies the directory onto the push-target repo's orphan evidence branch through internal/evidence and hands the PR body its links. A provider without derivable links does not push the branch.
  • internal/evidence owns the mechanism and its fail-closed rules: plumbing only (scratch GIT_INDEX_FILE + hash-object/write-tree/commit-tree), so HEAD, the index, and the worktree are untouched and a detached or shallow clone works; the parent is the just-fetched remote tip so the push is a plain fast-forward and never a force; an existing branch without the .no-mistakes-evidence marker at its tip is refused, which is what makes a wrong branch name (main) harmless. Every failure returns an error and the PR body falls back to local-path references rather than links that would not resolve.
  • PR links are pinned to the evidence COMMIT, not the branch, so a later run overwriting the same paths cannot change what an old PR shows. Link bases come from Repo.UpstreamURL/ForkURL, never the push URL, which can carry a credential.
  • test.evidence.branch is trusted-only in EffectiveRepoConfig (it names a ref the daemon pushes to); local_root/retention/max_runs are global-only (applyEvidenceStorageOverrides is called from Merge with GlobalConfig alone); the rest of test.evidence stays pushed-readable. Invalid branch names, relative local_root, unparseable retention, and negative max_runs all fail the config at parse time (validateTestRaw).
  • Regressions: internal/evidence/publish_test.go, internal/evidence/branch_test.go, internal/pipeline/steps/evidence_publish_test.go, TestPushStep_DoesNotPublishTestEvidenceIntoThePushedBranch, TestEffectiveRepoConfig_EvidenceBranchTrustedOnly, TestLoadGlobalConfig_InvalidEvidenceBranchFailsClosed, internal/config/evidence_storage_test.go.

no-mistakes Owns Its Own Scratch (never the shared system temp dir)

  • Evidence lives at <NM_HOME>/evidence/<runID> (paths.EvidenceDir/EvidenceRoot/RunEvidenceDir), never os.TempDir(). The daemon's service unit exports only HOME, PATH, and proxy vars, so TMPDIR is unset and os.TempDir() resolved to the shared /tmp - a systemd tmpfs on Ubuntu 24.10+, so evidence consumed RAM. The app root is disk-backed on all three platforms, so there is deliberately NO runtime.GOOS branch; do not add one.
  • One owner for the path: the executor resolves it (Executor.runEvidenceDir) into StepContext.EvidenceDir, and agent.WithSteering(a, evidenceRoot) takes it as an argument. Steps and the steering preamble must never rebuild it - two independent os.TempDir() copies is exactly the drift this replaced.
  • Cleanup is ours, in three layers: RunManager.cleanupRunEvidence removes a finished run's dir when empty (os.Remove, never RemoveAll - the test step creates the dir before the agent decides it has anything to write, and that litter was 94% of observed accumulation), reapEvidence bounds the directory by age and count oldest-first, and reapLegacyEvidence drains the pre-relocation temp directory under the same policy. All three reuse skipWorktreeCleanup's pending/running guard and are best effort. No OS temp timer is load-bearing.
  • HELD SCOPE: internal/eval/replay.go sandboxes stay in the system temp directory. They are the largest scratch this program creates, but a replay materializes its own nested NM_HOME and worktree while Store.Prune, the case records, and the object pools all live under <NM_HOME>/eval - so relocating the sandbox inside the app root nests it in the state it is replaying, which e2e TestEvalJourney refuses on purpose. Moving it needs a disk-backed root outside NM_HOME, which does not exist yet; do not "fix" it by weakening that assertion. Every remaining os.MkdirTemp("", ...) caller is auto-named and self-cleaning with defer; keep it that way.
  • Regressions: internal/paths/evidence_test.go, internal/config/evidence_storage_test.go, internal/daemon/evidence_reap_test.go, TestSteeringNamesTheConfiguredEvidenceRoot, TestTestEvidenceDir_DefaultResolutionStaysUnderTheAppRoot, e2e TestTestEvidenceLivesUnderAppRootNotSharedTemp / TestRunCleanupLeavesNoEmptyEvidenceDirectory.

Combined Document+Lint Housekeeping Pass

  • When commands.lint is empty, the document step performs both duties in one agent invocation and stashes the lint half on RunShared (consume-once); the lint step consumes it instead of paying a second cold pass. Neither duty is ever silently dropped: a skipped pass, untrusted structured output, or a lint fix round falls back to lint's own agent pass. Configured commands.lint stays a first-class deterministic gate. Uncategorized findings fail safe to the stricter documentation gate.
  • The document prompt enforces the placement policy (one owner per fact, stale duplicates become pointers, no AGENTS.md postmortems, scope limited to docs the change made stale). Do not reintroduce exhaustive-corpus-sweep language; it caused doc commits in 90 of 121 audited PRs. Contract test: TestDocumentStep_PromptAppliesPlacementPolicy; behavior tests: internal/pipeline/steps/housekeeping_test.go.

Local Eval Corpus Collection (internal/eval)

  • Collection is automatic and default-on through eval.capture_provenance / eval.auto_capture / eval.max_cases / eval.diversified_size in config.yaml, never an environment variable: the daemon's launchd/systemd unit is re-rendered on install and update and preserves only proxy variables (internal/daemon/service.go proxyEnvKeys), so an env-gated corpus silently stops collecting after an update. The keys are global-only - Merge copies them straight from GlobalConfig, and an eval block in a repo's .no-mistakes.yaml is ignored.
  • Provenance is unrecoverable: executor.go writes it with the review round or never. A round recorded with capture_provenance off can never be captured, so the rejection names the setting rather than the round's age.
  • The trigger is RunManager.autoCaptureEvalCase, called last in the run goroutine after the outcome is already reported: it recovers its own panic (the enclosing recover would otherwise mark a finished run failed), bounds itself with evalAutoCaptureTimeout off the run context, serializes runs on evalCaptureMu (shared pool + registry), and logs rather than propagates. ErrNoCapturableReview separates "nothing to freeze" (DEBUG) from a real fault (WARN). Automatic and manual capture call the same eval.Capture. A merged PR also best-effort relabels already-captured cases via RunManager.relabelEvalRun (same mutex/timeout); eval relabel is the CLI path.
  • The unit of truth is finding-level gold, not park/pass, and it is keyed on the round's recorded fix-vs-skip decision plus merge state, never on whether a later round still raises the finding (a fix and a ship both make it disappear): a user-selected Fix is true-positive gold (no merge required); an auto-fix selection on a merged run is true-positive gold even if a later round re-raised or rewrote it; a raised auto-fix/ask-user finding the human did NOT select, on a merged run, is false-positive gold - deliberately reversing the older "never auto-FP from a skip" stance, because in this operator's corpus an approved-and-shipped finding IS a false positive; a human-added finding is false-negative gold; skip/approve/abort without a merge and any round with no recorded decision stay unlabeled / pending; no-op findings are never labeled; unmatched candidate findings stay queued - never inferred as false positives - and a confirmed post-PR miss ingested via eval miss ingest is also false-negative gold (recorded-post-pr-miss). Owner: internal/eval (goldFromRound, hasRecordedDecision, IngestPostPRMiss, ScoreCandidate); user-facing language is docs/src/content/docs/reference/eval.md.
  • diversified is gold-only and pinned (empty gold -> empty set + eval sets warning, never unlabeled fill). Those pins are the held-out official set; leftover labeled cases are tune. ListCases trims pins to the live eval.diversified_size cap (at most one per stratum when reconciling to 0 or a lower cap); RefreshDiversified is only for an explicit rebuild. Never fit matcher thresholds or review prompts on diversified. Report F1 as the headline metric only when false-positive gold exists; otherwise recall + precision bounds. RelabelRun recomputes derived merge labels and drops the obsolete ones. Matcher assignment is ONE globally optimal bipartite matching over all gold and candidate findings, weighted so an exact match outweighs any number of fuzzy ones; per-strength-tier greedy assignment understated recall and must not come back. Regressions: TestListCasesDiversified_*, TestGoldFromRoundLabelsByRecordedDecision, TestCaptureWritesAutoFixMergedAsTruePositive, TestCaptureWritesShippedUnfixedAsFalsePositive, TestCaptureWritesShippedUnfixedEvenWhenTheFinalRoundNoLongerRaisesIt, TestCaptureLabelsSelectedAutoFixAsTruePositiveEvenWhenLaterRoundReRaisesIt, TestRelabelReplacesShippedUnfixedWhenTheRoundLaterRecordsAFixDecision, TestMergeGoldClearsStoredShippedUnfixedWhenRecomputedUnlabeled, TestRelabelClearsStoredShippedUnfixedFPWhenRecomputedUnlabeled, TestScoreCandidateDoesNotLetFuzzyEarlierGoldStealExactLaterMatch, TestScoreCandidateRecoversMatchTheTieredMatcherLost, TestMaxWeightAssignmentMatchesBruteForceOptimum, TestEvaluationSummaryWithholdsHeadlineF1WithoutFalsePositiveGold, TestCaptureDoesNotLabelSkipOrApproveAsPass, TestCaptureWritesFalseNegativeGoldForUserAddedFinding, TestCaptureSkipsIncompleteReviewRoundAndKeepsCompletedSibling, TestIngestPostPRMissWritesFalseNegativeGoldOnGreenReview, TestCaptureAndReport*, CLI TestEvalCaptureAndSetsSpeakInFindingGoldTerms, TestEvalMissIngestLabelsFalseNegativeGold.
  • A case stores no Git bundle. Bundles were a full history copy per review pass (~8 MB each here) and cannot be trimmed, because a bundle built with negative refs records prerequisites an empty restore gate lacks. Cases of one repository instead share <NM_HOME>/eval/pools/<fingerprint>.git, pinned by refs/no-mistakes/eval/<caseID>/{head,source-head,base,trusted-config}; the marginal case costs ~8 KB. Store.Prune applies max_cases oldest-first but protects active replay reservations and cases with recorded evaluations, so the cap is a retention target rather than a hard bound.
  • Capture stays read-only against the gate, so objects reach the pool through a throwaway bare clone plus a refspec fetch - never a bare-object-id fetch, whose want policy is off by default and version-dependent.
  • Every eval subcommand is idempotent and tested so (internal/eval/idempotency_test.go, CLI TestEvalCaptureSetsReportAndRelabelAreIdempotentAtTheCLI): capture/relabel converge in place, sets reads self-stabilize their pins, and replay is additive-by-cohort but never rewrites case labels or manifests - queued unmatched-finding counts derive from the evaluations table (Store.pendingFindingCounts), never from a stored counter. The eval sets and eval run dashboards render in internal/cli/eval_render.go, sharing the stats box idioms (renderTitledBox); the diversified headline's instant self-score is SelfScoreRecordedReviews scoring each case's recorded review against its own gold.
  • Regressions: TestCaptureDoesNotCopyRepositoryHistoryPerCase, TestPruneBoundsTheCorpusOldestFirstAndKeepsEvaluatedCases, TestDropCaseObjectsReleasesOnlyItsOwnPins, TestAutoCaptureEvalCase* (internal/daemon), TestEvalDefaultsCollectWithoutSetup, TestRepoConfigCannotChangeEvalCollection, e2e TestEvalAutoCaptureJourney.

Telemetry Shape

  • Read-only surfaces (axi home/status/logs, status, runs) emit NO pageview and gate their command event through telemetry.ReadSurfaceGate (emit on state-fingerprint change, else at most once per 10 min, persisted at <NM_HOME>/telemetry-gate.json). Never reintroduce the pageview+command double emit for read surfaces - axi-status alone was 42% of all remote event rows. Mutation surfaces stay full-fidelity via trackAxiSurface/trackCommand.
  • Detailed performance evidence is LOCAL-ONLY (agent_invocations rows plus runs.parked_ms); never store prompts, outputs, diffs, or raw command arguments there (shape-guard test TestAgentInvocations_PrivacySafeShape) and never send run IDs, paths, session identities, or per-invocation records to Umami - the only remote perf data is three bounded counts on the terminal run finished event. The local/remote split is documented in docs/src/content/docs/reference/environment.md; read locally with no-mistakes stats.
  • Session-fidelity metric counts and timing boundaries have ONE authoritative home, internal/agent/invocationmetrics.go (tool-category classifier, InvocationMetrics, FreshInputTokens, PerRoundTokens, ModelTimeMS); the codex adapter fills them from its live exec --json event stream (codex_metrics.go) and the additive fidelity fields plus cache-creation usage are nullable so a not-reported datum is stored as NULL, never a fabricated zero. Codex's live stream exposes neither the model (resolved best-effort from the ~/.codex/sessions rollout) nor internal model-request counts (it batches one exec into a single turn.completed, so round-trips are counted from completed items and subprocess wait is the reader-timed tool-item interval); codex usage is cumulative across a resumed session, so per-round deltas subtract the same session's prior cumulative (Result.SessionUsageCumulative). Regressions: internal/agent/invocationmetrics_test.go, internal/agent/codex_metrics_test.go, internal/pipeline/instrument_fidelity_test.go, internal/db/agent_invocation_test.go (TestOpenMigratesSessionFidelityColumns).

Guarded Local Branch Synchronization (internal/branchsync)

  • sync, axi sync, and the TUI u action share one service whose only ordinary worktree mutation is a clean guarded move to an exact freshly verified pipeline push binding: strict fast-forward for behind branches, or an anchored reset to an equivalent diverged pipeline head when local unique work is already represented there. Under --recover, the worktree can only strict-fast-forward to the gate-preserved head, or adopt a diverged preserved head that preservedContainsLocalWork proves carries every local change. Passive status never fetches, and blocked states never reset, stash, merge, rebase, force, switch, delete, or update an external remote.
  • Give each network remote operation its own bounded child context derived from the caller: Refresh must not share one deadline across sequential git.LsRemote and git.FetchRemoteBranchToPrivateRef calls, and Apply uses the same per-operation budget for its final live check. The per-operation budget is Service.RemoteTimeout, sourced only from the operator's global branch_sync_remote_timeout setting (default config.DefaultBranchSyncRemoteTimeout, 60s); RepoConfig deliberately has no matching field. Recover's local-gate fetch is outside this network deadline contract. Regressions: TestRefreshSlowSuccessfulLsRemoteDoesNotStealFetchBudget, TestRefreshSlowButSuccessfulLsRemoteAloneExceedsItsOwnBudgetReportsOffline, TestRefreshRaisedRemoteTimeoutAcceptsTheSameLegitimateSlowLsRemote, TestRefreshParentCancellationStopsFetchAfterLsRemoteSucceeds, TestServiceRemoteTimeoutDefaultsToConfigDefault, TestLoadGlobal_InvalidBranchSyncRemoteTimeout, TestLoadRepo_BranchSyncRemoteTimeoutIsNotARepoSetting.
  • Successful pipeline pushes persist the exact SHA, credential-free target fingerprint/ref, and generation; legacy rows remain nullable and must never infer provenance from mutable head_sha. Structured PR lifecycle retires merged/closed branches. The service rechecks the invoking worktree, target, live remote equality, ancestry or equivalent-divergence proof, generation, and all mutable assumptions immediately before apply.
  • A TERMINAL run with unpublished pipeline commits (moved head) is recoverable only from verified, non-conflicting evidence: inspection and Recover share one eligibility model. Equal/ahead local ancestry can create the local anchor without requiring gate access, but available gate evidence must agree; importing a missing preserved head requires exact or safely anchorable gate evidence, a clean worktree, and either ancestry or the content-preservation proof below. Only then does inspection report blocked_pipeline_owned_recoverable + next_action recover_custody with the exact submitted/current-head and relation facts (active runs keep the plain block). Missing, non-commit, symbolic, or conflicting evidence, and import cases that are dirty or genuinely divergent, fail closed with manual reconciliation instead. sync --recover anchors the preserved head at refs/no-mistakes/recover/<run> before stamping runs.custody_returned_at. Cancellation RELEASES a terminal run that never changed the submitted head (head_sha == submitted_head_sha, no push, no custody stamp): selection keeps it visible so it never misreports as blocked_wrong_branch, and it classifies user_owned - no next_action, non-blocking exit, never represented as recoverable custody, --recover there is an idempotent no-op that mutates nothing, and a fresh axi run or separately authorized direct push is never blocked. Equal/ahead worktrees anchor locally without requiring gate access, but an available gate's existing recovery ref must agree with the recorded head; behind/diverged worktrees verify and fetch the preserved head from the run-specific recovery ref, fast-forwarding only a clean behind worktree. A cancelled validation routinely leaves a preserved head that is a REBASE of the local branch, which equality and ancestry read as plain divergence, so a clean diverged worktree is adopted when preservedContainsLocalWork proves containment. That proof is an executable merge-tree three-way merge whose result must equal the preserved head's tree, anchored on the merge-base - never runs.base_sha, the previous gate head. It deliberately does NOT use patch identity: patch IDs discard hunk locations and whitespace, so they cannot tell a genuine replay from a same-shaped edit to another identical block, and a containment claim built on them is not a proof. Everything undecidable escalates, including a rebase whose fix rounds also rewrote operator lines, where nothing separates a deliberate fix from a dropped change. Adoption anchors the pre-recovery local head at refs/no-mistakes/recover-local/<run>, then moves the branch with Git operations that fail closed on their own rather than after an observation - an atomic update-ref CAS plus read-tree -m -u, never check-then-act followed by reset --hard, which destroys anything landing in the gap. recoverAdoptPreserved owns the reasoning. Terminalization pins every verified unpublished head at refs/no-mistakes/recover/<run> before the managed worktree can be removed. Recovery reads that run-specific ref rather than requiring the gate branch to match, so aborts, rebases, and pre-push failures remain recoverable while an independently moved gate branch is preserved. Legacy recorded heads that still exist as dangling gate objects are anchored on recovery; a truly missing recorded head reports a distinct manual-reconciliation action instead of advertising an impossible recover_custody command. When the operator keeps a behind or diverged local head instead of taking the preserved head, --keep-local never touches the worktree and CAS-moves the gate branch to the kept head, staging objects via gate-side fetch - never a push, which would fire the receive hook and start a run. The full relation matrix and fail-safe rules live in the Recover doc comment in internal/branchsync/sync.go.
  • Public guidance is owned by internal/skill/skill.go plus live AXI strings, then regenerated with make skill. Core regressions live in internal/branchsync (incl. recover_test.go), internal/cli/sync_test.go, internal/tui/branch_sync_test.go, and e2e TestAxiBranchSyncJourney / TestAxiCustodyRecoveryJourney / TestAxiCustodyRecoveryAfterRebaseJourney / TestAxiPrePushAbortUnmovedHeadCustodyJourney.

Post-Review Head Continuity and Push Binding

  • Every step after Review in the fixed pipeline order (Test, Document, Lint, Push, PR, CI) calls assertPipelineHeadContinuity at entry. The helper is the single semantic owner: equal or descendant live heads continue; backward, sibling, and unverifiable heads fail before the step performs work. Regression: TestPostReviewStepsRefuseHeadClobberAtEntry.
  • A successfully completed full review atomically records runs.review_approved_head_sha; parked, failed, skipped, and legacy reviews carry no inferred authority. Push reads that durable binding, permits only the exact commit or a descendant, and pushes the verified immutable SHA rather than mutable HEAD. Never infer approval from runs.head_sha, a worktree, gate ref, or remote branch. Regressions: TestPushStep_RefusesPostReviewClobberWithoutLaterPipelineCommit, TestPushStep_BindsRemoteAndDatabaseToVerifiedCommitWhenHEADMovesDuringPush, TestExecutor_FullRereviewReplacesApprovalWithoutAuthorizingParkedRound.

Rebase Base & Force-Push Safety (data-loss prevention)

  • The whole job of this tool is to not lose people's code; favor refusing the push and surfacing a finding over any clever recovery. The comments in internal/pipeline/steps/forcepush.go own the full reasoning; the invariants are the next three bullets.
  • Rebase bases come from the freshly fetched authoritative remote refs, never local or stale state; and a branch built on unpushed local-default-branch commits parks with NeedsApproval + AutoFixable=false instead of silently widening the PR (detectBundledLocalDefaultCommits, #283).
  • Every force-push routes through resolveForcePushDecision, which re-reads the live remote head and allows the push only for a new branch, an already-equal remote, an unchanged lastSeenSHA, or remote commits already incorporated by patch-id (excluding ^baseSHA history the run knowingly rewrites). Anything else refuses, and a failed ls-remote/fetch fails closed; never degrade to a bare --force/--force-with-lease without an explicit anchor.
  • lastSeenSHA must stay the head the run last observed, never the live remote tip: the rebase step refreshes origin/<branch> only on a normal push, NOT on a force push, and the CI step passes Run.HeadSHA. Anchoring the lease to a SHA read immediately before pushing is the original #281 bug (it always passes and protects nothing); always-fetching the branch on force push recreates it. Never reintroduce either.
  • Regressions: TestCIStep_CommitAndPush_RefusesToClobberUnseenUpstreamCommit (#281), TestPushStep_RefusesToClobberAdvancedUpstreamBranch (#305), TestForcePushRun_RefusesToClobberOutOfBandBranchCommit, TestRebaseStep_DetectsUnpushedLocalDefaultBranchCommits (#283), TestResolveForcePushDecision_*.

macOS Release Signing (permanent identity)

  • Every official macOS release artifact - both darwin/arm64 and darwin/amd64 - is Developer ID Application signed on a macOS runner with a fixed identifier, hardened runtime, secure timestamp, and no entitlements, then strictly verified before it is archived or checksummed; the Linux and Windows release paths are unchanged.
  • The executable identifier com.kunchenguid.no-mistakes and Team ID 9T2J7MNUP9 are the permanent Developer ID identity and MUST NEVER change: they are the invariant of the identity-based designated requirement that lets macOS permission grants survive no-mistakes update, so changing either resets every grant once.
  • Signing runs only in the darwin build job gated behind the release-signing GitHub environment; the certificate is the base64 CSC_LINK secret unlocked with CSC_KEY_PASSWORD, imported into an ephemeral keychain with a runtime-generated password that is deleted on success and failure, and no other job may reference those secrets.
  • Signing happens before tarball creation and checksum generation, and the verify gate fails the release closed on any missing or ambiguous signature, wrong Team ID, non-permanent identifier, content-based (cdhash) requirement, missing hardened runtime or timestamp, or wrong architecture.
  • Mechanics live in .github/workflows/release.yml; the contract is pinned by the root TestReleaseWorkflow* static tests in workflow_release_signing_test.go, and secret values are never recorded here or in any test fixture.
  • Notarization, stapling, a PKG, Homebrew, and universal binaries are intentionally out of scope for this phase.

When Making Changes

  • Whenever you must bring in new dependencies, check latest documentation for knowledge, and discuss with the user.
  • Always use test driven development for bug fixes and feature development.

Maintaining this file

Keep this file for knowledge useful to almost every future agent session in this project. Do not repeat what the codebase already shows; point to the authoritative file or command instead. Prefer rewriting or pruning existing entries over appending new ones. When updating this file, preserve this bar for all agents and keep entries concise.