Skip to content

Promote develop to main: verify command, hub conformance round 4, version floor 1.1 - #45

Merged
ptr727 merged 12 commits into
mainfrom
develop
Aug 3, 2026
Merged

Promote develop to main: verify command, hub conformance round 4, version floor 1.1#45
ptr727 merged 12 commits into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Promotes twelve commits to main. This is the first promotion that carries a functional change: the verify command, a set of breaking exit-code changes, and the version.json floor raised to 1.1 to match.

What is in it

PR Change
#35 verify command: runs the Immich decoder inside the Immich image to confirm each file renders. Breaking exit-code changes come with it
#36 Consume the fleet prose rules from the hub, and wire the prose gate into validate-task.yml
#37 New OPERATIONS.md: how the repository is run, meaning the local gates, the release pipeline, and the external tools
#38 Merge-bot workflow: auto-merge in-repo bot pull requests, merge method chosen by base ref
#39 Re-vendor the six verbatim fleet-law regions from the hub
#40 Refresh the carried intent docs against the hub
#41 Restore the CRLF-default line-ending model, and pin the Dockerfile and pre-commit hook to LF
#42 D4.5 publish gate: build-docker needs build-executable, so a failed executable build can no longer push an image with no release
#43 Raise the version.json floor to 1.1
#44 Open the local gate chain with a dotnet tool restore
#46 Correct the exit-code and Docker claims in OPERATIONS.md
#47 Kill the leaked docker probe in the verify test helper

Versioning: this promotion does bump the floor

Unlike the previous two rounds, version.json moves, from 1.0 to 1.1, in #43. HISTORY.md already declared **Version: 1.1** for the verify command and the breaking exit-code changes, while the floor still read 1.0, so the next release cut from main would have been 1.0.18 and would have contradicted the changelog shipping with it. Latest release is 1.0.17.

That is the Release Model working as intended: the 2-digit line is raised by maintainer instruction for a functional change, and NBGV owns the patch position from git height. Only version.json changed. The Version and InformationalVersion properties in PhotoCleaner/PhotoCleaner.csproj are local-build placeholders that never reach a published artifact, since get-version-task.yml runs NBGV once and threads the computed values into the build jobs.

Two changes worth reading closely

The verify command carries no container parser of its own, deliberately. Such a parser condemns whatever it fails to understand, so an unfamiliar but valid format is indistinguishable from a damaged one from the inside. Running the Immich decoder makes that decoder the only judge of a file health, which is the judgment that actually matters for an Immich library. Docker is therefore required for verify.

The D4.5 publish gate closes a partial-publish hole. Before #42, a failed build-executable skipped github-release while build-docker still pushed and moved latest, leaving an image with no release behind it. build-docker now needs build-executable and guards with !failure() && !cancelled(), so a failed upstream build stops the push while a skipped one does not.

Review audit

Every constituent PR was audited across all its reviews rather than only the head one, because a suppressed finding carries no thread, so reviewThreads reads zero unresolved and mergeStateStatus reads CLEAN while it stands.

  • 54 suppressed rounds across #35, #36, #37 and #38; #39 through #44 carried none.
  • One was genuinely unanswered: the OPERATIONS.md tool-restore finding on #37 merged head. Confirmed against a cold package cache, fixed in #44, and answered on #37.
  • The four on #38 were all correct and deliberately not fixed here: they land on fleet-carried YAML, so a one-repo change would diverge from the canonical and be reverted by the next re-vendor. Raised upstream as ptr727/ProjectTemplate#521.
  • Two further findings came from Copilot on this PR, both correct and both fixed in #46. OPERATIONS.md claimed a per-file failure does not change the exit code, when ExitCode.Failed is 2 and seven call sites return it, and it claimed the application needs no Docker daemon at runtime, when verify requires one. Neither was catchable reviewing against develop, where that file is unchanged. Diffing against main is what surfaced them, which is an argument for reading a promotion diff as a whole-file review rather than a formality.
  • A third finding came in suppressed on this PR's own round-2 review, and was fixed in #47: the verify test helper started docker image inspect and returned on a 60-second timeout without ending the child, so the probe outlived the test. Notably the review-polling script reported suppressed=0 for that round, because its parser expects a <summary> shape and that review used a ### heading. Reading every review body directly is what caught it.
  • Spot-checked that the accepted fixes actually landed. The && 0 || 1 falsy-expression bug from #36 is fixed on develop, with both depths quoted and a comment recording why they must stay quoted.

Verification

  • Merge dry-run into main applies with zero conflicts across 49 files. No EOL trap on these paths despite #41 touching the line-ending model.
  • dotnet test: 372 passed, 0 failed, 0 skipped. The Docker-gated tests ran rather than silently skipping.
  • Build clean at 0 warnings with TreatWarningsAsErrors. dotnet format style --verify-no-changes: 0 of 57 files. dotnet husky run: pass.
  • markdownlint-cli2 over **/*.md: 0 issues in 11 files. editorconfig-checker clean. Hub prose_lint.py clean.
  • Hub audit returned zero defect, letter, or error findings (audit run 2026-08-03T17:46:41Z, hub 943b564). That run was against 252f5c5, so it covers #35 through #42 but not the four commits added since. Those four touch version.json, OPERATIONS.md and a test helper, none of which is hub-carried content, so no re-audit was run for them.

What happens after this merges

Merging publishes nothing. publish-release.yml runs on workflow_dispatch and a weekly schedule only, never on a human push. The next scheduled main run (Mon 02:00 UTC) or a manual dispatch cuts the release, which will be the first 1.1.x.

Merge with a plain merge commit. Never squash, and never --delete-branch, since the head of this PR is develop itself.

Issue #14 stays open: the merge-bot shipped in #38, so only the date badge remains.

ptr727 and others added 10 commits August 2, 2026 12:57
@ptr727 @claude
Add the merge-bot workflow (#38)
aed71c4 
Dependabot opens pull requests against both branches here, and nothing
merged them, so every dependency bump waited on a manual merge and the
action pins went stale between sweeps.

Carry the two jobs the contract requires. The fleet canonical also
carries merge-codegen and merge-upstream-version, and this repository
runs neither, so both are left out rather than carried as conditions
that can never match.

The App-token secrets the workflow needs are already configured in both
the Actions and Dependabot stores.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Add OPERATIONS.md (#37)
5aa4b13 
* Add OPERATIONS.md

The hub requires this file of every repo, checked for presence only, so
its content is entirely this repository's own. It exists because the
convention was emerging unevenly and the same operational material was
landing under ad-hoc names.

Carry the five declared headings with real content rather than a stub:
the local gates that mirror CI, the dispatch-driven release, the runtime
dependency verify has on Docker and on a database of its own, what the
three exit codes distinguish, and why exiftool validation warnings are
logged at debug level only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Say how the CI invocation differs from the local one

The runbook claimed CI runs the same commands, and it does not: locally
csharpier formats and writes, while CI runs it in check mode and only
verifies. A reader following the text would expect CI to fix formatting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe the commands this branch actually has

The file documented the verify command, the exiftool -validate flag, and
a per-file-failure exit code of 2. None of those exist on develop: they
are on the unmerged verify branch, and this branch was cut from develop.

Describe what is here instead. Exit codes are 0 and 1, a per-file
failure does not change them, exiftool is invoked without -validate, and
the application needs no Docker daemon at runtime.

The verify operational content follows once that work merges.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Restore the line endings on OPERATIONS.md

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Name the two places CI differs, and the lint step the snippet omitted

The text claimed CI runs dotnet test unchanged, and it adds coverage
collection so coverlet can emit the report Codecov consumes.

The lint snippet listed three of the four Lint tasks, leaving out
actionlint, so following it ran less than the local surface it claimed
to match.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Say that the local linters and CI's are the same rules, not the same builds

The local commands pull :latest while CI reaches three of the four
through SHA-pinned wrappers, so a local result can differ from CI once
an upstream release lands ahead of the pin. Name CI as authoritative and
the difference as a version gap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Recommend the key file rather than naming it the only mechanism

The CLI accepts an inline --apikey too, and the two are mutually
exclusive by validator, so stating the file form as the only one was
wrong. Recommend it and say why.

Drop the trailing claim that a local Docker linter run matches what CI
resolved, which contradicted the paragraph above it saying local pulls
latest while CI reaches three of the four through pinned wrappers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Name the parse error among the exit-1 causes

BypassStartup short-circuits when the parse result carries errors, so a
command-line mistake exits 1 before any work starts. The list named only
cancellation and unhandled exceptions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Consume the fleet prose rules from the hub (#36)
4a7403d 
* Consume the fleet prose rules from the hub

The prose rules that govern this repository live in the hub, and until
now nothing here ran them, so a comment or a sentence breaking a
documented rule reached main with every linter green.

Consume the hub's composite action rather than vendoring its checker, so
a rule change lands in one place instead of in every repository holding
a copy. A develop-targeted run reads the rules from hub develop, so an
unpromoted change is exercised here before it is promoted, and every
other run uses the copy bundled at the pinned commit so a released build
stays reproducible.

The gate reports only lines a change touches, so the repository's
existing prose backlog blocks nothing and is corrected as each file is
next edited.

Checkout gains full history in the lint job, because diffing against the
base branch needs that branch present.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Point the prose gate at the branch it merges into

The gate runs on push, because this repository has no pull_request
trigger, so there is no event base to read and the first run diffed
against an empty ref. Unresolvable, it reported the whole repository
instead of the lines this change touches.

Name the base explicitly as the branch being merged into, and skip main,
which only receives promotion merges already gated on develop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Skip the prose gate on a publish run

A publish reaches validate-task through build-release-task, where the
content was already gated when it was pushed. Re-reading it there would
diff the whole unpromoted delta against main and could fail a release on
prose that already passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Cut the publish-run comment to one sentence per line

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe what this workflow does, and mark the pin temporary

The comment described the action's main-run behavior, which this
workflow never reaches because it skips main outright. A maintainer
reading it would assume a prose gate runs on main.

The pin targets an unmerged hub commit, and that intent lived only in
the pull request description, which does not survive the merge. State it
inline instead, including that Dependabot cannot bump a pin resolving to
no tag, so the repoint is manual.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Only pay for full history on the runs that use it

The lint job always full-cloned, including publish runs where the prose
gate is skipped, which is time and network a release does not need. Tie
the fetch depth to the same condition the gate uses, so the two cannot
drift apart.

Repoint the pin to the hub branch head, which carries the fix for a
multi-line paths input that scanned only its first entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Quote the fetch depths so the true branch is not falsy

An unquoted 0 is falsy in an Actions expression, so the ternary
collapsed to 1 whenever the condition held and every run shallow-cloned,
leaving the prose gate no base branch to diff against.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Cut the wrapped comment sentences to one per line

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Repoint the prose gate at the hub commit that carries it

The pin named the head of an unmerged hub branch, which would have gone
unreachable once that branch was squashed and deleted, breaking this
repository's gate later with nothing here to explain it.

ptr727/ProjectTemplate#520 has landed, so point at the commit on hub
develop instead. It still carries no release tag, so Dependabot cannot
bump it yet and the note says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe the pin as a SHA rather than as a branch

A pin that calls itself develop stops being true the moment develop
moves. Name what it contains instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Add a verify command that confirms Immich can render each file (#35)
39c896b 
* Add a verify command and always-on exiftool validation

Adds `verify`, a standalone pipeline step that answers whether Immich can
generate a preview for each file, and turns on exiftool `-validate` in the
metadata read that `process` and `import` already perform.

Fixes #25, where ~8,200 HEIC files were byte-complete, passed every existing
check, uploaded successfully, and then failed thumbnail generation forever. An
`iloc` box re-encoded from version 1 to version 0 loses `construction_method`,
so the `grid` descriptors Apple stores in `idat` are read as absolute file
offsets and land on the `ftyp` header.

That incident is only one of three classes. Two more are open against Immich:
DNGs libraw cannot parse, and RAW from cameras it does not yet recognize.
Neither is corruption, so no parser can predict them - only running the decoder
Immich runs can. `verify` therefore has two passes:

- A structural ISO-BMFF box walk, in-process and free, reporting extents past
  EOF, an `iloc` v0 alongside an `idat` box, and `grid` geometry that disagrees
  with the `dimg` reference count. `--quick` stops here and needs no Docker.
- A decode pass that calls Immich's own compiled `MediaRepository`,
  `defaults`, and `ThumbnailConfig` inside the `immich-server` image, batching
  paths over stdin. Calling Immich's code rather than reimplementing its
  pipeline means behavior tracks Immich across releases.

A preflight runs before any file is judged, so an unreachable Docker or a
missing image exits Error rather than condemning the collection.

Measured on a real collection, `-validate` costs nothing on top of the existing
exiftool call, but ~75% of healthy files carry warnings, so only an error count
fails a file; warnings are logged at debug level.

Commands now share one exit-code contract: 0 success, 1 could not run, 2 ran to
completion with per-file failures. This is a behavior change - `process`
previously exited 0 while logging failed files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Document the verify database behavior

No behavior change. The verified state is a single bit rather than a record of
which check produced it, and two consequences of that were undocumented:

- A --quick run after a full run skips every file the full run passed, so it
  can legitimately do nothing. Correct, since a full run subsumes the
  structural checks, but surprising without a note. --reprocess forces it.
- The bit lives in the same is_processed column that process writes, so
  pointing verify --db at a Process.db silently skips every file as "already
  verified" when they were only processed. Nothing detects this, so the
  requirement to use a separate Verify.db is now stated rather than implied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Bring comments in the verify code to house style

The verify code was written with XML doc blocks on internal members and
multi-sentence prose comments, matching neither the documented rules nor the
surrounding code.

GOVERNANCE.md bans class/type/file header summary blocks, calls for one
sentence per line with no wrapping, and defaults to a single line. CODESTYLE.md
scopes XML documentation to public surfaces, and everything here is internal.
The pre-existing PhotoCleaner sources carry two XML summaries in total.

Removes every XML doc block from the new files, collapses the header blocks to
the single non-obvious point each type actually needs, and rewrites the wrapped
prose comments as one sentence per line. Net 196 lines of comment removed with
no code change; tests unchanged at 367 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Parallelize the verify partition pass

PartitionAsync hashed every file in a sequential foreach while every other bulk
path in the codebase uses Parallel.ForEachAsync bounded by --threads. On a real
264k-file tree that measured 446 files/min, putting roughly ten hours of
single-threaded hashing in front of the first verification.

Now matches IndexTask.ExecuteAsync: parallel, with a ConcurrentBag for the
candidate list and Interlocked counters. Measured 908 files/min on the same
storage. The remaining ceiling is the disk, not the code, since hashing reads
every byte of the tree off spinning disks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Verify per file in quick mode and per batch in full mode

Verification ran as three global barriers: hash every file, structurally check
every file, then decode in batches, recording database state only at the very
end. On a 264k-file tree that meant roughly five hours of hashing before the
first file was verified, every file read twice with nothing left in cache
between the two reads, and no progress retained if the run was interrupted.

Quick mode now processes one file at a time in parallel, matching the shape
import and process already use. Full mode does the same work per batch: hash
the batch, decode it in one container, then act on the results and record them.
Batches still run concurrently under --threads, so the container round trip is
amortized without holding the whole tree in front of the first result.

Two container flags follow from running this against a live Docker host. The
image's healthcheck cannot pass under an overridden entrypoint, so every
container reported unhealthy and alerted anything watching Docker; disable it.
And the image may be the same one an existing container already runs, so the
containers this tool starts carry a label and are never selected by image.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Cut wrapped prose comments back to one sentence per line

A sweep across every comment syntax the project ships found prose sentences
wrapped mid-thought throughout the C# sources, which the comment rules
disallow. Rewrites 32 of them as one sentence per line, cutting each block back
to its load-bearing point, and drops the two remaining XML documentation blocks
from non-public members.

Comments-only, no behavior change. Label-and-example blocks are left alone,
since a lead line followed by examples is not a wrapped sentence. The workflow
YAML and repo-config carry the same pattern but are verbatim fleet content, so
they are left for the hub rather than forked here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Decide the exiftool verdict from its output, not its exit code

The metadata read ran with CliWrap's default exit-code validation, but exiftool
exits non-zero on exactly the files whose verdict reports an error, while still
writing the JSON that carries it. The call therefore threw before the verdict
could be parsed, the file was counted as a pipeline failure, and the
fail-on-error branch was unreachable.

Disables exit-code validation for that read and throws only when exiftool
returns no JSON at all, so a genuine tool failure still surfaces. The error text
now rides along in the log rather than only a count.

Reverting the validation change fails the new test, so the case proves the fault
it names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Translate paths across the container boundary

The decode pass mounted the media tree at its own absolute host path and wrote
host paths to the container. A container path must be Linux-style, so that
mount is invalid on a Windows host even though the repo ships and documents a
Windows build.

Mounts at a fixed container path instead and translates each path across the
boundary, mapping the verdicts back by the same table. This drops the
same-path assumption rather than branching on the operating system, so one code
path serves every host.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Log container start and stop for the decode pass

The batch size trades container startup against how long a file waits between
being hashed and being decoded, and neither side of that trade was measurable.

Adds paired start and stop debug lines around each container invocation, so a
debug-level log carries the duration of every launch without a timer in the
code. The preflight carries no files, so its own pair isolates the fixed cost.

Measured over 200 synthetic launches: about 3.8s median fixed overhead, of
which roughly 2.3s is Immich's module graph rather than the container itself.
That part is a floor, since loading those modules is what makes this Immich's
decoder rather than an approximation. At the current batch size the work
outruns startup by about seventy times.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Correct what quick mode records in the database

Both the code comment and the README claimed a quick run records nothing. It
records content hashes through the shared index path whenever --db is given,
and those hashes are then cached for a later run. What it withholds is the
verified bit, so a full run still decodes the file.

Documentation only, no behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Count an unreadable file as failed rather than verified

The structural check caught every exception, logged a warning, and returned
true. A file it could not open was therefore counted as verified, so a
permission error or an I/O fault produced a clean result for a file nothing had
actually checked. For a command whose whole purpose is to answer whether a file
is good, a false clean is the worst possible outcome.

Now follows the per-file pattern the other tasks use: a file that no longer
exists is logged and uncounted, since that races with any other run, and any
other exception is an error against the failed count. Neither path reports the
file as verified, and neither reports it as invalid, because being unable to
read a file is not evidence of damage.

Restoring the old catch fails the new test on Verified being 1 rather than 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Say that exit 2 also covers files that could not be verified

The exit code table claimed a 2 meant files were found to be bad, which stopped
being true once an unreadable file started counting against the failed total. A
run that could not read a file now exits 2 while nothing was actually judged
bad, so a script reading that wording would draw the wrong conclusion.

Says invalid or could not be verified, and points at the two counts that tell
those apart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Raise the decode batch to 1024 files

Per-file decode cost spans fiftyfold across real media, from about 20ms for a
thumbnail to over a second for a raw frame, while container startup is a flat
four seconds. At 256 files a batch of thumbnails spent 37 percent of its time
starting the container, a multiplier of less than two.

1024 puts the cheap end at sevenfold and typical photographs near fiftyfold.
Going further would risk producing fewer batches than threads on a small tree,
which costs more in lost parallelism than it saves in startup.

Measured on three real corpora: 20ms per file for thumbnails, 168ms for
five megapixel JPEGs, and 1103ms for HEIC and video.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Match the missing-file branch on the exception alone

The filter also accepted any exception where File.Exists returned false, and
File.Exists returns false when the path cannot be read at all, not only when it
is absent. A permission error on an unreadable directory therefore landed in
the branch meant for files that went away, logged at information and counted
nowhere, so a run could exit 0 with files that were never checked.

Matching FileNotFoundException and DirectoryNotFoundException keeps that branch
to files that genuinely vanished and lets every other error reach the failed
count, which is what the surrounding pattern already intends.

Restoring the old filter fails the new test on Failed being 0 rather than 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Narrow the same missing-file filter in the process command

Applies the previous commit's correction to the only other place carrying the
pattern, so both command paths classify a missing file the same way. Import and
index never had the lenient branch, routing every exception to the failed
count, so the two are now the whole set.

Left as two inline filters rather than a shared predicate. The condition is a
single type pattern, and an exception filter reads better where it is caught
than behind a call.

The mechanism is proven by the verify test, which fails on the broad filter.
Reaching it through the process command needs a directory to become unreadable
between enumeration and processing, which is a race rather than something a
test can stage honestly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Document verify and replace the icloudpd examples with kei

Carries the maintainer's 1.1 release notes and rewrites the workflow example
around kei, since icloudpd is no longer maintained. kei keeps its settings in a
TOML file rather than on the command line, so the example leads with the config
and the later forms point at it: an interactive login to store the session, a
one-shot sync, and the service form that keeps mirroring on the watch interval.
The stack-specific parts of a real deployment are left out so the compose
example stays portable.

Adds the verify command to the overview, framed around the question metadata
checks cannot answer.

Also corrects three typos that the spelling gate covers, and adds the kei
author's handle to the dictionary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Build the index task once instead of per file

Verify constructed a new IndexTask for every file it considered. The class holds
no mutable state, so one instance serves the whole run, matching how the index
and import commands already build theirs.

The saving is an allocation per file against work measured in megabytes of
hashing, so this is consistency rather than a measurable gain. Sharing the
instance across the parallel loop is safe for the same reason it was safe to
rebuild it: nothing on it is written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Refine application description to enhance clarity across documentation and command line interface

* Cover the invalid import path with a test

Import gained an Invalid outcome for files exiftool reports errors on, but
nothing asserted it. The case now drives a real file through ImportTask and
checks that invalid is one while failed stays zero, that nothing is copied to
the output, and that the source is left alone.

Disabling the branch fails it on invalid being 0 rather than 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Count a vanished file as failed in import and verify

Only process rewrites the tree it walks, so only there is a name that has gone
missing the expected result of the command's own work. Import copies without
renaming its source and verify only reads, so under either of those a file that
disappears between indexing and use means something outside the run changed the
tree, and the run no longer covers what it was asked to.

Verify's lenient branch is removed, leaving one catch that counts everything as
failed, which also brings it in line with import and index. Process keeps its
branch, and all three now carry a comment saying which case they are and why.

Adds a case for a path that is gone by the time it is verified.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Fail an item info box that declares more entries than it holds

The entry loop stopped once the remaining bytes could not hold another header,
then reported success. A container whose iinf promises more entries than it
carries was therefore treated as sound, and because the missing entries are
where item types live, a grid item could go unrecognized and skip the geometry
check that exists to catch it.

Running out of box before the declared count is now a malformed structure.

Checked against real media before tightening, since a validator that flags a
good file is worse than one that misses: unchanged on the regression corpus,
and no new findings across roughly fifteen thousand healthy HEIC, MP4 and MOV
files from three collections.

Also carries the maintainer's workspace edit, sorting the extension
recommendations and adjusting the list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Clear the prose gate on the lines this branch changed

The spaced hyphen and the mid-sentence semicolon are both out, replaced by
commas, separate sentences, or parentheses rather than an em dash, which the
character set rules ban in the same terms.

Confined to the lines this branch touched, per the rule that existing prose is
corrected as a file is next edited rather than swept. In the Copilot
instructions that means the repository layout and pipeline sections only, none
of the three sections carried from the hub.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe the real concurrency and drop an unused enum

Two comments described verify as working through files and batches one after
another, when both paths run in parallel under the thread limit. A reader
reasoning about ordering, or about when a defect reaches the log, would have
been misled by them.

VerifyStatus was left over from an earlier shape of the code and never
referenced once the counts record took over.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Bring the prose to spec across the repository

The gate reports clean on the whole tree, down from 245 findings, ahead of the
repository going public.

The carried files were stale rather than wrong. Every one of them is clean at
the hub, so where a sentence differs only by the correction the hub already
made, its wording is adopted rather than reinvented, which brings the two back
into step instead of forking them. The runbook sections of the Copilot
instructions are re-carried whole, since the hub had rewritten one of them in
substance and not only in style.

Nothing was copied wholesale. The two sections of GOVERNANCE.md that describe
this repository's own tree and devcontainer, and the equivalent sections of the
Copilot instructions, are deliberate adaptations, and taking the hub's text
would have replaced this repository's layout with the hub's own. All eighteen
byte-locked sections were checked against the hub before and after, and match.

The rest is this repository's own prose, rewritten by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Keep a per-file verify failure from aborting the run

An exception thrown inside either parallel loop of VerifyTask escapes
Parallel.ForEachAsync and ends the whole command, so a single unreadable
file aborts a run that may span an entire library and reports nothing
about the files already verified. The hashing call that runs before the
structural check when a database is in use is the exposed path.

Guard the per-file body of both loops, logging the file and counting it
failed, so the command reaches its summary and exits 2 rather than 1.

The regression test sets a database, which is why the existing
unreadable-file test did not cover this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Compare ISO-BMFF bounds by subtracting so a crafted file cannot overflow

Three bounds checks add two attacker-controlled values before comparing.
Each addend is already non-negative, but their sum can exceed long.MaxValue
and wrap negative, so the comparison passes and a malformed file is
reported clean, which is the one outcome a validator must not produce.

An extent at offset long.MaxValue with length 2 returned None. Compare by
subtracting instead, in the extent bounds check, the grid descriptor range
check, and the top-level box walk, which is the only walk reading a 64-bit
size and so the only one able to overflow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe the types VerifyResult.cs actually declares

The architecture map credits the file with a VerifyStatus enum that exists
nowhere in the codebase, sending a reader looking for a type that was never
written. Name the two types it does declare.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Warn on unrecognized ISO-BMFF structure instead of condemning the file

The structural pass treated any structure the box walk could not follow as
a defect, which conflates two different things. A file it can name a defect
in is damaged; a file it merely cannot parse may just be a format the parser
has never met, and from the inside those look identical.

That asymmetry is measurable. Across 236,797 files the named-defect checks
produced 16,394 hits and no false positives, while the generic check
produced 7 hits, all false positives, on Samsung MP4s carrying a proprietary
SEF trailer after the last box. Every future vendor quirk would arrive the
same way, as a corruption report against someone else's valid library.

MalformedBoxStructure now counts Suspect, logs a warning, and lets the file
through to the decoder, which is the authority on whether Immich can use it.
Suspect never affects the exit code. Recognize the SEF trailer specifically
so the common case is silent rather than merely non-fatal.

Keeping the check advisory rather than dropping it matters, because a
truncated file decodes: Immich renders a thumbnail from bad-truncated.heic
and reports nothing wrong. This is the only signal that class of damage has.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Make the README help blocks match what the CLI prints

The blocks are presented as literal --help output, but eight option
descriptions across four commands had drifted from the code, so the
documented CLI and the real one disagreed. Copilot found one instance; the
same class covered process, import, index, and verify.

Four descriptions were wrong in the code rather than the docs, because the
options are single instances shared by every command that takes them. A
shared option cannot say "the directory path to process" when verify, index,
trash, and undo do not process anything, so those now read neutrally. The
long --quick text moved out of help, where the README option notes already
carry it in full.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Refuse to map a file outside the mount into a container path

ToContainerPath built a container path from a relative path it never
checked, so a file outside the mount root would produce a path containing
'..' that resolves somewhere else inside the container. Immich would then
judge a different file and report the verdict against the original name,
which is worse than an error because it looks like a result.

The file list is enumerated under the mount root today, so this is an
unchecked invariant rather than a live bug. TryToContainerPath now returns
false instead, and the caller logs the file and counts it failed rather
than throwing, since a throw there would abort the whole run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Fail closed when box nesting outruns the depth guard

WalkBoxes returned true past MaxBoxDepth, so a container nesting meta boxes
deeper than the guard stopped the walk and reported success. No iloc was
ever recorded, so Validate returned None and a file built specifically to
outrun the parser got a clean bill of health.

Real containers nest two deep, since only meta and iinf recurse and every
other box is skipped by declared size, so passing the guard means the
structure cannot be followed rather than that the file is unusual. Return
false, which now reports Suspect and defers to the decoder rather than
condemning the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Stop the grid check reading a descriptor it cannot locate

Construction method 1 means the item offset is relative to idat, so with no
idat box in the file there is no base to resolve against. IdatOffset stayed
at its default of zero, and the offset was read as absolute, pulling four
arbitrary bytes and treating them as an ImageGrid descriptor.

Those bytes then decided the tile geometry, so the check could invent a
GridTileCountMismatch out of unrelated data. That is the worst direction for
this feature, because a mismatch is a named defect that condemns the file.
A synthesized case returned GridTileCountMismatch before this change.

Report the contradiction instead, which is advisory and leaves the verdict
to the decoder.

Also require the box walk to consume its whole range, since a remainder too
small to hold a header belongs to no box. Measured first: across 20,000 real
files from the collection, every single one consumed its range exactly, so
this costs nothing on real media.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Import System.Globalization in the file that reads it

The namespace is a global using, so the file compiled without the import
and the import changes nothing at build time. It is kept because a reader
seeing NumberStyles and CultureInfo in this file has no local evidence of
where they come from, which is what led a reviewer to call the file
uncompilable.

Verified that no formatter strips it: csharpier, dotnet format style in
apply mode, and husky all leave it in place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Stop counting a dropped file twice, and refuse an unlocatable grid descriptor

Two independent problems, both reported against the same round.

The decode-batch failure handler counted one failure per entry in batch,
but a file dropped during path mapping was already counted there and never
entered mapped. Adding the mapping filter in 67e9a37 created the gap, since
mapped and batch were the same length before it. Count over mapped instead.

CheckGridGeometry read any construction method other than 1 as an absolute
offset, so method 2, which is relative to another item, pulled four
arbitrary bytes and let them decide the tile geometry. A synthesized case
returned GridTileCountMismatch, so the file was condemned by a named defect
derived from an accidental read. This is the same shape as the missing-idat
case fixed in 88455e3, and the remaining branch of it.

Report the contradiction instead, which is advisory and leaves the verdict
to the decoder.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Bound the dimg reference reads to their own box

ParseItemReference read from_item_ID and reference_count straight after the
dimg header without checking either fits inside the box. A dimg declaring
only its 8-byte header carries no fields at all, so both reads continued
into whatever followed, and the resulting count went into the map the grid
check consults.

Where those bytes land decides which way it goes wrong. A synthesized
header-only dimg filed the count under an item id nothing references, so
the grid item lost its entry and the file passed as clean. Bytes that
happen to decode to a real grid item's id give the opposite result, a tile
count invented from unrelated data, which the grid check reports as a named
defect.

Require the box to hold the two fields and the reference list it declares,
and report malformed structure otherwise, which is advisory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Make Immich's decoder the only judge, and drop the container parser

The hand-rolled ISO-BMFF validator is removed along with the --quick mode
it powered. Verify now runs Immich's decoder and nothing else.

The parser was a standing liability rather than a one-off bug source. Five
consecutive review rounds each found a real defect in it, every one the same
shape: a read that was not bounded by the structure it claimed to parse, fed
into the check that condemns a file by name. Two of those produced a tile
count invented from unrelated bytes, which is the worst outcome available
here, since it marks a healthy photo corrupt. The corpus it was tested
against is one collection, and the failure mode of a format it has never met
is indistinguishable from damage, so shipping it publicly meant condemning
other people's media on evidence it could not actually read.

Removing it costs the truncation signal, which decodes cleanly and so no
decoder reports. That is a real loss, and the deliberate price of not
guessing about files this tool has never seen.

Verify keeps the existence check, because a file that vanished mid-run still
counts as failed rather than damaged, and without it the missing path would
reach Immich and come back as unrenderable. The exiftool -validate read is
untouched.

Verify has no offline mode now, so the tests that drive it skip when the
image is absent, and the ones that judged synthetic bytes use real media.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Log a file missing during process at debug rather than information

Process rewrites the tree it walks, so a name that has gone is usually its
own earlier rename. It could equally be an external deletion, and the two
are not cheaply told apart, so the message neither fails the run nor claims
to know which happened. Debug is the level for something that is expected
most of the time and diagnostic the rest.

Import and verify keep counting a missing file as failed, since neither
modifies its input and a vanished file there can only be external. Verify
reaches that verdict through an explicit check rather than an exception,
because without a database it never opens the file, so the wording now
matches import's and the comments say why the mechanisms differ.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Keep a file missing during process at information, not debug

Process rewrites the tree it walks, so a name that has gone is usually its
own earlier rename, and it neither fails the run nor claims to know whether
an external deletion caused it instead. The line stays at information rather
than dropping to debug, so a run still accounts for why a file is gone
without the reader having to raise the level to find out.

Verify and import are deliberately louder: both log an error and count the
file as failed, because neither modifies its input, so a vanished file there
can only be an external change to the tree mid-run. Counting it failed is
what makes the command exit 2, and the error level agrees with that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Stop the preflight failure pointing at an option that no longer exists

Removing --quick left the preflight message telling the reader to pass it,
so the one message a user sees when Docker is missing offered a way out
that the CLI now rejects. It states the actual requirement instead: docker
and the Immich image, with no offline mode, because the decoder is the
whole check.

Also drop the System.Buffers.Binary import, unused since the tests that
synthesized box headers went with the parser.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Test that an unknown verify option is rejected, without naming one

The case names a specific flag that no released version ever accepted, so
the name kept a option alive in the codebase that a reader could never have
used. What is worth guarding is the general behavior: an option the command
does not define is an error rather than something silently ignored.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Count an unreadable file as failed whether or not a database is configured

With a database the hash read opens every file, so a permission or I/O
failure throws and the per-file guard counts it failed. Without one nothing
opened the file before the container did, so the same file was handed to
Immich, came back unrenderable, and was counted invalid. The verdict for a
tooling gap therefore depended on an unrelated flag, and in one of the two
paths reported a readable-media problem as damaged media.

Probe the file in the no-database path and let the existing guard count the
throw, so both paths reach the same verdict.

Also rename the launch profile that still said Quick. The two verify
profiles differ by whether they pass a database, so the name says that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Count a file that vanished before the decode as failed, not invalid

A file removed after its batch was assembled makes the decoder fail, and
that failure was reported as "Immich cannot render", so a file that is
simply gone was recorded as damaged media. Check the host before believing
a rejection, so the vanished case reaches the same failed count as every
other place the tree changes mid-run.

Also correct two claims in the verify flow. The media directory mounts at
the fixed container path rather than at its own absolute path, and paths are
translated onto it, which is what lets a host path that is not a valid
container path work at all. The command exits 2 for a failed file as well as
an invalid one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Split the untested-race note across two comment lines

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Reject a path carrying a line break before sending it to the container

Paths reach the container one per line, so a name containing a newline or a
carriage return cannot be expressed in that protocol. Without this the name
splits and the container is asked to decode fragments of it, and the run
reports the generic "no verification result returned" rather than the reason.

The counts are unchanged, because a split path matches no returned verdict
and so already counted failed. What this buys is an accurate reason and a
well-formed payload, not a different tally.

No path under the collection carries either character, so this guards a
hazard rather than fixing an observed failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Skip the Immich preflight when nothing needs decoding

Verify preflighted the Docker image before looking at what it had been
given, so a directory holding no supported media exited 1 on a host without
Docker, and paid four seconds of container startup on one that had it. The
command reported an infrastructure failure for work it was never going to do.

Preflight now runs only when at least one file carries a supported
extension. The partition loop is untouched, so non-media files are still
counted ignored and still drive the unknown-extension warnings.

Proven end to end with docker removed from PATH: the same directory of text
files exits 1 before this change and 0 after. The test is deliberately not
gated on the Immich image, because a host without it is the case this
protects, which is also what CI is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Mount the media directory in a form that survives the characters in a path

The volume spec is colon delimited, so a colon anywhere in the media
directory made docker reject the spec outright, and the whole batch failed
with "too many colons" against a directory that was perfectly valid.

Switching to --mount alone only moves the problem, because that flag is
comma delimited and a comma is far the more likely of the two in a photo
directory name: this collection holds 285 paths containing a comma and none
containing a colon, so the naive swap would trade a hazard that has never
occurred for one that already exists.

Quoting the whole source field as CSV takes both, with any embedded quote
doubled per CSV rules. Verified against directories named with a colon, a
comma, both, and both plus a quote.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Use a neutral example path in the kei instructions

The examples named a real media tree rather than an illustrative one, which
puts a reader's own directory layout in front of them and invites anyone
following along to copy a path that means nothing on their machine.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Defer the Immich preflight until a file actually reaches the decoder

Gating the preflight on any supported extension being present left a
database-backed run that skips every file still demanding Docker, so an
incremental run over an already-verified tree failed on a host without it.
The comment claimed nothing to decode means nothing needs Docker while the
code only honoured that for a tree holding no media at all.

The first batch to find a candidate now triggers the preflight, once, and
the rest await it. A run that decodes nothing never starts a container.

The earlier reason for not doing this was wrong. Batches are processed as
they are chunked, so the wait for a Docker failure is bounded by one batch
rather than by hashing the whole tree, which is seconds rather than hours.

Verified with docker off PATH: a fully cached run exits 0 having skipped its
files, and an uncached one still exits 1 with the message intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Probe readability for every file that reaches the decoder

The probe only ran when no database was configured, on the assumption that
the hash read would open the file otherwise. It does not: a row matching on
size and mtime returns cached hashes without reading the file, and stat
needs no read permission, so an unreadable file with a cached row went
straight to the decoder unopened.

Immich runs as another user and may well read it, so a gap in this tool's
own access came back as a verdict about the media, which is the outcome this
branch has been closing off everywhere else.

The probe now sits at the end of the skip decision, so it covers exactly the
files that get judged and costs nothing on the ignored, missing, and
already-verified paths that an incremental run spends its time on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Check readability in the shared metadata call, not only in verify

Reading a file's attributes needs no permission on its content, so size and
mtime prove nothing about whether the bytes can be reached. Verify was given
its own probe for that, which left the same hole open everywhere else.

Measured against an unreadable file before this change: process reported
Failed 0 and exited 0, and import reported Imported 1. Both claimed success
over a file they never read. Index was already correct, because it hashes
and therefore opens.

The probe now sits in the shared exiftool call, so process and import
inherit it, and verify calls the same helper for the path that hands the
file to Immich rather than reading it here.

Keeping it ahead of exiftool is what makes the distinction work. exiftool
reports a file it cannot open and a file it cannot parse the same way, as an
error alongside well formed JSON, so keying on that field would have turned
damaged media into a tooling failure. Garbage bytes in a readable file still
count Invalid, and only an unreadable one counts Failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Name the no-extension case, and stop the trash comment overstating

A file with no extension tracks as an empty string, so the summary printed
"Unknown file extension: ''", a quoted nothing that reads as a bug in the
message rather than a fact about the tree. All four commands feed the same
tracker, so naming the case there covers every one of them. No file in the
264,044 verified carried this, which makes it latent noise rather than an
observed defect.

The trash comment claimed a page either fetches or throws, so no partial
state exists. Pagination also stops early on a NextPage value it cannot use,
warning and then returning success, which leaves the database short of the
server. The comment now says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Exit 2 when trash syncs only part of the server

Pagination stops early on a NextPage value it cannot use, and the command
then reported success over a database holding fewer hashes than the server.
That database is what import --trashdb and process --trashdb consult to skip
files, so a short one silently re-imports assets that were trashed, and a
pipeline gating on the exit code had no way to see it.

The stop is now an error rather than a warning, and the command exits 2,
which is the existing meaning of a run that completed while carrying a
failure. Whatever pages were read are still kept, since a partial database
is more useful than none as long as the caller knows it is partial.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Drain the docker probe's pipes before waiting on it

The image-availability probe redirected both streams and read neither, so
the child could fill a pipe buffer and block on the write while the wait
blocked on the child. Measured at 5,662 bytes against a 64 KB buffer today,
which is why nothing has hung, but the margin belongs to the image rather
than to this code.

Both streams are now read before the wait, and the probe asks for an empty
rendering since only the exit code is wanted, so there is almost nothing to
buffer either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Name every exit code, list verify, and finish the trash comment

Three findings from suppressed review comments that had not been answered.

CommandRunner logged completion for anything that was not Failed, so an
Error would have reported the command complete. Nothing returns Error from
the work itself today and the catch blocks return it directly, so this is a
guard on a future caller rather than a live defect.

The architecture map still described five subcommands and omitted verify,
having been written before this branch added it.

The trash comment covered pagination stopping early but not a page that
throws, which leaves the database short as well, having kept the pages
before it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Re-vendor the six verbatim fleet-law regions from the hub (#39)
671812b 
The hub advanced on six byte-locked regions since the last conformance
round, and the audit reports each as a stale carry: a region matching a
past hub revision rather than the current canonical.

AGENTS.md "Context and Delegation Discipline" no longer ends a session on
a third review round, since a loop still producing findings is the
deliverable in progress, and it gains the rule that a wait separates
"met", "not yet", and "cannot be reached" instead of rendering all three
as silence. "Where the Rules Live" gains the router row for quoting data
into agent-authored text.

GOVERNANCE.md gains "Representative Data in Agent-Authored Text", absent
here entirely, which binds agent-authored text to constructed data rather
than data observed in the maintainer's environment. "Git and Commit
Rules" gains the rule that an authorization to commit carries the push,
because nothing reviews a local commit. "Verification Discipline" gains
the rule that a launched process is not a result. "PR Review Etiquette"
gains the five outcomes that close a finding, the rule that a
low-confidence finding is not a low-value one, the requirement that a
decline carry proof rather than an assertion, the answer format for a
suppressed finding, and the warning that a review's own overview cannot
be trusted to say whether findings exist.

The new "PR Review Etiquette" text points at scripts/pr_review.py, which
this repo does not carry. That is a known open question at the hub, and a
verbatim region is carried unchanged rather than locally patched.

Audit run 2026-08-03T16:52:36Z, hub 1ed0cc8, against develop@39c896b. All
six regions are promoted to hub main a9cd154.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Refresh the carried intent docs against the hub (#40)
65227ed 
The audit's mechanical check only presence-checks the sections of an
intent-fidelity carried file, so these four drifted silently while every
byte-locked region stayed green. Hand-diffing each against the hub found
stale paragraphs in all four, and one guarantee this repo describes but
does not implement.

CODESTYLE.md regains the Python section it had trimmed. The file's own
intro says a repo keeps it whole and that an unused-language section costs
nothing, so trimming it contradicted the sentence directly above the cut.
Four paragraphs re-vendor: the clean-compile bullet now says it is not the
whole gate and points at the repo's whole lint gate, MD033 now permits
details and summary, HISTORY.md is now framed as mirroring the README
opening rather than sharing a header, and the intro names the verification
discipline. The Python tasks-mirror reference pointed at a hub catalog path
this repo does not have, so it names the repo's own .vscode/tasks.json
instead, which is what the file's VS Code config rule requires.

WORKFLOW.md re-vendors D2.2, which now explains that the smoke check exits
early while the job still reports success, and why a job-level if: would be
wrong. D4.2 names GitCommitId, which is what get-version-task.yml already
outputs and what github-release already passes as target_commitish.

D1.1, D1.2, D1.4, D1.5 and D4.1 described machinery this repo does not
have: a paths-filter changes job, smoke-build needing the validation job,
and a publish-plan-task.yml. Each is adapted to what the tree actually
does. Both absences are stricter than the guarantee rather than looser,
since every push smoke-builds and no push publishes at all, so the fix is
to state the repo's shape rather than build machinery it does not want.
The S1, S2, S3, S5 and S6 trace scenarios are corrected the same way.

.github/copilot-instructions.md re-vendors two runbook sections. Triggering
and Polling still scoped the suppressed-finding query to the current head,
which is how a finding stops appearing without ever being answered, so it
now reads every round and marks which round each came from. The reviewer
login note gains the third spelling, the REST timeline's bare Copilot.
Bounded Retry Workflow gains the pending-request-nobody-picked-up state,
which is invisible from the reviews alone and indistinguishable from
patience, along with the clear-and-re-request recovery.

repo-config/README.md re-vendors one paragraph and is now byte-identical
to the hub.

D4.5 is absent from this repo's WORKFLOW.md and is deliberately not added
here. The pipeline does not satisfy it, so stating the guarantee before
implementing it would replace a silent gap with a false claim. It lands
with its fix.

Audit run 2026-08-03T16:52:36Z, hub 1ed0cc8, against develop@39c896b.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Restore the CRLF-default line-ending model and pin the Dockerfile (#41)
788f7b7 
The hub's model is a CRLF default with declared LF exceptions. This repo
had dropped the `[*]` `end_of_line = crlf` default and enumerated CRLF per
file type instead, which produces the same result for the types it listed
and leaves every other type with no declared ending at all: `.slnx`, the
`.code-workspace`, `LICENSE`, `.gitignore`, `.dockerignore`, and
`.editorconfig` itself were all uncovered. Restoring the default covers
them and makes the per-type CRLF lines redundant, so they go, along with
the `[*.{json,jsonc}]` and `[*.{cmd,bat,ps1}]` sections that carried
nothing else.

Two LF pins were missing, and one of them matters. `Docker/Dockerfile` had
no pin in either file although this repo ships one, and a CRLF there breaks
RUN heredocs and line continuations. It is LF in the tree today, so intent
held by accident rather than by rule; the pin is what keeps it that way
through a checkout or a renormalize. `.husky/pre-commit` is an
extensionless shebang script that matches no extension rule, so the CRLF
default would have claimed it. `.gitattributes` already pinned it and
`.editorconfig` did not, which is exactly the gap the restored default
exposes.

`.gitattributes` itself was LF while the hub's is CRLF, and under the
restored default that is a violation of the repo's own rule, so it is
rewritten as CRLF. Its comments regain the configure, renormalize, and
inspect commands the hub carries.

The hub's `catalog/`, `.github/actions/`, `scripts/*.py`,
`host-setup/`, `spec/*.py` and `uv.lock` pins are hub-only paths and stay
out.

Verified: `editorconfig-checker` passes over the whole tree, and
`git add --renormalize .` stages nothing beyond these two files, so the
new pins describe the tree exactly rather than proposing a conversion.
`git check-attr` confirms the Dockerfile pin resolves.

Audit run 2026-08-03T16:52:36Z, hub 1ed0cc8, against develop@39c896b.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Block a partial publish when a build fails (#42)
252f5c5 
WORKFLOW.md D4.5 requires that a build failure block every publish target.
This repo's WORKFLOW.md was missing the guarantee entirely, and the
pipeline did not satisfy it.

build-docker needed only [get-version, validate, validate-release] and
guarded solely on cancellation. The image builds from source and consumes
no executable artifact, so the two builds were independent and nothing
forced an ordering. On a real publish where build-executable failed,
github-release skipped, so no tag and no release were cut, while
build-docker was untouched and pushed the multi-arch image anyway, moving
`latest`. An image shipped with no release behind it.

build-docker becomes the terminal publish target: it needs
build-executable, and its `if` gains `!failure()`. The distinction that
matters is failed versus skipped. A failed upstream build must stop the
push, and a skipped one must not, because `validate` is skipped on every
smoke run and that run still has to build the image. The explicit
get-version and validate-release result checks stay, so a job that somehow
did not run cannot feed empty version inputs into a build.

The cost is on smoke, where build-docker now waits for build-executable
instead of running beside it, adding roughly the executable build's
duration to PR feedback. Correctness over a subset of the wall clock.

WORKFLOW.md gains D4.5 in the same commit rather than in the doc-refresh
PR ahead of it, so the contract and the code that satisfies it land
together instead of the file claiming a guarantee the pipeline breaks.

The failure path is not reachable from a pull request, since smoke never
publishes, so it is established from the needs graph rather than by
observation. actionlint passes.

Audit run 2026-08-03T16:52:36Z, hub 1ed0cc8, against develop@39c896b.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Raise the version floor to match the 1.1 declared in HISTORY.md (#43)
3d6ad1d 
HISTORY.md declares Version 1.1 for the verify command and the breaking
exit-code changes, while version.json still floored at 1.0, so the next
release cut from main would have been 1.0.18 and contradicted it.

Only version.json moves. The Version and InformationalVersion properties
in PhotoCleaner.csproj are local-build placeholders that CI overrides with
the values Nerdbank.GitVersioning computes, so they do not track this file.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727 @claude
Open the local gate chain with a dotnet tool restore (#44)
5e2eee0 
* Open the local gate chain with a dotnet tool restore

CSharpier and Husky.Net are local tools declared in .config/dotnet-tools.json,
so on a clone whose package cache does not already hold them the documented
chain failed on its first command with "Run dotnet tool restore to make the
csharpier command available" and exit 1.

CI already restored before its CSharpier step, and the prose called that out as
a difference between the two. It is no longer one, so that clause is dropped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Attribute the restore to the clone, not to the .NET Format task

The paragraph introduces the chain as the `.NET Format` task, so saying the
chain opens with a restore read as a claim about that task. It is not one:
`.NET Format` depends on `CSharpier Format` and `.NET Build`, and none of the
three restores, so the task fails on a fresh clone exactly as the shell chain
did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 18:38
Copilot AI reviewed Aug 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

OPERATIONS.md currently contains outdated statements about exit-code semantics and Docker runtime requirements that conflict with the new ExitCode contract and the verify command’s Docker dependency.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This promotion merges develop into main, bringing in the new verify command (Docker/Immich-based decode verification), standardizing command exit codes, enabling ExifTool validation in metadata reads, and completing a broad hub-conformance refresh (docs, workflows, line-ending model, and operational runbooks). It also raises the NBGV version.json floor to 1.1 to match the declared release notes.

Changes:

  • Add the verify command and its supporting in-container Immich verification protocol, including tests.
  • Standardize exit codes across commands (ExitCode) and add ExifTool -validate handling (invalid vs failed outcomes) in process/import.
  • Refresh governance/intent docs, workflows (including prose gate + publish gating), operational documentation, and configuration baselines; bump version.json to 1.1.
File summaries
File Description
version.json Raise NBGV version floor to 1.1.
repo-config/README.md Prose clarifications for repo-config purpose and rulesets.
PhotoCleanerTests/VerifyTaskTests.cs New unit/integration tests for VerifyTask parsing, mapping, and Docker-gated execution paths.
PhotoCleanerTests/UndoTaskTests.cs Comment wording tweak for clarity.
PhotoCleanerTests/TrashCommandTests.cs Add test ensuring invalid pagination (NextPage) results in exit code 2 / failed run.
PhotoCleanerTests/TempDirectoryFixture.cs Comment cleanups and clarification around exiftool exit-code suppression.
PhotoCleanerTests/ProcessTaskTests.cs Add tests for unreadable files and ExifTool validation warning/error behavior.
PhotoCleanerTests/IndexTaskTests.cs Comment clarification on rehash and --processed behavior.
PhotoCleanerTests/ImportTaskTests.cs Add invalid-file import test; update tuple shape to include invalid count.
PhotoCleanerTests/ExifToolJsonTests.cs Add tests for ExifToolJson.ParseValidate.
PhotoCleanerTests/DirectoryCleanerTests.cs Comment wording cleanup.
PhotoCleanerTests/CommandLineTests.cs Add CLI parsing/registration tests for the new verify subcommand.
PhotoCleaner/VerifyTask.cs Implement VerifyTask: batching, Docker preflight, path translation, and result tallying.
PhotoCleaner/VerifyResult.cs Add JSON model + source-gen context for verify script output protocol.
PhotoCleaner/VerifyCommand.cs Add verify command orchestration, logging, and exit-code handling.
PhotoCleaner/UndoTask.cs Comment refinements in undo algorithm documentation.
PhotoCleaner/UndoCommand.cs Return ExitCode based on per-file failures instead of always-success.
PhotoCleaner/TrashCommand.cs Treat invalid pagination as a failed (exit 2) partial sync rather than success.
PhotoCleaner/SkippedExtensionTracker.cs Improve logging for empty-extension case.
PhotoCleaner/ProcessTask.cs Add Invalid result and enforce ExifTool validation error handling.
PhotoCleaner/ProcessCommand.cs Track/report invalid count and return ExitCode.Failed when invalid/failed > 0.
PhotoCleaner/MediaUtilities.cs Enable ExifTool -validate -all, add EnsureReadable, and handle empty JSON output.
PhotoCleaner/IndexCommand.cs Return ExitCode.Failed when indexing reports failures.
PhotoCleaner/ImportTask.cs Add Invalid outcome for ExifTool validation errors; include invalid count in results.
PhotoCleaner/ImportCommand.cs Log invalid count and return ExitCode.Failed when invalid/failed > 0.
PhotoCleaner/ImmichVerifyScript.cs Add in-container Node script strings (preflight + verify) that call Immich internals.
PhotoCleaner/ExitCode.cs Introduce shared exit-code contract constants (0/1/2).
PhotoCleaner/ExifToolJson.cs Add ExifTool validation/error fields and implement ParseValidate.
PhotoCleaner/CommandRunner.cs Update runner to return/label command exit codes and unify error handling.
PhotoCleaner/CommandLine.cs Register verify command; refine help/option descriptions.
PhotoCleaner.code-workspace Update workspace settings and extension recommendations.
OPERATIONS.md Add operations/runbook documentation (local gates, lint tooling, release process).
HISTORY.md Add 1.1 release notes covering verify, validation, and exit-code changes.
GOVERNANCE.md Add representative-data rule text; refine review-loop and verification discipline guidance.
Docker/README.md Align Docker README description wording with repository description.
Docker/Dockerfile Align image label description wording with repository description.
cspell.json Add/adjust project vocabulary words.
CODESTYLE.md Refresh carried code-style rules; expand Python section and clarify lint/CI expectations.
AUDIT.md Prose clarification for audit scope and ruleset expectations.
AGENTS.md Refresh session-scope guidance and add rule-router row for representative data.
.vscode/launch.json Add debug launch configurations for verify.
.github/workflows/validate-task.yml Add base-branch history fetch for prose diffing; add hub prose gate step.
.github/workflows/merge-bot-pull-request.yml Add merge-bot workflow for Dependabot PRs (auto-merge enable/disable).
.github/workflows/build-release-task.yml Gate docker publish on successful executable build (needs + !failure()).
.gitattributes Restore line-ending governance and pin Dockerfiles + scripts to LF where required.
.editorconfig Restore CRLF default model with explicit LF exceptions; add rationale comments.
Review details
  • Files reviewed: 49/49 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread OPERATIONS.md Outdated
Comment thread OPERATIONS.md Outdated
@ptr727 ptr727 mentioned this pull request Aug 3, 2026
Merged
@ptr727 @claude
Correct the exit-code and Docker claims in OPERATIONS.md (#46)
0335970 
Two statements were written true and were left behind by the verify command.

The exit-code section said a per-file failure does not change the exit code and
listed only 0 and 1. ExitCode.Failed is 2 and every command returns it, so the
note asserted the opposite of the behavior and the list was missing a code. It
now points at the README table rather than restating it, and keeps only the
operational reading: 0 and 2 both mean the command ran to completion.

The tool section said the application needs no Docker daemon at runtime. verify
runs the Immich decoder inside the immich-server image and its preflight throws
when docker is missing, so Docker is a runtime dependency of that one command.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 19:04
Copilot AI reviewed Aug 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

A new test helper can leak a hanging docker subprocess on timeout; it should terminate the child process to avoid destabilizing the test run.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

PhotoCleanerTests/VerifyTaskTests.cs:502

  • In ImmichImageAvailable(), if docker image inspect hangs and WaitForExit times out, the method returns false without terminating the child process. That can leak a running docker process into the test run and potentially affect subsequent tests.
  • Files reviewed: 49/49 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@ptr727 ptr727 mentioned this pull request Aug 3, 2026
Merged
@ptr727

ptr727 commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Suppressed finding answered (round 2, on head 0335970)

The round-2 review carried no inline comment but did collapse one finding, and it was marked Not ready to approve on the strength of it. Correct, and fixed on develop in #47, which this promotion picks up since its head is develop.

Worth recording that the review-polling script reported suppressed=0 for this round. Its parser looks for the <summary>Suppressed comments (n)</summary> shape, and this review used a ### Suppressed comments (1) heading inside the details block instead. Reading every review body directly is what caught it, which is the whole reason the head-review signal is not trusted on its own.

PhotoCleanerTests/VerifyTaskTests.cs:502 - Fixed in #47

In ImmichImageAvailable(), if docker image inspect hangs and WaitForExit times out, the method returns false without terminating the child process. That can leak a running docker process into the test run and potentially affect subsequent tests.

The helper returns false on a 60-second timeout, and the using disposes the Process without ending the child, so the probe outlives the test that started it.

Checked with a standalone probe rather than assuming the disposal semantics: a child started the same way and left to Dispose was still present in /proc after the using block exited, and the same child was gone when Kill ran first. Kill(entireProcessTree: true) now runs before the false return, with an InvalidOperationException catch for the child exiting between the timeout and the kill.

@ptr727 @claude
Kill the docker probe when its wait times out (#47)
83f5101 
* Kill the docker probe when its wait times out

ImmichImageAvailable started `docker image inspect` and returned false on a
60-second timeout without ending the child. Disposing a Process frees its
handles only, so the probe outlived the test that started it and kept running
alongside the rest of the suite.

Confirmed the semantics with a standalone probe rather than assuming them: a
child started the same way and left to Dispose was still present in /proc after
the using block exited, and the same child was gone when Kill ran first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Wait out the killed probe and observe its drain tasks

Kill signals termination rather than performing it, so the previous commit
still returned while the child was alive. A bounded WaitForExit after the kill
is what makes the child gone by the time the helper returns.

The early return also left the two ReadToEndAsync drains unobserved. The kill
closes their pipes, so a bounded WaitAll finishes them, and the AggregateException
catch observes a drain that faulted on the closing pipe.

Measured the asynchrony rather than reasoning about it: across five trials a
killed child was still present in /proc when Kill returned, and gone after the
bounded wait in every one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 20:59
Copilot AI reviewed Aug 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Human review recommended

It promotes a large, cross-cutting change set (new command pipeline + shared exit-code contract + workflow and line-ending governance changes) that warrants final human sign-off despite no specific defects found in review.

Review details
  • Files reviewed: 49/49 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@ptr727
ptr727 merged commit c457ff3 into main Aug 3, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727