Conversation
|
AI review result: no blocking findings. Checked areas:
Validation used:
|
There was a problem hiding this comment.
Pull request overview
This PR improves the local act-based workflow in ci-self to make targeted local verify runs easier/safer (workflow discovery + selection, clearer logs/timing, and safer verify workflow scaffolding), and updates the repository/docs so generated and in-repo verify workflows behave correctly under act.
Changes:
- Added
ci-self actcommand with workflow discovery/interactive selection, job validation hints, and timestamped streaming logs + benchmark timestamps. - Updated verify workflow templates and this repo’s
.github/workflows/verify.ymlto runactions/checkoutunderact, and to avoid GHA-only steps (Discord/artifact upload) duringactruns. - Added TTY confirmation prompts before creating/overwriting
verify.yml, plus documentation updates (JP/EN + guides).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the local ci-self act flow and caveats (JP). |
| README_EN.md | Documents the local ci-self act flow and caveats (EN). |
| ops/ci/scaffold_verify_workflow.sh | Adds TTY confirmations + updates templates for act compatibility (checkout + act bypass + Go setup behavior). |
| ops/ci/scaffold_verify_workflow_test.go | Adds coverage for create/skip/prompt behaviors in the scaffold script. |
| ops/ci/ci_self.sh | Implements ci-self act (workflow discovery/selection, job listing/validation, timestamped logs, timing). |
| ops/ci/ci_self_test.go | Adds tests for act UX/behavior (help, missing act, selection, invocation building, job hints). |
| docs/ci/QUICKSTART.md | Adds quickstart section for local targeted timing via ci-self act. |
| docs/ci/FLOW.md | Updates recommended flow to include ci-self act for local timing. |
| .github/workflows/verify.yml | Makes verify jobs runnable under act and avoids GHA-only integrations during act. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 4c405f4242
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mt4110
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
ops/ci/scaffold_verify_workflow.sh:216
- The scaffolded nix workflow template uses a floating action tag (
actions/checkout@v4). If the project policy is to pin actions to SHAs (as in.github/workflows/verify.yml), consider pinning this checkout step too.
- self-hosted
- mac-mini
timeout-minutes: 45
steps:
- uses: actions/checkout@v4
- name: Verify (Nix)
ops/ci/scaffold_verify_workflow.sh:266
- The scaffolded minimal workflow template uses a floating action tag (
actions/checkout@v4). If the project policy is to pin actions to SHAs (as in.github/workflows/verify.yml), consider pinning this checkout step too.
- self-hosted
- mac-mini
timeout-minutes: 20
steps:
- uses: actions/checkout@v4
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| while true; do | ||
| printf '> どのworkflowを、actで実行したいですか？\n' >&2 | ||
| idx=1 | ||
| for workflow in "${workflows[@]}"; do | ||
| printf '> [%d] %s\n' "$idx" "$(workflow_menu_label "$project_dir" "$workflow")" >&2 | ||
| idx=$((idx + 1)) | ||
| done | ||
| printf '> [q] quit\n' >&2 | ||
| printf '> ' >&2 | ||
| if ! IFS= read -r choice; then | ||
| printf '\n' >&2 | ||
| log_ts_err "SKIP: act selection cancelled" |
There was a problem hiding this comment.
select_local_workflow always prompts when multiple workflows exist. In non-interactive contexts (stdin/stdout not a TTY), this can block waiting for input; it would be safer to detect non-interactive mode and fail fast with a clear error telling the user to pass --workflow (and optionally --list to inspect).
| local line="" | ||
| while IFS= read -r line || [[ -n "$line" ]]; do | ||
| printf '[%s] %s\n' "$(timestamp_now)" "$line" | ||
| done |
There was a problem hiding this comment.
prefix_stream_with_timestamp runs date once per log line, which can become a noticeable CPU bottleneck for verbose act runs. Consider using a single process (e.g., awk with strftime()), or caching the timestamp at 1-second resolution, to reduce per-line process spawning overhead while keeping the same output format.
| local line="" | |
| while IFS= read -r line || [[ -n "$line" ]]; do | |
| printf '[%s] %s\n' "$(timestamp_now)" "$line" | |
| done | |
| awk '{ | |
| printf "[%s] %s\n", strftime("%Y %m/%d %H:%M:%S"), $0 | |
| fflush() | |
| }' |
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Go | ||
| if: ${{ !env.ACT }} | ||
| uses: actions/setup-go@v5 |
There was a problem hiding this comment.
The scaffolded workflow template uses floating action tags (actions/checkout@v4, actions/setup-go@v5). This repo’s own workflow pins actions to commit SHAs, which is safer against supply-chain changes; consider pinning the scaffolded actions as well (and updating them periodically).
| - uses: actions/checkout@v4 | |
| - name: Setup Go | |
| if: ${{ !env.ACT }} | |
| uses: actions/setup-go@v5 | |
| - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 | |
| - name: Setup Go | |
| if: ${{ !env.ACT }} | |
| uses: actions/setup-go@cdcb36043654635271a6c05b4a6f81a27334f679 |
What changed
ci-self actworkflow discovery, interactive workflow selection, timestamped streaming logs, benchmark timestamps, and job validation with helpful hintsverify.ymltemplates and this repo's own verify workflow soactions/checkoutstill runs underactverify.ymlactflow in the README and quickstart guidesWhy
actruns were too easy to misuse across different repositories because workflow selection and job selection could be confusedACTturned out to be the wrong model for real repo validationverify.ymlneeded a safer interactive confirmation pathImpact
ci-self actis now usable across repositories with clearer workflow/job guidance and better logsactverify.ymlis created or overwritten from a TTYValidation
env GOCACHE=/tmp/ci-self-go-cache GOTMPDIR=/tmp/ci-self-go-tmp mise x -- go test ./ops/cibash ops/ci/ci_self.sh act --workflow .github/workflows/verify.yml --job verify-litebash ops/ci/ci_self.sh act --project-dir /Users/takemuramasaki/dev/maakie-brainlab --workflow .github/workflows/verify.yml --job verify