NO-ISSUE: Validate migration files before updating the hash

[jhernand]

Summary

• The update hashes command now validates migration file prefixes and checks for duplicate
  migration numbers before computing the hash, preventing developers from unknowingly working
  with an incorrect hash when their migrations have problems.
• The unit test failure message no longer reveals the expected hash value, directing developers
  to use uv run dev.py update hashes instead of manual updates.

Test plan

• Run uv run dev.py update hashes with valid migrations and verify the hash is updated.
• Introduce a duplicate migration number and verify the command reports the error and exits
  without updating the hash.
• Run ginkgo run internal/database and verify the hash test passes with a correct hash and
  fails with a helpful message when the hash is outdated.

Summary by CodeRabbit

• Bug Fixes
  • Improved migration hash update validation by verifying migration filename formats and detecting duplicate numeric prefixes.
  • Updated error handling so validation failures are reported more clearly and the command exits with a non-zero status when issues are found.
• Tests
  • Simplified the “out-of-date migrations hash” test failure message to direct running the hashes update command.
• Chores
  • Added the humanize library to support clearer, human-readable error output.

[openshift-ci-robot]

@jhernand: This pull request explicitly references no jira issue.

Details

In response to this:

Summary

• The update hashes command now validates migration file prefixes and checks for duplicate
  migration numbers before computing the hash, preventing developers from unknowingly working
  with an incorrect hash when their migrations have problems.
• The unit test failure message no longer reveals the expected hash value, directing developers
  to use uv run dev.py update hashes instead of manual updates.

Test plan

• Run uv run dev.py update hashes with valid migrations and verify the hash is updated.
• Introduce a duplicate migration number and verify the command reports the error and exits
  without updating the hash.
• Run ginkgo run internal/database and verify the hash test passes with a correct hash and
  fails with a helpful message when the hash is outdated.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhernand

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [jhernand]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

The PR validates migration filenames before hash generation, reports duplicate prefixes with human-readable output, updates the migrations hash test message, and adds the humanize dependency.

Changes

Migration hash validation

Layer / File(s)	Summary
Validate migration hashes dev/update.py, pyproject.toml	update hashes now imports sys and humanize, validates sorted *.up.sql migration filenames for numeric prefixes and duplicates, logs validation errors, and exits with status 1 before hashing; humanize==4.15.0 is added as a dependency.
Update hash mismatch message internal/database/database_migrations_test.go	The migrations hash mismatch test now reports the hash as outdated and instructs running uv run dev.py update hashes, without including the expected and stored hash values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• osac-project/fulfillment-service#450: Both PRs focus on migration filename integrity and numbering rules around database migrations.
• osac-project/fulfillment-service#747: Both PRs touch the update hashes workflow and the migrations checksum path.

Suggested reviewers

• larsks
• tzvatot

Poem

Little hashes line up neat,
Prefix checks tap a careful beat.
Humanized lists now name the split,
And tests say “update hashes” fit.

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: validating migration files before hash updates.
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.
No-Hardcoded-Secrets	✅ Passed	No hardcoded secrets or credentialed URLs were found in the changed files; the diff only adds validation, logging, and a dependency.
No-Weak-Crypto	✅ Passed	Only SHA-256 is used, and the touched files contain no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or custom crypto logic.
No-Injection-Vectors	✅ Passed	No unsafe SQL/shell/eval/pickle/yaml/os.system/dangerouslySetInnerHTML usage was introduced; changes are filename validation, logging, and a test message update.
Container-Privileges	✅ Passed	PR only changes dev/update.py, a Go test, and pyproject.toml; no touched manifest adds privileged, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs	✅ Passed	New logs only print migration filenames, counts, and the SHA-256 hash; no passwords, tokens, PII, session IDs, or host/customer data are exposed.
Ai-Attribution	✅ Passed	HEAD commit includes Assisted-by: Cursor plus Signed-off-by; no Co-Authored-By trailer was present.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@dev/update.py`:
- Around line 49-55: The migration filename prefix handling in update.py needs
to reject non-digit prefixes before converting to an integer. In the migration
parsing logic around migration_parts and migration_number, validate
migration_parts[0] with isdigit() (or otherwise catch ValueError) before calling
int(...), so names like foo_add_table.up.sql or signed prefixes like +1_... are
reported as validation errors instead of crashing. Keep the existing
logging/error_count flow and only proceed to the numeric checks when the prefix
is a valid unsigned digit string.

🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 9e3134a0-bdc0-4f53-9623-f6a6029707c7

📥 Commits

Reviewing files that changed from the base of the PR and between 075185e and d9e2921.

⛔ Files ignored due to path filters (1)

• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• dev/update.py
• internal/database/database_migrations_test.go
• pyproject.toml

[coderabbitai]

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 `@dev/update.py`:
- Around line 47-71: The duplicate-migration check in update.py is using the raw
string prefix from migration_file.name, so prefixes like 1 and 01 are treated as
different keys even though the migration parsing logic normalizes them to the
same number. Update the migration_index grouping in the prefix validation loop
to normalize the validated prefix before storing it, and ensure the duplicate
detection in the migration_prefix iteration compares the canonical numeric form
used by the database migration tests.

In `@pyproject.toml`:
- Line 7: The dependency constraint for humanize is too loose and can drift
beyond the locked version; update the manifest entry in pyproject.toml to match
the exact release already captured in uv.lock. Keep the change limited to the
humanize requirement so the project resolves consistently to the locked version.

🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a0206678-595a-44a2-9408-9c308aa18972

📥 Commits

Reviewing files that changed from the base of the PR and between d9e2921 and 0ec713f.

⛔ Files ignored due to path filters (1)

• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• dev/update.py
• internal/database/database_migrations_test.go
• pyproject.toml

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@dev/update.py`:
- Around line 39-73: The migration validation in hashes() only checks *.up.sql,
so it can miss malformed or duplicate .down.sql files and diverge from
internal/database/database_migrations_test.go. Update the validation in
dev/update.py to scan the full migrations set, normalize each file by migration
number plus direction (up/down), and report duplicates or invalid names before
proceeding; then keep the hashing step limited to the sorted .up.sql files only.

🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: b22a644d-a159-444a-95b2-3c31b22ddbee

📥 Commits

Reviewing files that changed from the base of the PR and between 0ec713f and d6113bd.

⛔ Files ignored due to path filters (1)

• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• dev/update.py
• internal/database/database_migrations_test.go
• pyproject.toml