feat: land third-party oc10 apps from the merged legacy import batches#72
Conversation
PRs #68 (batch 4) and #69 (batch 5) were merged into the import stack's own tip (feat/import-legacy-batch3-oc) rather than into main, and the PRs that connect that stack to main (#65–#67) are still open. As a result none of the third-party apps those batches added ever reached main, so they never appeared on the live catalog. This adds the 28 third-party apps from those two merged batches directly on top of main, together with their 23 publisher profiles and their entries in data/created.json and data/downloads-baseline.json. ownCloud-owned apps and the still-open stack PRs are intentionally out of scope. It also carries over the validate-screenshots change those batches introduced: the info.xml URL fallback is now best-effort (a rotted/unsupported source URL is warned and skipped, not failed), since those URLs are only an ingestion source and are never served. This lets legacy apps whose only screenshot source has rotted — checksum (gif), e2eeshare (404), rds (dead host) — still import. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
e0f471f to
a951a1b
Compare
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated review by Claude Code review agent.
Overview
This PR lands the 28 third-party oc10 apps from the two already-merged legacy import batches (#68 + #69) directly onto main, fixing the broken PR merge chain described in the description (the connecting stack PRs #65–#67 are still open, so those batches never reached main). The change is overwhelmingly data: 28 app release packages (Git LFS pointers + screenshots), 23 publisher profiles, and merges into data/created.json and data/downloads-baseline.json. The only logic change is making the validate-screenshots info.xml URL fallback best-effort.
I verified the data is internally consistent:
- 28
package.tar.gzfiles, each a well-formed Git LFS pointer (version,oid sha256:…,size). - Each of the 28 packages has a matching
created.jsonentry at the exact same version, and is referenced by exactly one publisher profile. No orphans in either direction. - All 23 publisher profiles have the required fields (
slug,name,enabled,apps) and everyslugmatches its directory name. - The 3 pre-existing entries (
onlyoffice,richdocuments,music) are preserved in bothcreated.jsonanddownloads-baseline.json—musiconly changes position due to alphabetical re-sort, not content. This matches the "partial-merged, existing entries untouched" claim. - All 23 publisher
websiteURLs are https; no plaintext/untrusted-scheme hosts.
The scope matches the description: ownCloud-owned apps and the open stack PRs are excluded.
Code quality / style
validate-screenshots.ts: the doc comment is rewritten clearly to explain the strict-local / best-effort-fallback split, and the rationale (committed files are what's served; info.xml URLs are only an ingestion source mirrored byingest-screenshots). The try/catch is minimal and correct, emitting aWARNwith the path, URL, and reason. Good.- The test is updated to match the new contract: the previously-rejecting case now expects
resolves.toBe(0)with a comment explaining why. Reasonable. - Data files are consistently formatted (2-space JSON, sorted keys).
Specific suggestions
-
Behavior-change blast radius —
validate-screenshotsis now strictly weaker for the fallback path. Previously a bad info.xml screenshot URL failed the gate; now any fetch/validation error (unreachable host, 404, wrong format, oversize, malformed image) is downgraded to a warning. That is intended here, but it also means a genuinely broken new upstream submission can silently pass with zero screenshots. Consider, in a follow-up, distinguishing "source rotted" (network/404 — skip) from "source served bad bytes" (format/size violation — arguably still fail), or at least surfacing the skipped count so CI flags it. Not blocking for this import. -
console.warnin CI may be invisible. Since the skip is best-effort, the only signal is aWARNline. If the deploy chain doesn't surface tool stdout/stderr prominently, a fully-rotted app importing with no screenshots could go unnoticed. A trailing summary (e.g.validated N, skipped M) would help operators. -
The PR body lists 28 apps and the diff confirms 28 packages — consider noting in the body that
created.jsonshows 29 added lines only becausemusicwas re-sorted, to preempt a reviewer miscount.
Potential issues / risks
- Third-party trust: these are imported third-party apps. Trust is inherited from the legacy ownCloud marketplace and the merged batch PRs — this PR does not add any new publisher integrity/signature enforcement, and the appstore model serves committed tarballs rather than re-fetching from publisher sites at runtime, so the runtime attack surface is the committed LFS objects. The
oid sha256in each LFS pointer pins the exact bytes, which is good. I could not validate the contents of the tarballs from the diff (LFS objects aren't fetched here); their integrity rests on the same review that merged #68/#69. Worth a conscious sign-off that those upstream packages were vetted. - No code/model/migration changes beyond the one CLI tool — no Django model or migration risk in this PR.
- Download baseline values are large integers carried over as-is; they only seed analytics baselines and are low-risk.
Overall this is a clean, well-scoped data landing with one defensible, well-documented test/behavior change. The main thing to keep an eye on is the weakened screenshot gate becoming a silent quality hole for future submissions; fine for this legacy import.
Why
Recently imported oc10 apps are missing from the live catalog
(https://deepdiver1975.github.io/appstore/#catalog). The deploy chain is fine —
the live site is in sync with
main, which only has 4 apps.The real cause is the PR merge chain, not GitHub Actions. The legacy import
was a 5-PR stack, and the two merged PRs landed in the wrong base:
#68 and #69 merged into the stack's own tip (
feat/import-legacy-batch3-oc),not into
main, and the PRs that connect the stack tomain(#65–#67) arestill open — so nothing those batches added ever reached
main.What
Lands the 28 third-party apps from the two already-merged batches (#68 + #69)
directly on top of
main, plus their 23 publisher profiles and their entriesin
data/created.jsonanddata/downloads-baseline.json(partial-merged so the3 existing entries are untouched).
Also carries over the
validate-screenshotschange those batches introduced:the info.xml URL fallback is now best-effort (a rotted/unsupported source URL
is warned and skipped, not failed), since those URLs are only an ingestion source
and are never served. This lets legacy apps whose only screenshot source has
rotted still import —
checksum(gif),e2eeshare(404),rds(dead host).ownCloud-owned apps and the still-open stack PRs (#65–#67) are intentionally
out of scope, per request.
Apps (28): afterlogic, bookmarks, bplog, carnet, checksum, cms_pico, duo,
e2eeshare, extract, federatedgroups, files_3dviewer, files_textviewer,
groupalert, metadata, nextbackup, notes, opencloudmesh, openotp_auth,
qownnotesapi, rds, sccuot_ng, sciencemesh, sensorlogger, syncblue, tasks,
twofactor_backup_codes, twofactor_privacyidea, wallpaper
Verification
generate-api.tsexits clean and emits 32 apps (4 + 28) — every releasevalidated.
website buildsucceeds: 82 pages incl. 28 app + 23 publisher pages.validate-screenshotsagainstorigin/mainpasses (rotted source URLs warnedand skipped).
After merge, the existing push-triggered chain (
deploy.yml,publish-app-assets.yml,ingest-screenshots.yml) rebuilds the site, createsthe GitHub Release assets, and ingests screenshots — same flow already proven
for the current 4 apps.
🤖 Generated with Claude Code