pixiv retired, placement reconciler removed, build race fixed, mirror pinned #257

Merged

bvandeusen merged 5 commits from dev into main 2026-09-22 07:43:12 -04:00

Conversation 0 Commits 5 Files Changed 46

+1030 -4207

5 Commits

Author	SHA1	Message	Date
bvandeusenandClaude Opus 5	aa2bb665b9	fix: serialise builds per branch, and pin :c-<sha> to the digest this run built (4290) ... Two independent defects from one missing mechanism. `grep -rn concurrency .forgejo/workflows/` returned nothing, so two pushes to a branch ran build.yml in full parallel with no ordering. Both jobs read `fc.revision` off the channel tag before either has pushed, so both miss the reuse check and both build. Whichever finishes LAST owns the tag. Half 1 — the rolling tag. A slower older build leaves `:dev` carrying content older than the commit that moved it. Family rule 146 says a rolling channel refreshes itself; this is the case where it quietly does not. Self-healing on the next push. Half 2, and the reason this is not filed low — `:c-<sha>`. The repoint step wrote every non-channel tag by copying the channel tag BY NAME, so the immutable rollback tag named whatever `:latest` pointed at when that step happened to run. Lose the race and `:c-<shaA>` names the OTHER run's bytes. Rule 145 makes that tag the rollback unit and immutable, so this does not break immutability — it makes the tag wrong from birth, and immutability then guarantees nobody ever corrects it. Nothing goes red; it surfaces the day someone rolls back and gets a commit they did not choose. Not self-healing. Two fixes, deliberately both: A. A workflow-level `concurrency` group keyed on `github.ref`, so dev and main never block each other. `cancel-in-progress: false` — queue, never cancel: cancelling could kill sign-extension mid-AMO-upload, leaving the version registered at AMO with no cached asset, which is the unrecoverable stuck state that job's rollback trap exists to prevent, reached by another door. Not keyed on BUILD_REF because the group is evaluated before any job starts and cannot read the `env` context. B. Each build-push-action step gains `id: build`, and the repoint step copies from `$IMAGE@${{ steps.build.outputs.digest }}` — the manifest THIS run pushed — rather than from the channel tag by name. On a reuse hit there is no digest and the channel tag remains correct by definition: "hit" means that tag already carries this commit's fc.revision. B is not redundant with A. A depends on this Gitea honouring a key whose failure mode is silent, and this file has been burned by exactly that before (the `format()` note records `true == 'true'` evaluating FALSE on run 5270, every lane green, the feature simply not happening). B holds the :c-<sha> correctness property whether or not A is honoured. The loop's exclusion is now keyed on CHANNEL_REF rather than on SOURCE, and that is load-bearing: SOURCE may now be a digest ref, which never equals a tag string, so testing against it would stop excluding the channel tag. imagetools would index-wrap it and `.Image.Config.Labels` would stop resolving, killing the reuse label permanently — #3183 arriving again. Neither half is verifiable on a dev push: dev's tag list is the channel tag alone, so the repoint correctly does nothing there. B is observable on the next merge to main (:c-<sha>'s digest must equal the build step's), and A by pushing twice in quick succession and reading the run list for a queued second run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-21 22:07:53 -04:00
bvandeusenandClaude Opus 5	bd92fb46b3	test: pin the JS<->Py artist-pattern mirror with a shared sample table (3093) ... `PLATFORM_ARTIST_PATTERNS` (extension/lib/platforms.js) and `_PLATFORM_PATTERNS` (extension_service.py) are two hand-kept copies of one table whose only guard was the comment "keep in sync by hand; reviewers catch drift" — the same guarantee manifest.json had before #3069, where deviantart sat in the manifest for seven weeks after the product dropped it. Drift here is worse than the manifest case, because the two copies gate opposite halves of ONE interaction: the JS copy decides whether the "Add to FC" button appears, the Python copy decides whether the resulting POST is accepted. JS looser than Py shows a button that 400s; Py looser than JS silently never offers a button for a URL the backend would take. #1485 (the Patreon /c/ and /cw/ shapes) was the second of those, and its fix had to be applied to both files by hand. The two-runtimes objection to a shared SOURCE file is fair, so this tests the invariant instead of the source. `extension/test/artist-url-samples.json` is one table of 25 URL samples — match (with the expected slug) and no_match, each with a `why` — read by BOTH suites and asserted against each one's own copy of the patterns. Neither runtime imports the other; a change to one copy alone turns the other runtime's suite red. Both halves also assert their own coverage: the sample platforms must equal the platforms that actually have an artist pattern, and every platform must have samples in both directions. Without that, deleting a platform's samples would make the guard pass by testing less. Discord is deliberately in neither table — it is channel-based and has no creator page to put a button on. The no_match half asserts `_derive` RAISES rather than merely missing the platform: it tries every pattern in turn, so a nav page some other platform's pattern happened to swallow would still be accepted by the backend — the same defect wearing a different platform name. Samples live under extension/test/ because that path is excluded from both the XPI file set and the extension version derivation (packaging.sh: NOT_PACKAGED_TRACKED and NOT_VERSION_RELEVANT both carry `test/**`), so adding samples ships no bytes and forces no re-sign. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-21 21:04:53 -04:00
bvandeusenandClaude Opus 5	24b10d0ffa	feat: retire pixiv entirely — delete its code, its ledgers, its credential (3977, 3978, 3979) ... Milestone #406 phase 2, with issue #3980 folded in. Phase 1 (2026-09-13) unregistered pixiv so nothing could reach it; the code has sat in the tree uncalled since. DeviantArt is why the second half is not left for later — #3069 retired it in code on 2026-08-27 and its stored session was still in the database seven weeks on. Step 5 — the code. Deletes pixiv_client, pixiv_downloader, pixiv_ingester, platforms/pixiv and their three test modules and fixture, then edits out every remaining reference: the dispatch entry, the campaign-id and verify branches in download_backends, the display-name branch in extension_service, and the comments that still described pixiv as live. The consolidation check the step asked for comes back negative: native_ingest_common has seven non-pixiv callers (patreon, subscribestar, membership_reconcile, membership_roster, ingest_core), so nothing there drops to a single user. Step 6 — the data, alembic 0102. Drops pixiv_seen_media and pixiv_failed_media, and deletes credential rows whose platform is not registered. Written as "not registered" rather than "pixiv" at the step's explicit ask, which is what makes one migration cover two retirements: the pixiv OAuth refresh token and DeviantArt's leftover session (#3980). It is also the only way either row can go — the credentials UI renders one card per platform from /api/platforms and looks the credential up by key, so an unregistered platform's row has no card and no Remove button. Pixiv's Source rows are KEPT, changing the milestone's original data table on the operator's call. `platform` is stored only on Source; neither Post nor ImageRecord carries it. Both FKs are ON DELETE SET NULL, so a delete would not lose the art — but it would drop every pixiv image into the gallery's __unsourced__ bucket and strip the platform chip off every pixiv post. The rows stay disabled (0097) and unregistered, so nothing schedules or downloads through them. Keeping them costs nothing and keeps the attribution that "the art already downloaded from pixiv stays" is about. Step 7 — the guard. test_pixiv_code_and_tables_are_gone asserts absence from the module table and from Base.metadata, not from prose (snippet #3352's trap). The extension and registry negative assertions were already in place from phase 1. The final sweep found one real residue step 4 missed: extension/README.md still advertised pixiv support and carried a "Pixiv OAuth" manual-test item. Also replaces the two deleted dispatch tests with one over the whole NATIVE_INGESTER_PLATFORMS set, so adding a platform and forgetting its ingester class now fails at unit level rather than as a mid-download KeyError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-21 20:46:15 -04:00
bvandeusenandClaude Opus 5	5aa8e3d81b	fix: a stopped source is not a failing one, and cannot be deep-scanned (4279) ... Ebi77 sat in the "1 source is failing" banner for six days with no action available, reading `stranded by recovery sweep (no terminal status after time_limit)`. Four things lined up: 1. The membership sweep did its job — saw `former_patron`, disabled the source, cleared its failure state. Clean at 02:50. 2. Twenty minutes later a deep scan was armed on it. `/backfill` had a credential pre-flight but NO `enabled` guard, while `/check` has carried one all along. The two trigger endpoints disagreed, and the ungated one is the one that arms the long walk. 3. Without a membership the walk cannot finish, never reaches a terminal status, and the recovery sweep strands it with consecutive_failures = 1. 4. Nothing could clear that. A disabled source is never scheduled, so no successful run resets the count; `SourceService.update` clears only on an explicit disable and it was already disabled; and the banner's Retry routes to `/check`, which refuses a disabled source. The card offered a button structurally incapable of acting on the only source it was showing. `failing_sources_clause()` now means "enabled AND erroring". That also settles a disagreement its two callers already had: the scheduler's count paired it with `enabled.is_(True)` and `SourceService.list(failing=True)` did not, so one counted Ebi77 and the other did not — exactly the drift the note above that function warns about, which is why the test belongs IN the predicate rather than beside it. The scheduler's now-duplicate clause is dropped so one place decides. `/backfill` gains the guard for start/recover/recapture. `stop` stays open on a disabled source, or arming becomes a one-way door. Migration 0101 clears failure state on sources that are already disabled — the predicate fixes what the surfaces report, not what the rows carry, and the rows are why the operator had no way out (lesson #4202). It matches what `update` already does on an explicit disable, so rows disabled by any other path come into line. Enabled sources are untouched: a real failure on a live source must keep showing, which the second new test pins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-21 19:25:42 -04:00
bvandeusenandClaude Opus 5	11a01a9686	revert: remove the placement reconciler — it manufactured the problem it solved ... Milestone #421 built a sweep that compared each image's `artist_id` to the name of the directory holding its file, and called every mismatch a misplaced image. It reported 33,789 of 63,605 as wrongly filed. That number described the comparison, not the library. What it actually was: 32,475 (97.1%) one artist's own folder, spelled differently — Telepurte/ vs telepurte/. Same artist, same art. 657 ( 2.0%) loose at the images root 328 ( 1.0%) in a folder named after a different artist And the 1% did not mean what the tool assumed either. `ImageProvenance` records the post and source every file was downloaded from — the authoritative answer, which the tool never consulted. Querying it for all 328: 144 provenance agrees with the record (move would be right) 87 provenance agrees with the FOLDER (the record is wrong; move wrong) 53 provenance names SEVERAL artists (no single correct folder) 41 no provenance at all 3 agrees with neither So the sweep would have misfiled or arbitrarily picked for ~41% of the only set it was really needed for. The system already knew where each file came from; the tool inferred it from a column and a directory name instead. Operator, 2026-09-21: *"the current system consistently records where items are and where they came from this is just complicating something works and doesn't need fixing."* Correct on both counts. Removed: the service, the tasks, the model and migration 0099's table, the /api/cleanup/layout and /placement/* endpoints, the Maintenance card and its store actions, and the tests. 0100 drops the table (rule #22 — no legacy). KEPT deliberately, per the operator: - `utils.paths.canonical_subdir` — new filesystem imports derive their directory from the artist's slug, matching what the downloader always did. Not part of this tool; removing it would be churn that fixes nothing. - The 327 files run 1 moved (InsoUwu/ -> insouwu/). Same artist either way, and the gallery renders them correctly. - Everything from #4223 (three-gate dedup, 256-bit pHash) and #4234 (backup credential exclusion). Those fixed problems that were actually reported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR	2026-09-21 18:17:13 -04:00