diff --git a/.editorconfig-checker.json b/.editorconfig-checker.json index 22add8b..a3478e9 100644 --- a/.editorconfig-checker.json +++ b/.editorconfig-checker.json @@ -1,4 +1,11 @@ { + "Exclude": [ + "(^|/)__pycache__/", + "(^|/)\\.mypy_cache/", + "(^|/)\\.pytest_cache/", + "(^|/)\\.ruff_cache/", + "(^|/)\\.venv/" + ], "Disable": { "Charset": true, "Indentation": true, diff --git a/.github/skills/repo-worktree/SKILL.md b/.github/skills/repo-worktree/SKILL.md index d4f9e0e..9e675f0 100644 --- a/.github/skills/repo-worktree/SKILL.md +++ b/.github/skills/repo-worktree/SKILL.md @@ -61,6 +61,18 @@ from `develop` unless the task is explicitly about `main`-only content, per `GOV "Branching Model". Fetch immediately before creating and base on the remote ref, because a clone is whatever it last fetched rather than the branch it names. +The base clone is a fetch source, not a place to do task work. `fetch` and `worktree add` run +against it for that purpose, and outside "Listing and Cleanup"'s own terminal step below, nothing +else does: never `checkout`, `pull`, `reset`, `commit`, or any other command that mutates its own +working tree, index, or HEAD while a task is in progress. That distinction is the one a real +incident missed, reusing a primary checkout as the working directory itself rather than only as +the source a worktree is created from. On Claude Code this is also a mechanical stop for most +of that list. `merge --ff-only`/`pull --ff-only` and a `checkout `/`switch ` carrying no +force flag stay exempt even there, matching this skill's own cleanup step below, which needs +exactly those. +Prose remains the only enforcement for a non-Claude-Code agent, and for the shapes the hook itself +exempts. + ## Creating a Worktree The fleet layout convention keeps every base clone and every in-flight task visible in one @@ -123,6 +135,13 @@ task branch from `origin/develop`. A standalone clone keeps its worktree and Git directory under the same writable root. It therefore supports edits, explicit-path staging, commits, and branch updates without sharing the base clone's index. +On Claude Code, a standalone clone is structurally a primary checkout to `gh-write-guard.py`'s own +rule 6 test (`--git-dir` equals `--git-common-dir` there too, since it is not a linked worktree of +anything), so the hook denies the very commits/edits this fallback exists to make. Set +`GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT` for the session before using one -- the same escape hatch +`host-setup/agent-safety/README.md`'s requirement 6 already documents -- since this fallback is +exactly the narrow, already-approval-gated case that grant exists for. + A temporary standalone clone is a degraded handoff, not an equivalent location. The base clone does not register it, `git worktree list` does not show it, and an IDE opened on the base clone does not discover its changes. The maintainer must navigate to it manually, and the operating diff --git a/.github/skills/resync-a-repo/SKILL.md b/.github/skills/resync-a-repo/SKILL.md index 9ac991f..6791899 100644 --- a/.github/skills/resync-a-repo/SKILL.md +++ b/.github/skills/resync-a-repo/SKILL.md @@ -37,8 +37,15 @@ anything else. The finding kind names the procedure the repo is owed. ## Reach the hub and measure before changing anything Fetch a hub checkout of your own immediately before reading it, per RESYNC.md section 1, since a -stale clone answers confidently instead of failing, and verify the host with -`python3 scripts/host_gate.py --repo `. Then run the audit end to end, +stale clone answers confidently instead of failing. Never operate against an existing checkout +already present at a known or shared path, the maintainer's own primary checkout included, even +one that looks current -- always fetch into a private worktree of your own, per `repo-worktree`. +On Claude Code this is now also a mechanical stop for most such commands (a `PreToolUse` hook +denies a mutating git operation run directly in a primary checkout), though this prose is still +the only enforcement for a non-Claude-Code agent, and for the narrow shapes the hook itself +exempts, so following it here is not optional even where no hook can catch a lapse. Verify the +host with `python3 scripts/host_gate.py --repo `. Then run the audit end +to end, RESYNC.md section 2, against the target's `main` branch, never `develop`. A finding is a snapshot, so quote the run stamp in anything derived from it and re-run before acting on a finding read earlier in the session. diff --git a/AGENTS.md b/AGENTS.md index 5d86c2a..51d87bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,7 @@ **PhotoCleaner** is a .NET 10 console application that prepares media files (photos and videos) for import into photo management systems such as Lightroom, Immich, and PhotoPrism. It analyzes and transforms files through validation, conversion, and verification phases, orchestrating external tools (ExifTool, FFmpeg) through CLI wrappers, with SQLite-backed deduplication and Immich trash integration. The repo also contains an xUnit test project (`PhotoCleanerTests/`) and a benchmark project (`PhotoCleanerBenchmarks/`). -This file is the entry point every coding agent reads first, and it holds only two things: the rules for managing context and delegation, which apply to every task, and a map of where every other rule lives. The rule text itself is in [`GOVERNANCE.md`](./GOVERNANCE.md), one section per topic. Code style lives in [`CODESTYLE.md`](./CODESTYLE.md), the CI/CD workflow contract in [`WORKFLOW.md`](./WORKFLOW.md), and this project's architecture, processing pipeline, per-command details, and test layout in [`ARCHITECTURE.md`](./ARCHITECTURE.md). +This file is the entry point every coding agent reads first, and it holds only three things: the bootstrap that says where the canonical rules live and which procedure to follow for the state this repository is actually in, the rules for managing context and delegation, which apply to every task, and a map of where every other rule lives. The rule text itself is in [`GOVERNANCE.md`](./GOVERNANCE.md), one section per topic. Code style lives in [`CODESTYLE.md`](./CODESTYLE.md), the CI/CD workflow contract in [`WORKFLOW.md`](./WORKFLOW.md), and this project's architecture, processing pipeline, per-command details, and test layout in [`ARCHITECTURE.md`](./ARCHITECTURE.md). Treat this file and `GOVERNANCE.md` as authoritative for cross-cutting rules, and do not restate their rules elsewhere. This project's own **code-style** conventions, the formatting and language rules a reviewer enforces, live in [`CODESTYLE.md`](./CODESTYLE.md), **not** in [`.github/copilot-instructions.md`](./.github/copilot-instructions.md), because that file targets GitHub Copilot / VS Code specifically, while this file and `GOVERNANCE.md` are the agent-agnostic ones every coding agent is directed to read, so any rule a reviewer must honor has to live in one of those two files to be provider-independent. This project's **design** conventions, its architecture, processing pipeline, and implementation patterns, are `ARCHITECTURE.md`'s own subject and are read there rather than restated here. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index bb9e2c4..0032b21 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -17,6 +17,7 @@ The specific rules in this file implement a few governing principles. Read these - **Durable knowledge lives in the committed docs, not in agent memory.** Anything a future agent must honor (a rule, a contract, a hard-won gotcha, a pattern worth repeating or one to avoid) belongs in a committed governance file (`AGENTS.md`, `CODESTYLE.md`, `WORKFLOW.md`, or a committed backlog such as a `README.md` TODO section). Agent memory does not survive a new session, a new machine, or a new environment, so it holds only environment-specific nuance and in-flight session state, never anything whose loss on reset would matter. A durable lesson left only in memory is lost to the next agent. - **Keep the governance current as you work.** When work surfaces something durable (a rule worth enforcing, a recurring gotcha, a positive pattern to repeat, a negative one to design out), record it in the governance docs as part of that change, rather than leaving it in a local note or routing around it with a one-off workaround. Where the governing doc is carried from a template this repo cannot edit directly, propose the change upstream instead of only fixing it locally. Governance is not static: it improves by agents folding good patterns in and designing bad ones out. +- **A durable rule earns a mechanical hook only where a hook can actually decide it, otherwise it stays prose.** Three conditions together, not any one alone. The failure recurs even after the governing prose was demonstrably read and understood, so it is not a discovery or loading problem a structural fix (getting the rule into context at all) would already solve. The triggering shape is decidable from the tool call's own text, arguments, and working directory alone, with no semantic or contextual judgment required. And the failure is destructive or hard to reverse rather than a quality miss. A worktree-isolation lapse met all three (the #1073 incident happened under prose the agent had already read, "is this command's target a primary checkout" is a plain directory comparison, and the harm is another task's swept or reverted work), so it was promoted to a `gh-write-guard` hook rule. A skill's own trigger going unread by the session at all, by contrast, is a loading problem, fixed by getting the rule into context (the `CLAUDE.md` importing `AGENTS.md`), not by a hook. And "was this review finding actually evidence-backed" fails the second condition outright: a hook sees only the command text, never the judgment call itself, so it can only ever nag, not decide, and that class of rule stays prose and a chained Skill trigger. This section keeps the full rules and is surfaced at its decision moment by the `agent-conduct` Skill at `.agents/skills/agent-conduct/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo. @@ -30,7 +31,7 @@ A state-changing GitHub call is the highest-blast-radius thing an agent does her - **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works: decide it should happen, make it happen, and read the result. Never append output-discarding redirection or a force-success tail to a mutation (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`), because the write's output is exactly what must be read. A write that appears to fail is **verified, not assumed harmless**, because the operation may have succeeded on the server while the client reported an error, so confirm the actual state before retrying or moving on. The ban targets hiding a *failure*. An ad-hoc call's response is the only signal you get, so `>/dev/null 2>&1`, `|| true`, and `|| echo`, which swallow the error stream or force success, are never acceptable on one. A committed script under `set -e` is a narrow exception: it may send a write's *stdout* to `/dev/null` to drop the success-response noise, because stderr stays visible and a failed write still aborts loudly (the hub's own `repo-config/configure.sh` does exactly this, and a repository reaches it there rather than carrying a copy). The exception is stdout-only suppression inside a reviewed, fail-loud script, never `2>&1` or a force-success tail, and never an ad-hoc command. - **A refused write is reported, never re-shaped, and the maintainer's say-so does not lift a refusal by the harness.** These are two different permissions and only one of them is the maintainer's to give. When the agent harness refuses a write, the maintainer authorizing it in conversation does not change the outcome, and the identical call is refused again, so a second attempt is not worth making and reading the second refusal as a flake is how an agent starts hunting for another shape of the same request. **That hunt is the failure this rule exists to stop.** Re-expressing a refused `gh` command as a raw `gh api -X POST` reaches the same endpoint with the same identity and the same blast radius, having defeated the one control that stopped it, and it is the more dangerous version because the agent believes it has permission. So a refused write is never re-attempted through a different API surface, a different tool, or a rephrasing, and it is never routed around by the agent writing itself a permission rule, which is self-authorization whatever the maintainer said. Two routes remain, both of them the maintainer's: they add the permission rule themselves, or they run the command themselves. Raise it as a blocked decision naming those two (see "Communicating with the User"), and where the work needs the result rather than the call, say what the agent will verify once the maintainer has run it. **A refusal is also a fact about the contract, not just about the session**: where a required verification can only be performed by a write the agent is refused, the document requiring it says so and names who runs it, since a check that is mandatory and unperformable is quietly dropped and then reported as done. - **Each task runs in its own checkout, in its own directory, on its own feature branch.** The unit is the task rather than the agent, since one agent moving between two repositories meets the same hazard as two agents sharing one tree, and a rule written per agent permits exactly the case that goes wrong. The commands that cross the boundary are the ordinary ones rather than the reckless ones, and each is correct in isolation: a blanket `git add -A` sweeps another task's uncommitted work into the commit, a `git reset --hard` deletes it, and a branch switch carries it into an unrelated change. The mechanical habit that holds the rule up is that a mutating command takes an absolute path, or a `cd` to one in the same invocation, rather than the working directory it inherited, because a read in the wrong directory is a wasted call and a write there is damage. -- **A task isolates into its own worktree before its first file edit, and a continuation re-isolates.** All new work begins by creating a unique git worktree (or clone) on its own feature branch, based on the branch work starts on for the repository's model per "Branching Model", which is `develop` unless the task is explicitly about `main`-only content. The primary checkout is the maintainer's own surface, so a session launched there isolates before writing rather than after noticing contention, and a session resuming a prior task creates a fresh worktree rather than resuming wherever its branch happens to be checked out, since a branch sitting checked out in a shared tree is exactly how two sessions end up in one checkout. The moment this rule binds is the first file edit, because the commit-time and review-time checks all run after another task's uncommitted work can already be swept. The worktree mechanics, the layout convention, and the cleanup are packaged as the `repo-worktree` Skill at `.agents/skills/repo-worktree/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo, and this section keeps the rule. +- **A task isolates into its own worktree before its first file edit, and a continuation re-isolates.** All new work begins by creating a unique git worktree (or clone) on its own feature branch, based on the branch work starts on for the repository's model per "Branching Model", which is `develop` unless the task is explicitly about `main`-only content. The primary checkout is the maintainer's own surface, so a session launched there isolates before writing rather than after noticing contention, and a session resuming a prior task creates a fresh worktree rather than resuming wherever its branch happens to be checked out, since a branch sitting checked out in a shared tree is exactly how two sessions end up in one checkout. The moment this rule binds is the first file edit, because the commit-time and review-time checks all run after another task's uncommitted work can already be swept. The worktree mechanics, the layout convention, and the cleanup are packaged as the `repo-worktree` Skill at `.agents/skills/repo-worktree/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo, and this section keeps the rule. For Claude Code sessions, the `gh-write-guard` hook now backstops a mechanical subset of this rule directly, denying a mutating git operation (`reset`, `add`, `commit`, and most of the rest, a documented handful of exemptions such as a fast-forward-only pull kept aside) run against a primary checkout rather than a worktree. Every other agent, and everything about isolation a hook cannot see (which checkout a read happens in, whether another task is live in the tree), still relies on this prose alone. The parenthetical "(or clone)" above names `repo-worktree`'s own standalone-clone fallback, used when a linked worktree is unavailable, and that fallback is itself structurally a primary checkout to the hook's own primary-vs-worktree test, so a session using it on Claude Code sets the hook's `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT` grant for that session, per `repo-worktree`'s own instructions, rather than being silently denied the commits the fallback exists to make. - **A checkout another task is live in is left rather than shared, and a footprint already left there is undone deliberately.** Two signals say someone else is in the tree, a branch that changes when nothing you did changed it, and an edit of yours reverted with no conflict, and the response to either is to stop rather than to re-apply the edit, which is the instinct and the wrong one. Leaving and cloning your own costs about a minute against an incident that costs the better part of an hour, so it is the cheap move rather than the cautious one. Once you have written there, leaving it alone arrives too late, so save your work aside, restore only the files you touched, verify the tree is clean, delete your branch from that clone, and then say plainly what was touched, since a regenerated report left behind reads as the other task's own and is committed by whoever runs the next blanket add. ## Representative Data in Agent-Authored Text diff --git a/WORKFLOW.md b/WORKFLOW.md index de7ddfc..fbc6bf3 100644 --- a/WORKFLOW.md +++ b/WORKFLOW.md @@ -202,7 +202,7 @@ The required behaviors, organized by domain. Each is a **MUST**, stated as input - **D8.1 Merge-bot.** Output: enables auto-merge on `opened`/`reopened` for **every** Dependabot tier including semver-major (the required checks are the gate, not the bump magnitude); dispatches `--squash`/`--merge` by the PR's base ref; disables on a maintainer-pushed `synchronize`; concurrency keyed on the **PR number**, not `github.ref`. *Prevents: two PRs colliding in auto-merge.* - **D8.2 CodeGen and Dependabot.** Output: codegen runs as a matrix over both branches and is deterministic from an external source. Dependabot targets both branches, security PRs to default. -- **D8.3 Upstream-version tracker.** Output: a scheduled resolver prints a JSON `name -> version` object to a committed state file, opens a rolling per-branch bump PR naming only the moved keys, the merge-bot auto-merges it. The `main` pin push publishes via the release gate, while a `develop` pin does not auto-publish. It ships via a `develop` dispatch (prerelease) or the next promotion to `main`. The tracker's `bump-branch-prefix` + `branches` MUST match a merge-bot rule, one of the built-in `-` head/base pairs or a `rules` entry the caller passes, or auto-merge silently never fires. +- **D8.3 Upstream-version tracker.** Output: a scheduled resolver prints a JSON `name -> version` object to a committed state file, opens a rolling per-branch bump PR naming only the moved keys, the merge-bot auto-merges it. The `main` pin push publishes via the release gate, while a `develop` pin does not auto-publish. It ships via a `develop` dispatch (prerelease) or the next promotion to `main`. The tracker's `bump-branch-prefix` + `branches` MUST match a merge-bot rule, one of the built-in `-` head/base pairs or a `rules` entry the caller passes, or auto-merge silently never fires. A tracker whose bump needs a human decision instead sets `auto-merge: false`, which prefixes the head so no merge-bot rule matches it, whatever `bump-branch-prefix` names. - **D8.4 An identity allowlist used as a gate fails loud.** Where a gate compares `github.actor` (or a PR author) against hard-coded bot identities, the non-matching branch on an otherwise-legitimate trigger **emits a `::warning::`** rather than falling through silently. Output: a run that declines to act on an unrecognized identity is visibly annotated. *Prevents: the App being renamed, replaced, or reinstalled under a new slug, after which the comparison quietly evaluates false and the gate stops firing, a green and silent run that looks identical to a healthy one.* The masking matters most where a second path hides the loss: a weekly schedule keeps publishing, so the only symptom is release *timeliness*, easily missed for months. Where the failure is self-announcing instead (the merge-bot simply stops merging, so bot PRs visibly pile up) an annotation is optional. Resolving the identity at run time (mint an App token, read `GET /app`) removes the hard-coded string entirely and is the escalation if an allowlist proves fragile in practice. ### D9 - Style / Static (See Section 2)