Skip to content

Slack credential validation, dead config cleanup, Docker digest pinning#42

Merged
wpak-ai merged 6 commits into
developfrom
bugfix/slack-credential-docker-pin
May 20, 2026
Merged

Slack credential validation, dead config cleanup, Docker digest pinning#42
wpak-ai merged 6 commits into
developfrom
bugfix/slack-credential-docker-pin

Conversation

@henry0816191
Copy link
Copy Markdown
Collaborator

@henry0816191 henry0816191 commented May 19, 2026

Summary

Fail fast on missing Slack credentials at settings load, remove dead configuration, and harden Docker builds for reproducible supply chain.

### Issue 35 — Settings / dead config

  • Enforce non-empty SLACK_BOT_TOKEN and SLACK_SIGNING_SECRET via @model_validator on Settings (raises ConfigurationError at import unless _PAPERSCOUT_TESTING=1)
  • Remove duplicate Slack guard from __main__.py (validation happens when settings is constructed)
  • Remove unused enable_bulk_openstd from Settings, .env.example, tests, and docs
  • Remove stale unused paperscout.db.pool module global
  • Reorganize .env.example with required vs optional credential comments

Issue 36 — Docker reproducibility

  • Pin python:3.12-slim to digest in builder and runtime stages
  • Keep multi-stage build: uv sync --frozen from uv.lock (hash-verified deps; no floating pip install .)
  • Add test / production Dockerfile targets; compose builds production only
  • Add CI docker job: build --target test and run pytest with coverage in-container
  • Document image rebuild workflow in CONTRIBUTING.md and deploy/SERVER_SETUP.md

Design notes

  • Slack-less runs: rejected for production; pytest/CI use _PAPERSCOUT_TESTING=1 to allow empty tokens (existing tests/test_config.py coverage).
  • Hash pinning: uses uv.lock + uv sync --frozen rather than requirements.txt + pip --require-hashes (aligned with issue 03 lockfile work).

Test plan

  • uv run pytest tests/ -q --cov=paperscout --cov-fail-under=90
  • uv run pre-commit run --all-files
  • uv lock --check
  • docker build --target test -t paperscout:test . and in-container pytest (CI docker job)

Links

Summary by CodeRabbit

  • New Features

    • Added OPS_ALERT_CHANNEL (empty channel disables posting). Added ENABLE_ISO_PROBE and confirmed ENABLE_BULK_WG21 toggle.
  • Chores

    • Removed ENABLE_BULK_OPENSTD from templates/docs. Pinned Python base image, added containerized test stage, updated CI to run tests in Docker. Updated compose/build config and dockerignore to keep tests for CI.
  • Documentation

    • Added image rebuild and deployment rebuild guidance.
  • Behavior

    • Startup now exits with error if database URL is unset.

Review Change Stack

@henry0816191 henry0816191 self-assigned this May 19, 2026
@henry0816191 henry0816191 requested a review from wpak-ai as a code owner May 19, 2026 15:10
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ad737185-59a2-468a-92b4-1dae85bacafb

📥 Commits

Reviewing files that changed from the base of the PR and between 78cf1f3 and 8c82d73.

📒 Files selected for processing (14)
  • .dockerignore
  • .env.example
  • .github/workflows/ci.yml
  • CONTRIBUTING.md
  • Dockerfile
  • README.md
  • deploy/SERVER_SETUP.md
  • docker-compose.yml
  • docs/handoff.md
  • docs/onboarding.md
  • src/paperscout/__main__.py
  • src/paperscout/config.py
  • src/paperscout/db.py
  • tests/conftest.py
💤 Files with no reviewable changes (6)
  • README.md
  • src/paperscout/db.py
  • src/paperscout/config.py
  • docs/onboarding.md
  • tests/conftest.py
  • src/paperscout/main.py

📝 Walkthrough

Walkthrough

Removes unused enable_bulk_openstd, adds an early DATABASE_URL startup guard, removes module-level DB pool, pins Docker base images and adds test-oriented build stages, adds a CI job that runs pytest inside the test image, and updates env templates and docs accordingly.

Changes

Settings validation, Docker reproducibility, and documentation cleanup

Layer / File(s) Summary
Settings field refactor and startup validation
src/paperscout/config.py, tests/conftest.py, src/paperscout/__main__.py, src/paperscout/db.py
Removes dead enable_bulk_openstd from Settings and test defaults; adds an early DATABASE_URL startup validation that logs an error and exits; removes the module-level DB pool global; adjusts __main__ imports/startup flow.
Environment template and documentation updates
.env.example, README.md, CONTRIBUTING.md, docs/onboarding.md, docs/handoff.md, deploy/SERVER_SETUP.md, .dockerignore, docker-compose.yml
Reframes .env.example sections as required/optional; removes ENABLE_BULK_OPENSTD, adds ENABLE_ISO_PROBE and OPS_ALERT_CHANNEL; updates scheduling docs and onboarding/handoff text; documents frozen uv.lock rebuilds and digest-pinned base images; keeps tests/ in build context with notes; expands compose build block.
Docker image reproducibility and multi-stage builds
Dockerfile
Pins python:3.12-slim bases by digest; adds test-builder stage to install dev deps into shared venv; adds test stage that copies venv and tests and clears ENTRYPOINT; names final production stage explicitly.
CI workflow docker test job
.github/workflows/ci.yml
Pins actions/checkout to specific commit SHAs and sets persist-credentials:false across jobs; adds a docker job that builds the test target image and runs pytest inside the container with coverage enforcement.

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant DockerEngine
  participant TestContainer
  participant Pytest
  GitHubActions->>DockerEngine: build image (target=test) -> paperscout:ci
  DockerEngine->>TestContainer: run container with env vars, COVERAGE_FILE=/tmp/.coverage
  TestContainer->>Pytest: execute pytest tests/ -q --cov=paperscout --cov-fail-under=90
  Pytest->>TestContainer: write /tmp/.coverage and exit status
  TestContainer->>GitHubActions: return exit status and coverage result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • cppalliance/paperscout#21: Related __main__.py startup-sequence changes touching startup behavior and Slack status enqueuing.

Suggested labels

bug

Suggested reviewers

  • wpak-ai

Poem

🐰 I nibble locks and pin the base,

tests tucked safe in build-time space,
startup sniffed for DB's key,
docs refreshed for builders to see,
the rabbit hops — CI sings with grace.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the three main changes: Slack credential validation, dead config cleanup, and Docker digest pinning.
Description check ✅ Passed Description includes required sections: detailed Summary organized by issues, complete Test plan with checkmarks, and Related issues links.
Linked Issues check ✅ Passed All requirements from Issue #35 and #36 are met: Slack validator added, dead config removed, Docker digest pinning implemented, test job added, and rebuild docs created.
Out of Scope Changes check ✅ Passed All changes are directly aligned with Issues #35 and #36; no unrelated modifications to unrelated code areas detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/slack-credential-docker-pin

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@Dockerfile`:
- Line 20: The Dockerfile's COPY instruction "COPY tests/ tests/" fails because
tests/ is excluded from the build context; update the build context so Docker
can see tests/ before that COPY runs by removing or negating tests/ from
.dockerignore (or otherwise including tests/ in the build context) so the "COPY
tests/ tests/" line in the Dockerfile succeeds; ensure .dockerignore contains an
explicit exception like "!tests/" or simply remove the tests/ exclusion so the
tests directory is available during docker build.
- Around line 48-55: The test stage inherits runtime's ENTRYPOINT
(["python","-m","paperscout"]) so CI args become arguments to that entrypoint
instead of running pytest; in the Dockerfile test stage (the stage named "test")
clear or override the ENTRYPOINT (e.g. add ENTRYPOINT [] or set ENTRYPOINT to
["python","-m","pytest"]) after the USER paperscout line so running `docker run
... python -m pytest` executes pytest directly rather than being passed to the
paperscout entrypoint.
🪄 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: 10b261e0-f3db-4982-9cb3-c1adbfee79c4

📥 Commits

Reviewing files that changed from the base of the PR and between 78cf1f3 and cd74635.

📒 Files selected for processing (13)
  • .env.example
  • .github/workflows/ci.yml
  • CONTRIBUTING.md
  • Dockerfile
  • README.md
  • deploy/SERVER_SETUP.md
  • docker-compose.yml
  • docs/handoff.md
  • docs/onboarding.md
  • src/paperscout/__main__.py
  • src/paperscout/config.py
  • src/paperscout/db.py
  • tests/conftest.py
💤 Files with no reviewable changes (6)
  • src/paperscout/db.py
  • tests/conftest.py
  • docs/onboarding.md
  • src/paperscout/main.py
  • src/paperscout/config.py
  • README.md

Comment thread Dockerfile
Comment thread Dockerfile
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

125-133: ⚡ Quick win

Consider adding coverage report output for easier debugging.

The test command uses -q and lacks a --cov-report flag. If coverage falls below the 90% threshold, developers won't see which modules or lines are uncovered. Adding --cov-report=term would provide actionable feedback when the job fails.

📊 Suggested enhancement
       - name: Run tests in container
         run: |
           docker run --rm --entrypoint python \
             -e _PAPERSCOUT_TESTING=1 \
             -e SLACK_BOT_TOKEN=xoxb-ci-placeholder \
             -e SLACK_SIGNING_SECRET=ci-placeholder-secret \
             -e COVERAGE_FILE=/tmp/.coverage \
             paperscout:ci \
-            -m pytest tests/ -q --cov=paperscout --cov-fail-under=90
+            -m pytest tests/ -q --cov=paperscout --cov-report=term --cov-fail-under=90
🤖 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 @.github/workflows/ci.yml around lines 125 - 133, The CI test step running
the containerized pytest invocation uses -m pytest tests/ -q --cov=paperscout
--cov-fail-under=90 but omits a coverage report, so update the docker run
command invocation to include an explicit coverage report flag (for example add
--cov-report=term or --cov-report=term-missing) to the pytest args so failing
coverage shows which modules/lines are missing; locate the docker run block that
sets environment vars and the pytest command and append the chosen --cov-report
option to the pytest flags.
🤖 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.

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 125-133: The CI test step running the containerized pytest
invocation uses -m pytest tests/ -q --cov=paperscout --cov-fail-under=90 but
omits a coverage report, so update the docker run command invocation to include
an explicit coverage report flag (for example add --cov-report=term or
--cov-report=term-missing) to the pytest args so failing coverage shows which
modules/lines are missing; locate the docker run block that sets environment
vars and the pytest command and append the chosen --cov-report option to the
pytest flags.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 093e0078-65fb-4842-b0dd-62deed0eacb1

📥 Commits

Reviewing files that changed from the base of the PR and between 71b9da5 and d35f6ba.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • CONTRIBUTING.md
  • Dockerfile
✅ Files skipped from review due to trivial changes (1)
  • CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • Dockerfile

@henry0816191
Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
Dockerfile (1)

9-9: ⚡ Quick win

Pin the uv tool image by digest as well.

Line 9 still pulls ghcr.io/astral-sh/uv:0.8.3 by tag, which weakens reproducibility compared with the digest-pinned Python base images.

Suggested change
-COPY --from=ghcr.io/astral-sh/uv:0.8.3 /uv /usr/local/bin/uv
+COPY --from=ghcr.io/astral-sh/uv:0.8.3@sha256:<resolved-digest> /uv /usr/local/bin/uv
🤖 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 `@Dockerfile` at line 9, Update the COPY source to use the uv image pinned by
digest instead of the tag to ensure reproducible builds: replace the current
COPY --from=ghcr.io/astral-sh/uv:0.8.3 /uv /usr/local/bin/uv reference with a
digest-pinned reference (e.g., ghcr.io/astral-sh/uv@sha256:<digest>) or point to
a named build stage created from that digest; modify the Dockerfile COPY
invocation (the COPY --from=... /uv /usr/local/bin/uv line) accordingly and
ensure the chosen sha256 digest corresponds to the 0.8.3 release you expect.
🤖 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 `@CONTRIBUTING.md`:
- Around line 61-62: The Docker command "docker build -t paperscout:test ." is
misleading because it tags a full production build as :test; update the
CONTRIBUTING.md entry so the command either explicitly targets the test stage
(add --target test) like "docker build --target test -t paperscout:test ." or
rename the tag to reflect a production build (e.g., "docker build -t
paperscout:latest ."); pick one approach and make the corresponding replacement
for the line containing the "docker build -t paperscout:test ." command.

---

Nitpick comments:
In `@Dockerfile`:
- Line 9: Update the COPY source to use the uv image pinned by digest instead of
the tag to ensure reproducible builds: replace the current COPY
--from=ghcr.io/astral-sh/uv:0.8.3 /uv /usr/local/bin/uv reference with a
digest-pinned reference (e.g., ghcr.io/astral-sh/uv@sha256:<digest>) or point to
a named build stage created from that digest; modify the Dockerfile COPY
invocation (the COPY --from=... /uv /usr/local/bin/uv line) accordingly and
ensure the chosen sha256 digest corresponds to the 0.8.3 release you expect.
🪄 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: 4b1d4cbb-3b30-48cc-8634-6869a8860e3f

📥 Commits

Reviewing files that changed from the base of the PR and between 78cf1f3 and d35f6ba.

📒 Files selected for processing (14)
  • .dockerignore
  • .env.example
  • .github/workflows/ci.yml
  • CONTRIBUTING.md
  • Dockerfile
  • README.md
  • deploy/SERVER_SETUP.md
  • docker-compose.yml
  • docs/handoff.md
  • docs/onboarding.md
  • src/paperscout/__main__.py
  • src/paperscout/config.py
  • src/paperscout/db.py
  • tests/conftest.py
💤 Files with no reviewable changes (6)
  • src/paperscout/db.py
  • src/paperscout/config.py
  • docs/onboarding.md
  • tests/conftest.py
  • README.md
  • src/paperscout/main.py

Comment thread CONTRIBUTING.md Outdated
@henry0816191
Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review again

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

@henry0816191: Triggering another full review of PR #42 now!

✅ Actions performed

Full review triggered.

@wpak-ai
Copy link
Copy Markdown
Contributor

wpak-ai commented May 19, 2026

@coderabbitai resume

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

✅ Actions performed

Reviews resumed.

@wpak-ai
Copy link
Copy Markdown
Contributor

wpak-ai commented May 19, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @.env.example:
- Around line 62-63: Add an explicit empty OPS_ALERT_CHANNEL key to the
.env.example so it is discoverable (follow the same pattern used by
NOTIFICATION_CHANNEL); replace the commented example "# OPS_ALERT_CHANNEL=" with
a bare "OPS_ALERT_CHANNEL=" line in the template to make it clear the key exists
and can be set by operators.

In @.github/workflows/ci.yml:
- Line 120: Replace the unpinned checkout action "uses: actions/checkout@v6"
with a pinned commit SHA for that actions/checkout v6 release and explicitly
disable credential persistence; i.e., change "uses: actions/checkout@v6" to
"uses: actions/checkout@<commit-sha-for-v6>" and add a with block containing
"persist-credentials: false" (and optionally "fetch-depth: 0") so credentials
are not persisted and the action is pinned.
🪄 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: 43c242eb-a067-4194-b188-0fb772efa80b

📥 Commits

Reviewing files that changed from the base of the PR and between 78cf1f3 and d9b5dc1.

📒 Files selected for processing (14)
  • .dockerignore
  • .env.example
  • .github/workflows/ci.yml
  • CONTRIBUTING.md
  • Dockerfile
  • README.md
  • deploy/SERVER_SETUP.md
  • docker-compose.yml
  • docs/handoff.md
  • docs/onboarding.md
  • src/paperscout/__main__.py
  • src/paperscout/config.py
  • src/paperscout/db.py
  • tests/conftest.py
💤 Files with no reviewable changes (6)
  • docs/onboarding.md
  • README.md
  • src/paperscout/db.py
  • tests/conftest.py
  • src/paperscout/main.py
  • src/paperscout/config.py

Comment thread .env.example Outdated
Comment thread .github/workflows/ci.yml Outdated
@henry0816191
Copy link
Copy Markdown
Collaborator Author

@coderabbitai continue to review
don't skip

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

@henry0816191: Understood — triggering a full review of PR #42 now and will not skip any files. The review will cover all changed files including the Dockerfile, CI workflow, config changes, credential validation logic, and documentation updates.

(ミ✿ω✿ミ)

✅ Actions performed

Full review triggered.

@wpak-ai wpak-ai merged commit dc319cd into develop May 20, 2026
10 checks passed
@wpak-ai wpak-ai deleted the bugfix/slack-credential-docker-pin branch May 20, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker Build Uses pip install Without Hash Pinning Settings Allows Partially-Configured Credentials

2 participants