Skip to content

AudioBookShelf#25

Open
KhadeejaQubra wants to merge 1 commit into
Yundera:mainfrom
KhadeejaQubra:main
Open

AudioBookShelf#25
KhadeejaQubra wants to merge 1 commit into
Yundera:mainfrom
KhadeejaQubra:main

Conversation

@KhadeejaQubra

Copy link
Copy Markdown

This docker-compose file sets up the Audiobookshelf service with initialization for required directories and permissions.

This docker-compose file sets up the Audiobookshelf service with initialization for required directories and permissions.
@Maelisse2002

Copy link
Copy Markdown
Collaborator

🤖 AI Pre-Check

Decision: ⚠️ ai-reviewed:needs-review (new app — AI pre-check is advisory; human flow owns the decision)
Tech review (incl. security): needed — new app submission
Commit: 08192db | Checklist source: CONTRIBUTING.md@main

Apps in this PR

  • audiobookshelf — new

AI static checks

  • ❌ Specific version tag (no :latest) — ghcr.io/advplyr/audiobookshelf:latest and busybox:latest both unpinned
  • ✅ No hardcoded credentials
  • ✅ Volumes — config/metadata under /DATA/AppData/audiobookshelf/; media under /DATA/Media/ (sanctioned media-app pattern)
  • cpu_shares set on all services — missing on both audiobookshelf and audiobookshelf-init
  • ❌ Permission strategy — audiobookshelf mounts user dirs (/DATA/Media/Audiobooks, /DATA/Media/Podcasts) but sets neither user: nor PUID:PGID; CONTRIBUTING requires user: $PUID:$PGID for user-directory access
  • x-casaos icon / thumbnail / screenshots referenced — none present in metadata
  • ❌ Asset files present & URLs point to Yundera/AppStore@main — no icon.png / thumbnail.png / screenshots in the PR (only docker-compose.yml)
  • ➖ pre/post-install-cmd pinned & --user $PUID:$PGID — n/a (uses an init container, not x-casaos.*-install-cmd)

→ Tier 2 must verify (human)

  • Works immediately after installation
  • Fresh installation tested
  • Uninstall / reinstall preserves data
  • Migration from previous version succeeds

Notes for reviewers

New app submission, so this verdict is advisory only — it stays on the human flow (PH + Cristian). Static analysis found several blocking-class issues to address before merge: unpinned :latest tags, no cpu_shares, missing icon/thumbnail/screenshot assets, and the main service accessing /DATA/Media without user: $PUID:$PGID. There is no rationale.md, yet the app relies on Audiobookshelf's own first-launch onboarding for auth (an acceptable pattern à la Jellyfin/Immich, but CONTRIBUTING says it must be documented in rationale.md). It also publishes a host port (${WEBUI_PORT:-13378}) rather than using the Caddy pcs-network label pattern — tech review should confirm web-UI routing works on PCS.

Next step

→ Tier 2 functional review, then tech + security review (reason: new app). Please also fix the ❌ items above and push again to re-trigger the AI pre-check.


Generated by AI pre-check. Checklist read live from CONTRIBUTING.md on main. Labels are the machine-readable verdict; this comment is the human-readable explanation. Humans own the merge.

@Maelisse2002 Maelisse2002 added ai-reviewed:needs-review AI pre-check: ambiguity flagged tech-review:needed Tech + security review required labels Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed:needs-review AI pre-check: ambiguity flagged tech-review:needed Tech + security review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants