Release 1.2.0 - #112
Open
GianniCarlo wants to merge 74 commits into
Open
Release 1.2.0#112GianniCarlo wants to merge 74 commits into
GianniCarlo wants to merge 74 commits into
Conversation
Two surfaces ignored the sticky sort and walked raw orderRank: - getAdjacentItem fed the player's next/previous buttons and end-of-book auto-advance from rank-ordered siblings, so under an automatic sort the player jumped to a different book than the visible neighbor — on the phone, Auto, and Wear alike. The repository now carries a property-wired effectiveSortResolver (the manager depends on the repository, so a constructor arg would be a cycle; null keeps rank order for targets without sort prefs) and orders the playable siblings by the rule before indexing. The auto-advance path also stops constructing a bare inline RoomLibraryRepository and goes through the shared getRepository instance, which playNext/playPrevious already use. - Android Auto's browse tree listed the Library tab and folder drill-downs in rank order. onGetChildren now applies the location's effective sort via the previously-unused LibrarySortManager .sortedForDisplay one-shot, before pagination so page slices stay stable. Recent and in-car search keep recency order by design. A new observeSortPreferences flow (filtered store snapshot) drives coarse cache invalidation: any sort change refreshes the Library node, mirroring the existing Recent-tab notifyChildrenChanged pattern. BOUND volumes are untouched (they resolve Unresolved → Custom → rank, which is the playback timeline). AdjacentItemSortTest pins the fallback (no resolver ⇒ rank), the rule walks for title and most-recent, Custom staying rank-ordered, per-folder independence, folder exclusion from the walk, and sortedForDisplay's automatic/Custom split.
The watch rendered every library level in raw orderRank because nothing on wear constructed the sort machinery: no LibrarySortManager, no preference sync processors, an empty DataStore. A folder sorted by Title on the phone listed in custom/import order on the watch. Pull-only preference sync, mirroring the iOS watch: - WearSyncServiceHost registers PreferenceFetchProcessor (the active path — it writes pulled values into the watch's own DataStore) and PreferenceUploadProcessor (processor-set parity with the phone, so a preference task can never sit unhandled). StandaloneViewModel enqueues the preferences fetch alongside its contents fetch on open/refresh; the factory debounces to one pull per 30s per launch. - WearApp builds a LibrarySortManager over the watch's repositories and wires the repository's effectiveSortResolver, so end-of-book auto-advance on the watch follows the visible order too. - StandaloneViewModel applies the same view transform as the phone's list: items combined with the level's effective sort, rule-ordered when automatic, rank-ordered under Custom (and when no manager is wired — tests). The per-path effective-sort glue is hoisted from LibraryViewModel into LibrarySortManager.observeEffectiveSort(path) so phone and wear share one implementation. Free accounts never pull (the fetch requires the sync engine, which only runs for PRO — the standalone library is PRO-gated anyway) and nothing on the watch writes sort preferences.
Make next/previous and Android Auto follow the visible library sort
Bring the Wear standalone library up to sort parity
…e embedded covers Import made two allocations the size of the embedded cover, one line apart in ImportManager.createBookItem: AudioChapterExtractor read the whole `moov` to find the chapter track, and ArtworkManager read the picture whole via MediaMetadataRetriever.embeddedPicture. A 29 MB cover against 10-13 MB of heap headroom is Sentry ANDROID-BOOKPLAYER-17/-18. The MP4 box and ID3 frame walkers now return byte ranges (ContainerReaders.kt); the extractor materializes only the chapter trak, EmbeddedCoverLocator finds covr/APIC without reading it, and ArtworkManager decodes the cover through a stream with inSampleSize. CoverArtResolver routes through the same path. Verified on an API 31 emulator at a 64 MB heap growth limit: both former OOM sites now import a 60 MB-cover m4b with chapters and artwork.
…ash-looping A zero-filled or truncated preferences proto (typically a disk-full or interrupted write) threw CorruptionException on every launch until reinstall (Sentry ANDROID-BOOKPLAYER-13). ReplaceFileCorruptionHandler resets to defaults and logs a warning; losing playback settings is the lesser harm.
…ro.md) Scripts that reproduce production crashes on a low-end API 31 AVD and verify fixes before/after: AVD creation, heap growth limit control (with the sys.boot_completed reset that stop/start needs), DataStore corruption, huge-cover and huge-moov m4b generators, and an import driver that walks Open-with -> Accept -> Library through the accessibility tree and reports rows and artwork.
An oversized remote cover (over the 8 MB browse cap) is reported as EmbeddedArtwork.Failed, not None: the picture exists and was only skipped, and CoverArtResolver negative-caches None as "artless" for the process, which would have hidden the cover even after the book was downloaded. MockWebServer tests cover the remote locate+save path and the oversized skip.
The streaming decode path only caught Exception, so an OutOfMemoryError from BitmapFactory.decodeStream / createScaledBitmap (an extreme aspect ratio keeps inSampleSize at 1) could escape saveEmbeddedArtwork and crash the import this branch hardens. decodeAndSave now reports a tri-state: OOM maps to EmbeddedArtwork.Failed (transient, never negative-cached as "no art"), an undecodable picture to None.
Import survives huge embedded covers; DataStore corruption recovery; emulator crash-repro rig
chapters.bookUuid and playback_sessions.bookUuid are FOREIGN KEYs to library_items. Both writers run asynchronously after a play/load, so the book can have been deleted or replaced by a sync pull by the time they land; the failed insert escaped a coroutine with no handler and took the process down (Sentry ANDROID-BOOKPLAYER-15 via replaceChaptersForBook, -19 via StatisticsDao.insertSession). Both DAO writes now check the parent row inside the same transaction and write nothing when it is gone. Statistics coroutines also run under a CoroutineExceptionHandler: bookkeeping must never take playback down, whatever the database throws.
… the playback service media3 keeps session ids in a process-wide registry and refuses a duplicate. The service used the default "" id, so one session that was registered but never released in the same process made every later service creation die with "Session ID must be unique" at MediaLibrarySession.Builder.build() (Sentry ANDROID-BOOKPLAYER-1A; both reports are OPPO devices, consistent with the OEM retrying service creation without killing the process). Each instance now takes a fresh bookplayer-<n> id (controllers connect through the ComponentName, never the id), and onDestroy releases the session even if the player's release throws.
…-session-id Skip child-row writes for vanished books (-19/-15); unique media session id (-1A)
…udioManager 1.11.0 fixes the out-of-bounds timeline merge in MediaUtils.mergePlayerInfo — the top open crash on 1.1.2 (Sentry ANDROID-BOOKPLAYER-Q, an IllegalStateException in PlayerInfo.Builder.build on the app's own MediaController). BookTimelinePlayer derives every index from BoundTimeline.chapterLocalOf, whose clamping BoundTimelineTest pins, so the race was upstream. Consequences of the bump handled here: - MediaNotification.Provider gained a required getNotificationChannelInfo(); the Wear OngoingMediaNotificationProvider delegates it to the default provider. - 1.10 stopped honouring device-volume commands from a MediaController for local playback, which was the watch crown's path. DeviceVolume now adjusts STREAM_MUSIC through AudioManager and observes the platform volume broadcast only while a UI collects the fraction; ExoPlayer's setDeviceVolumeControlEnabled goes away with it.
media3 1.11.0 (fixes the mergePlayerInfo crash, -Q); watch crown volume via AudioManager
… full Sentry ANDROID-BOOKPLAYER-12 / -10 / -1D / -1G / -S / -V are one condition: the device has no free space, and the first write to fail took the process down (at zero bytes, that was the database open at launch — SQLITE_IOERR_SHMSIZE on PRAGMA journal_mode). StorageMonitor is the process-wide storage state. It is fed two ways: measured (free bytes on the data volume; below 32 MB is critical) and observed (a write that failed for lack of space — SQLiteFullException, SQLITE_IOERR_SHMSIZE, ENOSPC anywhere in the cause chain — flips critical and stays sticky until a later measurement sees 64 MB again). Its exceptionHandler records full-disk failures and forwards everything else to the default uncaught handler, so real bugs still crash and still reach Sentry. What the state drives in :core: - PlaybackManager refuses to start while critical and pauses running playback the moment a progress write fails (an audiobook player that cannot save your place must not pretend to). This covers every surface — app, notification, Auto, Wear, Bluetooth — because it sits in the controller listener. Recovery clears the block; it does not auto-resume. - StoragePolicy (pure, tested) runs no sync task while critical — every task ends in a database write — and holds file downloads while a transfer is known not to fit. - DownloadFileProcessor pre-flights Content-Length against a 64 MB reserve instead of filling the disk and taking the database down with it. - The long-lived scopes that write (PlaybackManager, TaskConcurrencyManager, SubscriptionManager, SleepTimerManager) carry the handler. CoreContext.appContextOrNull lets the handler measure free space without forcing init order.
… full The phone and Wear surfaces for StorageMonitor: - Launch gate: MainActivity shows a storage screen (free space, "Free up space" → the system storage UI, "Check again") instead of the app when the volume is critical at launch, so nothing touches the database. It re-checks on resume and restarts the app once space is back. Intent handling (shortcuts, "open with") is skipped while gated. - MainScreen shows a banner while critical or while a transfer is waiting, re-measures every 10 s in that state, and shows the playback-blocked dialog. - BookPlayerApplication measures right after CoreContext.init, starts the sync host only when not critical, and restarts it when storage recovers. The Sentry account binding moves onto the supervised app scope: its handler-less IO scope opened the database and was the second exit at zero bytes. - ImportManager pre-flights each URI's length against the reserve (skipped, not copied) and reports a mid-copy ENOSPC instead of dying on it. - Every remaining long-lived scope that writes carries the handler: ThemeManager, the Wear publishers, the widget, shortcuts, the sync host, WearApp, the tile. Eight new strings; translations are handled separately.
Nothing referenced it — only the Gradle line — but its auto-initializer wrote to its own database at process start, which made it the very first thing to die with no free space (SQLiteFullException on WM.task-1 on the emulator). Re-add it with on-demand initialization when the WorkManager sync migration lands.
startForegroundService() gives the service a few seconds to call startForeground(), measured from the request, and onCreate runs on the main thread. The host used to open the database and build its 17 processors first, so the promotion waited on all of that plus whatever the main thread was already doing when the start landed (a recreate, an import sheet — which is what the storage-recovery restart does). Sentry ANDROID-BOOKPLAYER-1H / -X is that deadline expiring. Promotion is now the first statement after channel creation, and a dataSync-budget refusal short-circuits before any of that work. Hardening: the window is now just the main-thread queue delay. (An emulator run appeared to reproduce -1H on the recovery path; it turned out to be the guest's storage freezing for two minutes after a 4.6 GB fill file was freed — see fill-disk.sh — so this is not a verified fix for -1H.)
fill-disk.sh fills the emulator's data volume to a given root-side figure (or to zero) and frees it again. The header records what cost time: `df` as root counts ~140 MB of root-reserved blocks the app's StatFs does not, removing the WAL to force the shm-size failure discards unflushed rows, and at zero bytes the crash reporter itself cannot store reports. The docs section records the launch / mid-playback / import scenarios with before-and-after results, plus the recipes for -19/-15, -1A and -Q from the earlier fixes.
openStorageSettings falls back to Settings.ACTION_SETTINGS, which every device resolves, so "Free up space" is never a silent no-op.
fix: storage-full guard — launch gate, playback block, transfer holds
Sentry ANDROID-BOOKPLAYER-11: a Background ANR with the main thread parked in LoudnessEnhancer.<init> → AudioFlinger::createEffect. Creating or releasing an audio effect is a synchronous binder call into audioserver, and we made it from ExoPlayer's onAudioSessionIdChanged listener, which runs on the application looper. On a low-end Redmi (Android 14) that call stalled long enough for the system to kill the process. LoudnessBooster owns the effect on its own single-thread executor. The service only posts attach / setEnabled / release commands, so a stalled audioserver stalls that thread and nothing else. Commands apply in order: "attach to B" after "attach to A" ends with B live and A released, the boost setting is remembered across re-attach, and release drops later commands. The LoudnessEnhancer surface it uses is an interface, so the tests drive a fake: a factory that blocks on a latch must not block attach(), plus ordering, an unavailable-effect failure, and release semantics. Not reproducible on the emulator (its audioserver never stalls); the contract is what the tests pin down.
Emulator crash reproductions and chaos runs were landing in the production issue list as fresh fingerprints (environment:dev on the 1.1.3+20 release; -1N, -1K, -1M, -1J were archived for that reason, and -1H's count was inflated). The dev flavor now compiles SENTRY_REPORTING=false unless local.properties has SENTRY_DEV_REPORTING=true; prod is always on. Documented in CLAUDE.md and docs/crash-repro.md.
…h is attributable
Sentry ANDROID-BOOKPLAYER-1E ("Bad notification for startForeground", one TECNO on
Android 12) carries no cause on Android 12+ and its stack has no app frames, so the
report cannot say which service was promoting. Both sites now leave an `fgs`
breadcrumb: TaskConcurrencyServiceHost before startForeground (and on a denied
promotion), and AudioPlayerService.onUpdateNotification when media3 is about to
promote with the media notification. Nothing to fix until it recurs with that
context attached.
fix: LoudnessEnhancer off the main thread, dev builds out of Sentry, promotion breadcrumbs
… builds The Play Console's R8 configuration panel flagged both as missing (Full Mode and Resource Shrinking were already on). Neither needs the AGP 9 upgrade the panel also suggests: both are one-line changes on AGP 8.13. - android.r8.optimizedResourceShrinking=true: R8 shrinks code and resources as one reference graph, so resources referenced only from unused code go too (AGP 8.12+, the default from 9.0). - -repackageclasses in the app and wear rules: obfuscated classes move to the unnamed package, dropping package-name strings from the DEX (the AGP 9.1 default). Kept classes are untouched, so the cross-process keeps (wear ongoing activity) hold. Measured on unsigned prodRelease: phone APK 16.44 → 16.28 MB (67 of 321 resource files removed, 8278 of 11149 classes repackaged); watch 8.79 → 8.43 MB (72 of 443 resource files, 6668 of 9049 classes). Mapping identity of every by-name surface is unchanged.
…es its name Play rejected wear 100007 (1.1.2, the first minified build) for "missing ongoing activity": SystemUI deserializes OngoingActivity state by class name in its own process, and R8 had renamed that machinery. Nothing in the build, the tests or CI could see it; only Play review did. The keeps fixed it, but the only guard since has been a manual mapping.txt grep. scripts/audit-mapping.sh reads the prodRelease mapping and fails when any by-name class was renamed or moved: every manifest component of the module (and the class an activity-alias targets), Room's AppDatabase/_Impl, and on wear everything under androidx.wear.ongoing and androidx.versionedparcelable. R8 synthetics are skipped. CI runs it after the minified prodRelease assemble; the release workflow runs it right after the bundle build, before anything is attached or uploaded. Verified: passes on develop's baseline and on the repackaged build; fails with the wear keeps removed.
…ension fix: never guess a media-server item's file extension on virtual import
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } catch (e: SessionExpiredException) { | ||
| _sessionExpiredServerName.value = currentServer.name | ||
| return null | ||
| } catch (e: Exception) { |
There was a problem hiding this comment.
🔵 INFO — The broad catch (e: Exception) in prepareStreamImport also catches CancellationException, so if the enclosing coroutine is cancelled while getFileExtensions is in flight (e.g. the user leaves the screen), cancellation is swallowed and surfaced as a generic error instead of propagating. The network services in this same PR carefully rethrow it (catch (e: CancellationException) { throw e }). Consider adding that rethrow here (and at the pre-existing sites on lines 104/164) for consistency. Low impact.
✅ Claude PR Review —
|
The README read like internal engineering notes ("Multi-Queue Engine",
"Tiered Access Policy") and undersold the app — no mention of Android Auto,
Wear OS, the widget, chapters, bookmarks, the sleep timer, Hardcover, media
servers or the 11 supported languages.
Rewritten to the iOS README's shape and voice (Import / Manage / Listen /
BookPlayer Pro / Roadmap / Locales, then Contributing / Maintainers /
Contributors / Community, then Dependencies / License / Legal), with the
feature list checked against the code. Adds the shared header banner, the
Play Store badge, and the current five-panel screenshot strip (1776px wide,
quantized: 4.2 MB source -> 193 KB). Community points bug reports at the
#bugs-and-feedback Discord forum channel, matching iOS.
Also drops library.png — an unreferenced duplicate of the empty-library
placeholder sitting at the repo root.
Separately, the Claude reviewer now skips fork PRs. Runs from a fork get no
repo secrets and a read-only token, so it would post a red X on every outside
contributor's first PR. Same guard the iOS repo already carries.
docs: rewrite the README for the open-source launch
| _sessionExpiredServerName.value = currentServer.name | ||
| return null | ||
| } catch (e: Exception) { | ||
| _error.value = e.message?.let { UiText.DynamicString(it) } |
There was a problem hiding this comment.
🔵 INFO — On failure the raw e.message from the service (e.g. "Jellyfin API error fetching items: 500 ...") is surfaced to the user verbatim as UiText.DynamicString, an untranslated developer string. It carries only HTTP status/reason (no server body, so no HTML/JSON leak), but consider routing non-session failures through the existing R.string.media_servers_error_failed_to_fetch_library instead of the raw message for a localized UX.
| * by a stale reading. | ||
| */ | ||
| private fun blockedByStorage(): Boolean { | ||
| val critical = appContext?.let { StorageMonitor.refresh(it).isCritical } ?: StorageMonitor.isCritical |
There was a problem hiding this comment.
🔵 INFO — blockedByStorage() calls StorageMonitor.refresh() (a synchronous StatFs(...).availableBytes syscall inside synchronized(this)) from play()/load() on the main/transport thread, tripping StrictMode disk-read on every play. It's a single sub-ms stat by design (re-measure to avoid a stale reading), so impact is minor; if you want to keep the UI thread clean, gate on the already-observed StorageMonitor.state.value and kick refresh() to Dispatchers.IO.
| null | ||
| } | ||
| if (located != null) { | ||
| return decodeAndSave(destFile) { ByteRangeInputStream(FileByteSource(audioFile), located.start, located.length) }.toEmbeddedArtwork() |
There was a problem hiding this comment.
🔵 INFO — When EmbeddedCoverLocator.locate returns a range but decodeAndSave yields UNDECODABLE, the result maps to EmbeddedArtwork.None ("definitive, no art") and does not fall back to MediaMetadataRetriever the way the null-locate branch does — so a mis-located-but-actually-present cover could silently lose art the old platform path would have found. Well-mitigated by EmbeddedCoverLocatorTest (located ranges start with real JPEG/PNG signatures), so real-world triggering is unlikely. Optional hardening: map UNDECODABLE from a located range to Failed (retry via the platform reader) rather than None.
…pass, state record, 233 tests Replaces the first-generation Claude review harness with the hardened one. The agent runs under `permissionMode: 'default'` behind a `canUseTool` gate whose Bash is a grammar, not a shell emulator: it accepts only what it can prove it has parsed as bash would, with commands and their flags allowlisted in full spelling because getopt_long accepts any unambiguous prefix. Nothing that follows a symlink, never returns, reads stdin, or takes filenames from a file gets through. `settingSources` is empty, the agent's environment is built by allowlist, and the two tokens that can write to the pull request are deleted from this process for the duration of every agent call — so the isolation does not depend on how the SDK spawns. Identity is stated rather than inferred. The prompt lists the findings still open from earlier pushes and the agent may answer `same_as`; the fallback fingerprint is corroborated against what the thread actually says, and a finding matched to a thread that does not carry its text gets that text posted, so no identity decision can bury a finding's wording. Nothing closes a thread except a judgement: a second pass reads each still-open finding against the current code and answers fixed, present, not applicable, accepted (by a maintainer — never the author), insufficient or duplicate. Absence closes nothing. A close whose reason cannot be posted on the thread is undone, because a thread left resolved with no marker and no record is read next round as a maintainer's own resolve. The harness writes down what it did. A hidden state record in the summary comment carries which thread holds which finding and what was closed and why; the next round reads it instead of doing archaeology over its own comments, and the record survives a failed read, a truncated listing, an edited body, and a round that could not write its summary — which now fails loudly instead of exiting green with nothing on the PR. Every clock is bounded and the bounds are tested against each other: the review and verification passes, the write phase's network retries, the GitHub client's ladders and page loops, and the workflow's step and job caps, which `test/workflow.test.mjs` computes from the harness's own constants so the numbers move together or the build fails. A conservation law (`test/conservation.test.mjs`) fuzzes rounds against a GitHub whose state drifts, lies about `same_as`, and refuses each kind of write in turn, and states the one rule everything else serves: a reported finding is on the pull request, or the round failed visibly. Everything the model writes is untrusted at the write boundary and redacted on the way to the log; a result-shaped example quoted inside a finding cannot be adopted as the round's answer. The SDK is pinned exactly, because the sandbox rests on option names that no stubbed test can see change. This commit is the portable half only. `review-guide.md` is the one file that knows which repository it is in; `review.mjs` no longer carries a second copy of that. The Android CI gating that grew alongside this work on the previous branch is deliberately not here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
github.mjs's two retry warnings quoted a thrown error's message outside the
redact() boundary review.mjs declares for every string that leaves the
process. The client cannot import redact (cycle), so it is injected:
setLogRedactor, installed by review.mjs at module scope, failing closed
(message withheld, name kept) until it is.
The test that enforces the boundary read only review.mjs and matched only a
plain `${x.message}`, so `${e.name || e.message}` was invisible to it. It now
reads both files, every `${...}` to its own closing brace, across multi-line
console calls, and requires the whole expression to be redact(...)'s argument.
Two seam tests pin the fail-closed default and the install; five mutations
killed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- A finding the model reports with no usable file/line/comment/severity was discarded with a run-log line only; the summary now carries the count, on all three summary paths (dry run, no thread listing, full round). The one way a reported finding could leave the PR without a trace is closed. - closeWithReason returned `why` and nobody read it; dropped, and the header bullet that still described the pre-round-29 behaviour (close stands, row carries the reason) rewritten to what the code does (close undone). - The `Previously raised` label deliberately pairs the thread's live path with its live line; said so where the record's path is preferred for keying. - conservation fuzzer: `filter(...) || []` was unreachable; removed. Tests: discarded-count wording unit test + the malformed-findings round asserts the summary line; three mutations killed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The node_modules ignore moves from the repository root into the harness
directory, so "copy this directory and the workflow" copies it too; the
root .gitignore is back to develop's. A workflow test pins it.
- The review prompt's open-findings list rendered an outdated thread's line
bare while the verifier's prompt labelled it stale; both now use one
STALE_ANCHOR_ATTR, and openFindings carries `stale` through.
- A test passed actionByFp an option it does not take (`unpostable` for
`unpostableFps`). Fixed, and the class is now checked: for every exported
function whose first parameter is an options object, a call spelling its
options as a literal may only use declared names. The check found 14 more
— every planRound({ provisional }) in the suite, a dead option since the
verification pass took over closing — cleaned up.
Four mutations killed (stale dropped, stale rendered bare, module .gitignore
deleted, misnamed option restored).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
review.mjs (3,053 lines, four programs in one file) becomes one module per seam, moved statement-for-statement with an AST tool, no logic rewritten: sandbox.mjs tool gate, path rules, agentEnv, withheld tokens, redact repo.mjs PER REPOSITORY: secret file names, secret shapes (finding) identity.mjs fingerprints, same_as, state record, markers, planRound prompts.mjs system + user prompt (loads review-guide.md) agent.mjs SDK options, run loop, model resolution, result parsers verify.mjs verification pass, the only closer summary.mjs sticky summary, record, size budget, notes, upsert config.mjs env read at call time review.mjs runReview: budgets and the order of operations (620 lines) Shared modules read env per call (config getters, diffPath()/agentCwd(), maxTurns()/maxOutputTokens(), captureSecretValues() at run start, setModel()) so the tests' per-scenario environments still hold. The text checkers (comments, option names, log redaction, budget prose) read every module instead of two named files. Round-4 findings, all agnostic: - repo.mjs: the repository's secret files and shapes leave the portable half; sandbox builds REPO_SECRET_PATH and redact's tail from them. - agent.mjs: the tool gate is also a PreToolUse hook (deny-only), so the path rules hold whether or not the SDK routes a Read to canUseTool. - smoke.mjs: the install check loads the SDK AND runs the native CLI binary for this runner (execute bit + --version), replacing a check that proved only that JavaScript installed. - README: layout table, porting = copy + edit two per-repository files, the 422-after-a-write-that-landed residual; package.json no longer names the repository. 241 tests; five mutations killed (hook absent, hook never denies, shapes skipped, path rule hardcoded, smoke step dropped). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…; address review feedback (round 5) The workflow moves to pull_request_target, so the base branch's copy of it runs. The review job checks the harness out from the base branch into harness/ and executes only that; the pull request's tree is a second checkout beside it (REVIEW_CHECKOUT) that the agent reads and the path rules confine it to. The secrets are scoped to a `reviewer` environment whose branch policy (develop, main) is what keeps any other pull_request workflow from reaching them — a repository setting, documented in the README along with what this does not close (the agent subprocess still holds the key) and the next step (OIDC to Bedrock / a GitHub App token). The pull request's own harness tests run in a second job with no secret, no environment and a read-only token, gated on the pull request touching the harness. A pull request that changes the harness is reviewed by the harness it changes from; merging promotes it. test/workflow.test.mjs reads jobs separately now and pins the split: the event, the environment, the base-ref checkout, that every node the review job runs is harness/, that REVIEW_CHECKOUT points at pr/, and that the tests job references no secret. Round-5 findings: - review.mjs: a finding element that is not an object (null, a string, an object whose comment is not a string) is discarded and counted instead of throwing past the parse's try/catch into a red check. The conservation fuzzer's agent now emits such elements (20% of rounds), so the class stays covered — the mutation that removes the guard fails both the round test and the law. - summary.mjs: model text inside the summary's <details> block cannot close it (</details>, <summary> escaped; code spans left alone). - smoke.mjs: a spawn that could not run at all reports run.error. - sandbox.mjs: the agent-output dump cap is read per call, like every other knob in a shared module. 246 tests; eight mutations killed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
reviewer: port the hardened Claude review harness
GianniCarlo
had a problem deploying
to
reviewer
September 11, 2026 17:45 — with 
GitHub Actions
Failure
GitHub Actions
Failure
… suite names no repository Porting the harness to bookplayer-support-pipeline (its PR #16) found two tests in the shared suite that were still this repository's: the redaction test asserted Sentry/RevenueCat/client-id strings by hand, and the read-tool deny checks named local.properties, keystore.properties and google-services.json. A copy of the suite failed on a repository without them. REPO_SECRET_SHAPES entries are objects now — pattern, replacement, and the `example` that proves the shape plus a `keeps` look-alike that must pass — and the suite runs both for every entry, so a shape cannot be listed without working and cannot eat prose. The deny checks iterate REPO_SECRET_FILES through Read, Grep and Glob, and the template copy of each name. The two harness directories are byte-identical again apart from repo.mjs and the README's local-run coordinates. Mutations: an example the pattern cannot redact fails the suite; a name added to REPO_SECRET_FILES is refused by every read tool with no test edit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The shape test asserts that THIS entry's pattern redacts its example: the example is run through a fresh RegExp of the pattern and the boundary's answer must equal that, so a generic rule or a sibling shape catching it cannot stand in. Fields are validated (RegExp, global, non-empty strings); `example`/`keeps` accept a string or an array. - The Sentry shape lists all three DSN forms the old assertions covered (modern ingest host, legacy without and with a secret). - The deny loop exercises Grep's `glob` field, the one an agent sweeps for a file by name with. Mutations: an example only a generic rule redacts fails the suite; dropping `glob` from Grep's path fields fails it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…its reason Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
reviewer: the per-repository shapes carry their own proof; the shared suite names no repository
GianniCarlo
had a problem deploying
to
reviewer
September 11, 2026 21:46 — with
GitHub Actions
Failure
GitHub Actions
Failure
actions/checkout v5 -> 3d3c42e5 (v7.0.1) and actions/setup-node v4 -> 82076278 (v7.0.0): a mutable tag lets whoever holds it swap the code that runs inside the job holding the secrets; a commit cannot move, and v7 runs on the Node 24 action runtime instead of the deprecated Node 20 one. test/workflow.test.mjs now fails on any unpinned `uses:` or a pin without its version comment. checkout v7 refuses a fork's head under pull_request_target unless allow-unsafe-pr-checkout is set; both jobs already skip fork pull requests at the job level, so the input stays unset and is documented as the second guard. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The pin test accepts subdirectory actions (owner/repo/dir@sha), skips local `./` actions, and takes any `# v…` version comment, not only three-component ones. - Its name and comment say what it covers: the reviewer workflow only. The harness is portable, so it does not assert on the repository's other workflows; and it is a shape check — nothing local verifies that the commit is the tag in the comment. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The pin test also matches the bare `- uses:` step form, which has no `name:` key; an unpinned action written that way was skipped silently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ci: pin the reviewer workflow's actions to Node 24 commit SHAs
GianniCarlo
had a problem deploying
to
reviewer
September 12, 2026 16:20 — with
GitHub Actions
Failure
GitHub Actions
Failure
release.yml: check out full history and, after the bundle build, run `sentry-cli releases set-commits --auto` so Sentry's GitHub integration can resolve `Fixes ANDROID-BOOKPLAYER-<id>` commits in the release that actually ships them (the gradle plugin already creates the release entry at build time). Metadata only, so continue-on-error: it must never block a ship. sentry-deploy.yml (new): on push to main, finalize the current release and record a production deploy. main only moves after the store has published the build, so this is the reliable 'published' marker the weekly crash-triage routine reads next to the traffic gate. Idempotent; fails only when the release entry is missing, i.e. main moved without a build behind it.
…UDE.md Two Git rules. Crash-fix commits carry `Fixes ANDROID-BOOKPLAYER-<id>` in the commit body (not only the PR description: merges keep just the subject), never plain-Resolve an issue in the Sentry UI (1.0.0+14 stragglers reopen it within hours), and the REST inRelease call is for backfilling trailer-less fixes only. Merging into main records a Sentry production deploy, which is only a reliable signal because the release PR merges after the store publishes, never before.
GianniCarlo
had a problem deploying
to
reviewer
September 20, 2026 12:30 — with
GitHub Actions
Failure
GitHub Actions
Failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phone
Improvements
– Improved flow to connect Jellyfin and Audiobookshelf servers
– Quick Connect sign-in for Jellyfin
– Single sign-on (SSO) for Audiobookshelf
– Next/Previous and Android Auto follow your library sort
Bugfixes
– Out of storage now pauses playback instead of crashing
– Fixed crashes with huge cover art and deleted books
– Volume Boost no longer freezes the app
– Media-server imports keep the real file type
If you experience any issues, please reach us at support@bookplayer.app
Wear
Improvements
– Your library on the watch follows the sort order you chose on the phone
Bugfixes
– Running out of storage pauses playback instead of crashing
– Fixed rare crashes when starting playback or after deleting a book
If you experience any issues, please reach us at support@bookplayer.app