Conversation
Scheduled runs of the nightly CI, IBD perf and RPC perf workflows checked out the retired 0.1.x branch even though the workflow file runs from the default branch, and each job repeated its own ref selection. Each workflow now begins with an inline resolve-source job that resolves the maintained 1.x.x branch to an immutable commit with git ls-remote before any checkout or repository-local action. A manual dispatch keeps its selected ref and event commit. Every source job depends on that job, refuses to check out anything but the resolved commit, verifies the actual HEAD after the checkout, and reports the requested ref, resolved commit, actual HEAD and workflow revision separately in the step summary and benchmark metadata. Concurrency groups and run names follow the same selection. The scanner keeps its HEAD-derived source commit and its independent origin/main comparison base. ci/checks/test_scheduled_validation_contract.py executes the real resolver, guard, verify and metadata step scripts against temporary Git repositories whose default-branch, maintained-branch and manual-selection commits differ, evaluates the checkout wiring under scheduled and manual contexts, and covers resolver input rejection and checked-out HEAD mismatches. The workflow YAML is loaded with YAML 1.2 booleans so the on trigger key stays a string.
… job Only the native fuzz entry of the nightly matrix sets QBIT_FUZZ_MUTATE_MIN_TIME=60; every other matrix entry, the scanner job and the perf workflows leave it empty. The workflow only carries the request: ci/test/03_test_script.sh must read the variable to add the second test/fuzz/test_runner.py invocation with --require_qbit_corpus --mutate_min_time=60 on the six fixed qbit corpus targets, and ci/test/00_setup_env_native_fuzz.sh must export it so the container env file forwards it. Both live in the runner interface change this depends on and must land together with it. The contract test evaluates the nightly job environment for every matrix entry so the value cannot move to or spread beyond the native fuzz entry.
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. |
The application test times all synchronous GUI teardown, including work that happens after the pending send worker has observed cancellation and joined. Slow teardown can therefore fail the five-second assertion even when the cancellation contract succeeds. Use the existing watchdog to enforce the cancellation deadline, anchored before shutdown starts. Keep total teardown time as diagnostic output, assert worker completion before teardown returns, and preserve the wallet lifetime and cancellation checks. Validation: reproduce the old failure with a post-cancellation delay, verify the fixed test accepts it, and verify missed cancellation still fails the watchdog.
![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: 88c08aa7fc
ℹ️ 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".
A post-return read of create_finished can observe a worker that completed after sendEvent returned. It cannot establish the required lifetime order. Record worker completion and the synchronous dialog destruction callback with sequence numbers under the synthetic wallet mutex. Require completion to precede destruction inside teardown, independently of scheduling after sendEvent returns. Report each event's timing and remove obsolete elapsed state while preserving the five-second cancellation watchdog.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses #152 by selecting maintained
1.x.xsource for scheduled heavy validation and recording the exact commit built. Depends on #164; merge that runner change first.refs/heads/1.x.xto a full commit SHA; manual runs preserve the selected branch/tag and event commit.origin/maincomparison base. Enable the companion seeded mutation phase only on the native-fuzz nightly matrix entry, at 120 seconds per target.This release-branch PR does not complete schedule delivery: GitHub schedules use the default branch. The workflow revisions must also reach
main, then a scheduled run must demonstrate the maintained source and correct provenance before #152 is closed.Stabilize the shared Qt shutdown regression test: enforce the five-second send-worker cancellation deadline, preserve wallet lifetime checks, and report total GUI teardown time separately. Slow teardown after successful cancellation no longer causes a false failure.
Resolve the Qt ordering review: record worker completion before the dialog’s synchronous destruction, so the test no longer infers lifetime order from a mutex read after teardown returns. The shared fix is also in fuzz: seed qbit targets and verify replay and mutation #164.
Testing
Review follow-up: a controlled late-completion record passes the old Qt assertion and fails the revised one. The clean Linux Debug Qt suite passed all 12 test groups; slow teardown still passes, and missed cancellation still fails the five-second watchdog. Full repository Docker lint passed on the updated signed head.
CI follow-up: the shared Qt test passed a Linux Debug build, all 12 QtTest suites through CTest, and three clean repeat runs. A 5.5-second delay after worker cancellation reproduces the old failure and passes with the fix; deliberately disabled cancellation still fails the five-second watchdog, with and without the extra delay. Full repository Docker lint passed on the updated signed commit. Hosted CentOS validation is rerunning; local verification used GCC 13.3 / Qt 6.4.2.
GIT_CONFIG_GLOBAL=/dev/null python3 ci/checks/test_scheduled_validation_contract.py: 10 tests passed again after merging current1.x.x; also passed on Linux during implementation.Deliberate workflow mutations failed the relevant contracts: retired/trigger source selection, wrong metadata, overridden manual selection, removed dependency/verification, unqualified refs, wrong matrix placement, and reverting the mutation budget to 60 seconds.
Docker actionlint 1.7.12 passed for the changed workflows. Full repository Docker lint passed on the publication head.
The companion runner's real Linux integration passed at 120 seconds with positive post-initialization execution counts for all six targets.
Built locally: shared Qt regression-test fix in a Linux Debug build.
Ran focused unit or functional tests for the changed area.
Ran lint or formatting checks relevant to this change.
Not run. Reason: hosted manual/scheduled runs, self-hosted tool availability, and the live authenticated resolver path remain to be exercised after delivery.
Target Branch
mainor a maintainer-requested release branch such as0.1.x. Target:1.x.x; separate default-branch delivery remains required.Risk / Review Notes
Notes: CI source selection and provenance change. Ref values reach shell through environment variables. Resolution fails closed, and checkout refuses empty output to prevent fallback to the triggering commit. The contract requires PyYAML/Git and is not wired into a required gate. Merge #164 first so the nightly mutation setting has its consumer.
Docs / Process Impact
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.