Repository navigation
fix(ci): package changed integrations from merge diff - #508
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99a5a9f443
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Shubhank-Jonnada
changed the title
|
@codex review |
![autohive-ai[bot]](https://avatars.githubusercontent.com/in/4787664?s=60&v=4){kind=small}
There was a problem hiding this comment.
| Result | P1 | P2 | P3 |
|---|---|---|---|
| Review incomplete | 0 | 0 | 0 |
Review incomplete
Summary
Review target: a155058
Review is incomplete: prior Autohive findings were not supplied, and the Admin release-cursor ordering contract was unavailable to verify out-of-order publication from newly concurrent packaging runs. Selection syntax checks passed, but HiveUp 2.5.0 was unavailable for execution tests.
Review powered by Autohive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a155058c9b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Package only integrations whose config.json version changed in | ||
| # this push. Failed runs can be rerun directly; the workflow does not | ||
| # need a bootstrap release or cross-run recovery baseline. | ||
| previous_commit_sha="$EVENT_BEFORE" |
There was a problem hiding this comment.
With the workflow-level concurrency block removed, GitHub runs multiple instances of this workflow concurrently by default (GitHub Actions concurrency documentation). If two rapid merges bump the same integration from 1.0.0→1.0.1 and then 1.0.1→1.0.2, the newer job can publish first while the slower older job publishes afterward; because docs/integration-release-workflow.md says Admin keeps the newest ZIP per folder, that completion order can make the older package win during sync. Queue these runs serially, such as with concurrency and queue: max, so every push is retained without publishing releases out of commit order.
Useful? React with 👍 / 👎.
NinosMan
left a comment
NinosMan
left a comment
There was a problem hiding this comment.
Approved. I have left a comment about the release behaviour, but that isn't blocking. I think HiveUp should enforce version bumping and it shouldn't be a concern of the package and release action. Just something to keep in mind 😄
| # Package only integrations whose config.json version changed in | ||
| # this push. Failed runs can be rerun directly; the workflow does not |
There was a problem hiding this comment.
Not entirely sure if this is the correct behavior. If a PR happens to be merged without a version bump, this would mean that the sync tool doesn't resolve the current code changes that exist in master. The behaviour would also be invisible to the user.
There was a problem hiding this comment.
I agree, but in some cases we may only want to push README updates or other non-code changes, so those should not create a release.
And yes, I agree with the point above: HiveUp should enforce version bumps and make them mandatory for integration code changes. That would fix this issue.
There was a problem hiding this comment.
Yeah, I think I agree that HiveUp’s PR validation should require a version bump for integration code or dependency changes, while README-only changes should not necessarily require one.
The packaging workflow should remain version-driven.
|
I think Codex’s concurrency finding is blocking, but the downstream failure is slightly different from the one described. The Admin sync implementation in Autohive #5487 sorts releases by commit ancestry, so an older ZIP doesn’t overwrite a newer one when both releases are discovered in the same sync. The gap is an older release arriving after the cursor has advanced:
Removing concurrency limits makes this possible during normal packaging. The documented “rerun the failed workflow” recovery path can produce the same situation even with serialized runs. Before merging, I’d like the producer/consumer contract to handle late batches, with a regression test covering sync B first, then discover A, ideally using different integrations so A’s missing update cannot be treated as superseded. The existing out-of-order test covers both releases being available before the first sync, which doesn’t exercise this case. |
|
Thanks @TheRealAgentK, the missing case was the late ancestor release after the Admin cursor had already advanced. This is fixed on the consumer side in Autohive #5487, commit |
3 participants
Packages each push to
masterfromgithub.event.beforeto the pushed commit. Only new integration folders or increasedconfig.jsonversions produce ZIPs and a schema 3 integrity manifest. Commits without version bumps produce no release.Removes bootstrap/release-history baseline discovery and the workflow concurrency group so rapid merges do not cancel pending packaging runs. Manual
workflow_dispatchstill supports selected or full snapshot packaging. If a run fails, rerun that run directly; later pushes do not automatically recover its packages.Validation
2.5.0tag.Merge and rollout
Tooling #62, integrations #441, and the HiveUp
2.5.0tag are already available.Merge and deploy Autohive #5487 before this PR so the consumer safely accepts older batches published after newer ones. Its SQL scripts accompany deployment. Then smoke-test a version-bump push, concurrent/out-of-order completion, and rerunning a failed packaging job.
Version-bump enforcement for integration code changes remains a separate HiveUp follow-up, as discussed in review. README-only changes need not create releases.