Ops: AI-native reliability OS + repro bundle contracts#45
Conversation
|
Unable to trigger custom agent "Code Reviewer". You have run out of credits 😔 |
github-actions
bot
added
area:runtime
📝 WalkthroughWalkthroughIntroduces an evidence-first runtime debugging framework: repro-bundle schema, collection and validation tooling, CI/policy workflows enforcing PR evidence, updated issue/PR templates, visual-audit and duplication-check workflows, agent operating contracts, runbooks, and supporting scripts and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (PR/Run)
participant Run as Live Validation (run-live-validation.ps1)
participant Collector as Collector (collect-mod-repro-bundle.ps1)
participant Repo as Repository (repro-bundle.json/.md)
participant CI as GitHub Actions (ci.yml / policy-contract.yml)
participant Validator as Validator (validate-repro-bundle.ps1)
Dev->>Run: trigger live validation (RunId, Scope, EmitReproBundle)
Run->>Collector: assemble repro bundle (process, tests, diagnostics)
Collector->>Repo: write repro-bundle.json and repro-bundle.md
Dev->>CI: open PR / push
CI->>Repo: fetch repro-bundle path from PR body or artifacts
CI->>Validator: validate repro-bundle.json against schema (Strict optional)
Validator-->>CI: validation result (pass/fail + errors)
CI-->>Dev: enforce policy (fail PR if required evidence missing/invalid)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Codacy's Analysis Summary39 new issues (≤ 0 issue) Review Pull Request in Codacy →
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/run-live-validation.ps1 (1)
116-127:⚠️ Potential issue | 🟡 MinorAssigning to
$argsshadows PowerShell's automatic variable.
$argsis a built-in automatic variable in PowerShell that holds unbound arguments. Overwriting it can cause unexpected behavior if this function ever needs to access unbound parameters or is called in certain contexts.🛠️ Rename to avoid shadowing
- $args = @( + $dotnetArgs = @( "test", "tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj", "-c", $Configuration, "--filter", $Filter, "--logger", "trx;LogFileName=$TrxName", "--results-directory", $runResultsDirectory ) if ($NoBuild) { - $args += "--no-build" + $dotnetArgs += "--no-build" } - & $dotnetExe `@args` + & $dotnetExe `@dotnetArgs`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/run-live-validation.ps1` around lines 116 - 127, The script currently assigns to the automatic PowerShell variable $args which shadows built-in unbound-argument behavior; rename that variable (e.g., $testArgs or $dotnetArgs) wherever it's defined and used (the array that contains "test", the csproj path, configuration, filter, logger, results-directory and the conditional append of "--no-build") and update all subsequent references to the new name so the script no longer overwrites the automatic $args variable.
🧹 Nitpick comments (4)
.github/workflows/duplication-check.yml (1)
29-35: Consider addingif-no-files-foundhandling for artifact upload robustness.The
.jscpd.jsoncorrectly configures output tojscpd-report, which matches the upload path. However, adding explicit handling improves workflow resilience against unexpected edge cases.Suggested improvement
- name: Upload jscpd report if: always() uses: actions/upload-artifact@v4 with: name: jscpd-report path: | jscpd-report + if-no-files-found: warn🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/duplication-check.yml around lines 29 - 35, The artifact upload step "Upload jscpd report" using actions/upload-artifact@v4 should handle missing files to avoid failing the workflow; update that step (the step named "Upload jscpd report", which currently uploads path "jscpd-report") to include the upload-artifact input if-no-files-found (e.g., "warn" or "ignore") under its with: block so the action won't hard-fail when the jscpd-report directory/file is absent.tools/compare-visual-pack.ps1 (1)
109-110: ConsiderWrite-Informationfor better compatibility (optional).PSScriptAnalyzer flags
Write-Hostusage. For CI scripts this is generally acceptable since the output is intended for console display. However,Write-Informationwith-InformationAction Continueprovides better compatibility across different PowerShell hosts if needed in the future.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/compare-visual-pack.ps1` around lines 109 - 110, Replace the two Write-Host calls with Write-Information to satisfy PSScriptAnalyzer and improve host compatibility: emit the same messages using Write-Information and include -InformationAction Continue so the output appears in console-host and CI environments; locate the lines that reference the OutputPath and status variables and change those invocations (the current Write-Host "visual compare report: $OutputPath" and Write-Host "status: $status") to use Write-Information with a descriptive message and -InformationAction Continue.tools/validate-repro-bundle.ps1 (1)
76-93: RedundanttrxPathempty check.Line 82 calls
Require-Field -Object $test -Field "trxPath"(without-AllowNull), which already validates thattrxPathis non-null and non-empty. Lines 90-92 then redundantly check for emptytrxPathagain.♻️ Remove redundant validation
if ($allowedOutcomes -notcontains [string]$test.outcome) { Add-Error "liveTests[$($test.name)] outcome invalid: $($test.outcome)" } - - if ([string]::IsNullOrWhiteSpace([string]$test.trxPath)) { - Add-Error "liveTests[$($test.name)] trxPath is empty" - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-repro-bundle.ps1` around lines 76 - 93, The trxPath emptiness check is redundant because Require-Field -Object $test -Field "trxPath" already enforces non-null/non-empty; remove the extra block that calls [string]::IsNullOrWhiteSpace([string]$test.trxPath) and its Add-Error call so only Require-Field validates trxPath for items in liveTests, leaving the other validations (Require-Field for name/outcome/message and outcome membership check) intact.tools/collect-mod-repro-bundle.ps1 (1)
16-33: Consider extracting sharedResolve-PythonCommandfunction.This function is duplicated between
collect-mod-repro-bundle.ps1andrun-live-validation.ps1. Consider extracting it to a shared module (e.g.,tools/lib/resolve-commands.ps1) and dot-sourcing it from both scripts to reduce duplication and simplify maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/collect-mod-repro-bundle.ps1` around lines 16 - 33, Extract the duplicated Resolve-PythonCommand function into a single shared script module (e.g., resolve-commands.ps1) and remove the local copies from both scripts; then dot-source the new module from each script so they call the centralized Resolve-PythonCommand implementation. Ensure the new module exports the same function name (Resolve-PythonCommand), keep the exact return shape (arrays for command + args), and update both calling scripts to rely on the shared function instead of their inline copies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/ISSUE_TEMPLATE/bug.yml:
- Around line 71-84: The YAML descriptions for the fields with id
"launch_context" and "diagnostics" are not quoted, which can break YAML parsing;
update the attributes.description values for launch_context and diagnostics to
be wrapped in quotes (e.g., "Include launchKind, reasonCode, confidence, and
detected STEAMMOD/MODPATH data." and "Include
runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency state,
helper readiness, symbol health summary.") so the strings are valid YAML while
preserving the existing text.
- Around line 21-40: The YAML in the issue template uses raw backticks in the
description fields which breaks parsing; update the three description values
(the ones for the fields with id repro_bundle_json and repro_bundle_md and the
earlier description at line 21) to be valid YAML strings by quoting them (single
or double quotes) or escaping the backticks so the literal text like
`TestResults/runs/<runId>/repro-bundle.json` and
`TestResults/runs/<runId>/repro-bundle.md` are wrapped in quotes; ensure all
three description placeholders/values are changed consistently so the template
parses correctly.
In @.github/workflows/visual-audit.yml:
- Around line 59-63: The step "Applitools availability note" uses a step-level
if with the secrets context which is not allowed; change to export the secret
into an env var (e.g., set env: APPLITOOLS_API_KEY: ${{
secrets.APPLITOOLS_API_KEY }} at the job or step level) and remove the
step-level if, or keep the env mapping and replace the if with a runtime check
inside the pwsh script (use $env:APPLITOOLS_API_KEY in the "run" block) so the
availability message only prints when the variable is present.
In `@tools/collect-mod-repro-bundle.ps1`:
- Around line 42-51: The variable $matches shadows PowerShell's automatic
$matches and should be renamed to avoid confusion; update the call to
[regex]::Matches($CommandLine, "STEAMMOD\s*=\s*([0-9]{4,})", ...) to assign to a
new name (e.g., $regexMatches) and update the foreach loop to iterate over that
new variable (change foreach ($match in $matches) to foreach ($match in
$regexMatches)) while leaving all other logic (the HashSet $ids, $match.Groups
usage, and the return) unchanged.
- Around line 145-156: The script is assigning to the automatic PowerShell
variable $args which causes shadowing; rename that variable (e.g. $pythonArgs)
and update all uses in this block so you build the argument array into
$pythonArgs instead of $args (modify the $args += lines and any later invocation
that consumes it), keeping the logic that appends $pythonCmd[1..] and the
tools/detect-launch-context.py arguments that use $Process and $ProfileRootPath.
- Around line 397-425: The here-string written to $bundleMdPath currently
escapes nine variables (e.g., `$RunId`, `$Scope`, `$classification`,
`$($launchContext.profileId)`, `$($launchContext.reasonCode)`,
`$($launchContext.confidence)`, `$($launchContext.launchKind)`,
`$($runtimeMode.effective)`, `$($runtimeMode.reasonCode)`) with backticks which
prevents PowerShell interpolation; remove the backticks so the here-string
interpolates those expressions directly (leave the surrounding @"..."@
here-string intact) so the generated repro-bundle.md contains the actual runtime
values instead of literal variable names.
In `@tools/run-live-validation.ps1`:
- Line 5: The [switch] parameter $NoBuild currently defaults to $true which
makes the flag unusable; rename the parameter to [switch]$Build (default $false)
or change $NoBuild to default $false, then update all usages where the script
checks $NoBuild (or where you previously skipped the build) to use the new
$Build logic (invert the conditional: run the build when $Build is true, or
replace checks like "if (-not $NoBuild)" with "if ($Build)") so callers can
simply pass -Build to opt into building.
---
Outside diff comments:
In `@tools/run-live-validation.ps1`:
- Around line 116-127: The script currently assigns to the automatic PowerShell
variable $args which shadows built-in unbound-argument behavior; rename that
variable (e.g., $testArgs or $dotnetArgs) wherever it's defined and used (the
array that contains "test", the csproj path, configuration, filter, logger,
results-directory and the conditional append of "--no-build") and update all
subsequent references to the new name so the script no longer overwrites the
automatic $args variable.
---
Nitpick comments:
In @.github/workflows/duplication-check.yml:
- Around line 29-35: The artifact upload step "Upload jscpd report" using
actions/upload-artifact@v4 should handle missing files to avoid failing the
workflow; update that step (the step named "Upload jscpd report", which
currently uploads path "jscpd-report") to include the upload-artifact input
if-no-files-found (e.g., "warn" or "ignore") under its with: block so the action
won't hard-fail when the jscpd-report directory/file is absent.
In `@tools/collect-mod-repro-bundle.ps1`:
- Around line 16-33: Extract the duplicated Resolve-PythonCommand function into
a single shared script module (e.g., resolve-commands.ps1) and remove the local
copies from both scripts; then dot-source the new module from each script so
they call the centralized Resolve-PythonCommand implementation. Ensure the new
module exports the same function name (Resolve-PythonCommand), keep the exact
return shape (arrays for command + args), and update both calling scripts to
rely on the shared function instead of their inline copies.
In `@tools/compare-visual-pack.ps1`:
- Around line 109-110: Replace the two Write-Host calls with Write-Information
to satisfy PSScriptAnalyzer and improve host compatibility: emit the same
messages using Write-Information and include -InformationAction Continue so the
output appears in console-host and CI environments; locate the lines that
reference the OutputPath and status variables and change those invocations (the
current Write-Host "visual compare report: $OutputPath" and Write-Host "status:
$status") to use Write-Information with a descriptive message and
-InformationAction Continue.
In `@tools/validate-repro-bundle.ps1`:
- Around line 76-93: The trxPath emptiness check is redundant because
Require-Field -Object $test -Field "trxPath" already enforces
non-null/non-empty; remove the extra block that calls
[string]::IsNullOrWhiteSpace([string]$test.trxPath) and its Add-Error call so
only Require-Field validates trxPath for items in liveTests, leaving the other
validations (Require-Field for name/outcome/message and outcome membership
check) intact.
| description: Value from `tools/run-live-validation.ps1 -RunId ...` | ||
| placeholder: 20260217-010203 | ||
| validations: | ||
| required: true | ||
| - type: input | ||
| id: repro_bundle_json | ||
| attributes: | ||
| label: Repro bundle JSON path/link | ||
| description: `TestResults/runs/<runId>/repro-bundle.json` or uploaded artifact URL | ||
| placeholder: TestResults/runs/20260217-010203/repro-bundle.json | ||
| validations: | ||
| required: true | ||
| - type: input | ||
| id: repro_bundle_md | ||
| attributes: | ||
| label: Repro bundle markdown path/link | ||
| description: `TestResults/runs/<runId>/repro-bundle.md` | ||
| placeholder: TestResults/runs/20260217-010203/repro-bundle.md | ||
| validations: | ||
| required: true |
There was a problem hiding this comment.
YAML syntax error: backtick characters break template parsing.
The backtick characters () in the description` fields on lines 21, 29, and 37 are invalid YAML syntax. GitHub Actions issue templates require valid YAML, and these will cause parsing failures when users try to create bug reports.
Wrap these values in quotes or use single quotes to escape the content properly.
🐛 Proposed fix
- type: input
id: run_id
attributes:
label: Run ID
- description: Value from `tools/run-live-validation.ps1 -RunId ...`
+ description: "Value from tools/run-live-validation.ps1 -RunId ..."
placeholder: 20260217-010203
validations:
required: true
- type: input
id: repro_bundle_json
attributes:
label: Repro bundle JSON path/link
- description: `TestResults/runs/<runId>/repro-bundle.json` or uploaded artifact URL
+ description: "TestResults/runs/<runId>/repro-bundle.json or uploaded artifact URL"
placeholder: TestResults/runs/20260217-010203/repro-bundle.json
validations:
required: true
- type: input
id: repro_bundle_md
attributes:
label: Repro bundle markdown path/link
- description: `TestResults/runs/<runId>/repro-bundle.md`
+ description: "TestResults/runs/<runId>/repro-bundle.md"
placeholder: TestResults/runs/20260217-010203/repro-bundle.md📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: Value from `tools/run-live-validation.ps1 -RunId ...` | |
| placeholder: 20260217-010203 | |
| validations: | |
| required: true | |
| - type: input | |
| id: repro_bundle_json | |
| attributes: | |
| label: Repro bundle JSON path/link | |
| description: `TestResults/runs/<runId>/repro-bundle.json` or uploaded artifact URL | |
| placeholder: TestResults/runs/20260217-010203/repro-bundle.json | |
| validations: | |
| required: true | |
| - type: input | |
| id: repro_bundle_md | |
| attributes: | |
| label: Repro bundle markdown path/link | |
| description: `TestResults/runs/<runId>/repro-bundle.md` | |
| placeholder: TestResults/runs/20260217-010203/repro-bundle.md | |
| validations: | |
| required: true | |
| description: "Value from tools/run-live-validation.ps1 -RunId ..." | |
| placeholder: 20260217-010203 | |
| validations: | |
| required: true | |
| - type: input | |
| id: repro_bundle_json | |
| attributes: | |
| label: Repro bundle JSON path/link | |
| description: "TestResults/runs/<runId>/repro-bundle.json or uploaded artifact URL" | |
| placeholder: TestResults/runs/20260217-010203/repro-bundle.json | |
| validations: | |
| required: true | |
| - type: input | |
| id: repro_bundle_md | |
| attributes: | |
| label: Repro bundle markdown path/link | |
| description: "TestResults/runs/<runId>/repro-bundle.md" | |
| placeholder: TestResults/runs/20260217-010203/repro-bundle.md | |
| validations: | |
| required: true |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 29-29: syntax error: found character '`' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/ISSUE_TEMPLATE/bug.yml around lines 21 - 40, The YAML in the issue
template uses raw backticks in the description fields which breaks parsing;
update the three description values (the ones for the fields with id
repro_bundle_json and repro_bundle_md and the earlier description at line 21) to
be valid YAML strings by quoting them (single or double quotes) or escaping the
backticks so the literal text like `TestResults/runs/<runId>/repro-bundle.json`
and `TestResults/runs/<runId>/repro-bundle.md` are wrapped in quotes; ensure all
three description placeholders/values are changed consistently so the template
parses correctly.
| - type: textarea | ||
| id: launch_context | ||
| attributes: | ||
| label: Launch context diagnostics | ||
| description: Include launchKind, reasonCode, confidence, and detected STEAMMOD/MODPATH data. | ||
| validations: | ||
| required: true | ||
| - type: textarea | ||
| id: diagnostics | ||
| attributes: | ||
| label: Diagnostics / logs | ||
| description: Include launchKind, reasonCode, confidence, dependencyValidation if available. | ||
| description: Include runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency state, helper readiness, symbol health summary. | ||
| validations: | ||
| required: true |
There was a problem hiding this comment.
Same YAML syntax issue in launch_context and diagnostics descriptions.
The description on line 75 and 82 should also be quoted to ensure valid YAML.
🐛 Proposed fix
- type: textarea
id: launch_context
attributes:
label: Launch context diagnostics
- description: Include launchKind, reasonCode, confidence, and detected STEAMMOD/MODPATH data.
+ description: "Include launchKind, reasonCode, confidence, and detected STEAMMOD/MODPATH data."
validations:
required: true
- type: textarea
id: diagnostics
attributes:
label: Diagnostics / logs
- description: Include runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency state, helper readiness, symbol health summary.
+ description: "Include runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency state, helper readiness, symbol health summary."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - type: textarea | |
| id: launch_context | |
| attributes: | |
| label: Launch context diagnostics | |
| description: Include launchKind, reasonCode, confidence, and detected STEAMMOD/MODPATH data. | |
| validations: | |
| required: true | |
| - type: textarea | |
| id: diagnostics | |
| attributes: | |
| label: Diagnostics / logs | |
| description: Include launchKind, reasonCode, confidence, dependencyValidation if available. | |
| description: Include runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency state, helper readiness, symbol health summary. | |
| validations: | |
| required: true | |
| - type: textarea | |
| id: launch_context | |
| attributes: | |
| label: Launch context diagnostics | |
| description: "Include launchKind, reasonCode, confidence, and detected STEAMMOD/MODPATH data." | |
| validations: | |
| required: true | |
| - type: textarea | |
| id: diagnostics | |
| attributes: | |
| label: Diagnostics / logs | |
| description: "Include runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency state, helper readiness, symbol health summary." | |
| validations: | |
| required: true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/ISSUE_TEMPLATE/bug.yml around lines 71 - 84, The YAML descriptions
for the fields with id "launch_context" and "diagnostics" are not quoted, which
can break YAML parsing; update the attributes.description values for
launch_context and diagnostics to be wrapped in quotes (e.g., "Include
launchKind, reasonCode, confidence, and detected STEAMMOD/MODPATH data." and
"Include runtimeModeHint/runtimeModeEffective/runtimeModeReasonCode, dependency
state, helper readiness, symbol health summary.") so the strings are valid YAML
while preserving the existing text.
| - name: Applitools availability note | ||
| if: ${{ secrets.APPLITOOLS_API_KEY != '' }} | ||
| shell: pwsh | ||
| run: | | ||
| Write-Host "APPLITOOLS_API_KEY is configured. Visual pack can be uploaded for external review." |
There was a problem hiding this comment.
secrets context is not available in step-level if expressions.
The secrets context cannot be used directly in step if conditions. This will cause the workflow to fail. Use an environment variable approach instead.
🐛 Proposed fix using environment variable
- name: Applitools availability note
- if: ${{ secrets.APPLITOOLS_API_KEY != '' }}
+ if: ${{ env.HAS_APPLITOOLS_KEY == 'true' }}
shell: pwsh
+ env:
+ HAS_APPLITOOLS_KEY: ${{ secrets.APPLITOOLS_API_KEY != '' }}
run: |
Write-Host "APPLITOOLS_API_KEY is configured. Visual pack can be uploaded for external review."Alternative: Check the secret as an env var inside the script:
- name: Applitools availability note
- if: ${{ secrets.APPLITOOLS_API_KEY != '' }}
shell: pwsh
+ env:
+ APPLITOOLS_API_KEY: ${{ secrets.APPLITOOLS_API_KEY }}
run: |
- Write-Host "APPLITOOLS_API_KEY is configured. Visual pack can be uploaded for external review."
+ if ($env:APPLITOOLS_API_KEY) {
+ Write-Host "APPLITOOLS_API_KEY is configured. Visual pack can be uploaded for external review."
+ }🧰 Tools
🪛 actionlint (1.7.10)
[error] 60-60: context "secrets" is not allowed here. available contexts are "env", "github", "inputs", "job", "matrix", "needs", "runner", "steps", "strategy", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/visual-audit.yml around lines 59 - 63, The step
"Applitools availability note" uses a step-level if with the secrets context
which is not allowed; change to export the secret into an env var (e.g., set
env: APPLITOOLS_API_KEY: ${{ secrets.APPLITOOLS_API_KEY }} at the job or step
level) and remove the step-level if, or keep the env mapping and replace the if
with a runtime check inside the pwsh script (use $env:APPLITOOLS_API_KEY in the
"run" block) so the availability message only prints when the variable is
present.
| $matches = [regex]::Matches($CommandLine, "STEAMMOD\s*=\s*([0-9]{4,})", [System.Text.RegularExpressions.RegexOptions]::IgnoreCase) | ||
| $ids = New-Object System.Collections.Generic.HashSet[string]([StringComparer]::OrdinalIgnoreCase) | ||
| foreach ($match in $matches) { | ||
| if ($match.Groups.Count -gt 1) { | ||
| [void]$ids.Add($match.Groups[1].Value) | ||
| } | ||
| } | ||
|
|
||
| return @($ids) | ||
| } |
There was a problem hiding this comment.
Assigning to $matches shadows PowerShell's automatic variable.
$matches is a built-in automatic variable populated by the -match operator. While this code uses [regex]::Matches() (the .NET method), shadowing the automatic variable can cause confusion and unexpected behavior if any downstream code uses the -match operator.
🛠️ Rename to avoid shadowing
- $matches = [regex]::Matches($CommandLine, "STEAMMOD\s*=\s*([0-9]{4,})", [System.Text.RegularExpressions.RegexOptions]::IgnoreCase)
+ $regexMatches = [regex]::Matches($CommandLine, "STEAMMOD\s*=\s*([0-9]{4,})", [System.Text.RegularExpressions.RegexOptions]::IgnoreCase)
$ids = New-Object System.Collections.Generic.HashSet[string]([StringComparer]::OrdinalIgnoreCase)
- foreach ($match in $matches) {
+ foreach ($match in $regexMatches) {
if ($match.Groups.Count -gt 1) {
[void]$ids.Add($match.Groups[1].Value)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $matches = [regex]::Matches($CommandLine, "STEAMMOD\s*=\s*([0-9]{4,})", [System.Text.RegularExpressions.RegexOptions]::IgnoreCase) | |
| $ids = New-Object System.Collections.Generic.HashSet[string]([StringComparer]::OrdinalIgnoreCase) | |
| foreach ($match in $matches) { | |
| if ($match.Groups.Count -gt 1) { | |
| [void]$ids.Add($match.Groups[1].Value) | |
| } | |
| } | |
| return @($ids) | |
| } | |
| $regexMatches = [regex]::Matches($CommandLine, "STEAMMOD\s*=\s*([0-9]{4,})", [System.Text.RegularExpressions.RegexOptions]::IgnoreCase) | |
| $ids = New-Object System.Collections.Generic.HashSet[string]([StringComparer]::OrdinalIgnoreCase) | |
| foreach ($match in $regexMatches) { | |
| if ($match.Groups.Count -gt 1) { | |
| [void]$ids.Add($match.Groups[1].Value) | |
| } | |
| } | |
| return @($ids) | |
| } |
🧰 Tools
🪛 PSScriptAnalyzer (1.23.0)
[warning] 42-42: The Variable 'matches' is an automatic variable that is built into PowerShell, assigning to it might have undesired side effects. If assignment is not by design, please use a different name.
(PSAvoidAssignmentToAutomaticVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/collect-mod-repro-bundle.ps1` around lines 42 - 51, The variable
$matches shadows PowerShell's automatic $matches and should be renamed to avoid
confusion; update the call to [regex]::Matches($CommandLine,
"STEAMMOD\s*=\s*([0-9]{4,})", ...) to assign to a new name (e.g., $regexMatches)
and update the foreach loop to iterate over that new variable (change foreach
($match in $matches) to foreach ($match in $regexMatches)) while leaving all
other logic (the HashSet $ids, $match.Groups usage, and the return) unchanged.
| $args = @() | ||
| if ($pythonCmd.Count -gt 1) { | ||
| $args += $pythonCmd[1..($pythonCmd.Count - 1)] | ||
| } | ||
| $args += @( | ||
| "tools/detect-launch-context.py", | ||
| "--command-line", $Process.commandLine, | ||
| "--process-name", $Process.name, | ||
| "--process-path", $Process.path, | ||
| "--profile-root", $ProfileRootPath, | ||
| "--pretty" | ||
| ) |
There was a problem hiding this comment.
Assigning to $args shadows PowerShell's automatic variable.
Same issue as in run-live-validation.ps1. Rename to $pythonArgs or similar.
🛠️ Rename to avoid shadowing
- $args = @()
+ $pythonArgs = @()
if ($pythonCmd.Count -gt 1) {
- $args += $pythonCmd[1..($pythonCmd.Count - 1)]
+ $pythonArgs += $pythonCmd[1..($pythonCmd.Count - 1)]
}
- $args += @(
+ $pythonArgs += @(
"tools/detect-launch-context.py",
"--command-line", $Process.commandLine,
"--process-name", $Process.name,
"--process-path", $Process.path,
"--profile-root", $ProfileRootPath,
"--pretty"
)
try {
- $output = & $pythonCmd[0] `@args` 2>&1
+ $output = & $pythonCmd[0] `@pythonArgs` 2>&1🧰 Tools
🪛 PSScriptAnalyzer (1.23.0)
[warning] 145-145: The Variable 'args' is an automatic variable that is built into PowerShell, assigning to it might have undesired side effects. If assignment is not by design, please use a different name.
(PSAvoidAssignmentToAutomaticVariable)
[warning] 147-147: The Variable 'args' is an automatic variable that is built into PowerShell, assigning to it might have undesired side effects. If assignment is not by design, please use a different name.
(PSAvoidAssignmentToAutomaticVariable)
[warning] 149-149: The Variable 'args' is an automatic variable that is built into PowerShell, assigning to it might have undesired side effects. If assignment is not by design, please use a different name.
(PSAvoidAssignmentToAutomaticVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/collect-mod-repro-bundle.ps1` around lines 145 - 156, The script is
assigning to the automatic PowerShell variable $args which causes shadowing;
rename that variable (e.g. $pythonArgs) and update all uses in this block so you
build the argument array into $pythonArgs instead of $args (modify the $args +=
lines and any later invocation that consumes it), keeping the logic that appends
$pythonCmd[1..] and the tools/detect-launch-context.py arguments that use
$Process and $ProfileRootPath.
| @" | ||
| # Repro Bundle Summary | ||
|
|
||
| - runId: `$RunId` | ||
| - scope: `$Scope` | ||
| - classification: `$classification` | ||
| - launch profile: `$($launchContext.profileId)` | ||
| - launch reason: `$($launchContext.reasonCode)` | ||
| - confidence: `$($launchContext.confidence)` | ||
| - launch kind: `$($launchContext.launchKind)` | ||
| - runtime mode effective: `$($runtimeMode.effective)` | ||
| - runtime mode reason: `$($runtimeMode.reasonCode)` | ||
|
|
||
| ## Process Snapshot | ||
|
|
||
| | PID | Name | SteamModIds | Command Line | | ||
| |---|---|---|---| | ||
| $((@($processSnapshot | ForEach-Object { "| $($_.pid) | $($_.name) | $(@($_.steamModIds) -join ',') | $([string]$_.commandLine).Replace('|','/') |" }) -join "`n")) | ||
|
|
||
| ## Live Tests | ||
|
|
||
| | Test | Outcome | Pass/Fail/Skip | TRX | Message | | ||
| |---|---|---|---|---| | ||
| $($liveRows -join "`n") | ||
|
|
||
| ## Next Action | ||
|
|
||
| $nextAction | ||
| "@ | Set-Content -Path $bundleMdPath |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the file size and context around the disputed lines
wc -l tools/collect-mod-repro-bundle.ps1Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 107
🏁 Script executed:
# View the context around lines 397-425
sed -n '380,430p' tools/collect-mod-repro-bundle.ps1 | cat -nRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 1979
🏁 Script executed:
# Search for any test fixtures or sample output related to repro bundle
fd -t f "repro" --search-path . | grep -E "\.(md|txt|json)$|fixture|sample|test"Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 151
🏁 Script executed:
# Look at the broader context to see how bundleMdPath is used
rg "bundleMdPath" -B 5 -A 5 tools/collect-mod-repro-bundle.ps1Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 664
🏁 Script executed:
# View the repro bundle sample fixture
cat tools/fixtures/repro_bundle_sample.jsonRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 1229
🏁 Script executed:
# Also check if there are any markdown samples or tests
fd -t f "\.(md|test)" tools/ | head -20Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 84
🏁 Script executed:
# Search for documentation about the repro bundle structure
rg "repro.bundle|repro-bundle" -A 3 --max-count 10 README.md docs/ 2>/dev/null || echo "No docs found"Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 3234
🏁 Script executed:
# Extract just the here-string content to see all variable references
sed -n '397,425p' tools/collect-mod-repro-bundle.ps1 | grep -E '^\s*-|^\$'Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 644
🏁 Script executed:
# Count backtick escapes in the here-string
sed -n '397,425p' tools/collect-mod-repro-bundle.ps1 | grep -c '`\$'Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 70
Remove backticks from all 9 variable references to enable interpolation in markdown output.
The here-string uses backtick escaping on 9 variables which prevents PowerShell from interpolating them:
$RunId,$Scope,$classification$($launchContext.profileId),$($launchContext.reasonCode),$($launchContext.confidence),$($launchContext.launchKind)$($runtimeMode.effective),$($runtimeMode.reasonCode)
This causes the generated repro-bundle.md to contain literal variable names (e.g., $RunId) instead of actual runtime values. Since this markdown is critical evidence for issue tracking, remove the backticks from these lines to allow proper interpolation. The fixture sample confirms that values should be interpolated (e.g., runId: sample-20260217, not runId: $RunId).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/collect-mod-repro-bundle.ps1` around lines 397 - 425, The here-string
written to $bundleMdPath currently escapes nine variables (e.g., `$RunId`,
`$Scope`, `$classification`, `$($launchContext.profileId)`,
`$($launchContext.reasonCode)`, `$($launchContext.confidence)`,
`$($launchContext.launchKind)`, `$($runtimeMode.effective)`,
`$($runtimeMode.reasonCode)`) with backticks which prevents PowerShell
interpolation; remove the backticks so the here-string interpolates those
expressions directly (leave the surrounding @"..."@ here-string intact) so the
generated repro-bundle.md contains the actual runtime values instead of literal
variable names.
| [string]$ResultsDirectory = "TestResults", | ||
| [string]$ProfileRoot = "profiles/default", | ||
| [switch]$NoBuild = $true | ||
| [switch]$NoBuild = $true, |
There was a problem hiding this comment.
Switch parameter defaulting to $true causes usability issues.
A [switch] parameter that defaults to $true is problematic because:
- Specifying
-NoBuildhas no effect (it's already true) - Users cannot easily disable it; they must use
-NoBuild:$false
If the intent is "no build by default, opt-in to build," consider renaming to [switch]$Build (defaults to $false), then invert the logic at usage.
🛠️ Suggested fix
- [switch]$NoBuild = $true,
+ [switch]$Build,Then at line 125-127:
- if ($NoBuild) {
+ if (-not $Build) {
$args += "--no-build"
}🧰 Tools
🪛 PSScriptAnalyzer (1.23.0)
[warning] 5-5: File 'run-live-validation.ps1' has a switch parameter default to true.
(PSAvoidDefaultValueSwitchParameter)
[warning] 5-5: The parameter 'NoBuild' has been declared but not used.
(PSReviewUnusedParameter)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/run-live-validation.ps1` at line 5, The [switch] parameter $NoBuild
currently defaults to $true which makes the flag unusable; rename the parameter
to [switch]$Build (default $false) or change $NoBuild to default $false, then
update all usages where the script checks $NoBuild (or where you previously
skipped the build) to use the new $Build logic (invert the conditional: run the
build when $Build is true, or replace checks like "if (-not $NoBuild)" with "if
($Build)") so callers can simply pass -Build to opt into building.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3a380d269
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ($processItems.Count -eq 0) { | ||
| return "blocked_environment" | ||
| } |
There was a problem hiding this comment.
Get-Classification returns blocked_environment as soon as processSnapshot is empty, before it checks whether relevant live tests already passed. If the game process exits between test execution and bundle collection, a run with passing TRX results is misclassified as environment-blocked, which sends triage down the wrong path and can hide real pass/fail status in repro bundles.
Useful? React with 👍 / 👎.
| $candidate = "TestResults/runs/$runId/visual-pack" | ||
| } |
There was a problem hiding this comment.
The workflow defaults candidate to TestResults/runs/<run_id>/visual-pack, but on a fresh Actions runner only repository files exist after checkout and this path is not created anywhere in the job. Because compare-visual-pack.ps1 throws when CandidateDir is missing, the default workflow path fails unless callers manually override candidate_dir to an existing directory.
Useful? React with 👍 / 👎.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
|
|
||
| ## Required Evidence | ||
| 1. Runtime/tooling/test changes must include deterministic test evidence or an explicit justified skip. | ||
| 2. Mod/runtime bugfixes must include a reproducible bundle: |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 2. Mod/runtime bugfixes must include a reproducible bundle: | |
| 2. Mod/runtime bugfixes must include a reproducible bundle: | |
| - `TestResults/runs/<runId>/visual-pack/<surface>/<checkpoint>.png` | ||
|
|
||
| Naming guidance: | ||
| - `<surface>`: `live-ops`, `runtime-tab`, `attach-dialog`, `tactical`, `galactic` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `<surface>`: `live-ops`, `runtime-tab`, `attach-dialog`, `tactical`, `galactic` | |
| - `<surface>`: `live-ops`, `runtime-tab`, `attach-dialog`, `tactical`, `galactic` |
| 1. Intake issue with profile/build/scope context. | ||
| 2. Run live validation tooling to emit `repro-bundle.json` and `repro-bundle.md`. | ||
| 3. Classify using explicit failure categories: | ||
| - `passed` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `passed` | |
| - `passed` |
| Applies to files under `tests/`. | ||
|
|
||
| ## Required Evidence | ||
| 1. Deterministic tests for new behavior. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. Deterministic tests for new behavior. | |
| 1. Deterministic tests for new behavior. |
|
|
||
| ## PR Readiness | ||
| A PR touching runtime/tooling/tests must include: | ||
| 1. affected profile IDs |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. affected profile IDs | |
| 1. affected profile IDs |
| 6. Close issue only when linked evidence confirms acceptance criteria. | ||
|
|
||
| ## Safety Rules | ||
| 1. No blind fixed-address runtime actions. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. No blind fixed-address runtime actions. | |
| 1. No blind fixed-address runtime actions. |
| Applies to files under `tools/`. | ||
|
|
||
| ## Required Evidence | ||
| 1. Every tooling script change must include a deterministic fixture or smoke command. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. Every tooling script change must include a deterministic fixture or smoke command. | |
| 1. Every tooling script change must include a deterministic fixture or smoke command. |
| - `failed` | ||
| - `blocked_environment` | ||
| - `blocked_profile_mismatch` | ||
| 4. Implement fix on branch. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 4. Implement fix on branch. | |
| 4. Implement fix on branch. |
| 3. Include repro bundle evidence for live-only fixes. | ||
|
|
||
| ## Runtime Safety Rules | ||
| 1. Signature-first; never rely on blind fixed addresses. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. Signature-first; never rely on blind fixed addresses. | |
| 1. Signature-first; never rely on blind fixed addresses. |
| ## Required Evidence | ||
| 1. Runtime/tooling/test changes must include deterministic test evidence or an explicit justified skip. | ||
| 2. Mod/runtime bugfixes must include a reproducible bundle: | ||
| - `TestResults/runs/<runId>/repro-bundle.json` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `TestResults/runs/<runId>/repro-bundle.json` | |
| - `TestResults/runs/<runId>/repro-bundle.json` |
| 3. No pass-by-return in live tests. | ||
|
|
||
| ## Test Rules | ||
| 1. Keep deterministic CI filter valid. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. Keep deterministic CI filter valid. | |
| 1. Keep deterministic CI filter valid. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/sonarcloud.yml (1)
38-53: AI summary inconsistency + token exposure in CLI arguments.Inconsistent summary: The AI-generated summary states this workflow uses
SonarSource/sonarqube-scan-action; the actual implementation uses the manualdotnet-sonarscanner begin/build/endpattern, which is a materially different integration.Token in CLI argument:
${{ secrets.SONAR_TOKEN }}is interpolated inline as/d:sonar.token="..."in both thebeginandendshell commands. This exposes the token value as a command-line argument visible in process listings (ps aux) on the runner. The scanner also supports authentication via theSONAR_TOKENenvironment variable, which avoids the process-list exposure. Combined with the fix proposed above (job-levelenv: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}), the scanner picks it up automatically and the explicit/d:sonar.token=flags can be removed from both steps:♻️ Proposed fix: use SONAR_TOKEN env var instead of CLI flag
- name: Sonar Begin shell: bash run: | dotnet-sonarscanner begin \ /k:"Prekzursil_SWFOC-Mod-Menu" \ /o:"prekzursil" \ - /d:sonar.token="${{ secrets.SONAR_TOKEN }}" \ /d:sonar.host.url="https://sonarcloud.io" - name: Build run: dotnet build SwfocTrainer.sln -c Release --no-restore - name: Sonar End shell: bash run: | - dotnet-sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}" + dotnet-sonarscanner end(Requires
SONAR_TOKENset at the job level as shown in the fix above.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sonarcloud.yml around lines 38 - 53, The workflow currently passes the secret via CLI (/d:sonar.token="${{ secrets.SONAR_TOKEN }}") to the dotnet-sonarscanner in both the "dotnet-sonarscanner begin" and "dotnet-sonarscanner end" steps, which exposes the token to process listings; remove the explicit /d:sonar.token= flags from both the begin and end invocations and instead set SONAR_TOKEN at the job level (env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}) so the scanner reads the token from the environment; also update the workflow summary/description to reflect that the manual dotnet-sonarscanner begin/build/end pattern is used (not the SonarSource/sonarqube-scan-action).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sonarcloud.yml:
- Around line 11-12: The workflow currently only grants permissions: contents:
read which prevents SonarCloud PR decoration; update the permissions block to
include pull-requests: write in addition to contents: read and ensure the
GITHUB_TOKEN is provided to the scanner (either export GITHUB_TOKEN as an env
var at the job level or pass it into the SonarCloud scanner step) so SonarCloud
can post analysis results and quality-gate status back to pull requests.
- Line 16: Remove the job-level conditional that uses secrets.SONAR_TOKEN (which
is invalid at job scope) and instead expose the secret via an env variable on
the job (e.g., set env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}), then delete
the job-level if: ${{ secrets.SONAR_TOKEN != '' }} and add step-level guards on
the Sonar-related steps using if: env.SONAR_TOKEN != '' so those steps only run
when the secret is present (apply to all steps that perform Sonar analysis or
upload).
---
Nitpick comments:
In @.github/workflows/sonarcloud.yml:
- Around line 38-53: The workflow currently passes the secret via CLI
(/d:sonar.token="${{ secrets.SONAR_TOKEN }}") to the dotnet-sonarscanner in both
the "dotnet-sonarscanner begin" and "dotnet-sonarscanner end" steps, which
exposes the token to process listings; remove the explicit /d:sonar.token= flags
from both the begin and end invocations and instead set SONAR_TOKEN at the job
level (env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}) so the scanner reads the
token from the environment; also update the workflow summary/description to
reflect that the manual dotnet-sonarscanner begin/build/end pattern is used (not
the SonarSource/sonarqube-scan-action).
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
Missing pull-requests: write permission will break PR decoration.
With only contents: read declared, SonarCloud cannot post analysis results or quality-gate status back to pull requests. The GITHUB_TOKEN also needs to be surfaced to the scanner (or set as GITHUB_TOKEN env var at the job level) for PR decoration to work.
🔧 Proposed fix: add pull-requests: write and GITHUB_TOKEN
permissions:
contents: read
+ pull-requests: write
jobs:
sonarcloud:
runs-on: ubuntu-latest
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: read | |
| permissions: | |
| contents: read | |
| pull-requests: write | |
| jobs: | |
| sonarcloud: | |
| runs-on: ubuntu-latest | |
| env: | |
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sonarcloud.yml around lines 11 - 12, The workflow
currently only grants permissions: contents: read which prevents SonarCloud PR
decoration; update the permissions block to include pull-requests: write in
addition to contents: read and ensure the GITHUB_TOKEN is provided to the
scanner (either export GITHUB_TOKEN as an env var at the job level or pass it
into the SonarCloud scanner step) so SonarCloud can post analysis results and
quality-gate status back to pull requests.
|
|
||
| jobs: | ||
| sonarcloud: | ||
| if: ${{ secrets.SONAR_TOKEN != '' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the workflow file to confirm the code at line 16
if [ -f .github/workflows/sonarcloud.yml ]; then
echo "=== File exists, reading content with line numbers ==="
cat -n .github/workflows/sonarcloud.yml | head -60
else
echo "=== File not found, checking for workflow files ==="
find . -name "*.yml" -o -name "*.yaml" | grep -E "workflows|github" | head -20
fiRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 1699
🌐 Web query:
GitHub Actions job level if conditional secrets context available
💡 Result:
No—the secrets context is not available in a job-level if: (e.g., jobs.<job_id>.if:), so expressions like this will fail validation:
jobs:
build:
if: ${{ secrets.MY_SECRET == 'true' }} # ❌ Unrecognized named-value: 'secrets'GitHub explicitly notes that secrets can’t be directly referenced in if: conditionals (at the job level) and recommends alternative patterns. [1]
What to do instead
- Gate at step level (where
secretsis available), and early-exit / skip the sensitive steps:- Put the secret into
env:and useif:on steps, or compute a non-secret boolean output and branch on that. [1][2]
- Put the secret into
- If your real goal is “turn a job on/off”, use a non-secret control like:
vars.*(repository/org/environment variables) orinputs.*(workflow_dispatch / reusable workflow inputs), which are allowed in job-level condition contexts per the context-availability rules. [2]
Sources: GitHub Docs on secrets + conditionals [1], and GitHub Docs context availability reference [2].
References:
[1] GitHub Docs – Using secrets in GitHub Actions (notes secrets can’t be directly referenced in if: conditionals) (docs.github.com)
[2] GitHub Docs – Contexts reference / context availability table (docs.github.com)
Citations:
- 1: https://docs.github.com/en/actions/how-tos/administering-github-actions/sharing-workflows-secrets-and-runners-with-your-organization?utm_source=openai
- 2: https://docs.github.com/actions/reference/accessing-contextual-information-about-workflow-runs?utm_source=openai
Job-level if: ${{ secrets.SONAR_TOKEN != '' }} causes workflow validation failure—secrets context is unavailable at job scope.
GitHub Actions does not allow the secrets context in job-level if: conditionals. This will cause an immediate validation error ("Unrecognized named-value: 'secrets'") and the job will not run.
Move the guard to step-level conditionals via an environment variable:
🔧 Fix: move secret check to step-level with env variable
jobs:
sonarcloud:
- if: ${{ secrets.SONAR_TOKEN != '' }}
runs-on: ubuntu-latest
+ env:
+ SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
steps:
- name: Checkout
uses: actions/checkout@v4Then add if: env.SONAR_TOKEN != '' to the Sonar-specific steps:
- name: Sonar Begin
+ if: env.SONAR_TOKEN != ''
shell: bash
run: |
dotnet-sonarscanner begin ...
- name: Sonar End
+ if: env.SONAR_TOKEN != ''
shell: bash
run: |
dotnet-sonarscanner end ...🧰 Tools
🪛 actionlint (1.7.11)
[error] 16-16: context "secrets" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sonarcloud.yml at line 16, Remove the job-level
conditional that uses secrets.SONAR_TOKEN (which is invalid at job scope) and
instead expose the secret via an env variable on the job (e.g., set env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}), then delete the job-level if: ${{
secrets.SONAR_TOKEN != '' }} and add step-level guards on the Sonar-related
steps using if: env.SONAR_TOKEN != '' so those steps only run when the secret is
present (apply to all steps that perform Sonar analysis or upload).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26fdef295e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasBundleField = body.includes('repro bundle json:'); | ||
| const hasClassification = body.includes('classification:'); | ||
| const hasSkipJustification = body.includes('justified skip'); | ||
|
|
||
| if (!hasBundleField || (!hasClassification && !hasSkipJustification)) { |
There was a problem hiding this comment.
This gate currently treats evidence as present if the PR body merely contains the literal strings repro bundle json: and classification:, which are always present in the default PR template placeholders, so PRs that touch src/SwfocTrainer.Runtime/, tools/, or tests/ can pass without supplying any real artifact path or classification value. In practice this makes the new evidence policy non-enforcing for most template-based PRs and allows missing runtime/tooling validation evidence to merge unnoticed.
Useful? React with 👍 / 👎.
1 participant
User description
Summary
AGENTS.mdoperating contracts for evidence-first runtime/mod worktools/schemas/repro-bundle.schema.json) with collection/validation scriptstools/run-live-validation.ps1with per-run isolation, scope selection, bundle emission, strict flags, and explicit artifact pathsValidation
dotnet build SwfocTrainer.sln -c Release --nologodotnet test tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj -c Release --no-build --filter "FullyQualifiedName!~SwfocTrainer.Tests.Profiles.Live&FullyQualifiedName!~RuntimeAttachSmokeTests" --nologopowershell.exe -NoProfile -ExecutionPolicy Bypass -File "tools/validate-policy-contracts.ps1"powershell.exe -NoProfile -ExecutionPolicy Bypass -File "tools/validate-repro-bundle.ps1" -BundlePath "tools/fixtures/repro_bundle_sample.json" -SchemaPath "tools/schemas/repro-bundle.schema.json" -Strictpowershell.exe -NoProfile -ExecutionPolicy Bypass -Command "& './tools/run-live-validation.ps1' -Configuration Release -NoBuild -Scope TACTICAL -RunId 'smoke-20260217-7' -EmitReproBundle \$true"powershell.exe -NoProfile -ExecutionPolicy Bypass -File "tools/compare-visual-pack.ps1" -BaselineDir "artifacts/visual-baselines" -CandidateDir "TestResults/runs/smoke-20260217-7" -OutputPath "TestResults/runs/smoke-20260217-7/visual-compare.json"Linked Issues
PR Type
Enhancement, Documentation
Description
Introduces repro-bundle contract with collection/validation scripts for evidence-first runtime/mod work
Adds scoped
AGENTS.mdoperating contracts for repository, runtime, tooling, and testsUpgrades live-validation tooling with per-run isolation, scope selection, bundle emission, and strict validation
Extends CI with repro-bundle schema smoke test and policy enforcement workflow
Adds external audit layer: visual compare workflow/script, SonarCloud workflow/config, jscpd duplication detection
Hardens issue/PR templates to require runId, bundle links, reason codes, and classifications
Updates runbooks and documentation to make reliability loop executable and enforceable
Diagram Walkthrough
File Walkthrough
14 files
New script to collect reproducible mod/runtime bundlesNew visual artifact comparison and diff detectionEnhanced with per-run isolation, scope selection, bundle emissionNew policy contract validation for AGENTS.md and templatesNew schema and semantic validation for repro bundlesHardened to require runId, bundle links, and classificationAdded runId and bundle evidence fields with validationAdded affected profiles and reliability evidence sectionsAdded repro-bundle schema smoke test stepNew jscpd duplication detection workflowNew policy contract validation and PR evidence enforcementNew SonarCloud code quality scanning workflowNew visual pack comparison and audit workflowNew JSON schema for repro bundle contract validation2 files
New jscpd configuration for duplication detectionNew SonarCloud project configuration13 files
New repository-level AI-native operating contractAdded repro bundle evidence requirement for runtime changesAdded repro bundle tooling section and usage examplesUpdated with bundle evidence format and reliability rulesNew evidence-first operating loop and decision rulesNew guide for SonarCloud and Applitools integrationRefactored with scope selection and bundle validation stepsAdded board lane policy and bundle evidence closure rulesAdded tooling contract tests and bundle validation examplesNew visual pack naming and comparison flow guideNew runtime-specific agent operating contractNew tests-specific agent operating contractNew tooling-specific agent operating contract1 files
New sample repro bundle for schema validation testingSummary by CodeRabbit
New Features
Documentation
Policy Evidence Tokens
Repro bundle JSON:
tools/fixtures/repro_bundle_sample.jsonClassification:
justified skip(ops/docs/tooling slice; no new live runtime behavior in this PR)