Conversation
Stream live canary output while preserving artifact logs and document OpenAI Codex lane setup.
There was a problem hiding this comment.
Code Review
This pull request establishes a 'Live Canary' regression system for IronClaw, adding documentation and automation scripts for multiple testing lanes, including smoke tests, persona-based workflows, and release upgrade verifications. The review feedback suggests several technical refinements to the new shell scripts: ensuring macOS compatibility for the artifact scrubber, accounting for shallow clones in CI environments, and optimizing test execution by removing redundant build steps.
|
|
||
| if [[ -s "${matches_file}" ]]; then | ||
| echo "Potential secret material found in live canary artifacts:" | ||
| sed -E 's/(bearer[[:space:]]+)[^[:space:]]+/\1<REDACTED>/Ig; s/(token[[:space:]]*[:=][[:space:]]*)[^[:space:]]+/\1<REDACTED>/Ig; s/(key[[:space:]]*[:=][[:space:]]*)[^[:space:]]+/\1<REDACTED>/Ig; s/(secret[[:space:]]*[:=][[:space:]]*)[^[:space:]]+/\1<REDACTED>/Ig' "${matches_file}" | head -200 |
There was a problem hiding this comment.
The I flag in the sed command is a GNU extension for case-insensitive substitution and is not supported by BSD sed (the default on macOS). This will cause the script to fail with a 'transform flag expected' error on macOS-based runners or local developer machines. Since the matches were already identified case-insensitively by grep -i on line 37, you can either remove the I flag or use a more portable approach like perl -pe for the redaction display.
References
- The project specifically targets macOS (darwin), so shell commands should use BSD-compatible syntax for consistency and portability across developer environments.
| DB_PATH="${DB_PATH:-${WORK_ROOT}/upgrade-canary.db}" | ||
|
|
||
| if [[ -z "${PREVIOUS_REF}" ]]; then | ||
| PREVIOUS_REF="$(git describe --tags --abbrev=0 2>/dev/null || true)" |
There was a problem hiding this comment.
In CI environments like GitHub Actions, repositories are often checked out as shallow clones (fetch-depth: 1). In such cases, git describe will fail to find any tags, causing the script to exit because PREVIOUS_REF cannot be auto-detected. It is recommended to document that this script requires a full clone or a sufficient fetch depth to correctly identify the previous release tag.
| echo "[upgrade-canary] building previous release" | ||
| ( | ||
| cd "${PREVIOUS_DIR}" | ||
| cargo build --no-default-features --features libsql |
There was a problem hiding this comment.
The cargo build command here (and on line 58) is redundant because the subsequent cargo test command will automatically trigger a build of the required targets. Removing these extra build steps can save significant time and disk space, especially since they are performed in temporary directories without a shared cache.
Summary
Notes