Fix UNKNOWN_SKIP classification and expose protected affected-item explanations #10
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A completed preview reported 156 UNKNOWN_SKIP items despite zero asset errors and zero pending items. The user-facing text says the importer supplied no reason, but Bridge can also fail to translate an existing reason.
Concrete code gap in the packaged Immich-Go source: app/upload/run.go handles BetterOnServer by recording a discarded asset with ProcessedMetadataUpdated. deploy/importer/bridge_report.go does not map that event in its discard switch. This is a possible contributor, not proof that all 156 items followed that path. UNKNOWN_SKIP can also cover count reconciliation.
Acceptance criteria:
Starting implementation for #10 and the shared diagnostics work in #11 after reading #14 and both issue threads (no existing comments). I will audit discard calls in the pinned importer, distinguish missing upstream reasons, unmapped events and accounting reconciliation, and add bounded owner-only affected-item details. I will also retain redacted, bounded actual importer logs per attempt through processor cleanup, with authorization, retention/deletion and cancellation tests. Shared support reports remain aggregate-only. No running pilot service will be rebuilt or restarted as part of implementation; validation will use disposable test resources. Prior discarded diagnostics cannot be recovered.
Implementation findings and scope update: the pinned source uses ProcessedMetadataUpdated for both PROCESSED items and the BetterOnServer DISCARDED path. Classification now uses final tracker state; an actual pinned-importer BetterOnServer fixture passes. All production discard call sites are audited by a regression test. New reports distinguish UPSTREAM_REASON_UNKNOWN, BRIDGE_EVENT_UNMAPPED and SKIP_ACCOUNTING_DISCREPANCY; legacy UNKNOWN_SKIP no longer assigns blame to upstream. This does not establish the cause of all 156 historical skips.
The shared #11 implementation captures the actual INFO/JSON log through a subprocess pipe and the existing ephemeral per-attempt proxy grant. No extra processor mounts or writable application access are added. The app redacts before storage and requires ownership of both attempt and collection, with no administrator override. Collection deletion and seven-day expiry remove private diagnostics; normal worker cleanup preserves them. Cancellation has bounded graceful draining and explicitly reports incomplete capture after failures.
Following the requested 150 GB / 23,000-image scale check, limits were increased from the initial small proposal to 64 MiB of retained log and 25,000 affected items, with 16 MiB of incoming item data (24 MiB after controlled explanations), 100-item UI pages and a bounded log excerpt. Synthetic 23,000-image/name-and-metadata fixtures fit without truncation. These are diagnostic limits, not media import limits, and truncation remains explicit. Full real-library performance is not claimed.
Backend tests/vet, the pinned-importer fixtures, Docker isolation, UI lint/build and disposable browser coverage have passed. Security review, final race checks and commit/handoff recording are in progress. Nothing has been published or deployed and no existing pilot service has been restarted.
Additional validation finding: the refreshed Grype database now reports four advisories for the existing Echo v4.16.0 dependency. Upstream v4.16.1 patches these issues: https://github.com/labstack/echo/releases/tag/v4.16.1 . A minimal patch-version update is included so the current image advisory gate can pass; the affected backend/browser/image checks will be rerun. This records a dependency scan finding, not proof of exploitation in Bridge. The importer, runner and existing module-only advisory policy remain separate.
Implemented and locally verified for #10 and #11.
Local commit:
59cde963d5a391326dec3aef2d365c9c11de7c4a— Explain skip outcomes and retain private importer diagnostics.Branch:
codex/issues-10-11-diagnostics. The commit has not been pushed; no images have been published and no pilot service has been restarted or deployed. The working tree is clean.Changes:
Checks actually passed:
Limitations and next handoff:
The historical 156 skips cannot be individually explained retrospectively from discarded logs, and this fix does not prove they all followed BetterOnServer. A real 150 GB collection has not been run. Native ARM64/full release CI remains outstanding. Keep these issues open for the publication/pilot checkpoint: review and push the local branch, pass the required native AMD64/ARM64 gates, then coordinate deployment of app, runner and processor together while uploads/tasks are idle. Run a new owner preview and inspect the retained private details before recommending the full pilot import. Follow #14 for the subsequent priority order.
Publication/CI handoff update: commit
59cde963d5is now pushed on codex/issues-10-11-diagnostics. Draft PR #15 is open: #15 . This supersedes the earlier local-only publication status.Forgejo created the trusted-push validation run for that exact revision: https://git.sharedroot.net/jchapman/Immich-Bridge/actions/runs/2 . Its source (amd64) and source (arm64) jobs are waiting for bridge-ci-amd64 and bridge-ci-arm64; dependent suites and the release gate are blocked behind those jobs. The repository runner list is empty. Existing local verification remains recorded above; native CI has not passed.
Next step is to choose and register dedicated native Linux CI hosts following docs/ci.md, then let the queued workflow run and fix any failures. Runner placement has been requested from the owner. The native gate remains required. No merge, pilot deployment or service restart has been performed; run a fresh owner preview after the gate and a coordinated deployment.
Review handoff update: PR #15 is now ready for review (no longer a draft), at unchanged commit
59cde963d5a391326dec3aef2d365c9c11de7c4a: #15 . Final code review found no blocking issue, the working tree is clean, and the completed local AMD64 Docker validation remains the evidence recorded above.The owner approved proceeding with ARM compatibility assumed for this change. The implementation adds ordinary Go/file/HTTP behavior without CPU-specific changes. This supersedes the earlier request for the owner to select additional CI machines as the next development step. Native ARM64 execution is still unverified; the Forgejo workflow remains queued, and no passing native gate is claimed. Repository release rules have not been changed.
Next handoff: review the ready PR, then coordinate the app/runner/processor installation and a new owner preview. Keep #10/#11 open through that pilot checkpoint. No merge, image publication, pilot deployment, service restart or real-library import has been performed. The historical skips cannot be reconstructed from discarded logs; the 150 GB real-library performance limitation still applies.
Scope correction from the owner: retain the complete relevant importer log, without sampling or size-based deletion. Every WARN-or-higher record must be flagged for review, and severe records/capture failures must be prominent. Retain private logs until explicit owner acknowledgement that all logs will be deleted, or seven-day expiry. Replace the current log/item truncation and excerpt-only viewer with server-paged review. Credential redaction and owner isolation remain required. Implementing this revision on PR #15; prior capacity tests do not establish full-fidelity retention. No pilot service will be restarted during development.
Implementation findings for the revised requirements: new captures retain all DEBUG-and-higher records, console output and all discarded-item identities without the former log/line/item sample caps. Server paging limits browser responses rather than retained evidence. WARN and higher records are indexed; ERROR/critical records and capture failures are prominent. Filenames/paths and ordinary URL details remain, while actual credentials and signed access parameters are redacted. Explicit owner acknowledgement deletes all private diagnostics; otherwise retention runs until seven days after the attempt ends. Upload cleanup and successful results do not delete logs.
Full capture exposed an upstream v0.32.0 completion bug: resumeJobs sends job-control commands even with --pause-immich-jobs=false. Bridge blocks the command, producing an ERROR despite zero aggregate item errors. The packaged importer now returns early from resumeJobs when that option is false; the proxy remains restrictive. This is a specific behavior fix, not a severity suppression. Source-contract and actual-importer fixtures are being updated to assert the disabled option sends no job-control request. The matching modified source version is 0.32.0-bridge.4.
Validation in progress covers records beyond 16 KiB, logs beyond 64 MiB, more than 25,000 affected items, exact filename retention, full-page traversal, every WARN-or-higher record, explicit deletion, post-completion expiry, storage/transfer failures and cancellation. No pilot service has been restarted or deployed.
Completed development handoff: commit
f71e9528889f9c2529bdeaa17a006285acd06319is pushed. PR #15 is ready for review (no longer a draft): #15 . This supersedes the earlier bounded-retention implementation and in-progress scope comments.New captures preserve the complete DEBUG-and-higher log, both console streams and every final discarded-item identity without total-byte, line-length or item-sample caps. Filenames, paths, original errors, duplicate JSON fields and exact numeric metadata remain available; actual credentials and signed access parameters are redacted. All WARN-or-higher entries are indexed for review. ERROR/critical records remain errors, do not qualify a preview for import, and cannot be hidden by a zero item-error count. The basic wizard remains intact, with a short review notice and View log details link to original messages, filenames, technical records, server paging and complete streaming downloads. The shareable aggregate support report remains separate from private evidence.
All private diagnostics remain until explicit owner acknowledgement that ALL private logs/details will be deleted, or seven days after the attempt ends. Active attempts and normal upload cleanup cannot delete them. Explicit collection deletion includes those logs in its acknowledgement. Failed storage/transfer or an incomplete final manifest stops the attempt and exposes incomplete diagnostics while preserving available evidence. Retention/deletion recover after restart; there is no size-based pruning or automatic import replay. Both collection and attempt ownership are required, including for downloads and deletion; administrators have no override.
The modified importer is
0.32.0-bridge.4. It includes the actual upstream job-resume fix described above: disabled job pausing no longer attempts job-control commands on completion. The restrictive proxy is unchanged. The matching source archive, checksum, notices, vendor source and applied guard were verified locally.Completed local AMD64 Docker Desktop validation:
Limits and deployment state: this is implemented, locally tested and pushed for review, not merged, release-image-published or deployed. No pilot service was restarted and no real library was imported. The 23,000-image record fixture is not a measured 150 GB import. Full retention needs sufficient staging capacity, and individual very long records still require redaction memory. Previously discarded logs cannot be recovered. The owner approved proceeding with assumed ARM compatibility for these ordinary Go/file/HTTP changes; native ARM64 remains unverified. Forgejo validation is waiting, with no passing native release gate claimed and no change to release rules.
Next step: review PR #15, then coordinate an idle installation of app, runner and processor together and run a new owner preview to verify the original pilot symptoms and inspect the retained evidence. Keep #10 and #11 open through that pilot checkpoint. Do not interrupt active uploads or tasks.
Integration handoff: PR #15 is merged into main in
15993b0. The final combined source passed local backend/vet/race, UI/browser, container/isolation/HTTPS, source/image policy and secret checks. Detailed merge/conflict evidence and limitations: #14 (comment). Native Forgejo AMD64/ARM64 release validation remains queued; no release images or deployment are claimed. This issue remains open for its pilot checkpoint. Run a fresh owner preview and inspect retained skip explanations before recommending the pilot import. Historical discarded diagnostics cannot be recovered. No active upload/task or pilot service was interrupted.Final development handoff: PR #15 is merged into main in
15993b0, preserving feature commit(s)59cde96, f71e952.Discard classification now uses final pinned tracker state and event context, including BetterOnServer/ProcessedMetadataUpdated. Reports distinguish unknown upstream reasons, unmapped Bridge events and accounting discrepancies without unsupported blame. Protected affected-item details retain all identities; shareable reports remain aggregate-only.
Validation: Pinned-source discard audits, supported mappings and actual importer BetterOnServer/filtered-item fixtures passed, with full retention, paging/redaction, owner isolation and browser checks. Historical discarded logs cannot establish the cause of all 156 original skips. The combined application also passed backend/vet/race, UI lint/build, container/HTTPS, source/image policies and secret checks; integration evidence.
Documentation is updated on main through PR #21, merge
5ed7191: feature guide, merged scope/evidence, and remaining pilot checks. All nine documentation/policy tests and link/anchor/workflow checks passed.Closing at the owner's explicit request after merge, local validation and documentation review. This supersedes the earlier request to keep this development issue open through deployment. Native Forgejo AMD64/ARM64 release gates, coordinated idle installation and applicable real-provider/owner pilot checks remain in the release checklist and coordination issue #14. No deployment, native ARM64 pass, real-library capacity result or gate waiver is claimed. PR #19 and scanning remain on hold.