feat(inputs): Add os-list input

[Dolly132]

No description provided.

[Copilot]

🟡 Changes recommended

Fix Windows label detection and preserve or document the changed default runner.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds configurable OS selection to the reusable extension build workflow.

Changes:

• Adds the os-list workflow input and dynamic build matrix.
• Derives compiler settings per runner.
• Passes compiler settings to AMBuild.

File summaries

File	Summary
.github/workflows/shared_build_release_am_extension.yml	Adds OS selection and compiler configuration.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Rushaway]

Review: feat(inputs): Add os-list input

Nice idea — making the OS matrix caller-configurable is the right direction for a reusable workflow. But the current state has a blocking bug and a couple of design issues worth addressing before merge.

🔴 Blocking — invalid YAML, workflow won't load

.github/workflows/shared_build_release_am_extension.yml L22-25:

      os-list:
        description: 'JSON array of OSes to build on, e.g. ["ubuntu-22.04"]'
      default: '["ubuntu-24.04", "windows-2022"]'   # <- 6 spaces, should be 8
        type: string

default: is indented one level too shallow (6 spaces vs 8), so it's a sibling of os-list: instead of a child. Reproduced locally:

yaml.parser.ParserError: while parsing a block mapping
  expected <block end>, but found '<block mapping start>'
  in "...", line 25, column 9

GitHub Actions will reject this as "Invalid workflow file" and every repo that calls this reusable workflow breaks immediately. The "resolve copilot suggestion" commit did not fix this. Fix:

      os-list:
        description: 'JSON array of OSes to build on, e.g. ["ubuntu-22.04", "windows-2022"]'
        default: '["ubuntu-24.04", "windows-2022"]'
        type: string

Also: the description example says ubuntu-22.04 while default uses ubuntu-24.04 — align them so the docstring isn't misleading.

🟠 Design — compiler selection is now hardcoded and less flexible

Set compiler step (L44-54):

The old matrix.include let each OS declare its own cc/cxx. The new step collapses that to a 2-way branch: windows-* -> msvc, everything else -> clang-14.

The whole point of os-list is to let callers pass other runners (macos-14, a future ubuntu-26.04, etc.). Any non-Windows runner without clang-14 on PATH (macOS has none; newer Ubuntu images ship clang-18) will fail with clang-14: command not found. Consider either:

• keeping a per-OS compiler map (a second JSON input, or a case on matrix.os), or
• at minimum documenting that os-list only supports windows-* and ubuntu-* today.

🟡 Simplification — the extra step + GITHUB_ENV round-trip isn't needed

If you keep the 2-way logic, an inline expression on the action with: block removes the whole Set compiler step:

      - name: Build extension
        uses: srcdslab/action-ambuild@v1
        with:
          os: ${{ matrix.os }}
          cc:  ${{ startsWith(matrix.os, 'windows') && 'msvc' || 'clang-14' }}
          cxx: ${{ startsWith(matrix.os, 'windows') && 'msvc' || 'clang++-14' }}

This also eliminates the next point.

🟡 Minor — script-injection pattern

L47: ${{ matrix.os }} (ultimately caller-controlled via os-list) is interpolated directly into a run: bash script. Low practical risk here since runs-on would also need a matching runner label, but actionlint/zizmor will flag it. Use env: indirection, or just drop the step per the point above.

🟡 Minor — no guard on malformed os-list

L32: fromJSON(inputs.os-list) aborts matrix expansion with a cryptic "Invalid JSON" if a caller passes an empty string or a typo'd array. Since there's a default, keeping the input non-required is fine, but a precise format note in the description helps.

---

Verdict: request changes — the YAML bug is a hard blocker. The compiler-hardcoding is the main design discussion; the rest are nice-to-haves.

_{🤖 Generated with Claude Code}

[Rushaway]

Pushed b351107 to this branch with the fixes:

• YAML fix (blocking): default: on the os-list input was indented to 6 spaces, making it a sibling of os-list: and the whole file invalid YAML. Now at 8 spaces. Verified the file parses.
• Description: clarified wording and aligned the example with the actual default.
• Compiler selection: removed the Set compiler step + GITHUB_ENV round-trip in favour of an inline expression on the Build extension inputs:

  cc:  ${{ startsWith(matrix.os, 'windows') && 'msvc' || 'clang-14' }}
  cxx: ${{ startsWith(matrix.os, 'windows') && 'msvc' || 'clang++-14' }}

  Same behaviour for ubuntu-* / windows-*, one less step, and it drops the matrix.os -> run: interpolation.

Still an open design decision (not blocking): the compiler is hardcoded to clang-14 for any non-Windows runner, so os-list can't currently be used with macOS or a runner needing a different clang. If that flexibility is wanted, a follow-up could add a per-OS compiler map or a second input.

_{🤖 Generated with Claude Code}