* no-mistakes: apply CI fixes * no-mistakes(document): Clarify recovery anchor conflict handling
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 plusgo vet)go test -race ./...(the e2e suite is behind thee2ebuild tag and excluded)make e2ewhen touching agent integrations, the e2e harness, or recorded fixturesgo build -o ./bin/no-mistakes ./cmd/no-mistakes
Fork Routing
repos.upstream_urlis the parent repository used for PR base routing;repos.fork_urlis an optional GitHub fork push target.no-mistakes init --fork-url <url>expectsoriginto point at the GitHub parent repository and<url>at the contributor fork; plainno-mistakes initpreserves 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'soriginremote at run time because the DBupstream_urlis 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
--repopointed at the parent and use--head <fork_owner>:<branch>whenfork_urlis set; existing-PR lookup must list by the bare branch and filter head-owner fields, never pass<owner>:<branch>togh 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_urlfor 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.URLsVerifiedis 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.InitWithForkruns the upstream URL throughsafeurl.Redactbefore every DB persist (UpdateRepoMetadata*,InsertRepoWithIDAndFork) and the "gate initialized" log line; the bare gate'soriginremote still carries the full credentialled URL (viaprovisionGate) so carved worktrees authenticate. Because the DB copy is redacted, push and CI auto-fix push must recover the credential from the worktree'soriginremote at run time (resolvePushURL/resolveUpstreamURL), never fromRepo.UpstreamURL/Repo.PushURL().- Step-failure errors (
executor.goFailStep/log/IPC emit) and the Bitbucket resolve-repo error are redacted viasafeurl.RedactText/safeurl.Redactso a credentialled URL wrapped into an error can never reach a step log orruns.error. Reuseinternal/safeurlfor new redaction sites rather than adding a git-local helper; it is already wired intogit.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 listno longer accepts--state opened, and the daemon's detached-HEAD worktree breaksglab ci get, so pipeline jobs are read via the branch-independentglab api .../pipelines/<id>/jobsREST endpoint. - The comments in
internal/scm/gitlab/gitlab.goown the full rationale for each trap; extend them there when you hit new glab version drift.
Documentation
- Keep
README.mdconcise 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.mdanddocs/src/content/docs/reference/repo-config.mdown configuration keys,docs/src/content/docs/reference/environment.mdowns environment variables and the telemetry local/remote split,docs/src/content/docs/concepts/daemon.mdowns the daemon lifecycle model, and guides pages explain purpose and link to those owners instead of restating tables and examples. - The
document.instructionsblock in.no-mistakes.yamlstates this ownership map for the pipeline's document step; update it when ownership moves.
Agent-Guidance Surfaces
skills/no-mistakes/SKILL.mdis generated: the source of truth is thebodyconstant ininternal/skill/skill.go. Edit the body, thenmake skill;make lintfails CI on drift. Never editSKILL.mddirectly.no-mistakes initships this rendering to agents at user level.- Agent-driving guidance is owned by the skill body and the live
axioutput strings (internal/cli/axi*.go);docs/src/content/docs/guides/agents.mdcarries only the canonical invariant sentences pinned byinternal/cli/axi_guidance_test.goplus a pointer to the skill. When you change driving guidance, change the skill body and the point-of-useaxistrings 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: 0inconfig.goautoFixDefaults), so blocking and ask-user review findings park for an agent decision; keep the skill, the liveaxigatenote, and docs qualified if you touch review auto-fix.
Unified Agent Tuning (internal/agentcfg)
agentcfgis 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-messagemodel/variant, acpx--modelforcursor/acp:<target>). Add a harness there, not in an adapter or in eval.rovodevandantigravityare deliberately declared unmappable, so a request for them is a config error rather than a flag that is silently ignored.agent.NewWithOptionsis the one funnel: it validatesOptions.Profileand splices the mapped args after the operator's rawagent_args_overrideargs, 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_overrideflag that already pins a knob natively wins and the mapped value is not emitted, which is what keeps every pre-agent_configconfiguration byte-identical and stops a harness receiving one knob twice.agent_configis global-only for the same reason asagent_args_override. - Eval candidates are
agent,model=<model>[,effort=<level>](the previousagent+modelspelling is refused with a migration message), effort is part of the persisted candidate identity, andagentNeutralGlobalConfigstripsagent,agent_args_override, andagent_configso 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.Contextthrough long-running, subprocess, and networked work; preferexec.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 installscmd.Cancelto kill the whole tree, so grandchildren (test workers, build watchers) cannot outlive cancellation and hold the next run's worktree locked. cmd.Cancelcovers only cancellation; on clean exit or error the group is not reaped, and leaked grandchildren accumulate until the OS OOM-kills the daemon (surfacing asdaemon crashed during executionwith no stack trace). Useshellenv.RunShellCommand/OutputShellCommand/CombinedOutputShellCommandfor one-shot commands, orStartShellCommandplusTerminateShellCommandGroupwhen handling pipes manually; the helper doc comments ininternal/shellenvown the details.ConfigureShellCommandalso installs a 5scmd.WaitDelaybackstop so a grandchild holding an inherited pipe cannot wedgecmd.Waitforever. 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/procreapis the identity-based backstop: it matches a process by the run worktree its cwd resolves under (deliberately never argv, which a legitimategit worktree removealso 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>/worktreesby 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 throughprocreap.SweepRunWorktree(s)(run cleanup and setup failure viaRunManager.removeRunWorktree, startup cleanup, eject), scoped and therefore without age floor or run-active check; the unscoped startup sweep inrecoverOnStartupkeeps theorphanProcessMinAgefloor. 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.ConfigureShellCommandalready calls it; one-shot commands built directly must call it themselves. Regressions:TestHarden*ininternal/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/gatecontextis the single classifier for recursive pipeline control. It combines canonical registered gate common-directory identity with OS-authenticated IPC peer ancestry;NO_MISTAKES_GATEis 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. Editinternal/skill/skill.go, then runmake skill; never edit the generated skill directly.
Filesystem and Paths
- Use
filepath.Join; respectNM_HOMEfor app state; directories are0o755and files0o644by convention. - On macOS, path comparisons may need symlink resolution (
/varvs/private/var); useworktrees.Canonical/worktrees.Containswherever run worktree paths are compared, so one spelling matches everywhere. - Run worktree placement (
worktree_roots) is owned byinternal/worktrees. Configuration decides it exactly once, at run creation (Layout.DirinRunManager.startRunWithIntentSource), and the result is persisted inruns.worktree_dir; every later consumer - resume, step diff, startup cleanup,procreap, eject, gatecontext attribution - must read it back throughworktrees.RecordedDirand 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.CheckPlacementis the single policy for an unusable root (insideNM_HOME, inside any registered checkout);config.ValidateWorktreeRootsowns what the config can judge alone. The daemon refuses to start on an unusable placement, soinit --worktree-rootmust refuse exactly the same set or it prints a paste that takes the operator's CLI down, and EVERYinitrefuses to register a checkout that contains a configured root - the same state reached from the other direction. User-facing semantics live indocs/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 throughgit.Run, which detects a bare git dir and prepends--git-dir=<dir>; never shell out to git in a bare gate repo relying oncmd.Diror-Cdiscovery (issue #362). - Startup gate migration is DB-authoritative with a strict validated
<id>.gitlegacy fallback; it must reject non-gates before hook or Git mutation and usegit.RunBareso 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
ghfrom 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 makesgh pr <verb>infer the cwd branch (main) and returnno pull requests found for branch maineven when the feature PR's checks are green.GetChecks,GetPRState,GetMergeableState, andUpdatePRroute through the sharedprSelector(number, else URL, else fail closed) — never append a barepr.Number/pr.URLthat can be empty. This is theghanalogue 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
--gatevalue must never come from a bare$(pwd): Git can invokepost-receivefrom 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), andnormalizeNotifyGatePathininternal/cli/daemon_cmd.gois 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.lockis acquired as the very first action inRunWithOptions, 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
waitForDaemonStopandstopDetachedDaemon; regressions: e2eTestDaemonStopLeavesNoDaemonProcessOwningTheRoot,TestDaemonRestartReplacesTheDaemonWithExactlyOneOwner. - Independent layers:
internal/ipclisten()dials the socket before unlinking it and refuses to steal a live one; client probes bound the dial withdaemon_connect_timeoutand fail fast on a dead or wedged socket instead of starting a replacement daemon (EnsureDaemonsurfaces the error with adaemon startrecovery hint; the health RPC itself is bounded separately byipc.DefaultDialTimeout). - Daemon execution is explicit-only (
no-mistakes daemon run --root); never let inherited environment reinterpret probes like--versionorstatusas daemon workers. - Startup worktree cleanup is DB-aware: never remove a worktree whose run row is
pendingorrunning;startRuninserts 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 theinternal/daemon/lock.goanddaemon.gocomments. 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/logstoreowns all daemon-process byte and retention bounds. Lifecycle output useslogs/daemon.log, managed Rovo Dev/OpenCode stdout and stderr uselogs/managed-server.log, and service bootstrap/direct crash output useslogs/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.gois the sole owner of event reconciliation, reconnect, duplicate-event coalescing, and the slow lost-event heartbeat. Do not reintroduce fixed-intervalget_runpolling. 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.gois 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 coalescingstream_gapthat 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_runsnapshot carries a monotonicStateRev;runSnapshotsamples 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, andupdaterefuse 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 sharedlifecycle.ActiveRuns/lifecycle.RunListhelpers, and require an explicit--force.update -yanswers 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
logLifecycleInvocationto<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
e2ebuild tag;make e2erunsscripts/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 byinternal/e2edaemon: exact inventory, concurrency cap (NM_E2E_DAEMON_MAX, default 2), bounded argv checks, and reapers in harness Cleanup, packageTestMain, andscripts/e2e.shEXIT/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-mistakesservice. Regressions:internal/e2edaemon/*_test.go. - Packages whose tests shell out to git unset
GIT_CONFIG_COUNTinTestMainso ambientGIT_CONFIG_*injection from agent harnesses cannot leak in; a test exercising injected config re-sets it witht.Setenv(seeinternal/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-wideTestMainthat pointsNM_HOMEandHOMEat 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-mistakesroot undergo test; tests that touch app state must setNM_HOMEto a temp dir, and only the production-default path test may opt in withNO_MISTAKES_ALLOW_DEFAULT_ROOT_IN_TESTS=1.- Isolate filesystem and environment state with
t.TempDir()andt.Setenv(). - The Windows CI leg is process-spawn bound, not compute bound: git-backed packages cost roughly 10x their Linux time (
internal/git5.7s -> 53s,internal/branchsync31s -> 415s). The Windows matrix is split into a git-heavy shard and a core remainder so each job's wall stays insidetimeout-minutes: 40and a hang still surfaces asgo test -timeout(15m) rather than an evidence-free job cancel. Keep long git-heavy packages off the serial critical path (internal/branchsyncrunst.Parallel()for exactly that reason) and keep the Defender scan-exclusion step inci.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
-raceintermittently reportsgit <cmd>: signal: segmentation fault. That is not a git or repo bug:~/Library/Logs/DiagnosticReports/*.ipsrecords the crash asprocName: <pkg>.test, parentProc: <pkg>.test, asi: "crashed on child side of fork pre-exec"- the forked child dies beforeexecve. 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/procreapreaps those by cwd.
Repo Config Trust Boundary (security)
- The daemon runs
commands.*from.no-mistakes.yamlverbatim viash -c, andagentselects which process launches with the maintainer's credentials. The code-executing selection fields (commands.{test,lint,format}andagent) 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. Seeinternal/daemon/manager.gostartRun,loadTrustedRepoConfig, andassertGateTrustedConfigReadable. 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), andci.rerun_transient(how many times a transiently failed check may be re-run) are also trusted-only, regardless ofallow_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-runci.rerun_transientauthorizes bills another provider-side workflow run to the repository, so a contributor must not be able to raise it (the operator's own globalci.rerun_transientis 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_instructionsis matched against the COMPLETE changed-file set, never theignore_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 toRepo.DefaultBranchwhen unset) is trusted-default-branch-only, but unlike the fields in the bullet above it is the deliberate exception that also honors theallow_repo_commands: trueopt-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 viascm.PRBaseBranchReader) is authoritative for CI merge-conflict repair and base-branch tip monitoring over a since-changedpr.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 bydocs/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_commandsis per-repo, read only from the trusted default-branch copy, and defaultsfalse; a contributor cannot self-enable it from a pushed branch. The e2e harness models a trusted single-developer environment and commitsallow_repo_commands: trueviaSetupOpts.AllowRepoCommands; security tests passfalse.- Regressions:
TestLoadTrustedRepoConfig_FailClosedOnFetchFailure,TestLoadTrustedRepoConfig_PinnedSHAReadsFreshDefaultBranch,TestEffectiveRepoConfig_DocumentPolicyTrustedOnly,TestEffectiveRepoConfig_ReviewPathInstructionsTrustedOnly,TestMatchPathInstructions_PushedIgnorePatternsCannotSuppressTrustedRule,TestReviewStep_PushedIgnorePatternsCannotSuppressPathInstructions,TestEffectiveRepoConfig_DisableProjectSettingsTrustedOnly,TestEffectiveRepoConfig_CIRerunTransientTrustedOnly,TestAssertGateTrustedConfigReadable_*,TestNewPipelineAgent_OptOut_*,TestLoadRecoveredConfig_BoundsFetchAndFailsClosed, e2eTestRepoConfigCommandsFromDefaultBranch(incl.pushed_branch_cannot_self_enable), e2eTestReviewPathInstructionsJourney.
CI Monitor Lifecycle
ci_timeoutis an idle timeout, not an absolute deadline: onlytimeoutAnchorre-arms when the upstream default-branch tip advances,startedstays fixed for poll pacing, and re-arm only ever extends the deadline (fail-safe on transient base-tip failures). Value semantics (0unset, negative unlimited sentinel, keyword parsing) live inconfig.go; keepconfig.DefaultCITimeoutanddefaultConfigYAMLin sync (TestDefaultConfigYAML_MatchesGoDefaults). User-facing semantics are owned bydocs/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: truewith zero registered checks (internal/pipeline/steps/ci.godecides whether the declaration applies;internal/cimonitorowns 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 onlyNM_HOMEand 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. Bareaxi abortstays 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
pendingorrunningrows that already hold terminal PR state before parked-run planning and generic crash recovery. Regressions:TestUpdateRunPRStateFinalizesActiveTerminalOutcomes,TestUpdateRunPRStateIgnoresDuplicateAndDelayedRegressions,TestReconcileTerminalPRRunsFinalizesLegacyActiveRows,TestRecoverOnStartup_FinalizesLegacyTerminalPRRun, e2eTestTerminalPRRunDisappearsFromActiveListing. - A provider-reported
cancelledcheck 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 asask-userinstead of entering theauto_fix.ciloop, 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 equalsruns.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 deliberateTIMED_OUT/STALEexclusions, rollup-lag grace, and retirement live ininternal/pipeline/steps/ci_transient.go; provider support is the optionalscm.CheckRerunner(GitHub only) and user-facing semantics are owned bydocs/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. Acancelbucket - GitHubCANCELLED, GitLabcanceled, BitbucketSTOPPED, and how GitHub reports a job killed by its owntimeout-minutes- is a published conclusion that nothing will replace, so with no rerun outstanding it parks atciUnresolvedCancelledOutcome(cancelledWithoutRerun) instead of waiting. Conflating the two is the #628 regression that hung real runs for their wholeci_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_sinceis non-nil iff a step is actually parked at anawaiting_approval/fix_reviewgate: the executor sets it on gate entry, clears it whenwaitForApprovalreturns, andRecoverStaleRunsclears it on crash recovery. It is observability only (rendered asawaiting_agent: parked <duration>inaxi status) and never changes gate resolution, auto-resume, or the--yesdefault.- Tests:
internal/db/run_test.go,internal/pipeline/executor_approval_test.go,internal/cli/axi_test.go, e2eTestAxiParkedAwaitingAgentSignal.
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: falseforces everything cold. Persistence is minimum metadata only, never prompts or transcripts;SessionRoleReviewerremains only so crash recovery accepts legacy persisted rows, which are never resumed. codex exec resumehas a narrower flag surface thancodex 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 = "[]"plusselection_source = user_declinedon a gated round with findings (executor.gorecordDeclinedRound,db.SetStepRoundDeclined); a round with no findings records no decision. The conditional write must never erase an existing selection. User-facing semantics are owned bydocs/src/content/docs/reference/pipeline-steps.md. - A decline is stored as the COMPLEMENT of the selection, never as its own list;
declinedFindingLinesderives it and deliberately excludesauto_fixselections, whose complement is findings still awaiting a decision (rendered underauto_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 bypipeline.BindBranchDecisions, unlike review-onlyBindUncertifiedPipelineRange). 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;
assertPipelineHeadContinuityandassertReviewApprovedPushHeadremain lineage-only.ci_fix.goandrebase.gobuild prompts withoutroundHistoryPromptSection, 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 emitfixRoundProvenanceClauseeven whenFixing==false, so the replacement reviewer is not cold. Rerun proceeds; there is no refusal or--ack-uncertified-reviewgate. - 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.testis the same contract when set: targeted baseline, not CI-parity complete-suite configuration; docs owner isdocs/src/content/docs/reference/repo-config.md(commands.test), step behavior owner isdocs/src/content/docs/reference/pipeline-steps.md(Test). This repository dogfoods an emptycommands.testso the agent-driven targeted path is the default; do not reintroducego 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 bytest_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 throughCommandContext; the missing piece was the Test step never setting one. Docs owner isdocs/src/content/docs/reference/global-config.md. Every other pipeline agent invocation is bounded byagent_timeout(default 30m, global-only) atpipeline.RunAgent/ the executortimeoutAgentseam, so a new agent-spawning step cannot hang a run by forgetting a deadline. Review keepsreview_agent_timeoutas a per-round budget; an existing sooner deadline is honored rather than capped. The invocation context is scoped only toAgent.Run; a late successful return after the deadline is rejected. Docs owner isdocs/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 --intentpersistsSource==db.RunIntentSourceAgent("agent", score 1); a transcript match persists the agent name ("claude"/"codex"/...). The executor propagates it asStepContext.IntentSourcealongsideUserIntent(executor.go). userIntentPromptSectionbranches 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 theStripAdversarial+RedactSecretspipeline 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 addsintentConformanceReviewClausefor agent-source intent only: a fixer change that contradicts the criteria (removes intent-required or adds intent-forbidden behavior) MUST become anask-userfinding, 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 (
StepReviewbeforeStepPush/StepPR/StepCI).pipelineDeliveryPhaseClauseplusstripDeferredPipelineOwnedDeliveryFindings(pipeline_delivery.go, applied inreview.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
actionfails closed toask-user, not auto-fix (types/findings.goActionOrDefault);HasAskUserFindingsusesActionOrDefaultso it agrees withAutoFixableFindings(an unclassified finding is never auto-fixed and is always caught as ask-user).MergeUserOverridesstill stamps user-added findings auto-fix on purpose. - The deterministic net-deleted-author-lines git-diff backstop is intentionally not built;
review.goowns 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, e2eTestIntentJourney(inferred-source framing), e2eTestReviewPipelineOwnedPRCriterionDoesNotPark/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. Withtest.evidence.store_in_repoand a derivable GitHub link base, the PR step callspublishRunEvidence(internal/pipeline/steps/evidence_publish.go), which copies the directory onto the push-target repo's orphan evidence branch throughinternal/evidenceand hands the PR body its links. A provider without derivable links does not push the branch. internal/evidenceowns the mechanism and its fail-closed rules: plumbing only (scratchGIT_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-evidencemarker 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.branchis trusted-only inEffectiveRepoConfig(it names a ref the daemon pushes to);local_root/retention/max_runsare global-only (applyEvidenceStorageOverridesis called fromMergewithGlobalConfigalone); the rest oftest.evidencestays pushed-readable. Invalid branch names, relativelocal_root, unparseableretention, and negativemax_runsall 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), neveros.TempDir(). The daemon's service unit exports only HOME, PATH, and proxy vars, soTMPDIRis unset andos.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 NOruntime.GOOSbranch; do not add one. - One owner for the path: the executor resolves it (
Executor.runEvidenceDir) intoStepContext.EvidenceDir, andagent.WithSteering(a, evidenceRoot)takes it as an argument. Steps and the steering preamble must never rebuild it - two independentos.TempDir()copies is exactly the drift this replaced. - Cleanup is ours, in three layers:
RunManager.cleanupRunEvidenceremoves a finished run's dir when empty (os.Remove, neverRemoveAll- the test step creates the dir before the agent decides it has anything to write, and that litter was 94% of observed accumulation),reapEvidencebounds the directory by age and count oldest-first, andreapLegacyEvidencedrains the pre-relocation temp directory under the same policy. All three reuseskipWorktreeCleanup's pending/running guard and are best effort. No OS temp timer is load-bearing. - HELD SCOPE:
internal/eval/replay.gosandboxes 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 whileStore.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 e2eTestEvalJourneyrefuses 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 remainingos.MkdirTemp("", ...)caller is auto-named and self-cleaning withdefer; 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, e2eTestTestEvidenceLivesUnderAppRootNotSharedTemp/TestRunCleanupLeavesNoEmptyEvidenceDirectory.
Combined Document+Lint Housekeeping Pass
- When
commands.lintis empty, the document step performs both duties in one agent invocation and stashes the lint half onRunShared(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. Configuredcommands.lintstays 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_sizeinconfig.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.goproxyEnvKeys), so an env-gated corpus silently stops collecting after an update. The keys are global-only -Mergecopies them straight fromGlobalConfig, and anevalblock in a repo's.no-mistakes.yamlis ignored. - Provenance is unrecoverable:
executor.gowrites it with the review round or never. A round recorded withcapture_provenanceoff 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 withevalAutoCaptureTimeoutoff the run context, serializes runs onevalCaptureMu(shared pool + registry), and logs rather than propagates.ErrNoCapturableReviewseparates "nothing to freeze" (DEBUG) from a real fault (WARN). Automatic and manual capture call the sameeval.Capture. A merged PR also best-effort relabels already-captured cases viaRunManager.relabelEvalRun(same mutex/timeout);eval relabelis 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-userfinding 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-opfindings are never labeled; unmatched candidate findings stay queued - never inferred as false positives - and a confirmed post-PR miss ingested viaeval miss ingestis also false-negative gold (recorded-post-pr-miss). Owner:internal/eval(goldFromRound,hasRecordedDecision,IngestPostPRMiss,ScoreCandidate); user-facing language isdocs/src/content/docs/reference/eval.md. diversifiedis gold-only and pinned (empty gold -> empty set +eval setswarning, never unlabeled fill). Those pins are the held-out official set; leftover labeled cases aretune. ListCases trims pins to the liveeval.diversified_sizecap (at most one per stratum when reconciling to 0 or a lower cap);RefreshDiversifiedis only for an explicit rebuild. Never fit matcher thresholds or review prompts ondiversified. 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*, CLITestEvalCaptureAndSetsSpeakInFindingGoldTerms,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 byrefs/no-mistakes/eval/<caseID>/{head,source-head,base,trusted-config}; the marginal case costs ~8 KB.Store.Pruneappliesmax_casesoldest-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, CLITestEvalCaptureSetsReportAndRelabelAreIdempotentAtTheCLI): 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. Theeval setsandeval rundashboards render ininternal/cli/eval_render.go, sharing the stats box idioms (renderTitledBox); the diversified headline's instant self-score isSelfScoreRecordedReviewsscoring each case's recorded review against its own gold. - Regressions:
TestCaptureDoesNotCopyRepositoryHistoryPerCase,TestPruneBoundsTheCorpusOldestFirstAndKeepsEvaluatedCases,TestDropCaseObjectsReleasesOnlyItsOwnPins,TestAutoCaptureEvalCase*(internal/daemon),TestEvalDefaultsCollectWithoutSetup,TestRepoConfigCannotChangeEvalCollection, e2eTestEvalAutoCaptureJourney.
Telemetry Shape
- Read-only surfaces (
axihome/status/logs,status,runs) emit NO pageview and gate their command event throughtelemetry.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-statusalone was 42% of all remote event rows. Mutation surfaces stay full-fidelity viatrackAxiSurface/trackCommand. - Detailed performance evidence is LOCAL-ONLY (
agent_invocationsrows plusruns.parked_ms); never store prompts, outputs, diffs, or raw command arguments there (shape-guard testTestAgentInvocations_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 terminalrun finishedevent. The local/remote split is documented indocs/src/content/docs/reference/environment.md; read locally withno-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 liveexec --jsonevent 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/sessionsrollout) nor internal model-request counts (it batches one exec into a singleturn.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 TUIuaction 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 thatpreservedContainsLocalWorkproves 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:
Refreshmust not share one deadline across sequentialgit.LsRemoteandgit.FetchRemoteBranchToPrivateRefcalls, andApplyuses the same per-operation budget for its final live check. The per-operation budget isService.RemoteTimeout, sourced only from the operator's globalbranch_sync_remote_timeoutsetting (defaultconfig.DefaultBranchSyncRemoteTimeout, 60s);RepoConfigdeliberately 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
Recovershare 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 reportblocked_pipeline_owned_recoverable+next_action recover_custodywith 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 --recoveranchors the preserved head atrefs/no-mistakes/recover/<run>before stampingruns.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 asblocked_wrong_branch, and it classifiesuser_owned- nonext_action, non-blocking exit, never represented as recoverable custody,--recoverthere is an idempotent no-op that mutates nothing, and a freshaxi runor 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 whenpreservedContainsLocalWorkproves containment. That proof is an executablemerge-treethree-way merge whose result must equal the preserved head's tree, anchored on the merge-base - neverruns.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 atrefs/no-mistakes/recover-local/<run>, then moves the branch with Git operations that fail closed on their own rather than after an observation - an atomicupdate-refCAS plusread-tree -m -u, never check-then-act followed byreset --hard, which destroys anything landing in the gap.recoverAdoptPreservedowns the reasoning. Terminalization pins every verified unpublished head atrefs/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 impossiblerecover_custodycommand. When the operator keeps a behind or diverged local head instead of taking the preserved head,--keep-localnever 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 theRecoverdoc comment ininternal/branchsync/sync.go. - Public guidance is owned by
internal/skill/skill.goplus live AXI strings, then regenerated withmake skill. Core regressions live ininternal/branchsync(incl.recover_test.go),internal/cli/sync_test.go,internal/tui/branch_sync_test.go, and e2eTestAxiBranchSyncJourney/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
assertPipelineHeadContinuityat 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 mutableHEAD. Never infer approval fromruns.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.goown 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=falseinstead 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 unchangedlastSeenSHA, or remote commits already incorporated by patch-id (excluding^baseSHAhistory the run knowingly rewrites). Anything else refuses, and a failed ls-remote/fetch fails closed; never degrade to a bare--force/--force-with-leasewithout an explicit anchor. lastSeenSHAmust stay the head the run last observed, never the live remote tip: the rebase step refreshesorigin/<branch>only on a normal push, NOT on a force push, and the CI step passesRun.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/arm64anddarwin/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-mistakesand Team ID9T2J7MNUP9are the permanent Developer ID identity and MUST NEVER change: they are the invariant of the identity-based designated requirement that lets macOS permission grants surviveno-mistakes update, so changing either resets every grant once. - Signing runs only in the darwin build job gated behind the
release-signingGitHub environment; the certificate is the base64CSC_LINKsecret unlocked withCSC_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 rootTestReleaseWorkflow*static tests inworkflow_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.