Conversation
…SM build, security audit permissions Agent-Logs-Url: https://github.com/knitli/thread/sessions/7cdcccda-a218-4dce-9fa7-3367d0eed087 Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Agent-Logs-Url: https://github.com/knitli/thread/sessions/7cdcccda-a218-4dce-9fa7-3367d0eed087 Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Reviewer's GuideResolves multiple CI failures and a license compliance issue by adjusting doctest imports, splitting nextest runs by OS to avoid macOS Docker limitations, stubbing C stdio for wasm32 builds in CI, granting additional permissions to the security audit job, and extending REUSE annotations to cover previously unannotated files. Flow diagram for OS-specific nextest runs and WASM stdio stub setupflowchart TD
A["Start tests job"] --> B{runner.os}
B -->|runner.os != macOS| C["Run cargo nextest run --workspace --exclude thread-language --all-features --no-fail-fast"]
C --> D["Run cargo nextest run -p thread-language --features all-parsers,matching --no-fail-fast"]
B -->|runner.os == macOS| E["Run cargo nextest run --workspace --exclude thread-language --all-features --no-fail-fast -E not (package(thread-flow) and test(postgres))"]
E --> F["Run cargo nextest run -p thread-language --features all-parsers,matching --no-fail-fast"]
subgraph wasm_build["WASM build job"]
G["Install clang"] --> H["Compute CLANG_RES = clang --print-resource-dir"]
H --> I["Create /tmp/wasm-stubs directory"]
I --> J["Write minimal stdio.h stub\n(FILE typedef, stdin/stdout/stderr, fprintf, vfprintf, printf no-ops)"]
J --> K["Append CFLAGS_wasm32_unknown_unknown=-I/tmp/wasm-stubs -I${CLANG_RES}/include to GITHUB_ENV"]
K --> L["Install wasm-pack"]
L --> M["Build WASM artifacts using wasm-pack"]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple CI failures by updating workflow logic, ensuring Rust doctests compile, and expanding REUSE license annotations to cover previously unannotated paths/files.
Changes:
- Fix a doctest compilation failure by importing the
Doctrait in the doctest preamble. - Adjust CI to handle macOS runner limitations (Docker unavailable) and to unblock WASM builds by providing a minimal
stdio.hstub forwasm32-unknown-unknown. - Extend
REUSE.tomlannotations to cover additional directories/files missing SPDX headers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
REUSE.toml |
Adds aggregate SPDX annotations for additional paths/files to satisfy REUSE compliance without editing each file. |
crates/ast-engine/src/tree_sitter/mod.rs |
Updates doctest preamble to import the Doc trait so root_node() resolves. |
.github/workflows/ci.yml |
Splits nextest execution by OS (macOS vs others), adds WASM C header stub setup, and grants checks: write to the security audit job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
|
@copilot we still have quite a few failures across the board here: https://github.com/knitli/thread/actions/runs/23946130256/job/69844387826?pr=140 -- please resolve |
…ense files, WASM C stubs Agent-Logs-Url: https://github.com/knitli/thread/sessions/af60cbe1-26cf-4b98-bc5d-e1377d4159a6 Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Addressed in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
bashandbone
left a comment
bashandbone
left a comment
There was a problem hiding this comment.
Beautiful. Nice work.
Summary
Diagnoses and resolves CI failures on the main branch across multiple platforms and targets.
Failures & Fixes
1. Doctest failure (ubuntu stable / beta / nightly)
Root cause:
StrDoc::root_node()is a method from theDoctrait, but the trait wasn't imported in the doctest scope.Fix: Added
# use thread_ast_engine::Doc;to the doctest hidden preamble.2.
thread-flowdoctest failures (all platforms)Root cause: Fixing the
thread-ast-enginedoctest exposed 5 pre-existing doc-compilation errors inthread-flow: aRapishSettypo (should beRapidSet), three uses ofRapidSet::from([...])which doesn't work with the customrapidhash::RandomState, and astd::collections::HashSetpassed to a method that expects&RapidSet<PathBuf>.Fix: Removed the typo import; changed array-init to
[...].into_iter().collect(); replacedstd::collections::HashSetwithRapidSetin the affected doctests.3. macOS postgres tests (macos-latest stable)
Root cause:
incremental_postgres_testsusestestcontainersto spin up Docker containers. macOS GitHub Actions runners do not include Docker.Fix: Split the nextest step into two conditional steps. On macOS, pass
-E "not (package(thread-flow) and binary(incremental_postgres_tests))"to skip the Docker-dependent tests by binary name. Linux and Windows runners continue to run the full suite.4. License compliance (REUSE)
Root cause: 35 files in
.claude/plugins/ctx/,ctx-plugin/,.vscode/,crates/flow/benches/,get_comments.js, andspecs/were missing SPDX copyright/license headers. REUSE.toml glob annotations fixed.claude/plugins/ctx/and.vscode/paths, but thectx-plugin/**glob annotation was not honoured byfsfe/reuse-action@v2.Fix: Added
[[annotations]]entries toREUSE.tomlfor the paths that work, and added.licensesidecar files (the project's established pattern) for the 19 remaining non-compliant files.5. WASM build failure
Root cause:
tree-sitter 0.25.10C sources include multiple standard C library headers (stdio.h,stdlib.h,string.h,time.h,ctype.h,wctype.h,assert.h). Thewasm32-unknown-unknowntarget has no OS and therefore no libc headers.Fix: Added a CI step that uses Python to generate minimal stubs for all 7 required headers and sets
CFLAGS_wasm32_unknown_unknownto include them alongside clang's built-in resource headers. Functions likemalloc/free/memcpyare declared (not stubbed out) so the wasm runtime provides real implementations at link time.6. Security audit permission error
Root cause:
rustsec/audit-check@v1.4.1needschecks: writeto post annotations back to GitHub. The workflow's globalpermissions: contents: readwas too restrictive.Fix: Added a job-level
permissionsblock tosecurity_auditwithchecks: write.Changes
crates/ast-engine/src/tree_sitter/mod.rs— addDocimport to doctestcrates/flow/src/incremental/graph.rs— fixRapidSet::from([...])in two doctestscrates/flow/src/incremental/invalidation.rs— removeRapishSettypo importcrates/flow/src/incremental/mod.rs— replaceHashSetwithRapidSetin doctestcrates/flow/src/incremental/types.rs— fixRapidSet::from([...])in doctestREUSE.toml— add[[annotations]]blocks for paths where glob matching workscrates/flow/benches/bench_graph_traversal.rs.license+ 18 other.licensesidecar files — REUSE compliance for remaining non-compliant files.github/workflows/ci.yml— conditional nextest steps (macOS binary filter), comprehensive WASM C header stubs, security_audit permissions