Conversation
…c147d55834722 From dependabot PR #328 — applying only the maint-76-claude-code-review.yml change. The agents-auto-pilot.yml change is excluded as that file is managed by the stranske/Workflows sync. https://claude.ai/code/session_01D7662TN52iZPqh1HgAFBRQ
- Add scripts/windows/request_counter_risk_remote.cmd — operator submits
a run request by dropping a .request file in a shared folder
- Add scripts/windows/process_counter_risk_remote.cmd — worker polls the
shared folder, runs the CLI for each request, marks .done/.failed
- Add docs/remote_trigger_testing.md — full flow documentation
- Update release.py to:
- Copy Runner.xlsm into bundle as counter_risk_runner.xlsm (operator
entrypoint as documented in deployment instructions)
- Copy remote cmd scripts into bundle root
- Update README_HOW_TO_RUN.md to reference all three entry points
Resolves the four missing items flagged in the deployment instructions review.
https://claude.ai/code/session_01D7662TN52iZPqh1HgAFBRQ
There was a problem hiding this comment.
Pull request overview
Adds missing operator/remote-trigger entry points to the release bundle and documents the shared-folder remote execution flow.
Changes:
- Bundle assembly now copies
Runner.xlsminto the release ascounter_risk_runner.xlsmand includes Windows remote trigger scripts. - Generated
README_HOW_TO_RUN.mdnow highlights the Excel runner and documents the remote request/worker entry points. - Adds requester/worker
.cmdscripts and a newdocs/remote_trigger_testing.mdguide for end-to-end testing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/counter_risk/build/release.py | Copies Excel runner + remote scripts into the bundle and updates the generated bundle README. |
| scripts/windows/request_counter_risk_remote.cmd | Adds requester script to create .request files with run parameters. |
| scripts/windows/process_counter_risk_remote.cmd | Adds worker script to poll/claim requests and execute counter-risk run. |
| docs/remote_trigger_testing.md | Documents prerequisites, lifecycle, and troubleshooting for the remote trigger flow. |
| for /f "tokens=2 delims==" %%I in ('wmic os get localdatetime /value 2^>nul') do set "DT=%%I" | ||
| set "TIMESTAMP=%DT:~0,8%_%DT:~8,6%" | ||
|
|
There was a problem hiding this comment.
Timestamp generation relies on wmic, but wmic may be unavailable/disabled on some supported Windows installations. If wmic fails, DT (and thus TIMESTAMP) can be empty, increasing collision risk for request filenames. Consider using a PowerShell Get-Date fallback (or another pure-CMD approach) and validating that TIMESTAMP was populated before writing the request file.
| for /f "tokens=2 delims==" %%I in ('wmic os get localdatetime /value 2^>nul') do set "DT=%%I" | |
| set "TIMESTAMP=%DT:~0,8%_%DT:~8,6%" | |
| set "DT=" | |
| for /f "tokens=2 delims==" %%I in ('wmic os get localdatetime /value 2^>nul') do set "DT=%%I" | |
| if defined DT ( | |
| rem WMIC succeeded; DT is in format yyyyMMddHHmmss.ffffff+zzz | |
| set "TIMESTAMP=%DT:~0,8%_%DT:~8,6%" | |
| ) else ( | |
| rem WMIC unavailable or failed; fall back to PowerShell Get-Date | |
| for /f %%I in ('powershell -NoProfile -Command "Get-Date -Format yyyyMMdd_HHmmss" 2^>nul') do set "TIMESTAMP=%%I" | |
| ) | |
| if "%TIMESTAMP%"=="" ( | |
| echo ERROR: Could not generate timestamp for request filename. | |
| echo Ensure WMIC or PowerShell is available on this system. | |
| goto :error | |
| ) |
| for %%F in ("%REQUEST_FOLDER%\*.request") do ( | ||
| set "FOUND=1" | ||
| set "REQ=%%F" | ||
| set "BASE=%%~nF" | ||
| set "DIR=%%~dpF" | ||
|
|
||
| echo [%TIME%] Found: %%~nxF | ||
|
|
There was a problem hiding this comment.
for %%F in ("%REQUEST_FOLDER%\*.request") will still iterate once with the literal wildcard string when there are no matching files, which will incorrectly set FOUND=1 and print a spurious "Found" line. Add an existence check inside the loop (or use a different enumeration pattern) so the loop body only runs for real files.
| :: Build and write a settings JSON to TEMP | ||
| set "SETTINGS_FILE=%TEMP%\counter-risk-runner-settings.json" | ||
| set "INPUT_ROOT_VAL=!INPUT_ROOT!" | ||
| if "!INPUT_ROOT_VAL!"=="" set "INPUT_ROOT_VAL=%SCRIPT_DIR%" | ||
| ( | ||
| echo { | ||
| echo "discovery_mode": "manual", | ||
| echo "formatting_profile": "default", | ||
| echo "input_root": "!INPUT_ROOT_VAL:\=\\!", | ||
| echo "output_root": "!OUTPUT_DIR:\=\\!", | ||
| echo "strict_policy": "warn" | ||
| echo } | ||
| ) > "!SETTINGS_FILE!" |
There was a problem hiding this comment.
The worker writes runner settings to a fixed path (%TEMP%\counter-risk-runner-settings.json). If you run multiple workers on the same machine (the docs mention this is safe for testing) or if runs overlap, workers can overwrite each other’s settings mid-run. Use a per-request unique settings filename (e.g., include !BASE! or a random suffix) and clean it up after the run completes.
docs/remote_trigger_testing.md
Outdated
| | Requirement | Requester | Worker | | ||
| |---|---|---| | ||
| | Bundle extracted locally | yes | yes | | ||
| | Macros allowed (Runner.xlsm) | yes (optional) | no | |
There was a problem hiding this comment.
This doc refers to Runner.xlsm, but the release bundle copies/renames the operator workbook to counter_risk_runner.xlsm (per release.py and the generated README). Update the documentation to use the shipped filename to avoid confusion during remote-trigger setup.
| | Macros allowed (Runner.xlsm) | yes (optional) | no | | |
| | Macros allowed (counter_risk_runner.xlsm) | yes (optional) | no | |
src/counter_risk/build/release.py
Outdated
| "## Remote request (worker machine required)", | ||
| "- Requester: run request_counter_risk_remote.cmd", | ||
| "- Worker: run process_counter_risk_remote.cmd", | ||
| "- See docs/remote_trigger_testing.md for full details.", |
There was a problem hiding this comment.
The generated bundle README points to docs/remote_trigger_testing.md, but assemble_release() does not copy the docs/ directory into the bundle. In the assembled release, this reference will be broken. Consider copying docs/remote_trigger_testing.md into the bundle (e.g., remote_trigger_testing.md at the bundle root) or updating the README text to point to a location that actually ships with the bundle.
| "- See docs/remote_trigger_testing.md for full details.", | |
| "- See the Remote trigger testing documentation for full details.", |
src/counter_risk/build/release.py
Outdated
| LOGGER.warning("Runner.xlsm not found at '%s'; skipping.", src) | ||
| return [] |
There was a problem hiding this comment.
_copy_runner_xlsm() silently skips Runner.xlsm when it is missing, but the bundle README now recommends the Excel runner as the primary entry point. This can produce an incomplete release bundle without failing the build. If Runner.xlsm is required for a valid operator bundle, consider failing fast (raise ValueError) when it is absent, or adjust the README generation to only advertise the Excel path when the file was actually copied.
| LOGGER.warning("Runner.xlsm not found at '%s'; skipping.", src) | |
| return [] | |
| raise ValueError(f"Required Excel runner not found at '{src}'.") |
| def _copy_runner_xlsm(root: Path, bundle_dir: Path) -> list[Path]: | ||
| src = root / "Runner.xlsm" | ||
| if not src.is_file(): | ||
| LOGGER.warning("Runner.xlsm not found at '%s'; skipping.", src) | ||
| return [] | ||
| dst = bundle_dir / "counter_risk_runner.xlsm" | ||
| shutil.copy2(src, dst) | ||
| return [dst] |
There was a problem hiding this comment.
New release-bundle behavior is introduced here (copying Runner.xlsm into the bundle and renaming it), but the existing tests/test_release_bundle.py coverage doesn’t assert that this artifact is actually included when present. Adding a test case that seeds a fake Runner.xlsm and verifies it is copied (and listed in manifest.json) would prevent regressions.
|
|
||
| :: --- Input root (optional) --- | ||
| echo. | ||
| set /p INPUT_ROOT=Enter input root directory (leave blank to use config default): |
There was a problem hiding this comment.
The prompt says leaving INPUT_ROOT blank will "use config default", but the worker script (process_counter_risk_remote.cmd) actually defaults input_root to the bundle folder (%SCRIPT_DIR%). This mismatch can confuse operators; please align the prompt text with the actual behavior (or change the worker default to truly use config defaults).
| set /p INPUT_ROOT=Enter input root directory (leave blank to use config default): | |
| set /p INPUT_ROOT=Enter input root directory (leave blank to use worker default/bundle folder): |
🤖 Keepalive Loop StatusPR #333 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
Keepalive Work Log (click to expand)
|
- request_counter_risk_remote.cmd: add PowerShell fallback for wmic timestamp generation; validate TIMESTAMP before use; align input_root prompt text with actual worker default (bundle folder) - process_counter_risk_remote.cmd: guard for loop body with `if exist` to prevent false FOUND=1 when no .request files match the wildcard; use per-request unique settings filename (%BASE%) to avoid collision when multiple workers run concurrently; delete settings file after run - docs/remote_trigger_testing.md: fix Runner.xlsm -> counter_risk_runner.xlsm in prerequisites table - release.py: fail fast (ValueError) when Runner.xlsm is absent; add _copy_remote_trigger_doc() to include remote_trigger_testing.md at bundle root; fix README reference from docs/ path to bundle-root path - tests/test_release_bundle.py: seed Runner.xlsm, remote scripts, and remote_trigger_testing.md in _write_fake_repo; add five new tests: copy_runner_xlsm copies / raises-when-missing, copy_remote_scripts, assemble_release includes all new artifacts in bundle + manifest, assemble_release fails fast when Runner.xlsm missing https://claude.ai/code/session_01D7662TN52iZPqh1HgAFBRQ
Automated Status Summary
Scope
Sync Summary
Files Updated
Tasks
Acceptance criteria
Head SHA: 51b0ad4
Latest Runs: ❔ in progress — Agents PR Meta
Required: gate: ⏸️ not started