Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions .github/skills/pr-review-conduct/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ description: >-
Governs opening, driving, and merging a pull request review loop in a ptr727/ProjectTemplate
fleet repo: requesting a review after a push, triaging findings (including suppressed
low-confidence ones), replying and resolving threads, and deciding whether a PR is actually
mergeable. Use this whenever about to merge a PR, enable auto-merge, ask the maintainer for
merge permission, push a fix and move on without re-checking review state, or judge a PR
"green" or "clean" from CI or mergeStateStatus alone. Triggers even when the request sounds
routine, such as "merge this" or "it's all green, go ahead," because mergeStateStatus: CLEAN
mergeable. Use this whenever about to open a PR, immediately after creating one, about to merge
a PR, enable auto-merge, ask the maintainer for merge permission, push a fix and move on without
re-checking review state, or judge a PR "green" or "clean" from CI or mergeStateStatus alone.
Triggers even when the request sounds routine, such as "open a PR," "merge this," or "it's all
green, go ahead," because PR creation starts the review loop and mergeStateStatus: CLEAN
can go clean once checks pass and every known thread is resolved, while still saying nothing
about whether the review that resolved those threads covered the current head SHA, read the
full diff, or left a suppressed low-confidence finding, which opens no thread at all,
Expand Down Expand Up @@ -56,21 +57,31 @@ full stop, whatever the merge-state field says.

## Expected review loop

1. Push changes to the PR branch.
2. Re-request a review for the **current head SHA**. Auto-trigger is unreliable, so request it
Opening a pull request starts this loop by default. Creating the PR is not a terminal handoff.
Only an explicit maintainer instruction may stop, defer, or alter the loop. A draft state, silence,
or a request that says only "open a PR" is not such an instruction.

Run every `scripts/pr_review.py` command below from a hub checkout. The script is hosted there and
is never carried into a downstream repository.

1. Push changes to the PR branch and open the pull request when it does not exist.
2. Run `scripts/pr_review.py status` once in the foreground and read its output.
3. Re-request a review for the **current head SHA**. Auto-trigger is unreliable, so request it
explicitly (mechanics in the Copilot runbook). The UI is a fallback only.
3. Wait for review activity on that head. A completed review raising **no findings** is a valid
terminal outcome, so don't re-trigger it or read silence as a missing review. A review whose
body says it declined to review is the one exception, and it is terminal the other way:
nothing follows it, and re-requesting the same head just repeats the decline.
4. Triage findings (see below).
5. Apply fixes or write a rationale for declines.
6. Reply to each thread and resolve what was addressed.
7. Re-run the loop after every fix push until no actionable finding remains.
4. Run a bounded `scripts/pr_review.py wait` in a background process and read its terminal output.
A completed review raising **no findings** is a valid terminal outcome, so do not re-trigger it
or read silence as a missing review. A review whose body says it declined to review is the one
exception, and it is terminal the other way. Nothing follows it, and re-requesting the same
head only repeats the decline.
5. Triage findings (see below).
6. Apply fixes or write a rationale for declines.
7. Reply to each thread and resolve what was addressed.
8. Re-run the loop after every fix push until the checks are green and no finding remains open.

Drive to green, a review confirmed on the latest head SHA and every actionable finding closed,
then apply the Merge Gate above. **Never exit the loop early.** A round count is not a stopping
condition, and neither is patience running out.
condition, and neither is patience running out. Reporting only that the PR was opened is an early
exit unless the maintainer explicitly instructed the agent not to monitor or drive its review.

## Every finding ends in one of five outcomes

Expand Down
22 changes: 13 additions & 9 deletions .github/skills/python-codestyle/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ description: >-
profile split, the uv/ruff/pyright/mypy/pytest toolchain, src layout, formatting and linting,
comment and docstring conventions, type hints, naming, imports, patterns to avoid, test
conventions, and versioning. Use this whenever writing, reviewing, or editing a .py file, a
pyproject.toml, or a uv.lock, whenever deciding whether a Python subtree is a shippable project
or a lint-only scripts tree, whenever choosing pyright versus mypy for a repo's CI gate, or
whenever writing or reviewing a pytest test. Triggers even when the task looks like a small
local fix ("just add a helper function", "silence this lint warning", "add a dependency"),
because the profile split, the ruff-is-authoritative rule, and the ban on backward-compat
shims or impossible-case error handling are each easy to violate one file at a time. Applies
only to a repo's Python side, a repo with no Python has no use for this Skill.
pyproject.toml, or a uv.lock, whenever running or choosing a Python formatting, lint, type-check,
or test command, whenever deciding whether a Python subtree is a shippable project or a lint-only
scripts tree, whenever choosing pyright versus mypy for a repo's CI gate, or whenever writing or
reviewing a Python test. Triggers even when the task looks like a small local fix ("just add a
helper function", "silence this lint warning", "add a dependency") or verification step ("run
the tests"), because choosing pytest before reading the profile turns an intentional unittest
suite into a false missing-dependency diagnosis. Applies only to a repo's Python side, a repo
with no Python has no use for this Skill.
---

# Python Codestyle
Expand All @@ -26,11 +27,14 @@ profiles, the toolchain, layout, and the language-level conventions.

## Two profiles

Read the repo's `pyproject.toml` shape and pick the profile before applying any other rule:
Read the repo's `OPERATIONS.md` local-verification commands before substituting a generic command.
Then read the `pyproject.toml` shape and pick the profile before running Python tooling or tests:

- **build** (Project): `[project]` + `[build-system]` + committed `uv.lock`. Uses `uv run`, pytest,
pyright strict (or mypy where the repo requires it).
- **lint-only** (Scripts): no `[project]`, no lockfile. Uses `uvx`, unittest, mypy as CI gate.
- **lint-only** (Scripts): no `[project]`, no lockfile. Uses `uvx` for third-party tools, unittest
for tests, and mypy as the CI gate. Do not run pytest or diagnose its absence as an environment
defect. Use the repository's exact coverage command and unittest scope from `OPERATIONS.md`.

For the full profile specification and per-repo adaptation axes (type checker, dependency
declaration, versioning, VS Code config), see `references/profiles.md`.
Expand Down
34 changes: 27 additions & 7 deletions .github/skills/repo-worktree/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ description: >-
branch a tool defaulted to), with a standalone-clone fallback when an executor cannot write
both the standard worktree and its Git metadata. Also wraps the mechanics:
creating a worktree with git worktree add, the fleet layout convention, listing what is in
flight, and removing a worktree and its branch after merge. Use this whenever about to create
or edit files in a fleet repo, whenever starting or resuming a task, whenever the task's
branch is already checked out in a shared checkout, and whenever creating, listing, or
removing a worktree. Triggers even when the session was launched in the primary checkout or
the change looks like a one-line fix, because the primary checkout is the maintainer's own
surface and the incident this guards against was two sessions sharing one checkout, each
session's blanket add committing the other's uncommitted files.
flight, preparing Husky.Net or Python pre-commit hooks in the new tree, and removing a
worktree and its branch after merge. Use this whenever about to create or edit files in a
fleet repo, whenever starting or resuming a task, whenever the task's branch is already
checked out in a shared checkout, and whenever creating, listing, or removing a worktree.
Triggers even when the session was launched in the primary checkout or the change looks like
a one-line fix, because the primary checkout is the maintainer's own surface and the incident
this guards against was two sessions sharing one checkout, each session's blanket add
committing the other's uncommitted files.
---

# Repo Worktree
Expand Down Expand Up @@ -150,6 +151,25 @@ a `name`, it creates the worktree under `.claude/worktrees/` inside the repo and
GitHub default branch, which is the wrong path and the wrong base here. Create the worktree with
`git worktree add` as above, then attach with `EnterWorktree` `path:`, not `name:`.

## Preparing Git Hooks

A new worktree holds tracked hook configuration but not every generated hook runtime. Prepare
the hooks immediately after creating or attaching the worktree, before the first commit. A
shared `core.hooksPath` value does not make generated files such as `.husky/_/husky.sh` appear
in the new tree.

- **Husky.Net:** When `.husky/pre-commit` sources `.husky/_/husky.sh` and the local .NET tool
manifest declares Husky.Net, run `dotnet tool restore`, then `dotnet husky install` from the
worktree root.
- **Python pre-commit:** When `.pre-commit-config.yaml` exists, install the repository's declared
Python environment, then run `pre-commit install` through that environment. A uv project runs
`uv sync --frozen`, then `uv run pre-commit install`.
- **Repository override:** Follow a repository's explicit hook-setup instructions when they
differ from these standard cases. Do not infer a replacement command from the language alone.

Treat hook preparation as worktree setup, not as recovery after a rejected commit. If setup
fails, report that boundary and fix the setup. Never bypass the hook to make the commit succeed.

## Listing and Cleanup

- `git worktree list`, run in any checkout of a repo, names that repo's base clone and every
Expand Down
46 changes: 36 additions & 10 deletions PhotoCleaner/ImportTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,12 @@ private async Task<ImportResult> ImportFileAsync(

string? sha256 = null;
string? sha1 = null;
FileRecord? cached = null;
if (database is not null || skipDatabase is not null || trashDatabase is not null)
{
// ResolveHashesAsync returns cached hashes when size and mtime still match disk.
// The cache is keyed by source path because import inserts source paths.
FileRecord? cached = database is null
cached = database is null
? null
: await database.GetByPathAsync(file, cancellationToken).ConfigureAwait(false);
Log.Debug("Hashing '{FilePath}'", file);
Expand Down Expand Up @@ -341,20 +342,45 @@ await MediaUtilities
// Lookups are by source content hash.
if (database is not null && sha256 is not null && sha1 is not null)
{
Log.Debug("Inserting source '{SourcePath}' with SHA-256 '{Sha256}'", file, sha256);
await database
.InsertAsync(
new FileRecord(
if (cached is null)
{
Log.Debug(
"Inserting source '{SourcePath}' with SHA-256 '{Sha256}'",
file,
sha256
);
await database
.InsertAsync(
new FileRecord(
file,
sha256,
sha1,
sourceInfo.Length,
sourceInfo.LastWriteTimeUtc.Ticks,
false
),
cancellationToken
)
.ConfigureAwait(false);
}
else
{
Log.Debug(
"Updating source '{SourcePath}' with SHA-256 '{Sha256}'",
file,
sha256
);
await database
.UpdateHashesAsync(
file,
sha256,
sha1,
sourceInfo.Length,
sourceInfo.LastWriteTimeUtc.Ticks,
false
),
cancellationToken
)
.ConfigureAwait(false);
cancellationToken
)
.ConfigureAwait(false);
}
}

return ImportResult.Imported;
Expand Down
77 changes: 77 additions & 0 deletions PhotoCleanerTests/ImportTaskTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,83 @@ public async Task ExecuteAsync_ImportTwice_SecondRunSkipsAndCreatesNoDuplicate()
}
}

[Fact]
public async Task ExecuteAsync_ReplacedSource_UpdatesRecordAndNextRunSkips()
{
string srcDir = TempDir();
string outDir = TempDir();
string dbPath = TempDb();
try
{
string jpg = Path.Combine(srcDir, "photo.jpg");
File.Copy(fixture.SourceFile(TempDirectoryFixture.SmallJpegFile), jpg);
await SetExifDateAsync(jpg, "2024:06:15 10:00:00");

await using Database db = new(dbPath);
await db.InitializeAsync(TestContext.Current.CancellationToken);
ImportTask task = new(
CreateOptions(outDir),
database: db,
skipDatabase: null,
trashDatabase: null,
new()
);

(int organized1, _, int skipped1, _, _, _, int failed1, _) = await task.ExecuteAsync(
[jpg],
new DirectoryInfo(srcDir),
TestContext.Current.CancellationToken
);
organized1.Should().Be(1);
skipped1.Should().Be(0);
failed1.Should().Be(0);

DateTime firstMtime = File.GetLastWriteTimeUtc(jpg);
await SetExifDateAsync(jpg, "2024:07:15 10:00:00");
File.SetLastWriteTimeUtc(jpg, firstMtime.AddSeconds(2));
FileInfo replacedInfo = new(jpg);
(string replacedSha256, string replacedSha1) = await Database.ComputeHashesAsync(
jpg,
cancellationToken: TestContext.Current.CancellationToken
);

(int organized2, _, int skipped2, _, _, _, int failed2, _) = await task.ExecuteAsync(
[jpg],
new DirectoryInfo(srcDir),
TestContext.Current.CancellationToken
);
organized2.Should().Be(1);
skipped2.Should().Be(0);
failed2.Should().Be(0);

FileRecord? row = await db.GetByPathAsync(
jpg,
cancellationToken: TestContext.Current.CancellationToken
);
row.Should().NotBeNull();
row.Sha256.Should().Be(replacedSha256);
row.Sha1.Should().Be(replacedSha1);
row.FileSize.Should().Be(replacedInfo.Length);
row.MtimeTicks.Should().Be(replacedInfo.LastWriteTimeUtc.Ticks);

(int organized3, _, int skipped3, _, _, _, int failed3, _) = await task.ExecuteAsync(
[jpg],
new DirectoryInfo(srcDir),
TestContext.Current.CancellationToken
);
organized3.Should().Be(0);
skipped3.Should().Be(1);
failed3.Should().Be(0);
File.Exists(Path.Combine(outDir, "2024-07", "photo_1.jpg")).Should().BeFalse();
}
finally
{
Directory.Delete(srcDir, recursive: true);
Directory.Delete(outDir, recursive: true);
File.Delete(dbPath);
}
}

// -- Helpers --------------------------------------------------------------

private static async Task<string[]> GetXmpSubjectAsync(string filePath)
Expand Down
Loading