feat: update relink container to Scout 2.1.0 - #15
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRelink container is upgraded to version 1.1.0 with Scout 2.1.0 and xi-mzidentml-converter. A new Dockerfile packages xiSEARCH 1.8.11, xiFDR 2.3.10, Scout 2.1.0, .NET 9.0, OpenJDK 21, and Python deps. CLI wrappers ( ChangesRelink 1.1.0
Sequence DiagramsequenceDiagram
participant User
participant Container
participant ScoutRunner
participant JavaRuntime
User->>Container: run `scout` / `xisearch` / `xifdr` (with args, --java-options)
Container->>ScoutRunner: exec /opt/scout/run_scout.sh (scout)
Container->>JavaRuntime: exec java -jar /opt/xisearch/xiSEARCH.jar (xisearch)
Container->>JavaRuntime: exec java -jar /opt/xisearch/xiFDR.jar (xifdr)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relink-1.1.0/Dockerfile (1)
76-80: Pinxi-mzidentml-converterandpyopenmsto match tested versions.The Dockerfile includes runtime verification checks (e.g.,
process_datasetCLI availability), which confirms the packages install and work. However, bothxi-mzidentml-converterandpyopenmsremain unpinned while other nearby dependencies (polars,matplotlib,seaborn,numpy) are pinned. This inconsistency means future rebuilds ofrelink:1.1.0may resolve different package versions, undermining reproducibility for a versioned release tag.♻️ Proposed pins
RUN pip install --no-cache-dir \ - pyopenms \ - xi-mzidentml-converter \ + pyopenms==<tested-version> \ + xi-mzidentml-converter==<tested-version> \ polars==1.35.1 \(The version printed during the current build verification run can be used as the tested pin.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relink-1.1.0/Dockerfile` around lines 76 - 80, Pin the Python packages xi-mzidentml-converter and pyopenms in the Dockerfile RUN pip install line to the tested versions to ensure reproducible builds; update the pip install invocation that currently lists "pyopenms" and "xi-mzidentml-converter" (the same tokens used in the diff) to include the specific version strings used during verification so the image always installs those exact releases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relink-1.1.0/Dockerfile`:
- Around line 113-116: The current validation in the Dockerfile uses echo "$out"
| grep -qvE ... which succeeds if any line does not match the fatal-pattern, so
mixed fatal+help output can incorrectly mark Scout as installed; change the
logic to explicitly fail when any fatal pattern appears in the output of
run_scout.sh (out variable) by checking with grep -qE "It was not
possible...|Could not execute...|A fatal error..." and exiting/failing the build
if that check returns true, otherwise proceed to print "Scout installed" —
locate the block using the out="$(timeout 5s /opt/scout/run_scout.sh --help 2>&1
|| true)"; echo "$out" | head -n 20; echo "$out" | grep... sequence and replace
the grep -qvE inversion with a direct grep -qE fatal-pattern check (or an
if/then) so any fatal output causes a non-zero exit.
---
Nitpick comments:
In `@relink-1.1.0/Dockerfile`:
- Around line 76-80: Pin the Python packages xi-mzidentml-converter and pyopenms
in the Dockerfile RUN pip install line to the tested versions to ensure
reproducible builds; update the pip install invocation that currently lists
"pyopenms" and "xi-mzidentml-converter" (the same tokens used in the diff) to
include the specific version strings used during verification so the image
always installs those exact releases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25465557-5059-40fd-a6d5-f21596739b36
📒 Files selected for processing (3)
.github/workflows/quantms-containers.ymlREADME.mdrelink-1.1.0/Dockerfile
…r-1-1-0 # Conflicts: # .github/workflows/quantms-containers.yml
Add a `relink` dispatcher exposing `scout`, `xisearch`, `xifdr`, and
`convert` subcommands on PATH (matches the `diann` UX) so users no
longer invoke tool-specific paths directly.
Fixes two Scout 2.1.0 startup bugs uncovered by Josh Beale:
1. The previous Dockerfile overwrote upstream's `run_scout.sh` with a
3-line stub that dropped the Python interpreter argument Scout's
`.dll` expects as args[0]. As a result `-search` was being passed
as the Python path and Scout fork-exec'd it as a binary, crashing
with `Win32Exception (2): trying to start process '-search'`. Keep
the upstream wrapper (which auto-detects Python and configures
MPFR/GMP) and only patch its `dotnet Scout_Unix.dll` line to use
an absolute dll path so it can run from any cwd.
2. CSMSL's `Protease` static initializer writes a defaults file to
`$XDG_CONFIG_HOME/CSMSL\Proteases.xml` and throws if the parent
dir is missing, aborting Scout at "Preparing database...".
Pre-create `/root/.config` in the image and have the dispatcher
`mkdir -p "${HOME:-/root}/.config"` at runtime so non-root /
Singularity HOME remappings also work.
Verified end-to-end against Scout's params templates with a tiny
FASTA/MGF: Scout completes the full search and writes `.buf` output.
Updated the Dockerfile verify step accordingly. README updated to
document the new `relink <subcommand>` interface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 79-85: The fenced code block containing the relink help lines (the
block that starts with the three backticks and includes the lines "relink scout
[args...] Scout (cleavable XL-MS search)" through "relink help
Show usage") is missing a language tag; update the opening fence from ``` to
```text so the block is explicitly marked as plain text (this resolves the MD040
lint warning).
In `@relink-1.1.0/relink`:
- Around line 30-31: The script currently swallows errors creating the config
dir by using "mkdir -p \"${HOME:-/root}/.config\" 2>/dev/null || true"; remove
the "|| true" and the stderr redirection so failures are visible, and add an
explicit check of mkdir's exit status (or fail-fast) before calling exec
/opt/scout/run_scout.sh "$@" — i.e., ensure the mkdir -p invocation returns
non-zero on error and, if it fails, print a clear error to stderr and exit with
a non-zero code rather than proceeding to exec run_scout.sh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54ad9894-2163-4b8f-aa3a-23e4c9a3b016
📒 Files selected for processing (4)
.github/workflows/quantms-containers.ymlREADME.mdrelink-1.1.0/Dockerfilerelink-1.1.0/relink
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/quantms-containers.yml
- relink-1.1.0/Dockerfile
…tions Replace the unified `relink <subcommand>` dispatcher with per-tool wrappers on PATH (matches the DIA-NN container's convention): scout Scout (XL-MS search) xisearch xiSEARCH xifdr xiFDR process_dataset xi-mzidentml-converter (already on PATH from pip) Invocation drops the `relink` prefix: docker run ghcr.io/bigbio/relink:1.1.0 scout -search ... docker run ghcr.io/bigbio/relink:1.1.0 xisearch --config=... ... The Java wrappers (`xisearch`, `xifdr`) accept `--java-options "..."` (GATK convention) for passing JVM flags such as -Xmx, -XX:..., -D... inside a single space-separated quoted string. This is robust across Nextflow / Singularity / Docker layered quoting where positional JVM-arg sniffing or `_JAVA_OPTIONS` env vars are awkward: xisearch --java-options "-Xmx16g -XX:+UseG1GC" --config=... The flag may be repeated and accumulates. `scout` retains the CSMSL `$HOME/.config` pre-creation that fixes the TypeInitializationException on first run. README updated with usage examples, a Nextflow recipe showing `task.memory` injection, and a note about the GATK convention. Verify step now exercises each command, both `--help` invocations and `--java-options` parsing on the Java tools. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reproduces Josh Beale's bug: running the relink image via Nextflow's
docker executor (which passes `-u $(id -u):$(id -g)`) crashes Scout at
"Preparing database..." with a CSMSL `Protease` TypeInitializationException
("Unable to prepare the database").
Two stacked failures, both caused by the host UID not having an entry in
/etc/passwd inside the container and HOME staying at the image default
(/root, not writable for that UID):
1. CSMSL writes `$XDG_CONFIG_HOME/CSMSL\Proteases.xml` on first run (where
$XDG_CONFIG_HOME falls back to $HOME/.config on Linux .NET). The
build-time `mkdir -p /root/.config` from the previous fix was owned
by root and didn't help the non-root caller. Patch the in-container
`run_scout.sh` to export `XDG_CONFIG_HOME=/tmp/csmsl-config` (always
writable, regardless of UID/HOME) before invoking dotnet. The
`${XDG_CONFIG_HOME:-/tmp/csmsl-config}` form means callers can still
override.
2. Scout's release archive ships as 0700/0600, so non-root UIDs couldn't
even read `run_scout.sh` (`bash: Permission denied`) — `chmod +x`
only set the exec bit. Apply `chmod -R a+rX /opt/scout` so the
script and dlls are world-readable.
Drop the now-redundant runtime `mkdir -p $HOME/.config` from the per-tool
`scout` wrapper and the build-time `mkdir -p /root/.config` — both were
trying to work around the same problem and neither helps under the
Nextflow invocation pattern.
Extend the in-Dockerfile verify step to re-run scout as UID 12345 with
HOME=/root (unwritable), exactly reproducing the Nextflow scenario, and
fail the build if a TypeInitializationException reappears.
Verified end-to-end: `docker run -u 12345:12345 -e HOME=/root ...
/opt/scout/run_scout.sh` reaches Scout's CLI parser cleanly and exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
93-113: ⚡ Quick winUse
:1.1.0in runnable examples to keep docs reproducible.Line 94 and several Relink usage snippets still pull/run
ghcr.io/bigbio/relink:latest. Since this PR documents a specific release, examples should default to:1.1.0to avoid drift in pipeline behavior.📌 Suggested doc tweak
-docker pull ghcr.io/bigbio/relink:latest +docker pull ghcr.io/bigbio/relink:1.1.0 -docker run --rm -v /path/to/data:/data ghcr.io/bigbio/relink:latest \ +docker run --rm -v /path/to/data:/data ghcr.io/bigbio/relink:1.1.0 \ -docker run -v /path/to/data:/data ghcr.io/bigbio/relink:latest \ +docker run -v /path/to/data:/data ghcr.io/bigbio/relink:1.1.0 \Also applies to: 241-242, 284-301
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 93 - 113, Replace all runnable examples that reference the image tag "ghcr.io/bigbio/relink:latest" with the fixed release tag "ghcr.io/bigbio/relink:1.1.0"; update the explicit docker pull line ("docker pull ghcr.io/bigbio/relink:latest") and every docker run invocation that uses "... ghcr.io/bigbio/relink:latest" (e.g., the scout, xisearch, xifdr, and process_dataset examples) so the examples remain reproducible by default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relink-1.1.0/Dockerfile`:
- Around line 61-67: Remove the insecure APT flags used in the apt-get
invocation (--allow-insecure-repositories and --allow-unauthenticated) and
instead ensure package signatures are trusted before installing
dotnet-runtime-9.0; update the Dockerfile's apt setup steps that run apt-get
update/install and replace the insecure approach by importing and trusting the
Microsoft package GPG key(s) (or switching to an official, signed
repository/mirror) and then run apt-get update && apt-get install -y
--no-install-recommends dotnet-runtime-9.0 libmpfr6 libgmp10 openjdk-21-jre,
finally cleaning /var/lib/apt/lists/* as before.
- Around line 27-33: The Dockerfile RUN block that downloads and unzips xiSEARCH
must verify SHA256 before extraction: add build ARGs (e.g., XISEARCH_SHA256) for
the expected checksum, download the .zip to /tmp/xiSEARCH.zip, compute and
compare sha256sum against XISEARCH_SHA256 and fail the build if it mismatches,
then proceed to unzip and copy xiSEARCH.jar and xiFDR.jar; apply the same
pattern for the Scout download block (use a SCOUT_SHA256 ARG and verify the
binary before extraction/installation).
---
Nitpick comments:
In `@README.md`:
- Around line 93-113: Replace all runnable examples that reference the image tag
"ghcr.io/bigbio/relink:latest" with the fixed release tag
"ghcr.io/bigbio/relink:1.1.0"; update the explicit docker pull line ("docker
pull ghcr.io/bigbio/relink:latest") and every docker run invocation that uses
"... ghcr.io/bigbio/relink:latest" (e.g., the scout, xisearch, xifdr, and
process_dataset examples) so the examples remain reproducible by default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae15ab52-9273-44bb-ad41-bfa8cac567e0
📒 Files selected for processing (5)
README.mdrelink-1.1.0/Dockerfilerelink-1.1.0/scoutrelink-1.1.0/xifdrrelink-1.1.0/xisearch
✅ Files skipped from review due to trivial changes (1)
- relink-1.1.0/scout
The previous fix (commit 499ccaf) exported XDG_CONFIG_HOME=/tmp/csmsl-config unconditionally so CSMSL's Protease static initializer would have a writable parent for its `CSMSL\Proteases.xml` config. But the in-Dockerfile verify runs Scout once as root before the non-root probe, and run_scout.sh's `mkdir -p "$XDG_CONFIG_HOME"` creates /tmp/csmsl-config owned by root with mode 0755. The subsequent runtime mkdir as a non-root UID is a no-op (dir exists) and openat() on the literal "CSMSL\Proteases.xml" filename then fails with EACCES — the same TypeInitializationException Josh Beale saw, just deferred until -search actually triggers Protease..ctor. Two fixes: 1. UID-namespace the path: XDG_CONFIG_HOME=/tmp/csmsl-config-$(id -u). The runtime mkdir now always creates a fresh dir owned by the caller, so non-root UIDs (Nextflow's `docker run -u $(id -u):$(id -g)`) get a writable home for CSMSL's defaults. 2. Verify step now runs `scout -search -no_filter` with a 1-protein stub FASTA under UID 12345. The previous verify ran scout with no args, which exits before touching CSMSL — the bug class went undetected. The new check requires both "Preparing database" and "lines parsed in the fasta DB" markers, and the Protease-regression grep is narrowed to the exact static-init signature so unrelated downstream failures (e.g. empty FASTA regex null) can't false-positive it. Verified locally with the exact pipeline command pattern from bigbio/relink modules/local/scout_search/main.nf: docker run -u 12345:12345 -e HOME=/root ... \ /opt/scout/run_scout.sh -search -no_filter \ search_params.json filter_params.json Scout now reaches and completes Protease/digestor setup ("2 lines parsed" → "885 lines parsed" after decoys+contaminants) instead of crashing in CSMSL.Proteomics.Protease..ctor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relink-1.1.0/Dockerfile (1)
49-122: ⚖️ Poor tradeoffConsider adding a non-root
USERto the runtime stage.The runtime image never sets
USER, so containers default to running as root, which Trivy flags (DS-0002). The verification block at lines 144–165 already proves Scout works under a non-root UID withXDG_CONFIG_HOME=/tmp/csmsl-config-$(id -u), so a default non-root user is feasible. Note Nextflow typically overrides with-u $(id -u):$(id -g)anyway, so this is primarily defense-in-depth for directdocker runusers; please apply only if it does not regress your callers.👤 Sketch of the change
WORKDIR /data/ + +# Run as non-root by default (callers like Nextflow can still override with -u). +RUN useradd -m -u 1000 relink && chown -R relink:relink /data +USER relink🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relink-1.1.0/Dockerfile` around lines 49 - 122, Add a non-root runtime user and make runtime-owned paths writable: create a user (e.g., relink or app) with a stable UID (1000), mkdir and chown /opt/scout /usr/local/bin (wrappers) and /data (and any XDG_CONFIG_HOME dir), and set USER to that non-root user in the runtime stage; ensure ENV XDG_CONFIG_HOME points at a writable path (e.g., /tmp/csmsl-config-1000) or create that directory and chown it so Scout's static initializer can run as the non-root UID; update ownership references for the copied files (/opt/scout, /opt/xisearch, /usr/local/bin/scout|xisearch|xifdr) so they are readable/executable by the new user and the WORKDIR /data remains writable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relink-1.1.0/Dockerfile`:
- Around line 41-43: The current Dockerfile command chain assigns scout_dir via
the find pipeline and then blindly runs cp -a and rm -rf; guard against an empty
or non-directory scout_dir to avoid copying the build-stage rootfs. Update the
block that sets scout_dir (the find ... | head -n 1 assignment) to validate that
scout_dir is non-empty and is a directory before running cp -a "${scout_dir}/."
/opt/scout/ and rm -rf "${scout_dir}" /opt/scout/__MACOSX; if the check fails,
emit a clear error and exit nonzero so the build fails fast instead of producing
a corrupted image. Ensure you reference the scout_dir variable and the cp/rm
steps (the lines using cp -a "${scout_dir}/." and rm -rf "${scout_dir}"
/opt/scout/__MACOSX) when adding the conditional guard.
---
Nitpick comments:
In `@relink-1.1.0/Dockerfile`:
- Around line 49-122: Add a non-root runtime user and make runtime-owned paths
writable: create a user (e.g., relink or app) with a stable UID (1000), mkdir
and chown /opt/scout /usr/local/bin (wrappers) and /data (and any
XDG_CONFIG_HOME dir), and set USER to that non-root user in the runtime stage;
ensure ENV XDG_CONFIG_HOME points at a writable path (e.g.,
/tmp/csmsl-config-1000) or create that directory and chown it so Scout's static
initializer can run as the non-root UID; update ownership references for the
copied files (/opt/scout, /opt/xisearch, /usr/local/bin/scout|xisearch|xifdr) so
they are readable/executable by the new user and the WORKDIR /data remains
writable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| scout_dir="$(find /opt/scout -mindepth 1 -maxdepth 1 -type d ! -name '__MACOSX' | head -n 1)" && \ | ||
| cp -a "${scout_dir}/." /opt/scout/ && \ | ||
| rm -rf "${scout_dir}" /opt/scout/__MACOSX && \ |
There was a problem hiding this comment.
Guard against an empty scout_dir — risks cp -a "/." /opt/scout/.
If the upstream archive ever extracts flat (no top-level directory) or only contains __MACOSX, find ... | head -n 1 yields an empty string. scout_dir="" is a successful assignment, so the && chain proceeds and you execute cp -a "/." /opt/scout/, which copies the entire build-stage rootfs into /opt/scout. The subsequent rm -rf "${scout_dir}" ... expands to rm -rf "" /opt/scout/__MACOSX and the test -s /opt/scout/run_scout.sh on line 44 may still succeed (it was extracted flat), so the build silently produces a bloated/corrupted image instead of failing.
🛡️ Proposed guard
- scout_dir="$(find /opt/scout -mindepth 1 -maxdepth 1 -type d ! -name '__MACOSX' | head -n 1)" && \
- cp -a "${scout_dir}/." /opt/scout/ && \
- rm -rf "${scout_dir}" /opt/scout/__MACOSX && \
+ scout_dir="$(find /opt/scout -mindepth 1 -maxdepth 1 -type d ! -name '__MACOSX' | head -n 1)" && \
+ if [ -n "${scout_dir}" ] && [ -d "${scout_dir}" ]; then \
+ cp -a "${scout_dir}/." /opt/scout/ && \
+ rm -rf "${scout_dir}"; \
+ fi && \
+ rm -rf /opt/scout/__MACOSX && \
test -s /opt/scout/run_scout.sh && \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@relink-1.1.0/Dockerfile` around lines 41 - 43, The current Dockerfile command
chain assigns scout_dir via the find pipeline and then blindly runs cp -a and rm
-rf; guard against an empty or non-directory scout_dir to avoid copying the
build-stage rootfs. Update the block that sets scout_dir (the find ... | head -n
1 assignment) to validate that scout_dir is non-empty and is a directory before
running cp -a "${scout_dir}/." /opt/scout/ and rm -rf "${scout_dir}"
/opt/scout/__MACOSX; if the check fails, emit a clear error and exit nonzero so
the build fails fast instead of producing a corrupted image. Ensure you
reference the scout_dir variable and the cp/rm steps (the lines using cp -a
"${scout_dir}/." and rm -rf "${scout_dir}" /opt/scout/__MACOSX) when adding the
conditional guard.
- New diann-2.5.1/Dockerfile (mirrors 2.5.0; pulls the public DIA-NN-2.5.1-Academia-Linux.zip from the 2.0 release tag). - CI: diann_2_5_1 paths filter, CHG_251 env, DIANN_ALL entry building ghcr.io/bigbio/diann:2.5.1 and moving the 'latest' tag from 2.5.0 to 2.5.1, and the matching jq selector. relink stays at 1.1.0; enterprise has no CI entry (local-only build).
…r-1-1-0 # Conflicts: # .github/workflows/quantms-containers.yml # diann-2.5.1/Dockerfile
Summary
Notes
Closes #8
Summary by CodeRabbit
New Features
Documentation
Chores