Skip to content

test(ci): cover spritz local smoke#224

Merged
onutc merged 1 commit intomainfrom
codex-spritz-ci-smoke-coverage
Apr 18, 2026
Merged

test(ci): cover spritz local smoke#224
onutc merged 1 commit intomainfrom
codex-spritz-ci-smoke-coverage

Conversation

@onutc
Copy link
Copy Markdown
Member

@onutc onutc commented Apr 18, 2026

Summary

  • add the ACP smoke script syntax checks to the e2e workflow
  • run the checked-in local smoke harness in CI without changing the local script
  • keep Peter's local flow unchanged while making the same smoke path enforceable in GitHub Actions

Validation

  • node --test e2e/*.test.mjs
  • node --check e2e/acp-smoke-lib.mjs
  • node --check e2e/acp-client.mjs
  • node --check e2e/instance-waiter.mjs
  • node --check e2e/acp-smoke.mjs
  • ./e2e/local-smoke.sh

@onutc onutc merged commit 549f237 into main Apr 18, 2026
2 checks passed
@onutc onutc deleted the codex-spritz-ci-smoke-coverage branch April 18, 2026 17:17
@gitrank-connector
Copy link
Copy Markdown

👍 GitRank PR Analysis

Score: 5 points

Metric Value
Component Other (1× multiplier)
Severity P3 - Low (5 base pts)
Final Score 5 × 1 = 5

Eligibility Checks

Check Status
Issue/Bug Fix
Fix Implementation
PR Documented
Tests ✅ (not required)
Lines Within Limit

Impact Summary

The PR adds syntax validation for ACP smoke test scripts to the e2e-unit-tests workflow and creates a new local-smoke-tests workflow that runs integration smoke tests in CI. This ensures code quality and prevents regressions in the smoke testing harness without changing the local development flow. The changes are purely infrastructure-focused with no modifications to production code.

Analysis Details

Component Classification: This PR modifies CI/CD workflows and adds smoke test infrastructure, which falls under miscellaneous/operational changes rather than a specific functional component.

Severity Justification: This is a low-severity contribution as it adds testing infrastructure and CI validation without fixing a critical bug or addressing a security vulnerability. It improves development practices but has minimal direct user impact.

Eligibility Notes: Tests are not required for this change type because it is a CI/CD configuration and workflow addition. The PR does include test execution steps within the workflows themselves, demonstrating validation. No issue reference is present, but the PR is not fixing a reported bug—it's adding preventive testing infrastructure. The implementation aligns with the stated goals of enforcing smoke test syntax checks and running local smoke tests in GitHub Actions.


Analyzed by GitRank 🤖

@onutc
Copy link
Copy Markdown
Member Author

onutc commented Apr 18, 2026

Final report:\n\n- Added the ACP smoke script coverage to CI.\n- Added a dedicated workflow that runs kind not found; downloading to /var/folders/82/1lv7n6lj01z3dvxrgdn3sryh0000gn/T//kind
node/spritz-e2e-control-plane condition met
namespace/spritz-system created
namespace/spritz created
customresourcedefinition.apiextensions.k8s.io/spritzbindings.spritz.sh created
customresourcedefinition.apiextensions.k8s.io/spritzconversations.spritz.sh created
customresourcedefinition.apiextensions.k8s.io/spritzes.spritz.sh created
warming go module cache...
waiting for API on port 8090...
waiting for spritz to become Ready...
deleted (204)
done (logs in /var/folders/82/1lv7n6lj01z3dvxrgdn3sryh0000gn/T//spritz-e2e-logs) unchanged.\n- Local validation passed: ✔ connectACPWebSocket correlates RPC responses and collects session updates (37.009583ms)
✔ connectACPWebSocket times out RPC calls without a response (1003.37875ms)
✔ withACPInstanceClient always closes the websocket client and stops the port-forward (1.010333ms)
✔ runACPInstancePrompt delegates the ACP flow through the instance client owner (0.845583ms)
✔ resolveSpzCommand prefers explicit binary env override (1.453375ms)
✔ resolveSpzCommand prefers the checked-out CLI before any global spz on PATH (0.0795ms)
✔ parsePresetList normalizes comma-delimited values (0.12225ms)
✔ parseSmokeArgs requires explicit presets instead of assuming example ids (0.22675ms)
✔ parseSmokeArgs normalizes provided preset ids (0.06325ms)
✔ parseSmokeArgs prefers the smoke namespace env over the generic namespace env (0.065041ms)
✔ parseSmokeArgs requires an explicit smoke api url and bearer token (0.082875ms)
✔ buildSmokeSpzEnvironment strips ambient spritz auth and profile state (0.328083ms)
✔ extractACPText flattens nested content blocks (0.096125ms)
✔ extractACPText falls back to resource uri when text is empty (0.083791ms)
✔ joinACPTextChunks preserves chunked tokens without inserted separators (0.064ms)
✔ buildIdempotencyKey and smoke token normalize preset names (0.083708ms)
✔ resolveACPEndpoint prefers discovered ACP port/path and normalizes the path (0.066166ms)
✔ resolveWebSocketConstructor returns a usable client constructor (0.058125ms)
✔ assertSmokeCreateResponse accepts canonicalized preset ids from the API (0.074208ms)
✔ isForbiddenFailure only accepts explicit forbidden command failures (0.083167ms)
✔ runCommand marks timed-out child processes (31.868708ms)
✔ runCommand escalates to SIGKILL when the child ignores SIGTERM (28.603917ms)
✔ waitForWebSocketOpen rejects and closes on handshake timeout (1002.557208ms)
✔ waitForWebSocketOpen resolves when the socket opens (2.282375ms)
✔ summarizeInstanceFailure prioritizes shared mount init failures (1.165083ms)
✔ summarizeInstanceFailure reports image pull failures distinctly (0.085458ms)
✔ waitForInstance returns the ready instance payload and discovered ACP endpoint (1.954333ms)
✔ waitForInstance recovers from transient kubectl polling errors (1.218916ms)
✔ waitForInstance fails with the last staged failure summary on timeout (9.470833ms)
ℹ tests 29
ℹ suites 0
ℹ pass 29
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 1147.818417, all four commands, and kind not found; downloading to /var/folders/82/1lv7n6lj01z3dvxrgdn3sryh0000gn/T//kind
port 8090 in use; switched to 49182
node/spritz-e2e-control-plane condition met
namespace/spritz-system created
namespace/spritz created
customresourcedefinition.apiextensions.k8s.io/spritzbindings.spritz.sh created
customresourcedefinition.apiextensions.k8s.io/spritzconversations.spritz.sh created
customresourcedefinition.apiextensions.k8s.io/spritzes.spritz.sh created
warming go module cache...
waiting for API on port 49182...
waiting for spritz to become Ready...
deleted (204)
done (logs in /var/folders/82/1lv7n6lj01z3dvxrgdn3sryh0000gn/T//spritz-e2e-logs).\n- GitHub Actions passed on the PR: and the new workflow.\n\nNote: local The added syntax-check step is fine, but the new local smoke workflow does not trigger for one of the API modules it actually compiles. That gap leaves real regressions untested, so the patch is not fully correct.

Review comment:

  • [P2] Trigger local smoke tests for ACP integration changes — /Users/onur/repos/spritz/.github/workflows/local-smoke-tests.yml:6-9
    When a PR only touches integrations/acptext/**, this workflow will be skipped even though ./e2e/local-smoke.sh launches go run . in api, and api/go.mod currently replaces spritz.sh/acptext with ../integrations/acptext. That means ACP integration regressions can land without ever running the new smoke job. Add that path to the pull_request and push filters so the workflow runs for all code it actually builds. could not be completed in this environment because the local Codex auth was broken ( / ).

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.

1 participant