Skip to content

GitHub workflow maintenance - #175

Merged
floitsch merged 1 commit into
mainfrom
ecosystem-review/db2c56eebf86-workflows
Oct 2, 2026
Merged

floitsch merged 1 commit into
mainfrom
ecosystem-review/db2c56eebf86-workflows

Conversation

@floitsch

@floitsch floitsch commented Oct 2, 2026

Copy link
Copy Markdown
Member

Scope

Part 2 of 2 of the prepared package review.

  • .github/workflows/ci.yml

  • .github/workflows/publish.yml

  • Fork pull requests do not trigger CI and the SDK minimum is duplicated: Add pull_request and resolve the oldest matrix SDK from package.yaml. Preserve the OS matrix, version labels, browser tests, and Linux Docker httpbin setup. Preserve checkout@v7 after verifying its upstream node24 action exists at https://raw.githubusercontent.com/actions/checkout/v7/action.yml. Dependabot is configured; the stale publishing action name is fixed in a separate finding.

  • Publishing references a renamed action repository: Use toitlang/action-publish@v1.5.0, preserving the release job and tag triggers. The local package skill template still uses the obsolete name; the verified canonical action takes precedence.

Related PR groups

  1. Code, tests and build changes — branch ecosystem-review/db2c56eebf86-implementation
  2. GitHub workflow maintenance — branch ecosystem-review/db2c56eebf86-workflows

Review/merge earlier groups first where their changes are required. This branch contains only this group; it does not include earlier groups. CI may need to be rerun after earlier PRs merge.

All branches start at reviewed commit dbe7effdb578273b940b2cd789ecd0ca0a588d85 and target main.

Validation

Each split patch is checked to apply independently to the reviewed base, and their combined tree must equal the full reviewed patch. The runtime tests below were run for the combined proposal, not independently for this split branch.

  • toit pkg install (tests and examples) — passed: Installed declared dependencies after sandbox cache/network escalation. Logs: logs/toitlang--pkg-http-install-escalated.log and logs/toitlang--pkg-http-examples-install.log. Restored installation-only lockfile changes.
  • ctest --test-dir build -j4 --output-on-failure -E "google|webdriver" (before fixes) — passed: 13/13 original local tests passed with alpha.199 after enabling localhost socket access. Initial sandbox execution was blocked from opening sockets. Log: logs/toitlang--pkg-http-baseline-escalated.log.
  • New regression tests on the original affected implementations — failed: Expected failures established case-sensitive header removal, over-read, ambiguous/negative lengths, writer overflow, cross-origin credentials, response framing, chunk metadata, semaphore double-release, empty continuation truncation, and missing terminal mask. Baseline logs: regressions-before, framing-before, websocket-before, redirect-before, response-before, and probes under logs/toitlang--pkg-http-*.
  • ctest --test-dir build -j4 --output-on-failure (alpha.199, ENABLE_HTTPBIN_TESTS=OFF) before the final URI/masking follow-up — passed: 20/20 tests passed, including deterministic regressions, localhost/retry/finalizer/concurrency tests, Chrome and Firefox browser interoperability, and both live Google TLS tests. Log: logs/toitlang--pkg-http-tests-final.log.
  • make test (with ENABLE_HTTPBIN_TESTS=OFF in the configured build) — passed: The documented entry point installed packages, reconfigured CMake, and passed all 21 available tests on alpha.199, including the final URI/masking fixes, Chrome, Firefox, and live TLS. Log: logs/toitlang--pkg-http-followup-make-test.log. Restored generated tests/package.lock changes afterward.
  • ctest --test-dir build/alpha190 -j4 --output-on-failure -E "google|webdriver" — passed: 17/17 local tests passed using official Linux SDK v2.0.0-alpha.190, including the final URI/masking changes and regressions. Log: logs/toitlang--pkg-http-followup-alpha190-tests.log. SDK available at sdks/alpha.190/toit.
  • toit analyze src/*.toit; (cd tests && toit analyze *.toit); toit analyze examples/*.toit — passed: Final source, test helpers, and examples analyze without errors. Log: logs/toitlang--pkg-http-followup-analysis.log. Earlier incorrectly combined project analysis was corrected by running each project separately.
  • toit pkg describe; parse CI YAML and execute its oldest/latest resolver; git diff --check — passed: Manifest recognized as HTTP with MIT license and alpha.190 minimum. YAML parsed, resolver emitted v2.0.0-alpha.190 and latest, and diff whitespace check passed. Logs: logs/toitlang--pkg-http-describe.log and logs/toitlang--pkg-http-final-checks.log.
  • docker info --format "{{.ServerVersion}}" — blocked: No Docker daemon socket exists even after escalation. The Docker httpbin integration test was not run; ENABLE_HTTPBIN_TESTS=OFF was retained for local validation. Log: logs/toitlang--pkg-http-docker.log.
  • Deterministic baseline probes for URI/masking issues — passed: Historical probes reproduced both issues before the follow-up fixes. Logs/source: logs/toitlang--pkg-http-remaining-probes.log and logs/toitlang--pkg-http-remaining-probes.toit. Both findings are now fixed and covered by passing regressions.
  • New URI and masking regressions before and after fixes — passed: Expanded parse-url-test fails with ILLEGAL_HOSTNAME before the fix; websocket-masking-test fails its fresh-key assertion on the constant-zero encoder. Final tests pass on alpha.199 and alpha.190. Logs: logs/toitlang--pkg-http-followup-before.log, logs/toitlang--pkg-http-followup-focused.log, and final suite logs.
  • Run framing regressions with the external runner’s extra SDK argument on alpha.199 and alpha.190 — passed: Both regression entry points run all cases successfully even when passed an extra executable-path argument. Log: logs/toitlang--pkg-http-external-arguments.log.
  • Parse publishing workflow YAML and inspect the canonical action reference; git diff --check — passed: The workflow uses action-publish@v1.5.0 with both existing tag patterns. Parent verified canonical repository/tag; GitHub rename documentation independently confirms action redirects are unsupported. Log: logs/toitlang--pkg-http-publish-validation.log. No publication was executed.
  • Run every cleanup-test.toit --case against an isolated snapshot of the previously prepared source — failed: Expected baseline failures: all 23 cases fail before this follow-up, including socket leaks, stranded writer state, nullable response cleanup, detach atomicity, preserved handler exceptions, and close interleavings. Snapshot: logs/toitlang--pkg-http-cleanup-before-source/. Detailed failures: logs/toitlang--pkg-http-cleanup-before.log. The working tree was never reset.
  • ctest --test-dir build -j4 --output-on-failure -E "google|webdriver" (alpha.199) — passed: Final 18/18 local tests passed, including all 23 new cleanup cases, previous wire-format/masking/framing/redirect tests, local WebSocket client/server concurrency, server lifecycle/retry tests and finalizers. Log: logs/toitlang--pkg-http-cleanup-alpha199-escalated-tests.log. Local socket access required escalation; initial sandbox failures were Operation not permitted, recorded separately.
  • ctest --test-dir build/alpha190 -j4 --output-on-failure -E "google|webdriver" — passed: Final 18/18 local tests passed on the declared minimum alpha.190 using the shared official SDK. Same focused scope as alpha.199. Log: logs/toitlang--pkg-http-cleanup-alpha190-escalated-tests.log.
  • toit run tests/cleanup-test.toit; toit analyze src/*.toit; toit analyze tests/cleanup-test.toit; git diff --check — passed: All 23 deterministic cases pass. The synthetic handler exception is intentionally traced by Server.run-connection_ and caught/asserted by the regression; it is not a test failure. Source and regression analysis and whitespace checks pass. Logs: logs/toitlang--pkg-http-cleanup-focused.log and logs/toitlang--pkg-http-cleanup-analysis.log.

Limits of the overall review

  • This is not a full HTTP/WebSocket conformance or fuzzing audit. Malformed header/control-frame validation and every possible transport exception or cancellation interleaving are not exhaustively verified; the cleanup follow-up adds targeted fault and concurrency coverage.
  • Docker/httpbin was unavailable. Windows/macOS jobs and hosted GitHub Actions were inspected but not executed locally.
  • ESP32 operation, RTC session resumption, self-signed TLS example execution, and hardware behavior were not exercised. The minimum SDK was tested on Linux only.
  • Duplicate Content-Length fields, including equal duplicate values, are now rejected deliberately rather than normalized. Applications relying on forwarding credentials across origins must set them explicitly through an appropriate higher-level policy.
  • Mask entropy follows the SDK crypto.random implementation. Its documentation states that ESP32 hardware randomness requires the WiFi or Bluetooth RF subsystem; hardware entropy quality was not tested here.
  • The cleanup follow-up reran the relevant local suites on alpha.190 and alpha.199. Browser/live-TLS/httpbin tests were not broadened or repeated; earlier browser and live-TLS results above predate the cleanup changes. Released HTTP 2.9.0 and 2.15.0 remain affected until these local fixes are published.

@floitsch
floitsch marked this pull request as ready for review October 2, 2026 16:53
@floitsch
floitsch merged commit e7f7817 into main Oct 2, 2026
13 checks passed
@floitsch
floitsch deleted the ecosystem-review/db2c56eebf86-workflows branch October 2, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant