Skip to content

Validate staging dry-run identity envs#827

Merged
ramimbo merged 1 commit into
ramimbo:mainfrom
RLTree:codex/staging-dry-run-identity-validation
Jun 2, 2026
Merged

Validate staging dry-run identity envs#827
ramimbo merged 1 commit into
ramimbo:mainfrom
RLTree:codex/staging-dry-run-identity-validation

Conversation

@RLTree
Copy link
Copy Markdown
Contributor

@RLTree RLTree commented Jun 2, 2026

Summary

  • validate MERGEWORK_DRY_RUN_REPO before the staging dry-run helper builds an issue URL or posts to staging
  • reject blank or malformed repo slugs unless they are in owner/name form
  • validate MERGEWORK_DRY_RUN_CONTRIBUTOR before building the webhook payload
  • add regression tests proving invalid dry-run identity inputs fail before any HTTP request

Bounty #761: #761
Source issue #813: #813
Refs #761

Validation

  • uv run --extra dev pytest tests/test_staging_webhook_dry_run.py -q -> 13 passed
  • uv run --extra dev ruff format --check scripts/staging_webhook_dry_run.py tests/test_staging_webhook_dry_run.py -> 2 files already formatted
  • uv run --extra dev ruff check scripts/staging_webhook_dry_run.py tests/test_staging_webhook_dry_run.py -> All checks passed
  • uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • uv run --extra dev mypy app -> Success: no issues found in 39 source files
  • git diff --check -> clean

Live duplicate/capacity check before PR:

No wallet, payout, treasury execution, bridge, exchange, cash-out, price, secrets, private details, or issue automation changes.

Summary by CodeRabbit

  • Refactor

    • Improved validation of staging dry-run configuration parameters with better error detection before processing requests.
  • Tests

    • Added comprehensive test coverage for dry-run configuration validation and default value handling.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3edaea08-5ee2-402e-97eb-659cce0c873a

📥 Commits

Reviewing files that changed from the base of the PR and between 12ca2f0 and e00d970.

📒 Files selected for processing (2)
  • scripts/staging_webhook_dry_run.py
  • tests/test_staging_webhook_dry_run.py

📝 Walkthrough

Walkthrough

The PR adds environment variable validation to the staging webhook dry-run script. New helper functions extract and validate the dry-run GitHub repository slug and contributor identity, enforcing owner/name format and non-empty values. Main integration delegates these reads to the helpers, and test coverage verifies defaults work correctly and malformed or blank inputs are rejected before HTTP calls.

Changes

Staging webhook dry-run environment validation

Layer / File(s) Summary
Repo slug validation contract and helper functions
scripts/staging_webhook_dry_run.py
Adds re import and compiled regex pattern _REPO_SLUG_RE enforcing owner/name format. Introduces _dry_run_repo() (reads MERGEWORK_DRY_RUN_REPO, defaults to ramimbo/mergework, validates slug format, raises RuntimeError on mismatch) and _dry_run_contributor() (reads MERGEWORK_DRY_RUN_CONTRIBUTOR, defaults to mergework-dry-run, enforces non-empty, raises RuntimeError if blank).
Main integration and validation test infrastructure
scripts/staging_webhook_dry_run.py, tests/test_staging_webhook_dry_run.py
Main function replaces inline environment reads with calls to _dry_run_repo() and _dry_run_contributor(). Test imports expanded to include new helpers and main entrypoint. Test utility _set_required_dry_run_env() centralizes environment setup. New tests verify helpers return expected defaults when env vars are unset, parametric tests confirm main() rejects malformed repo slugs and blank contributors before issuing HTTP calls, with error output captured for validation.

Possibly related issues

  • #813: The validation helpers and test coverage directly implement early rejection of malformed MERGEWORK_DRY_RUN_REPO and blank MERGEWORK_DRY_RUN_CONTRIBUTOR values as proposed.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: validating staging dry-run identity environment variables, which is the core focus of the code and test modifications.
Description check ✅ Passed The description includes all required template sections: a detailed summary, comprehensive validation evidence with command outputs, and proper issue/bounty references. All test checklist items are verified and documented.
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.
Mergework Public Artifact Hygiene ✅ Passed PR only modifies internal validation scripts with no changes to README, docs, or public artifacts; no investment, price, cash-out, or fabricated payout claims are introduced in the codebase.
Bounty Pr Focus ✅ Passed PR changes match stated objectives: validates MERGEWORK_DRY_RUN_REPO format and MERGEWORK_DRY_RUN_CONTRIBUTOR before HTTP calls with comprehensive parametric test coverage. No unrelated scope changes.

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


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

Copy link
Copy Markdown
Contributor

@keilogic keilogic left a comment

Choose a reason for hiding this comment

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

Reviewed current head e00d9702a0d4c974ee579d5791d2579bd6bf65b4 against origin/main 12ca2f0510abce4f3a8b3d77ae8596f47fa5c74c.

Files inspected: scripts/staging_webhook_dry_run.py and tests/test_staging_webhook_dry_run.py.

What I checked:

  • _dry_run_repo() preserves the default ramimbo/mergework, trims configured input, rejects blank values, and rejects malformed values before the issue URL or HTTP calls are built.
  • _dry_run_contributor() preserves the default mergework-dry-run, trims configured input, and rejects blank values before the webhook payload is posted.
  • The new negative tests monkeypatch httpx.post to fail if validation happens after an HTTP call, so they prove the fail-closed ordering for malformed repo and blank contributor inputs.
  • The changed files match source issue #813; the PR body references #761 and #813, and I am treating bounty eligibility as a maintainer decision rather than a code blocker.

Validation:

  • python -m pytest tests/test_staging_webhook_dry_run.py -q -> 13 passed
  • python -m ruff check scripts/staging_webhook_dry_run.py tests/test_staging_webhook_dry_run.py -> passed
  • python -m ruff format --check scripts/staging_webhook_dry_run.py tests/test_staging_webhook_dry_run.py -> 2 files already formatted
  • python -m mypy scripts/staging_webhook_dry_run.py -> success
  • python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check origin/main...HEAD -> clean
  • git merge-tree --write-tree origin/main HEAD -> clean tree 69a0941e9d813f2f121bc79fac170a49d2429ab0

GitHub reports mergeStateStatus=CLEAN; hosted Quality/readiness/docs/image and CodeRabbit are both successful. Duplicate search for #813, staging dry-run identity terms, MERGEWORK_DRY_RUN_REPO, and MERGEWORK_DRY_RUN_CONTRIBUTOR found only this open PR.

No blockers found in the reviewed scope.

@ramimbo ramimbo merged commit f3b9922 into ramimbo:main Jun 2, 2026
2 checks passed
@ramimbo ramimbo added mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded labels Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants