🧪 test(ci): harden Cucumber reliability#581
Merged
Merged
Conversation
scttbnsn
requested review from
ALARGECOMPANY and
biggest-littlest
as code owners
July 22, 2026 15:49
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (33)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (24)
📝 WalkthroughWalkthroughCI runs browser-free Cucumber against a QA-loaded Drydock image with manifest-based fixture readiness, retries, reports, and failure diagnostics. Tagged Cucumber scenarios now restore container, settings, and notification state. Browser route checks move to Playwright with route-name landmarks. The outer API rate limit accepts Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/ci-verify.ymlTraceback (most recent call last): e2e/package.jsonTraceback (most recent call last): test/qa-compose.ymlTraceback (most recent call last): Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 15:53
ddb8335 to
9ea5c79
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci-verify.yml:
- Around line 1151-1158: Add a finite --max-time limit to both diagnostic curl
invocations in the failure/cancellation diagnostics step, including the health
request and the containers request. Keep their existing URLs, headers, outputs,
and error redirections unchanged while ensuring stalled responses cannot consume
the remaining job budget.
In `@e2e/features/support/state-restoration.js`:
- Around line 33-53: Update createApiRequest to create an AbortController, pass
its signal to fetchImpl, and start an approximately 50-second timer that aborts
the request. Clear the timer in a finally block so it is released on success,
HTTP failure, parse failure, or fetch rejection, while preserving the existing
response handling and error messages.
In `@e2e/playwright/navigation.spec.ts`:
- Line 63: Update the visibility assertion using main.locator(':scope > *') so
it targets a single child element, using .first() if visibility of any child is
sufficient; otherwise assert the expected child count before checking
visibility.
In `@scripts/cucumber-scope.test.mjs`:
- Around line 36-37: Update the assertions in the navigation-spec test to verify
each path and routeName as a paired mapping within the same object entry, rather
than matching them independently across navigationSpec. Prefer matching the
complete serialized object or parsing the route map, while preserving the
existing path and routeName values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2be41689-b7b3-4704-a9be-4e6dc3a52315
📒 Files selected for processing (22)
.github/tests/ci-verify-workflow.test.ts.github/tests/release-cut-retry-workflow.test.ts.github/workflows/ci-verify.yml.gitignoreCONTRIBUTING.mddocs/cucumber-reliability.mde2e/config/cucumber-core-fixtures.txte2e/features/api-container.featuree2e/features/api-v14.featuree2e/features/step_definitions/ui.jse2e/features/support/init.jse2e/features/support/state-restoration-hooks.jse2e/features/support/state-restoration.jse2e/features/ui.featuree2e/package.jsone2e/playwright/navigation.spec.tse2e/tests/state-restoration.test.jsscripts/cucumber-scope.test.mjsscripts/start-drydock.shscripts/start-drydock.test.mjsui/src/layouts/AppLayout.vueui/tests/layouts/AppLayout.spec.ts
💤 Files with no reviewable changes (2)
- e2e/features/step_definitions/ui.js
- e2e/features/ui.feature
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 15:57
9ea5c79 to
5174808
Compare
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 15:59
5174808 to
bf29378
Compare
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 16:13
bf29378 to
f24a7d3
Compare
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 16:37
f24a7d3 to
86132a0
Compare
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 17:01
86132a0 to
466b443
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/api/container.test.ts (1)
2686-2699: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the refreshed-container not-found branch.
Add a case where
getContainers()returns an empty list, then assert a 404 response and thatwatchContaineris not called.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/container.test.ts` around lines 2686 - 2699, Add a test case around callWatchContainer where mockWatcher.getContainers returns an empty list; assert the response status is 404 and verify mockWatcher.watchContainer is not called, while preserving the existing refreshed-container success case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/container/handlers/actions.ts`:
- Around line 226-244: Update the watcher handling around getContainers and
watchContainer so watchContainer is never called without first obtaining a fresh
enumerable container list. Require getContainers as part of the handled watcher
contract, or route providers lacking it through an equivalent fresh-list path,
while preserving the existing 404 response when the requested container is
absent and passing the refreshed container to watchContainer.
In `@app/configuration/index.test.ts`:
- Around line 601-608: Update the test suite around getServerConfiguration to
add shared afterEach cleanup that unconditionally removes
configuration.ddEnvVars.DD_SERVER_RATELIMIT_MAX. Remove the test-local delete
and preserve the existing override assertion, ensuring cleanup runs even when
the test fails.
---
Nitpick comments:
In `@app/api/container.test.ts`:
- Around line 2686-2699: Add a test case around callWatchContainer where
mockWatcher.getContainers returns an empty list; assert the response status is
404 and verify mockWatcher.watchContainer is not called, while preserving the
existing refreshed-container success case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ddebc2eb-70b3-42cb-83fa-976ccebfb4ae
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (32)
.github/tests/ci-verify-workflow.test.ts.github/tests/e2e-playwright-workflow.test.ts.github/tests/release-cut-retry-workflow.test.ts.github/workflows/ci-verify.yml.gitignoreCONTRIBUTING.mdapp/api/api.test.tsapp/api/api.tsapp/api/container.test.tsapp/api/container/handlers/actions.tsapp/configuration/index.test.tsapp/configuration/index.tscontent/docs/current/configuration/server/index.mdxdocs/cucumber-reliability.mde2e/config/cucumber-core-fixtures.txte2e/features/api-container.featuree2e/features/api-v14.featuree2e/features/step_definitions/ui.jse2e/features/support/init.jse2e/features/support/state-restoration-hooks.jse2e/features/support/state-restoration.jse2e/features/ui.featuree2e/package.jsone2e/playwright/navigation.spec.tse2e/playwright/v16-health-preferences.spec.tse2e/tests/state-restoration.test.jsscripts/cucumber-scope.test.mjsscripts/start-drydock.shscripts/start-drydock.test.mjstest/qa-compose.ymlui/src/layouts/AppLayout.vueui/tests/layouts/AppLayout.spec.ts
💤 Files with no reviewable changes (2)
- e2e/features/step_definitions/ui.js
- e2e/features/ui.feature
🚧 Files skipped from review as they are similar to previous changes (19)
- e2e/features/api-container.feature
- ui/src/layouts/AppLayout.vue
- e2e/features/support/state-restoration-hooks.js
- e2e/package.json
- .github/tests/release-cut-retry-workflow.test.ts
- docs/cucumber-reliability.md
- scripts/cucumber-scope.test.mjs
- e2e/config/cucumber-core-fixtures.txt
- e2e/features/support/init.js
- e2e/features/api-v14.feature
- .gitignore
- e2e/tests/state-restoration.test.js
- e2e/features/support/state-restoration.js
- scripts/start-drydock.sh
- scripts/start-drydock.test.mjs
- CONTRIBUTING.md
- e2e/playwright/navigation.spec.ts
- .github/tests/ci-verify-workflow.test.ts
- .github/workflows/ci-verify.yml
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 17:09
466b443 to
efb1159
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/tests/ci-verify-workflow.test.ts:
- Around line 184-192: Update the workflow.jobs?.e2e assertion to enforce the
complete permissions allowlist rather than allowing additional keys: replace the
partial toMatchObject assertion with an exact equality assertion while
preserving the expected timeout-minutes, actions: read, and contents: read
values.
In @.github/tests/e2e-playwright-workflow.test.ts:
- Around line 52-69: Strengthen the test around the remote-bootstrap command in
Playwright QA so commented-out text cannot satisfy the assertions. Update the
checks in test('Playwright QA fails closed when required remote fixtures cannot
be seeded') to validate the shell entrypoint and require executable,
non-commented lines for set -eu and pull_with_retry, while preserving the
existing retry-limit assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b7b3220-9802-441f-a7c8-2d2ec2a669b3
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (33)
.github/tests/ci-verify-workflow.test.ts.github/tests/e2e-playwright-workflow.test.ts.github/tests/release-cut-retry-workflow.test.ts.github/workflows/ci-verify.yml.gitignoreCONTRIBUTING.mdapp/api/api.test.tsapp/api/api.tsapp/api/container.test.tsapp/api/container/handlers/actions.tsapp/configuration/index.test.tsapp/configuration/index.tscontent/docs/current/configuration/server/index.mdxdocs/cucumber-reliability.mde2e/config/cucumber-core-fixtures.txte2e/features/api-container.featuree2e/features/api-v14.featuree2e/features/step_definitions/ui.jse2e/features/support/init.jse2e/features/support/state-restoration-hooks.jse2e/features/support/state-restoration.jse2e/features/ui.featuree2e/package.jsone2e/playwright.config.tse2e/playwright/navigation.spec.tse2e/playwright/v16-health-preferences.spec.tse2e/tests/state-restoration.test.jsscripts/cucumber-scope.test.mjsscripts/start-drydock.shscripts/start-drydock.test.mjstest/qa-compose.ymlui/src/layouts/AppLayout.vueui/tests/layouts/AppLayout.spec.ts
💤 Files with no reviewable changes (2)
- e2e/features/step_definitions/ui.js
- e2e/features/ui.feature
🚧 Files skipped from review as they are similar to previous changes (23)
- e2e/config/cucumber-core-fixtures.txt
- e2e/features/api-container.feature
- content/docs/current/configuration/server/index.mdx
- e2e/features/support/state-restoration-hooks.js
- app/api/container/handlers/actions.ts
- ui/tests/layouts/AppLayout.spec.ts
- e2e/features/api-v14.feature
- e2e/package.json
- app/configuration/index.ts
- .gitignore
- docs/cucumber-reliability.md
- app/configuration/index.test.ts
- ui/src/layouts/AppLayout.vue
- app/api/api.ts
- e2e/playwright/v16-health-preferences.spec.ts
- app/api/container.test.ts
- scripts/cucumber-scope.test.mjs
- CONTRIBUTING.md
- e2e/playwright/navigation.spec.ts
- app/api/api.test.ts
- scripts/start-drydock.test.mjs
- scripts/start-drydock.sh
- .github/workflows/ci-verify.yml
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 17:22
efb1159 to
9cf689f
Compare
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 17:50
9cf689f to
9c9cb68
Compare
scttbnsn
force-pushed
the
fix/cucumber-reliability
branch
from
July 22, 2026 18:11
9c9cb68 to
91b5641
Compare
biggest-littlest
approved these changes
Jul 22, 2026
biggest-littlest
left a comment
biggest-littlest
left a comment
Member
There was a problem hiding this comment.
Reviewed the Cucumber failure-history audit, reliability fixes, regression coverage, and exact-SHA CI. Cucumber 50/50 scenarios (402/402 steps) and Playwright 31 passed/2 skipped are green without blanket retries.
ALARGECOMPANY
approved these changes
Jul 22, 2026
ALARGECOMPANY
left a comment
ALARGECOMPANY
left a comment
Member
There was a problem hiding this comment.
Independent approval: the historical taxonomy, deterministic fixes, setup attribution, diagnostics, and repeated exact-SHA verification are complete. All required checks are green.
Merged
scttbnsn
added a commit
that referenced
this pull request
Jul 22, 2026
## Summary Prepare **v1.6.0-rc.4** from the latest `main`. rc.4 delta over rc.3: - **#580 issue #498 visual follow-ups** - informational pinned Major/Minor/Patch states, "Image update" vocabulary and explanation, Host-first constrained layout, explicit Software Version metadata, browser-managed grouping, docs, regression coverage, and desktop/mobile visual QA. - **#581 Cucumber reliability** - complete retained-run audit, exact build-artifact reuse, six-fixture readiness manifest, scenario state restoration, structured reports/diagnostics, no blanket Cucumber/Playwright retries, fail-closed remote fixture seeding, and the targeted health-refresh product/race fixes. - Release identity advanced across the changelog, README, website/demo, current docs, API examples, quickstart, and guard tests. ## Verification - 123/123 repository script tests - 45/45 workflow invariant tests - 44/44 website script tests - Full pre-push gate: 100% app/UI coverage, type-check, Biome, Qlty, and app/UI builds - Local prerelease precheck completed; its five unchecked "shipped in v1.6.0" Discussion follow-ups remain GA closeout actions and are not RC blockers After merge: verify exact-`main` CI Verify and Playwright, then dispatch `release-cut.yml` with `release_tag=v1.6.0-rc.4`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Changelog ### 🔧 Changed - Prepared release identity for `v1.6.0-rc.4` across README, website/demo mocks, API documentation, Portwing examples, quickstart materials, and release updates. - Updated changelog comparison links and release identity guard tests for `v1.6.0-rc.4`. - Added release-gated Cucumber E2E failure-reporting highlights to the updates page. ## Concerns - Verify exact CI Verify and Playwright workflows after merge. - Post the five tracked Discussion replies against the published release candidate. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 participants
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
Verification
The historical evidence, failure taxonomy, reliability policy, and follow-up threshold are in docs/cucumber-reliability.md.
Changelog
scripts/start-drydock.sh+e2e/config/cucumber-core-fixtures.txt).docs/cucumber-reliability.md) based on retained CI Verify runs.@restores_container_state/@restores_settings_state/@restores_notification_state.e2e/tests/state-restoration.test.js+ supporting runner/tests).main[data-route-name](and updated layout/unit tests accordingly).health-transitionunhealthy detection.e2e-playwright.ymlworkflow file).ci-verifyto run Cucumber browser-free: use a QA-provisioned Drydock image artifact (no pinned Playwright Docker image; nodocker runfor Cucumber).ci-verifydependency installation to use bounded retries (nick-fields/retry) only around dependency install/setup steps (not blanket scenario retries).DD_E2E_IMAGE, support build-skipping with image existence checks, and gate readiness via fixture identity + required image metadata fields.getContainers()before callingwatchContainer.cucumber-core-fixtures.txtadds core fixtures;/api/v1/containersminimum length updated; v1.4 scenarios tagged for state restoration).e2e-playwright.yml).e2e/features/step_definitions/ui.js) and eliminated prior blanket--retry 1scenario behavior.start-drydock.shreadiness logic doesn’t embed PEM-shaped private-key material (and uses safe dummy/private-key templating paths).Concerns
docs/cucumber-reliability.mdstatistics/taxonomy/policy claims still match the intended retained CI Verify dataset and lenses.scripts/start-drydock.shreadiness checks (default manifest + required image metadata fields likeimage.registry.url/image.tag.value) remain aligned with the currently active Cucumber fixture contracts.hub_nginx_120, notification ruleupdate-available) and whitelisted mutable fields still match the current API shape for all scenarios using@restores_*.ci-verify-workflow.test.ts,release-cut-retry-workflow.test.ts) match actual step names/conditions and diagnostics/artifact paths (e.g.,e2e/reports).retries: 0) and CI trace retention is sufficient to diagnose single amplified failures.DD_SERVER_RATELIMIT_MAX=10000andDD_ICON_PROXY_RATE_LIMIT_MAX=1000should be applied only to the intended ephemeral/QA release-gate stack.