Skip to content

ci: fix downstream-basic trigger logic (#2474)#2489

Open
mohamadm0meni wants to merge 2 commits into
open-quantum-safe:mainfrom
mohamadm0meni:fix-2474-downstream-trigger
Open

ci: fix downstream-basic trigger logic (#2474)#2489
mohamadm0meni wants to merge 2 commits into
open-quantum-safe:mainfrom
mohamadm0meni:fix-2474-downstream-trigger

Conversation

@mohamadm0meni

@mohamadm0meni mohamadm0meni commented Jul 5, 2026

Copy link
Copy Markdown

Partially addresses #2474.
Replaces the CircleCI-based check for oqs-provider (decommissioned) and the substring/status-code check for the rest with a shared script, scripts/trigger_downstream_ci.py, that dispatches each downstream CI and polls the actual run to a terminal state before reporting success.
oqs-provider is temporarily excluded: none of its current workflows expose a repository_dispatch or workflow_dispatch trigger, so it cannot be wired in from this repo alone. A follow-up PR to oqs-provider is needed first.
Whether downstream compatibility should block liboqs PRs (vs. today's post-merge/manual checking) is a separate policy question, out of scope here.

@mohamadm0meni

Copy link
Copy Markdown
Author

Hello,

The CI workflows are currently pending manual approval. Could a maintainer please trigger them?

I will mark the PR as ready for review once the tests pass.

Thank you in advance.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 82.296% (+0.001%) from 82.295% — mohamadm0meni:fix-2474-downstream-trigger into open-quantum-safe:main

@mohamadm0meni mohamadm0meni marked this pull request as ready for review July 5, 2026 20:23
@baentsch

baentsch commented Jul 6, 2026

Copy link
Copy Markdown
Member

The CI workflows are currently pending manual approval. Could a maintainer please trigger them?

Done & passed, but I didn't see where the new code ran.

Also tagging @jplomas to take a look at this PR as this pertains to #2474

@jplomas

jplomas commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

If this works as it appears, it should be a small improvement over the existing setup but not a full fix for #2474.

I say "if" because the current CI will not confirm this until commit-to-main runs. downstream-basic is not triggered on PRs, which perhaps is part of the broader issue.

For liboqs/oqs-provider compatibility, I think we still need an explicit policy decision:

  1. If downstream compatibility should block liboqs PRs, then CI needs to build/test oqs-provider against the liboqs PR merge SHA, either directly inside liboqs CI or by triggering downstream with a liboqs ref and waiting/polling for the downstream result.

  2. If downstream compatibility is advisory/post-merge only, which I think is the implication in the current setup and docs, failures should create a more visible notification/issue.

I would suggest that the policy decision be made first, then a full fix be proposed.

@xuganyu96 xuganyu96 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.

Hi @mohamadm0meni, thank you for your contribution!

The intention behind these jobs is to check that changes in the upstream project (i.e. liboq) do not break downstream projects (i.e. oqs-provider, openssh, boringssl, python/cpp/go/rust wrappers). Based on my reading, the problem with the current setup is that it only triggers the downstream projects' GitHub Actions workflows but do not check their results. Instead, the current checking logic only "intends to ensure" that the workflows have been triggered. Even that is not implemented correctly. For example, the URL for triggering oqs-provider's CI pipeline is simply wrong: we don't use Circle CI anymore and have moved everything onto GitHub Actions.

Please consider the following points when writing a complete solution:

  • Do we want to interact with GitHub API (https://api.github.com) or use some wrappers like gh CLI? (I personally prefer interacting with the HTTP API directly)
  • Do we want to isolate the logic into a separate script file living under scripts/ instead of writing inline bash scripts in workflow YAML? The former carries an additional advantage of making local runs easier.
  • How to check the status of the triggered workflow run? Check GitHub API's documentation: what are some terminal states? Also consider setting a timeout so this job doesn't run indefinitely. Given these requirements, we might even want to consider using a richer language like Python instead of bash scripting

References:

cc: @baentsch @RodriM11 @dstebila @jplomas

Signed-off-by: Mohamad Momeni <mohamadmomeni@eng.uk.ac.ir>
Signed-off-by: Mohamad Momeni <mohamadmomeni@eng.uk.ac.ir>
@mohamadm0meni mohamadm0meni force-pushed the fix-2474-downstream-trigger branch from 849f588 to ffd8377 Compare July 7, 2026 11:18
@mohamadm0meni mohamadm0meni requested a review from xuganyu96 July 7, 2026 11:19
@mohamadm0meni

Copy link
Copy Markdown
Author

@xuganyu96 Thanks for the review. Updated to use the HTTP API directly via a stdlib-only Python script in scripts/, testable locally via --api-base. It now dispatches, locates the spawned run(s), and polls to a terminal status (with a 45-minute timeout), failing on anything other than success/neutral/skipped. CircleCI is removed entirely.
One open item: oqs-provider has no remote trigger today, so it's excluded pending a companion PR there — happy to send that next if you agree with the approach.

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.

5 participants