Conversation
… configuration resolution
…ved artifact handling
… README instructions
Created a `docs/` directory with the following descriptive markdown files: - `README.md`: Overview and index. - `getting-started.md`: Basic usage guide. - `configuration.md`: Detailed inputs and environment variable reference. - `architecture.md`: Explanation of the modular design and sub-actions. - `stacks.md`: Information on supported stacks (Wails v2, Deno). Also updated the root `README.md` to include a link to the new documentation directory.
Add descriptive markdown documentation
- Add .actrc with Docker image defaults for M3/ARM64 - Add Makefile with act test targets and local build targets - Add CLAUDE.md documenting the pipeline pattern - Add build-wails2 CI job that actually compiles (build: true) - Add TDD hello world fixtures: go-cli, go-http, node-http, deno-http, cpp-hello Co-Authored-By: Claude Opus 4.5 <[email protected]>
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughModular v3 redesign: root action now performs discovery and delegates to a directory orchestrator and stack wrappers (e.g., Wails v2). New sub-actions implement discovery, options, setup, build, sign, and package. Extensive CI self-tests, reusable workflows, Makefile, docs, and test fixtures added; old build-tests workflow removed and act defaults adjusted. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Root as Root Action
participant Disc as Discovery
participant Dir as Directory Orchestrator
participant Stack as Stack Wrapper (wails2)
participant Setup as Setup Toolchains
participant Build as Build Sub-action
participant Sign as Sign Action
participant Pack as Package Action
User->>Root: invoke action (inputs/env)
Root->>Disc: run discovery (detect markers, OS/ARCH)
Disc-->>Root: discovery outputs (PRIMARY_STACK_SUGGESTION, OS, ARCH, REF)
Root->>Dir: delegate (AUTO_STACK, STACK, PRIMARY_STACK_SUGGESTION)
Dir-->>Root: SELECTED_STACK
Root->>Stack: call selected stack wrapper
Stack->>Setup: provision toolchains (Go/Node/Deno)
Setup-->>Stack: toolchains ready
Stack->>Build: run build (wails build)
Build-->>Stack: artifacts produced
Stack->>Sign: optional sign/notarize (on tag & sign=true)
Sign-->>Stack: signed artifacts
Stack->>Pack: package & upload artifacts (compute ARTIFACT_NAME)
Pack-->>User: artifacts + release (on tag)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
✅ Pre-merge checks override appliedThe pre-merge checks have been overridden successfully. You can now proceed with the merge. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 34
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 305-314: The workflow step that invokes the root action (the block
containing "uses: ./", with inputs like build-name, build-platform,
app-working-directory, build, package, sign, nsis) passes an invalid input nsis:
false; remove the nsis input from that inputs list so only supported inputs are
provided. Locate the step near the "uses: ./" invocation (the same step that
precedes the "Assert orchestrator selected wails2" step) and delete the line
"nsis: false" to fix the invalid input error.
- Around line 1-9: Add a workflow-level permissions block to the top of the
"Build Action self-tests" workflow to restrict GITHUB_TOKEN scope; edit the
workflow that begins with "name: Build Action self-tests" and insert a top-level
permissions stanza (immediately under the name/on keys) specifying only the
least-privilege scopes required for your jobs (e.g., contents: read and any
other specific scopes your jobs need) so the GITHUB_TOKEN is not granted broad
default permissions.
- Around line 227-234: The workflow is passing an undefined input "nsis" to the
root action './' — either remove the "nsis: false" line from the job inputs or
add "nsis" as a declared input on the root action and forward it through the
intermediate action(s) to the wails2 stack; if choosing to propagate, add an
"nsis" input to the root action metadata and to ./actions, then pass that input
down to ./actions/build/wails2 so the final action (./actions/build/wails2)
receives and uses the nsis value.
In @.github/workflows/wails2.yml:
- Around line 95-110: The workflow currently hardcodes app-working-directory to
"tdd/wails2-root" instead of using the provided input; update the step where
inputs are passed (the block containing build, sign, package, nsis, etc.) to set
app-working-directory: ${{ inputs.app-working-directory }} so the input is
honored and callers can override the working directory; ensure there are no
conflicting hardcoded values elsewhere in the same job.
In `@action.yml`:
- Around line 6-42: The action.yml is missing an nsis input that the workflows
pass, causing undefined-input errors; add a new input named nsis under inputs
(e.g., nsis: description: "Enable NSIS installer support", required: false,
default: "false") and then propagate this input through the delegation to the
directory orchestrator (ensure the delegation step that forwards inputs includes
nsis in its with/inputs map so the orchestrator and any functions/classes
handling inputs receive nsis).
In `@actions/action.yml`:
- Around line 73-82: The workflow currently only runs the "Call Wails v2
wrapper" step when steps.sel.outputs.SELECTED_STACK == 'wails2', causing
cpp/unknown stacks to be silently skipped; add a fallback step that triggers
when SELECTED_STACK is not 'wails2' (e.g., if: steps.sel.outputs.SELECTED_STACK
!= 'wails2') that either fails the job with an explicit error message or runs a
no-op/placeholder action that logs "unsupported/ unimplemented stack: ${{
steps.sel.outputs.SELECTED_STACK }}", and ensure the existing step (uses:
./actions/build/wails2) remains unchanged so wails2 continues to run only for
that stack.
- Around line 3-44: The action.yml currently has empty description values for
multiple inputs which hurts discoverability; update the description for each
input (build, sign, package, build-name, build-platform, app-working-directory,
AUTO_STACK, AUTO_SETUP, STACK, PRIMARY_STACK_SUGGESTION) to concise, user-facing
sentences that explain purpose, expected values and defaults (e.g., "Whether to
run the build step (true/false)", "Name of the build artifact", "Target
platform, e.g., darwin/universal"), ensuring each input's description string is
non-empty and accurate to its behavior.
In `@actions/build/README.md`:
- Around line 12-26: In the "Usage examples" section, the two fenced YAML code
blocks need blank lines before and after each fence to satisfy MD031; update the
README.md so there is a blank line immediately above the first ```yaml fence and
a blank line immediately below its closing ```, and likewise add a blank line
above the second ```yaml fence and below its closing ```, ensuring both YAML
examples around the fenced blocks are separated by empty lines for proper
Markdown linting.
In `@actions/build/wails2/action.yml`:
- Around line 136-162: The signing inputs are never read because the resolver
only uses EV_* env vars; update the resolver to prefer action inputs using the
existing pick helper (and tobool for booleans) when populating signing
variables. Replace the direct env-only assignments for MAC_PW, MAC_ID, MAC_CERT,
MAC_CERT_PW, MAC_INS_ID, MAC_INS_CERT, MAC_INS_CERT_PW, WIN_CERT and WIN_CERT_PW
with calls to pick that choose between the corresponding IN_* input variables
(e.g. IN_SIGN_MACOS_APPLE_PASSWORD, IN_SIGN_MACOS_APP_ID,
IN_SIGN_MACOS_APP_CERT, IN_SIGN_MACOS_APP_CERT_PASSWORD,
IN_SIGN_MACOS_INSTALLER_ID, IN_SIGN_MACOS_INSTALLER_CERT,
IN_SIGN_MACOS_INSTALLER_CERT_PASSWORD, IN_SIGN_WINDOWS_CERT,
IN_SIGN_WINDOWS_CERT_PASSWORD) and the existing EV_* fallbacks so inputs are
honored when provided.
- Around line 31-70: The action.yml currently sets non-empty defaults (e.g.,
wails-version, go-version, deno-version, node-version, etc.) which causes the
shell resolver's pick() to always return the input value and block ENV
overrides; change those inputs' default values to empty strings in action.yml
(e.g., set default: "") and then update the shell resolver logic around the
pick() calls so it performs true inputs>env>defaults resolution: call pick(IN_*)
but if the returned value is empty, fallback to the corresponding EV_*
environment variable (e.g., check IN_WAILS_VERSION then EV_WAILS_VERSION), and
finally apply any hardcoded default only if both are empty; reference the pick()
usage and the IN_*/EV_* variables (e.g., IN_WAILS_VERSION, EV_WAILS_VERSION,
IN_GO_VERSION, EV_GO_VERSION, IN_DENO_VERSION, EV_DENO_VERSION) when making
these changes.
In `@actions/build/wails2/README.md`:
- Around line 9-23: The README contains multiple fenced code blocks that lack
blank lines before and after (MD031); update each fenced block (e.g., the "Usage
(recommended)" YAML block and the other fenced blocks around the examples) by
inserting a single blank line immediately before the opening ``` and a single
blank line immediately after the closing ``` to satisfy MD031 for all fenced
code blocks in the file.
- Around line 3-5: Update the guidance text in actions/build/wails2/README.md so
it no longer recommends pinning to v2.9.0; replace the note that suggests
`wails-version: "v2.9.0"` with guidance to upgrade to the current stable v2.11.0
or, if remaining on the 2.10.x line, to at least v2.10.1 (which fixes the
listenerOff and Darwin/universal build issues); ensure the line containing the
string `wails-version: "v2.9.0"` is changed and the note briefly explains these
two valid options.
In `@actions/discovery/action.yml`:
- Around line 128-131: The two find invocations are inconsistent: the check that
sets has_sub_npm uses find with -maxdepth 2 -mindepth 2 (only depth 2) while the
check that sets has_sub_mkdocs uses -maxdepth 2 -mindepth 1 (depth 1 or 2);
either align both to the same depth semantics or add an inline comment
explaining why mkdocs should include depth 1 but package.json must be depth 2.
Update the find invocation(s) accordingly or insert a clarifying comment next to
the find commands referencing the variables has_sub_npm and has_sub_mkdocs so
future readers know the intentional depth choice.
- Around line 180-181: The PowerShell subtree searches are inconsistent: the
Get-ChildItem call that looks for package.json uses a Where-Object filter to
exclude node_modules while the mkdocs.yml search does not; the bash
implementation doesn't exclude node_modules for either. To align behavior with
bash, remove the node_modules exclusion from the package.json search (the
Get-ChildItem pipeline that sets $has_sub_npm) so both Get-ChildItem calls (the
package.json and mkdocs.yml pipelines) have the same recursion and filtering
semantics.
- Around line 25-34: Currently the script exits on unknown DISTRO (the else
branch that echoes "Unsupported Linux distribution: $DISTRO" and runs exit 1);
change this to a graceful fallback: log a warning, attempt to install using the
latest known package set (reuse the 24.04 install command that includes
libwebkit2gtk-4.1-dev and gcc-aarch64-linux-gnu), and continue instead of
exiting, or optionally skip installations but do not call exit 1. Update the
else branch that handles DISTRO to emit a clear warning and run the fallback
install command (or a no-op path that logs skipping installs) so future Ubuntu
versions (e.g., 26.04) won’t break CI.
In `@actions/options/action.yml`:
- Around line 45-57: The composite action uses Bash 4+ associative arrays
(declare -A seen) which fail on macOS runners; replace the associative array
with a portable string-based set: remove declare -A seen and instead use a
variable like seen="" and inside the for-loop check membership with a pattern
match (e.g. if [[ -n "$t" && " $seen " != *" $t "* ]]; then seen="$seen $t";
norm_tags+=" $t"; fi), preserving the existing variables norm_tags, raw and the
loop behavior so duplicate tags are filtered without Bash 4 features.
In `@actions/options/README.md`:
- Around line 16-31: The fenced YAML code block in the README.md triggers
markdownlint rule MD031 because there are no blank lines surrounding the
triple-backtick fence; update the README.md to add one blank line before the
opening ```yaml and one blank line after the closing ``` so the code block is
separated from surrounding text, then commit the change (target the fenced block
shown under "Usage" in actions/options/README.md).
In `@actions/package/action.yml`:
- Around line 141-149: The upload-artifact step (uses:
actions/upload-artifact@v4) always includes artifact_meta.json in its path but
that file is only produced when inputs.include-meta == 'true'; update the
workflow so artifact_meta.json is only included when include-meta is true—either
split the upload into two steps (one for binaries and one conditional on
inputs.include-meta that uploads artifact_meta.json) or make the whole upload
step conditional on inputs.include-meta for metadata, ensuring the step that
references artifact_meta.json only runs when inputs.include-meta == 'true' to
avoid missing/stale-file errors.
- Around line 150-157: The Release step uses softprops/action-gh-release@v1 and
unconditionally includes artifact_meta.json; update the action to
softprops/action-gh-release@v2 and make inclusion of artifact_meta.json
conditional to inputs.include-meta == 'true'. Locate the Release job/step named
"Release" (and the upload artifact step that lists files/globs) and change the
uses from softprops/action-gh-release@v1 to softprops/action-gh-release@v2, and
modify the files list or the step logic so artifact_meta.json is only added when
inputs.include-meta == 'true' (or use a glob that safely ignores the file when
absent). Ensure the same conditional change is applied to the upload-artifact
step that currently references artifact_meta.json.
In `@actions/package/README.md`:
- Around line 18-30: The README.md contains a fenced YAML code block that lacks
blank lines before and after, causing markdownlint MD031; update the README.md
content around the shown fenced block so there is a blank line immediately
before the opening ```yaml and a blank line immediately after the closing ``` to
satisfy MD031 and preserve the existing YAML snippet and comments.
- Around line 15-16: Change the hyphenated adjective "auto-filled" to the single
word "autofilled" in the README text that describes optional fields (the line
containing "Optional (auto-filled when using the root action/wrapper): `os`,
`arch`, `tag`, `short-sha`, `ref`") so it reads "Optional (autofilled when using
the root action/wrapper): …"; no other semantic changes required to the
surrounding `include-meta` description.
In `@actions/setup/conan/action.yml`:
- Around line 15-25: In the "Install Conan via pip" run step, after installing
with $PIP, append the per-user script directory to GITHUB_PATH so the installed
conan is on PATH: detect OS and add ~/.local/bin for Linux/macOS and
%APPDATA%\Python\Scripts for Windows (use the appropriate echo >> $GITHUB_PATH
approach), keeping the existing $PIP/pip3 detection and inputs.version logic
intact so subsequent steps can invoke the conan command.
In `@actions/setup/deno/action.yml`:
- Around line 24-36: The case statement uses the Bash-4-specific expansion
${deno_enable,,} which breaks on macOS Bash 3.2; convert deno_enable to
lowercase in a portable way before the case (e.g., assign a lowercased version
of deno_enable using a POSIX-compatible command such as piping through tr or
printf into tr) and then use that lowercased variable in the case statement so
the true|1|yes|on checks work on macOS runners; update references to use the new
lowercased variable instead of ${deno_enable,,}.
In `@actions/setup/go/action.yml`:
- Around line 22-35: The workflow currently skips the actions/setup-go@v5 step
when inputs.wails-dev-build == 'true' but still runs Go-dependent steps "Install
Garble" and the `go version` run, causing failures on clean runners; fix by
either removing the conditional on the actions/setup-go step so
`actions/setup-go` always runs, or add the same guard (if:
inputs.wails-dev-build == 'false') to the "Install Garble" and `go version` run
steps so they only execute when Go has been provisioned (references:
actions/setup-go@v5, the "Install Garble" step, and the `go version` step).
In `@actions/setup/README.md`:
- Around line 43-66: The fenced YAML code block in the README (the
triple-backtick block starting with ```yaml and ending with ```) needs blank
lines added immediately before and after the fence to satisfy markdownlint
MD031; update the README.md so there is an empty line above the opening ```yaml
and an empty line below the closing ``` (and apply the same blank-line rule to
any other fenced blocks in the file) to ensure proper linting.
In `@actions/sign/action.yml`:
- Around line 107-109: The hardcoded signtool.exe path in the sign step (the two
invocations that call signtool.exe with inputs.sign-windows-cert-password and
inputs.build-name) can fail on different runner images; change the step to
dynamically locate signtool.exe instead of using the fixed "C:/Program Files
(x86)/Windows Kits/10/bin/10.0.17763.0/x86/signtool.exe" path—for example,
resolve the tool with vswhere (to find latest Windows SDK), search common
Program Files Windows Kits paths, or rely on signtool.exe on PATH, then use that
resolved path for both signing calls so the same binary signs .\build\bin\${{
inputs.build-name }}.exe and .\build\bin\${{ inputs.build-name
}}-amd64-installer.exe.
In `@actions/sign/README.md`:
- Around line 32-51: The fenced YAML example block starting with "- name: Sign
artifacts" and ending with the closing ``` needs blank lines immediately before
the opening ``` and after the closing ``` to satisfy MD031; update the README.md
usage section to insert one blank line above the opening fenced code block and
one blank line after the closing fence so the YAML block is surrounded by blank
lines.
In `@docs/getting-started.md`:
- Around line 7-55: Add a blank line immediately after each fenced code block in
the Getting Started examples to satisfy MD031: specifically insert a single
newline after the first YAML block under the intro (the auto-detect example),
after the "Build Only (No Packaging)" YAML block, after the "macOS Build" YAML
block, and after the "Windows Build" YAML block so each fenced block is followed
by an empty line before the next paragraph or heading.
In `@Makefile`:
- Around line 212-218: The build-go-versions Makefile target edits go.mod
in-place inside the loop which can leave the repo in a modified state if
interrupted; update the target (build-go-versions) to perform modifications on a
temporary copy or restore the original on exit (use a trap or save/restore of
the original go.mod) so each iteration edits a temp file (or copies the original
back at the end/failure) and ensure the final step restores go.mod to
GO111MODULE/1.21 state; reference the build-go-versions target and the cd
$(TDD)/go-cli && go mod edit -go=... invocation when implementing the atomic
cleanup.
In `@README.md`:
- Line 48: Update the incorrect documentation link in README.md that references
`actions/wails2/README.md`: replace that reference with the correct path
`actions/build/wails2/README.md` and verify the target README exists and the
link renders properly.
In `@tdd/cpp-root/CMakeLists.txt`:
- Around line 1-4: CMakeLists.txt currently calls add_executable(dummy main.cpp)
but main.cpp is missing and the file also lacks C++ standard settings; either
add a minimal main.cpp source into the tdd/cpp-root/ folder (matching expected
test fixture behavior) or modify CMakeLists.txt (project CppRootFixture) to
point to existing sources or remove the executable target, and add the C++
standard setting (e.g., set(CMAKE_CXX_STANDARD 17) or match tdd/cpp-hello) and
any target_compile_features used there so the fixture builds consistently.
In `@tdd/deno-http/main.ts`:
- Around line 9-10: The console.log currently runs before Deno.serve() so it may
report "Server running" even if startup fails; update the startup to log only
after the server is actually listening by either moving the console.log to run
after Deno.serve() resolves or by using Deno.serve's onListen option (pass an
onListen handler that logs the message), referencing the existing Deno.serve
call, the port variable and the handler function so the log fires when the
server is confirmed listening.
- Line 1: Change the default port used when reading PORT from the environment to
match the Node.js fixture by updating the fallback in the parseInt call
(variable name: port, expression: parseInt(Deno.env.get("PORT") || "8000")) to
"3000", and make the parseInt call explicit by adding the radix parameter
(parseInt(..., 10)); update the expression that reads Deno.env.get("PORT")
accordingly so port parsing is consistent and deterministic.
In `@tdd/node-http/package.json`:
- Around line 1-12: The package.json for the test fixture is missing the private
flag, so add a "private": true field to the top-level JSON (alongside "name",
"version", "description", etc.) to ensure this fixture package cannot be
accidentally published to npm; update the object to include the "private" key
with the boolean true value.
…in permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
1 participant
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 by CodeRabbit
New Features
Documentation
Infrastructure