[DX-3764] [CRE-3579] [CRE-3578] [CRE-3577] diagnose cmd + Skill Improvements + Fix Flaky Tests

[kalverra]

Improve fix-chainlink-tests Skill

• Rename from chainlink-diagnose-tests to fix-chainlink-tests, better aligned with best practices.
• Improve some edge cases and counting bugs in diagnose loop.
• Add some memory for fix attempts in the form of a fix-attempt-*.jsonl file to preserve past attempts and reduce context compaction consequences.
• Test postgres instance goes faster: https://github.com/smartcontractkit/chainlink/compare/postgresTestImprovements?expand=1#diff-90715ad3f71e06e08c4314d35148b9574ac20c98ff16c2db9afeefcd890dc035R107-R108

Faster (kinda not really) Postgres Tests

Tuned our test postgres instances to discard some production protections that don't help us, and only slow down tests. This wasn't very effective at speeding up tests, but it also didn't hurt. Left is before the changes, Right is after.

If anything, this indicates to me that our bottleneck for test speed IS NOT the postgres instance.

Fix Flaky Tests

Show off the skills

• Remove unnecessary subtest that was giving confusing test count results.

Fix flaky tests in core/services/workflows/syncer/ package

Tests in this package were using hardcoded DB keys, meaning they would stomp on each other all the time. The diagnose runs found these flake rates:

Flaky (8)
- github.com/smartcontractkit/chainlink/v2/core/services/workflows/
|-- syncer/ (3/25) 12.0%
|---- Test_workflowDeletedHandler (1/25) 4.0%
|---- Test_workflowDeletedHandler/success_deleting_existing_engine_and_spec (1/25) 4.0%
|---- Test_workflowPausedActivatedUpdatedHandler (2/25) 8.0%
|---- Test_workflowPausedActivatedUpdatedHandler/success_pausing_activating_and_updating_existing_engine_and_spec (2/25) 8.0%
|---- Test_workflowRegisteredHandler (3/25) 12.0%
|---- Test_workflowRegisteredHandler/correctly_generates_the_workflow_name (1/25) 4.0%
|---- Test_workflowRegisteredHandler/success_with_active_workflow_registered (2/25) 8.0%

Trunk.io has these tests and tickets listed for this package:

• Test_workflowDeletedHandler 24% flaky | CRE-3577
• Test_workflowDeletedHandler/success_deleting_existing_engine_and_spec 24% flaky | CRE-3578
• Test_StratReconciliation_RetriesWithBackoff 18% flaky | CRE-3579

diagnose runs with --iterations 100 after the fixes show a 0% flake rate!

Review

There are a lot of linting fixes in this PR that we did as part of running experiments that can largely be ignored for reviews. Focus on changes in:

• tools/test/: Updates to the diagnose harness and the AI skill.
• core/services/workflows/syncer/v2/handler_test.go: Flaky test fixes

[github-actions]

👋 kalverra, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

[github-actions]

CORA - Pending Reviewers

Codeowners Entry	Overall	Num Files	Owners
*	💬	1	@smartcontractkit/foundations, @smartcontractkit/core
/core/services/ocr*/	💬	7	@smartcontractkit/foundations, @smartcontractkit/core
/core/services/ocr2/plugins/ocr2keeper/	💬	2	@smartcontractkit/dev-services
/core/services/workflows/	🚫	13	@smartcontractkit/keystone
.tool-versions	💬	1	@smartcontractkit/core

Legend: ✅ Approved | ❌ Changes Requested | 💬 Commented | 🚫 Dismissed | ⏳ Pending | ❓ Unknown

For more details, see the full review summary.

[github-actions]

I see you updated files related to core. Please run make gocs in the root directory to add a changeset as well as in the text include at least one of the following tags:

• #added For any new functionality added.
• #breaking_change For any functionality that requires manual action for the node to boot.
• #bugfix For bug fixes.
• #changed For any change to the existing functionality.
• #db_update For any feature that introduces updates to database schema.
• #deprecation_notice For any upcoming deprecation functionality.
• #internal For changesets that need to be excluded from the final changelog.
• #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
• #removed For any functionality/config that is removed.
• #updated For any functionality that is updated.
• #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

[github-actions]

✅ No conflicts with other open PRs targeting develop

[Copilot]

Pull request overview

Risk Rating: MEDIUM

This PR improves the tools/test diagnose harness UX and performance by speeding up the ephemeral Postgres instance, enhancing progress/output behavior, and enriching the analyze/report pipeline (build-failure detection, slow reporting, runtime estimates, and metadata capture).

Changes:

• Speed up test Postgres containers (tuned settings + tmpfs) and persist additional run metadata (e.g., Postgres version, has DB).
• Improve diagnose runner output: better live-progress coordination, “analyzing” live timer, longest-possible runtime estimate, and stop-on-build-failure behavior.
• Expand analyze/reporting: detect build failures from go test -json, adjust slow reporting to include top packages, and tweak summary/overall formatting.

Scrupulous human review recommended (high-impact logic):

• tools/test/internal/runner/analyze.go: slow/top-packages merging and its interaction with summary metrics/CSV output.
• tools/test/internal/runner/runner.go: new fail-fast-on-build-failure behavior and new AI-output markers (lpr_s:*, bf_stop ...).
• tools/test/internal/db/db.go: Postgres container tuning (durability disabled, tmpfs) and any CI/platform implications.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tools/test/internal/runner/runner.go	Diagnose runner: build-failure stop, runtime estimate output, improved progress/analyzing behavior, run metadata updates.
tools/test/internal/runner/runner_test.go	Updated/added unit tests for analyzing progress, timeout parsing, runtime estimates, build-failure stop, serial progress mutex.
tools/test/internal/runner/diagnose_results_dir.go	Include -run pattern in diagnose results directory slug.
tools/test/internal/runner/diagnose_progress.go	More robust package-pattern detection when flags appear after packages; progress time clamping cleanup.
tools/test/internal/runner/diagnose_progress_test.go	Added regression tests for progress-line vs digest-line merging behavior.
tools/test/internal/runner/analyze.go	Build failure signals, slow-report restructuring (top packages), summary/overall formatting changes, new run meta fields.
tools/test/internal/runner/analyze_test.go	Added coverage for build-failure detection and severity color output in summary.
tools/test/internal/repo/repo.go	Use strings.SplitSeq iteration for go.mod parsing.
tools/test/internal/output/output.go	Add NewForTest helper to control live-inline behavior in unit tests.
tools/test/internal/output/output_test.go	Add test coverage for NewForTest live-inline behavior and AI-output interaction.
tools/test/internal/db/db.go	Speed up test Postgres container via config knobs and tmpfs.
tools/test/internal/config/config.go	Use strings.SplitSeq for fail-fast-on parsing.
tools/test/.agents/skills/chainlink-test-diagnosis/SKILL.md	Update agent skill guidance and references layout.
tools/test/.agents/skills/chainlink-test-diagnosis/references/flaky-patterns/filter.md	Add reference doc for a common flaky filter/logpoller pattern.
tools/test/.agents/skills/chainlink-test-diagnosis/eval/real-fix-shas.json	Add eval metadata file mapping PR SHA(s) to real fixes.
core/services/ocr2/plugins/ocr2keeper/integration_test.go	Switch pointer helpers to new(...) usage for config fields.
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/integration_test.go	Switch pointer helpers to new(...) usage for config fields.
core/services/ocr2/plugins/mercury/plugin_test.go	Switch pointer helpers to new(...) usage in test fixtures.
core/services/ocr2/plugins/mercury/helpers_test.go	Switch pointer helpers to new(...) usage; keep local ptr helper for remaining cases.
core/services/ocr2/plugins/llo/onchain_channel_definition_cache_integration_test.go	Use maps.Copy for merging definitions; switch SHA3 import usage.
core/services/ocr2/plugins/llo/integration_test.go	Misc test cleanups; config pointer creation updates; remove skipped subtest in favor of commented block.
core/services/ocr2/plugins/llo/helpers_test.go	Listener creation with net.ListenConfig; test assertions changed to avoid hard-failing HTTP handler after read errors; pointer helper removal.
core/services/ocr2/plugins/llo/config/config.go	Simplify validation control flow for channel definitions vs contract address.
core/services/ocr2/plugins/llo/config/config_test.go	Tighten error assertions with require.EqualError.
.gitignore	Ignore diagnose-attempted-fixes-*.jsonl.

[trunk-io]

     

View Full Report ↗︎ ⋅ Docs

[Copilot]

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated 3 comments.

[erikburt]

Should this maintain parity with the one we use in our unit tests? Otherwise, we may see different errors using the diagnose tool and what we see in practice.

And if these flags reduce flakes, then perhaps they should be present for the instance running with our unit tests?

[kalverra]

Agreed we want to keep parity. Made this PR to help us do so: smartcontractkit/.github#1549

[kalverra]

Added

[cl-sonarqube-production]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.3% Duplication on New Code

See analysis details on SonarQube