# 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 ` expects `origin` to point at the GitHub parent repository and `` at the contributor fork; plain `no-mistakes init` preserves an existing fork URL on idempotent refresh. - Push code must use `Repo.PushURL()` so configured forks receive branch updates. - GitHub PR code must keep `--repo` pointed at the parent and use `--head :` when `fork_url` is set; existing-PR lookup must list by the bare branch and filter head-owner fields, never pass `:` to `gh pr list --head`. - GitLab and Bitbucket fork MR/PR routing is intentionally out of scope until implemented end to end; if a legacy row has `fork_url` for those hosts, PR creation must skip instead of opening a self PR. **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 `, 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//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. - 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. **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_*`. - 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. **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`). **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=`; never shell out to git in a bare gate repo relying on `cmd.Dir` or `-C` discovery (issue #362). - Regressions: `TestRunOnBareRepoUnderSafeBareRepositoryExplicit`, `TestWorktreeAddRemoveOnBareRepoUnderSafeBareRepositoryExplicit`, `TestInitUnderSafeBareRepositoryExplicit`. **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 `/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. - 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. - 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`. **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 `/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` sweeps `./internal/e2e/...` and `./internal/pipeline/steps/...`, so keep new step-local e2e tests behind the tag too. - 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. - Isolate filesystem and environment state with `t.TempDir()` and `t.Setenv()`. **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) and `disable_project_settings` (the gate-agent project-instruction opt-out) are also trusted-only: a pushed branch must not weaken either boundary. When the opt-out is enabled, only adapters with verified effective suppression may launch. Non-executing fields (`ignore_patterns`, `auto_fix`, `intent`, `test`) are still read from the pushed branch. - `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_DisableProjectSettingsTrustedOnly`, `TestAssertGateTrustedConfigReadable_*`, `TestNewPipelineAgent_OptOut_*`, `TestLoadRecoveredConfig_BoundsFetchAndFailsClosed`, e2e `TestRepoConfigCommandsFromDefaultBranch` (incl. `pushed_branch_cannot_self_enable`). **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`. - Reap an orphaned monitor from outside its worktree with `no-mistakes axi abort --run `; it needs only `NM_HOME` plus the daemon, and an unknown id or stopped daemon is an idempotent no-op, not an error. Bare `axi abort` stays worktree/branch-scoped. **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 ` 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 reviewer session across the initial review and every full rereview, and a SEPARATE fixer session across review-fix turns; roles never share a session (the reviewer must never inherit the fixer's rationale), no other step uses sessions, and sessions are keyed strictly by run. Every review turn is still a full adversarial review of the complete branch diff. - Fail-safe rules: unsupported adapter runs cold; a failed resume drops the identity and re-runs the same turn in a fresh same-role session, never skipping the review; a cancelled ctx gets no fallback retry; `session_reuse: false` forces everything cold. Persistence is minimum metadata only, never prompts or transcripts. - `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`, `internal/agent/session_test.go`. **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`. **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. - 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/executor_intent_conformance_test.go`, `internal/types/findings_test.go`, e2e `TestIntentJourney` (inferred-source framing). **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`. **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 `/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`). **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/` 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.