Skip to content

fix: address safe-settings runtime issues from initial testing#141

Open
marcusburghardt wants to merge 4 commits into
complytime:mainfrom
marcusburghardt:fix/safe-settings-runtime-fixes
Open

fix: address safe-settings runtime issues from initial testing#141
marcusburghardt wants to merge 4 commits into
complytime:mainfrom
marcusburghardt:fix/safe-settings-runtime-fixes

Conversation

@marcusburghardt

Copy link
Copy Markdown
Member

Summary

Fix three runtime issues discovered during initial safe-settings testing against complytime-demos.

Related Issues

Review Hints

  • Review commits individually, each addresses a separate issue:

    1. fix(ci): Downgrade safe-settings to 2.1.18 (last Probot v13 version). Adds a version input to workflow_dispatch so newer versions can be tested without code changes.
    2. fix: Add missing pull_request parameters to the non-code repos ruleset. The GitHub API rejects partial parameter sets with HTTP 422.
    3. fix: Switch restrictedRepos from exclude to include (allowlist). Anchor all entries with ^...$ since safe-settings uses regex matching. Add complypack to code repos suborg and ruleset.
  • The version input defaults to the 2.1.18 commit SHA (594f3c706de6c4ddafb1a86dfa7468f19337e54f). To test a newer safe-settings version, enter a different SHA or tag in the workflow dispatch UI.

  • After merge, test with: Actions > "Safe Settings Sync" > Run workflow > dry-run=true, repos=complytime-demos

  • Full local validation: make sanity

safe-settings 2.1.19+ uses Probot v14 which has a breaking change:
the logger is initialized asynchronously but full-sync.js accesses
it synchronously, causing 'Cannot read properties of null'. This is
tracked upstream as github/safe-settings#955 with a fix in PR #961
(not yet merged).

Downgrade default to 2.1.18 (last Probot v13 version) and expose
the version as a workflow_dispatch input so newer versions can be
tested without code changes.

SHA 594f3c706de6c4ddafb1a86dfa7468f19337e54f verified via:
  gh api repos/github/safe-settings/git/ref/tags/2.1.18

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Assisted-by: OpenCode (claude-opus-4-6)
The GitHub API rejects org-level rulesets with partial pull_request
parameters (HTTP 422). Add the missing parameters with their intended
values (dismiss_stale_reviews_on_push, require_code_owner_review,
require_last_push_approval all set to false).

Update EXTERNALLY_DEFINED comment to clarify its expected behavior:
the rule is dropped on initial creation (by design, nothing to retain)
and takes effect on subsequent updates when checks exist externally.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Assisted-by: OpenCode (claude-opus-4-6)
Use an allowlist approach instead of exclude list. This ensures
only explicitly onboarded repos are managed by safe-settings,
without needing to enumerate every unmanaged repo by name.

Anchor all entries with ^ and $ for exact matching since
safe-settings uses regex matching for restrictedRepos.

Add complypack to the code repos suborg and ruleset conditions.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Assisted-by: OpenCode (claude-opus-4-6)
Add 'type: creation' rule to both code repos and non-code repos
rulesets. This restricts creation of refs matching the default
branch pattern, preventing recreation of the default branch if
deleted. Matches the existing behavior on complyctl and org-infra
repo-level rulesets.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Assisted-by: OpenCode (claude-opus-4-6)
@marcusburghardt

Copy link
Copy Markdown
Member Author

@jpower432 thanks for reviewing #140. Unfortunately, after bumping safe-settings version it revealed a known but being fixed in Upstream. So I downgraded it to 2.1.18 until the bug is fixed and included a input in the workflow so we can more easily test before bumping again.

I could successfully run it locally now and included some improvements from these tests in this PR.

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.

2 participants