Skip to content

fix: remote custom registry test#2547

Merged
daryllimyt merged 7 commits intomainfrom
codex/fix-remote-custom-registry-test
Apr 21, 2026
Merged

fix: remote custom registry test#2547
daryllimyt merged 7 commits intomainfrom
codex/fix-remote-custom-registry-test

Conversation

@daryllimyt
Copy link
Copy Markdown
Contributor

@daryllimyt daryllimyt commented Apr 21, 2026

Summary

  • Ensures the remote custom registry integration test syncs the builtin platform registry before workflow execution.
  • Detects and replaces/promotes away from the placeholder builtin tarball URI (s3://test/test.tar.gz).
  • Makes registry lock setup idempotent for platform registry rows used by xdist/unit tests.
  • Preserves an already-selected live platform registry version in registry_version_with_manifest, so temporal/integration tests do not clobber the API-synced builtin tarball with the fake fixture version.

Root Cause

Commit 5d1d745c1 made resolved workflow locks always include tracecat_registry as a runtime dependency. The remote custom registry test synced only the custom repo, so executor setup could try to download the builtin fixture placeholder tarball and fail with a 404.

The additional CI failures came from tests assuming platform registry setup was isolated/idempotent. Platform registry rows are global by origin, so parallel setup could collide on tracecat_registry, and the shared manifest fixture could overwrite a live platform registry current version with test-version and the fake tarball.

Validation

  • uv run pytest tests/unit/test_registry_lock_resolution.py -n 4
  • uv run pytest --collect-only -q tests/temporal/test_large_collection_regressions.py tests/integration/test_install_and_run_custom_remote_registry_flow.py
  • uv run ruff check tests/conftest.py tests/unit/test_registry_lock_resolution.py tests/integration/test_install_and_run_custom_remote_registry_flow.py
  • uv run ruff format --check tests/conftest.py tests/unit/test_registry_lock_resolution.py tests/integration/test_install_and_run_custom_remote_registry_flow.py
  • uv run basedpyright tests/conftest.py tests/unit/test_registry_lock_resolution.py tests/integration/test_install_and_run_custom_remote_registry_flow.py

CI is green for Ruff, Pyright, unit, registry, temporal, integration, custom registry install, CodeQL, Semgrep, and Terraform Cloud.


Summary by cubic

Fixes the remote custom registry integration test by syncing the builtin platform registry and ensuring a real tarball is selected, so tracecat_registry resolves to a downloadable tarball and executor setup avoids 404s. Also makes registry setup idempotent, repairs missing current selections at startup, and applies a safe fixture fallback in shared DBs.

  • Bug Fixes
    • Syncs the builtin platform registry (DEFAULT_REGISTRY_ORIGIN) before the workflow; creates it if missing, forces sync when needed, and promotes a version with a real tarball when the placeholder is current.
    • Safe fixture fallback: in per-test DBs, select the fixture platform version; in the default/shared DB, select it only if no current exists or it already matches the fixture; otherwise, wait for API/startup to select the live builtin version.
    • Makes platform setup idempotent: upserts repositories by origin (unique non-default origins), generates unique versions/tarballs, and reuses an existing current for the default repo; startup sync repairs a missing current pointer when the target version already exists.
    • Adds an early lock-resolution check and raises BuiltinRegistryHasNoSelectionError when the builtin has no selected version, so tests and workflows retry while sync is pending.

Written for commit dfe91a6. Summary will update on new commits.

@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 13:43 — with GitHub Actions Inactive
@zeropath-ai
Copy link
Copy Markdown

zeropath-ai Bot commented Apr 21, 2026

No security or compliance issues detected. Reviewed everything up to dfe91a6.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► tests/conftest.py
    Introduce fake_tarball_uri constant
    Conditionally set platform_repo.current_version_id
    Add _should_select_fixture_platform_current helper function
Enhancement ► tests/integration/test_install_and_run_custom_remote_registry_flow.py
    Add helper functions for asserting responses and handling builtin versions
    Implement _sync_builtin_platform_registry to ensure a real tarball
    Call _sync_builtin_platform_registry before testing remote custom registry
Enhancement ► tests/unit/test_registry_fixture_platform_current.py
    Add unit tests for _should_select_fixture_platform_current logic
Enhancement ► tests/unit/test_registry_lock_resolution.py
    Add test_missing_builtin_current_raises_retryable_sync_pending
    Modify _setup_platform_registry to handle DEFAULT_REGISTRY_ORIGIN uniquely
Enhancement ► tests/unit/test_registry_platform_startup.py
    Add test_startup_sync_promotes_existing_target_when_no_current_version
Refactor ► tracecat/registry/lock/service.py
    Move built-in version check logic to be earlier in the function
    Remove redundant built-in version check at the end of the function
Refactor ► tracecat/registry/sync/jobs.py
    Add logic to promote existing target version when current_version_id is None during sync

@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 13:43 — with GitHub Actions Inactive
@daryllimyt daryllimyt marked this pull request as ready for review April 21, 2026 13:44
@blacksmith-sh

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 13:54 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 13:54 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:05 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:05 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/conftest.py">

<violation number="1" location="tests/conftest.py:973">
P1: Do not promote the fake seeded version when no builtin selection exists; that can leave `s3://test/test.tar.gz` mounted as current and workflows will still fail to fetch the tarball.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread tests/conftest.py Outdated
@daryllimyt daryllimyt changed the title [codex] fix remote custom registry test fix: remote custom registry test Apr 21, 2026
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:23 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:23 — with GitHub Actions Inactive
Copy link
Copy Markdown

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

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: 437a5bab91

ℹ️ 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 tests/conftest.py Outdated
@blacksmith-sh

This comment has been minimized.

@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:44 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:44 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:54 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 14:55 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 15:07 — with GitHub Actions Inactive
@daryllimyt daryllimyt temporarily deployed to internal-registry-ci April 21, 2026 15:07 — with GitHub Actions Inactive
@daryllimyt daryllimyt merged commit a827295 into main Apr 21, 2026
16 checks passed
@daryllimyt daryllimyt deleted the codex/fix-remote-custom-registry-test branch April 21, 2026 15:21
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