Skip to content

fix: chores and reliability fixes (#123-#125, #128, #132, #137-#139)#141

Merged
michaelmcnees merged 2 commits into
mainfrom
fix/chores-reliability
May 30, 2026
Merged

fix: chores and reliability fixes (#123-#125, #128, #132, #137-#139)#141
michaelmcnees merged 2 commits into
mainfrom
fix/chores-reliability

Conversation

@michaelmcnees

@michaelmcnees michaelmcnees commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test plan

  • go build ./... and go vet ./... pass clean
  • secret.Store tests pass with audit event coverage
  • TestEmailReceive_FetchesUnseenMessages no longer flakes on SMTP EOF
  • Helm chart renders with runAsNonRoot: true + numeric UID without runAsUser admission error
  • /readyz returns 200 while worker/reaper are between poll cycles

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added audit event tracking for credential creation and deletion with configurable actor attribution.
  • Bug Fixes

    • Improved email delivery reliability with automatic retry logic.
  • Documentation

    • Updated release workflow references.
    • Clarified metrics endpoint authentication and Prometheus bearer token configuration.
  • Chores

    • Added a release step to generate a NOTICE file and include it in archives.
    • Pinned container base image digests and switched to numeric user/group IDs.
    • Updated Helm chart security context.

Review Change Stack

- #123: rename release-please.yml → changeset-version.yml; update RELEASING.md
- #124: pin alpine base image to digest in Dockerfile.goreleaser
- #125: add go-licenses NOTICE generation to release pipeline and goreleaser archives
- #128: audit events for secret.Store Create and Delete (Actor field, transactional EmitTx)
- #132: retry smtp.SendMail up to 3× in TestEmailReceive_FetchesUnseenMessages
- #137: numeric UID (10001) in Dockerfile and Helm podSecurityContext so runAsNonRoot resolves
- #138: remove worker/reaper liveness checkers from /readyz; probe checks DB only
- #139: revert unauthenticated /metrics; add comment explaining bearer-token Prometheus path;
        annotate serviceMonitor.bearerTokenSecret in values.yaml

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3bf0958d-6b35-4b14-ab87-eb058d2c50bc

📥 Commits

Reviewing files that changed from the base of the PR and between 17dc0f2 and 1b31d84.

📒 Files selected for processing (3)
  • .github/workflows/release-engine.yml
  • packages/engine/Dockerfile.goreleaser
  • packages/engine/internal/server/server.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/engine/Dockerfile.goreleaser
  • .github/workflows/release-engine.yml
  • packages/engine/internal/server/server.go

📝 Walkthrough

Walkthrough

Adds NOTICE generation to the release workflow and includes NOTICE in goreleaser archives; pins container base and uses numeric UID/GID; documents Prometheus bearer token usage; adds Store.Actor and transactional audit events for credential ops; simplifies /readyz to DB-only; retries SMTP in an email integration test.

Changes

Release, Deployment, and Audit Infrastructure

Layer / File(s) Summary
Release Pipeline and License Reporting
.github/workflows/release-engine.yml, packages/engine/.goreleaser.yaml, RELEASING.md
Adds a workflow step that runs go-licenses report ./... in packages/engine to generate NOTICE; includes NOTICE in goreleaser archives; updates RELEASING.md to reference changeset-version.yml.
Container and Deployment Security Hardening
packages/engine/Dockerfile.goreleaser, packages/helm-chart/values.yaml, packages/engine/internal/auth/middleware.go
Pins Alpine base image by digest, switches runtime user to 10001:10001, sets Helm podSecurityContext.runAsUser/runAsGroup to 10001, and clarifies Prometheus bearer token config for /metrics.
Credential Audit Trail with Actor Attribution
packages/engine/internal/secret/store.go
Adds Actor string on Store with a default helper; Create and Delete emit audit.Event (CredentialCreated/CredentialDeleted) inside DB transactions and return errors on audit failures.
Server Readiness Check Simplification
packages/engine/internal/server/server.go
Removes worker/reaper liveness adapter types and threshold logic; registers /readyz via health.ReadyzHandler(s.DB) relying only on DB connectivity.
Email Delivery Test Resilience
packages/engine/internal/connector/email_receive_test.go
Replaces a single smtp.SendMail call with a 3-attempt retry loop with 500ms delay and logged failures to improve test reliability.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Possibly related issues


Poem

🐰 I hopped through CI and build-time trees,

NOTICE gathered from dependency breeze.
Audit stamps on secrets now show,
Containers pinned where safe winds blow.
Retries keep mail from falling through. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title uses vague, generic phrasing ('chores and reliability fixes') with a list of issue numbers that doesn't convey the meaningful substance of the changes. Rewrite the title to highlight the primary change—e.g., 'Add audit events to credential store and improve release pipeline reliability' or focus on the most impactful change from a user perspective.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 fix/chores-reliability

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.

❤️ Share

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17dc0f2b31

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/engine/internal/server/server.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/release-engine.yml (1)

47-51: ⚡ Quick win

Pin go-licenses in CI for reproducible builds.

@latest is non-deterministic; the latest stable release is v2.0.1, so pin to that version.

📌 Proposed fix to pin go-licenses version
      - name: Generate NOTICE
        run: |
-         go install github.com/google/go-licenses@latest
+         go install github.com/google/go-licenses@v2.0.1
          go-licenses report ./... > NOTICE
        working-directory: packages/engine
🤖 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/release-engine.yml around lines 47 - 51, The CI step
"Generate NOTICE" installs go-licenses with an unstable `@latest` tag; update the
step to pin the tool to v2.0.1 by changing the go install invocation for
github.com/google/go-licenses to use `@v2.0.1` (the "Generate NOTICE" job/run that
executes "go install github.com/google/go-licenses@latest" and then "go-licenses
report ./... > NOTICE" in the packages/engine working-directory).
🤖 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 `@packages/engine/Dockerfile.goreleaser`:
- Line 1: The pinned Alpine image digest in the Dockerfile (the FROM directive
in packages/engine/Dockerfile.goreleaser) is incorrect for alpine:3.21; update
the FROM line to either use the correct Docker Hub digest for alpine:3.21
(sha256:1f390db9d9746cc317e16a98a8be2854c599abcbf04d45b6320b040e72034e33), or
remove the digest and/or explicitly add a platform (e.g.,
--platform=linux/amd64) to ensure you pull the intended manifest; adjust the
FROM directive accordingly so the image digest matches the intended
architecture/manifest.

In `@packages/engine/internal/server/server.go`:
- Line 241: The inline comment "Health endpoints (registered after engine
components so checkers are populated)." is stale because /readyz no longer uses
liveness checkers; update the comment near the health endpoints registration to
remove the "so checkers are populated" rationale and make it accurate (e.g.,
"Health endpoints (registered after engine components)." or similar) and ensure
any mention of /readyz and checkers is corrected to reflect the current
behavior.

---

Nitpick comments:
In @.github/workflows/release-engine.yml:
- Around line 47-51: The CI step "Generate NOTICE" installs go-licenses with an
unstable `@latest` tag; update the step to pin the tool to v2.0.1 by changing the
go install invocation for github.com/google/go-licenses to use `@v2.0.1` (the
"Generate NOTICE" job/run that executes "go install
github.com/google/go-licenses@latest" and then "go-licenses report ./... >
NOTICE" in the packages/engine working-directory).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8264f82f-8ca8-4022-8545-b01404db6282

📥 Commits

Reviewing files that changed from the base of the PR and between 1bd8cbf and 17dc0f2.

📒 Files selected for processing (10)
  • .github/workflows/changeset-version.yml
  • .github/workflows/release-engine.yml
  • RELEASING.md
  • packages/engine/.goreleaser.yaml
  • packages/engine/Dockerfile.goreleaser
  • packages/engine/internal/auth/middleware.go
  • packages/engine/internal/connector/email_receive_test.go
  • packages/engine/internal/secret/store.go
  • packages/engine/internal/server/server.go
  • packages/helm-chart/values.yaml

Comment thread packages/engine/Dockerfile.goreleaser Outdated
Comment thread packages/engine/internal/server/server.go Outdated
- Pin alpine:3.21 to correct digest (1f390db) in Dockerfile.goreleaser
- Pin go-licenses to @v2.0.1 instead of @latest in release-engine.yml
- Update stale comment in server.go: /readyz no longer uses worker/reaper
  liveness checkers; explain the intentional design decision

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@michaelmcnees michaelmcnees merged commit 2139b4f into main May 30, 2026
30 checks passed
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.

1 participant