Repository navigation
GitHub workflow maintenance - #175
Merged
Merged
Conversation
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
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.
Scope
Part 2 of 2 of the prepared package review.
.github/workflows/ci.yml.github/workflows/publish.ymlFork 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
ecosystem-review/db2c56eebf86-implementationecosystem-review/db2c56eebf86-workflowsReview/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
dbe7effdb578273b940b2cd789ecd0ca0a588d85and targetmain.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