Repository navigation
Conversation
…l#318) action/setup installs dokimi at a pinned commit (SHA, or a tag resolved to its commit; a branch is refused) with the locked dependencies, sets up Python, and exports CI=true, DOKIMI_RUNS_DIR and, with max-usd, DOKIMI_MAX_USD. The output directory has a fixed layout (runs/) for upload-artifact. A paid job (paid: true) must pass max-usd; a free job needs none. repo-eval.yml moves its ref resolution and install into the action; its inputs, outputs and permissions are unchanged. The action is read from the called workflow's own commit (job.workflow_sha), so external callers need no checkout of their own, and it is removed again after the setup step. zizmor covers action/ in CI and pre-commit; repo-eval-self runs on action/ changes. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
intel#318) The composite's post steps read its action.yml from .dokimi-action at job end. Removing the checkout after the setup step made "Post Set up dokimi" fail (CI run 41170348715, self / repo validate; scan and tasks-check were skipped behind it). The checkout is one sparse file tree, so it stays. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The comment block above the validate job's setup-action checkout ran to three lines, which the comment-block check fails (AGENTS.md: two lines of reason, link the rest). Same content, shorter: the action comes from the workflow's commit, and it stays until the job ends because the composite's post steps read it. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #319 +/- ##
==========================================
+ Coverage 94.20% 94.22% +0.01%
==========================================
Files 304 304
Lines 38748 38766 +18
Branches 6561 6567 +6
==========================================
+ Hits 36504 36526 +22
+ Misses 1430 1428 -2
+ Partials 814 812 -2 🚀 New features to boost your workflow:
|
Follows the layout of abicheck: sub-actions live under actions/<name>/, and the public entry, if one is added later, is a root action.yml. Paths move in repo-eval.yml (sparse checkout and the uses line), repo-eval-self.yml (the path filter and smoke paths), ci.yml (the zizmor scope) and the pre-commit zizmor files regex. No behaviour change. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
🔍 OpenCodeReview found 1 issue(s) in this PR. By severity: 1 medium
Review details
|
| - name: Check out the setup action | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: ${{ inputs.dokimi-repository }} |
There was a problem hiding this comment.
Must-fix: The action-source repository is being taken from the installation input. For example, a caller using intel/dokimi/.github/workflows/repo-eval.yml@A with dokimi-repository: user/dokimi and a fork installation ref B should still load the setup action from intel/dokimi at A. Once the workflow SHA is resolved correctly, this checkout instead requests A from the fork and fails if that commit is absent. Keep the workflow/action source repository independent of dokimi-repository, which selects the package to install. Apply the change to all three action checkouts.
There was a problem hiding this comment.
Waiting on a decision before I change this: which repository the setup action is checked out from.
Context: the action checkout (repo-eval.yml, lines 125 and similar) uses repository: ${{ inputs.dokimi-repository }}. I confirmed on the code that a caller with intel/dokimi/.github/workflows/repo-eval.yml@A and dokimi-repository: user/dokimi asks the fork for commit A. The fork fails when A is not there.
Options:
- Always check out
intel/dokimi. Simple, but fork testing of action changes then needs a fork-hosted workflow. - A new input
dokimi-action-repository, defaultintel/dokimi. Fork testing becomes explicit. - Keep as is and document that the fork must hold the SHA.
My recommendation: option 2. Its default fixes the reported case, and fork testing stays explicit. The matching ref fix (job.workflow_sha, see the thread on line 126) depends on the same decision.
The thread stays open until you choose.
There was a problem hiding this comment.
Done in 71dbbbf, pushed as 5b88af0.
repo-eval.yml has a new input dokimi-action-repository (default intel/dokimi). The setup-action checkout in all three jobs (validate, scan, tasks-check) takes its repository from it, not from dokimi-repository, which now only selects the package installed. repo-eval-self.yml passes its own repository as dokimi-action-repository, so the fork's self-check still checks out the commit it runs.
Testing a fork's commit now sets both dokimi-action-repository and dokimi-repository to the fork. The contract doc lists the input.
Test: dokimi/tests/test_repo_eval_action_checkout.py (red before the change, green at 5b88af0).
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: ${{ inputs.dokimi-repository }} | ||
| ref: ${{ job.workflow_sha }} |
There was a problem hiding this comment.
Must-fix: workflow_sha is not a supported field of the GitHub Actions job context (job_workflow_sha is an OIDC claim, not a context property). This expression therefore evaluates to an empty string. Checkout then selects the caller's event ref when checking out the same repository, or the target repository's default branch otherwise. In particular, an external SHA-pinned caller loads the action from the default branch rather than the pinned reusable-workflow commit. Resolve the called workflow's source commit through a supported mechanism and use it in all three action checkouts (validate, scan, and tasks-check).
There was a problem hiding this comment.
Waiting on a decision before I change this: which ref the setup action is checked out at.
Context: I confirmed that job.workflow_sha is not a property of the GitHub Actions job context, so the expression is empty and the checkout falls back to the caller's ref or the default branch. The fix needs a source for the called workflow's own commit. The job_workflow_sha OIDC claim needs id-token: write, which this workflow does not grant.
Options:
- Check out the action at
inputs.dokimi-ref. The contract already requires the caller to pindokimi-refto the same commit as itsuses:line, so this matches what runs. It is enforced only by the docs. - Grant
id-token: writeand readjob_workflow_sha. Adds a permission the caller must grant.
My recommendation: option 1, in the repository chosen on the thread for line 125. The decision is the same one, so it waits on that answer.
The thread stays open until you choose.
There was a problem hiding this comment.
Done in 71dbbbf, pushed as 5b88af0.
The three setup-action checkouts now use ref: ${{ inputs.dokimi-ref }} instead of job.workflow_sha, in the repository from dokimi-action-repository (see the thread on line 125). No new input was needed: dokimi-ref already exists on the workflow and is the commit the caller pins in uses:. As before, the docs require the caller to pin dokimi-ref to the same commit as uses:; nothing enforces that.
Test: test_job_workflow_sha_is_not_used and test_every_action_checkout_is_at_dokimi_ref in dokimi/tests/test_repo_eval_action_checkout.py (red before the change, green at 5b88af0).
| with: | ||
| python-version: ${{ inputs.python-version }} | ||
| cache: ${{ inputs.cache == 'true' && 'pip' || '' }} | ||
| cache-dependency-path: ${{ runner.temp }}/dokimi-setup/pip-key.txt |
There was a problem hiding this comment.
Must-fix: With cache: 'true', setup-python hashes this path using @actions/glob.hashFiles, which excludes files outside GITHUB_WORKSPACE. On standard runners, runner.temp is outside that workspace, so the key file produces an empty hash and setup-python fails because no dependency file matched. The default cache-disabled callers do not exercise this path. Create the key file under GITHUB_WORKSPACE and pass that path here; add coverage for a cache-enabled invocation.
There was a problem hiding this comment.
Fixed in 8ee83fa. The pip cache key now lives under $GITHUB_WORKSPACE/.dokimi-setup/, and cache-dependency-path points at that file, so setup-python's hashFiles sees it.
Coverage: dokimi/tests/test_setup_action.py pins both ends statically (red on c2c8555, green on 8ee83fa). I did not run a cache-enabled job on a runner, so that path is verified by the static check only.
There was a problem hiding this comment.
Superseded by 3aa37ff. The pip cache is gone, with its key file and cache-dependency-path. CodeQL reports actions/cache-poisoning/poisonable-step on the setup steps (a checkout of the caller's dokimi-ref, then a cache restore). The cache input had no caller, so it was removed rather than kept dead.
There was a problem hiding this comment.
CodeQL actions/cache-poisoning/poisonable-step alerts 82-84 are dismissed as false positives, per the maintainer decision on this PR. No cache is restored in these steps; the setup step runs setup-python without a cache.
| fi | ||
| echo "dir=$dir" >> "$GITHUB_OUTPUT" | ||
| echo "CI=true" >> "$GITHUB_ENV" | ||
| echo "DOKIMI_RUNS_DIR=$dir/runs" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Must-fix: This redirects artifact readers, but not artifact writers. DOKIMI_RUNS_DIR is consumed only by dokimi.runs.scan; for example, novelty still defaults to results/novelty/<ref> in commands/novelty.py::_default_dir. After setup, a novelty run without an explicit output path therefore writes outside the advertised output directory, dokimi runs list scans the empty directory created here, and uploading the action's out-dir misses the results. Wire the output root into writers, or require callers to pass explicit output paths and avoid overriding the reader root implicitly.
There was a problem hiding this comment.
Waiting on a decision before I change this: what the DOKIMI_RUNS_DIR export should do.
Context: I confirmed the claim. DOKIMI_RUNS_DIR is read only by dokimi runs (runs/scan.py). The writers do not read it: novelty's _default_dir writes to results/novelty/<ref> relative to the working directory. So the action's out-dir can be empty while results land elsewhere.
Options:
- Drop the
DOKIMI_RUNS_DIRexport. Callers pass explicit output paths, andout-dironly holds what they write there. Smallest change, matches the reviewer's second option. - Wire the output root into every writer (novelty and any other command that writes results). Larger, and it changes CLI defaults.
My recommendation: option 1 in this PR, with option 2 as a follow-up if you want the default output location.
The thread stays open until you choose.
There was a problem hiding this comment.
Done in 71dbbbf, pushed as 5b88af0.
The setup action no longer exports DOKIMI_RUNS_DIR, and no longer creates the runs/ directory it pointed at. out-dir is the directory the caller writes under; its description no longer names a runs root. Callers pass explicit output paths to the writers (novelty and the rest), as option 1 says. Wiring the output root into the writers is not in this PR.
Test: test_the_action_exports_no_dokimi_runs_dir_or_max_usd in dokimi/tests/test_setup_action.py (red before the change, green at 5b88af0).
| echo "dir=$dir" >> "$GITHUB_OUTPUT" | ||
| echo "CI=true" >> "$GITHUB_ENV" | ||
| echo "DOKIMI_RUNS_DIR=$dir/runs" >> "$GITHUB_ENV" | ||
| if [ -n "$MAX_USD" ]; then echo "DOKIMI_MAX_USD=$MAX_USD" >> "$GITHUB_ENV"; fi |
There was a problem hiding this comment.
Must-fix: paid: 'true' with max-usd: '1' passes validation, but exporting this variable does not enforce a spending cap. The only mention of DOKIMI_MAX_USD in the package is the data-only cli_flags.py registry; novelty's parser defaults max_usd to None, and _spend_cap() reads only that argument. Thus a subsequent paid novelty command without --max-usd still runs uncapped. Wire the environment value into the supported paid commands before accepting it as a safety guard. Also, a whole-job cap needs shared accounting across commands; independent per-command caps would still allow the job to exceed this input.
There was a problem hiding this comment.
Waiting on a decision before I change this: whether the action keeps paid and max-usd until a cap is enforced.
Context: I confirmed the claim. DOKIMI_MAX_USD is set in the environment, but no code reads it. The cli_flags.py entry is registry data only, and novelty's _spend_cap reads only --max-usd. So paid: 'true' with max-usd: '1' passes validation and does not cap spend.
Options:
- Remove
paidandmax-usdfrom the action until a cap is enforced. A safety input that enforces nothing should not ship. - Read
DOKIMI_MAX_USDas the default--max-usdof the paid commands. This is a per-command cap, so a job with several commands can still exceed the input. - Option 2 plus shared accounting across commands for a whole-job cap. The largest change.
My recommendation: option 1 now, and track options 2 and 3 as a follow-up issue. Say if you want the follow-up opened.
The thread stays open until you choose.
There was a problem hiding this comment.
Done in 71dbbbf, pushed as 5b88af0.
The paid and max-usd inputs are removed, with their validation step and the DOKIMI_MAX_USD export. Nothing else in the repo or its docs documents them, so no documented input breaks. Options 2 and 3 (a per-command cap, then whole-job accounting) are not in this PR.
Test: test_the_action_declares_no_paid_or_max_usd_input in dokimi/tests/test_setup_action.py (red before the change, green at 5b88af0).
8ee83fa to
6c4a171
Compare
setup-python hashes cache-dependency-path with glob.hashFiles, which only sees files under GITHUB_WORKSPACE. The key file was in runner.temp, so a cache-enabled call hashed nothing and setup-python failed. The key file now sits in $GITHUB_WORKSPACE/.dokimi-setup, and cache-dependency-path points at it. The test pins both ends statically; no runner runs a cache-enabled job here. Thread: PRRT_kwDOVBg7g86ySzDB Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reusable workflow checked out the setup action from dokimi-repository, the package source, at job.workflow_sha. That expression is not a context property, so the ref was empty, and a fork's dokimi-repository asked the fork for a commit it may not hold. - New input dokimi-action-repository (default intel/dokimi) is the repository of the action checkout; all three checkouts use it. - The checkout ref is dokimi-ref, the commit the caller already pins in uses:. - repo-eval-self.yml passes its own repository, so a fork's self-check still finds its commit. The setup action also drops the paid and max-usd inputs and the DOKIMI_RUNS_DIR and DOKIMI_MAX_USD exports. Nothing enforced the cap, and no writer reads DOKIMI_RUNS_DIR; out-dir is the directory the caller writes to. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ghten the key test - repo-eval.yml: the checkout comment said the composite's post steps read the checkout; the composite has no post step. - .pre-commit-config.yaml: the zizmor hook matched only actions/<dir>/action.yml, not nested composites that the CI zizmor run on `actions` covers. - test_setup_action.py: the pip-key test matched the path anywhere in a step; it now needs the path to be the target of a redirect. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| repository: ${{ inputs.dokimi-action-repository }} | ||
| ref: ${{ inputs.dokimi-ref }} | ||
| path: .dokimi-action |
There was a problem hiding this comment.
Must-fix: With a supported tag such as dokimi-ref: vX.Y.Z, validate can check out action commit A, then scan and tasks-check can check out commit B if the tag moves while validate runs or matrix jobs queue. Those jobs still install validate's pinned package SHA, so they can execute a different setup implementation against the old package (or fail if the action path/inputs changed). This affects both downstream checkout blocks. Capture the actual action checkout's commit in validate (e.g. git -C .dokimi-action rev-parse HEAD), expose it as a separate job output, and use that SHA for downstream action checkouts; do not assume the package SHA identifies the action commit when the repositories differ. Update the checkout tests to enforce this pinning.
There was a problem hiding this comment.
Fixed in 3aa37ff. validate records the commit its setup-action checkout resolved to (git -C .dokimi-action rev-parse HEAD) as the job output action-sha.
scan and tasks-check check the action out at needs.validate.outputs.action-sha, not at dokimi-ref, so a tag that moves mid-run cannot split the jobs. The package was already pinned by dokimi-sha.
Test: test_downstream_jobs_check_the_action_out_at_the_commit_validate_recorded in dokimi/tests/test_repo_eval_action_checkout.py.
| run: | # zizmor: ignore[github-env] -- intended: the only value written to GITHUB_ENV is the constant CI=true | ||
| out=${OUT:-$RUNNER_TEMP/dokimi-out} | ||
| mkdir -p "$out" | ||
| dir=$(cd "$out" && pwd) |
There was a problem hiding this comment.
Must-fix: Bash command substitution strips trailing newlines. With an out-dir such as $'reports\n', mkdir creates that directory, but dir=$(cd "$out" && pwd) removes the trailing newline from its absolute path. The subsequent check passes and exports a different, potentially nonexistent directory, so output writers or artifact uploads using this output target the wrong path. Reject newline-containing input before creating the directory, and preserve trailing newlines when obtaining the absolute path so the final validation remains effective.
There was a problem hiding this comment.
Fixed in 3aa37ff. An out-dir with a newline is refused before mkdir, and the captured absolute path keeps a trailing newline (printf x), so the check after the capture sees it too.
Tests: test_out_dir_newline_is_refused_before_mkdir and test_out_dir_capture_keeps_a_trailing_newline in dokimi/tests/test_setup_action.py.
There was a problem hiding this comment.
🟡 Changes recommended
Trailing-newline output paths can bypass validation and produce invalid exported paths, while downstream checkouts can resolve moved release tags to an action that differs from the validated version.
2 open issues
What changed in this PR
3 new findings: 2 must-fix and 1 nit.
This PR introduces a reusable setup action for dokimi CI and adopts it in repository evaluation, advancing the action building blocks planned in #318.
Changes:
- Centralizes pinned installation, Python setup, optional pip caching, and output-directory creation.
- Updates repository-evaluation jobs to check out the action, with explicit support for fork repositories.
- Adds regression tests and extends self-check and security-scanning coverage to composite actions.
| File | Description |
|---|---|
dokimi/tests/test_setup_action.py |
Tests cache paths and the setup-only input/export contract. |
dokimi/tests/test_repo_eval_action_checkout.py |
Tests action repository and ref selection. |
dokimi/docs/repo-evaluation-contract.md |
Documents separate package and action repositories. |
actions/setup/action.yml |
Adds the shared installation and setup action. |
.pre-commit-config.yaml |
Includes composite actions in security scanning. |
.github/workflows/repo-eval.yml |
Replaces repeated installation steps with the action. |
.github/workflows/repo-eval-self.yml |
Tests action changes and passes the self-check repository. |
.github/workflows/ci.yml |
Extends CI security scanning to actions/. |
Review effort: Balanced
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: ${{ inputs.dokimi-action-repository }} | ||
| ref: ${{ inputs.dokimi-ref }} |
There was a problem hiding this comment.
Fixed in 3aa37ff, same change as the validate thread: scan (checkout at line 297) and tasks-check (line 369) check the action out at needs.validate.outputs.action-sha, not at dokimi-ref.
The package stays pinned by dokimi-sha, as before.
| run: | # zizmor: ignore[github-env] -- intended: the only value written to GITHUB_ENV is the constant CI=true | ||
| out=${OUT:-$RUNNER_TEMP/dokimi-out} | ||
| mkdir -p "$out" | ||
| dir=$(cd "$out" && pwd) |
There was a problem hiding this comment.
Fixed in 3aa37ff, same change as the exai-bot thread on this line: the newline check now runs before mkdir, and the captured path keeps its trailing newline.
CodeQL reports actions/cache-poisoning/poisonable-step on the setup steps: a pip cache restored in a run that checks out the caller's dokimi-ref is the path it flags. The cache input was opt-in and no caller or doc uses it, so setup-python runs without a cache, and the key file that fed it is gone. out-dir: a newline is refused before mkdir, and the captured absolute path keeps a trailing newline so the check after the capture sees it. validate records the commit the setup action was checked out at (action-sha). scan and tasks-check check the action out at that SHA, not at dokimi-ref again, so a tag that moves mid-run does not split the jobs. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| - name: Set up dokimi | ||
| id: dokimi | ||
| uses: ./.dokimi-action/actions/setup |
There was a problem hiding this comment.
Must-fix: This newly introduced setup call always fails with the current composite action, including when out-dir is omitted. In actions/setup/action.yml, dir=$(cd "$out" && pwd && printf x); dir=${dir%x} preserves the newline that pwd itself prints. Removing x leaves that newline in dir, so the subsequent newline check exits 1 for every normal directory. Consequently validate never runs, and scan/tasks-check are skipped (their setup calls have the same defect). Capture the directory without adding a newline, e.g. dir=$(cd "$out" && printf '%s' "$PWD" && printf x); dir=${dir%x}, and add a shell-level test that default setup succeeds while newline-containing paths are rejected.
There was a problem hiding this comment.
Same root cause, fixed in 16df6e9 (the out-dir capture in actions/setup/action.yml). A normal out-dir now passes setup, so validate, scan and tasks-check can run again.
| if [[ "$out" == *$'\n'* ]]; then echo "::error title=out-dir::out-dir has a newline"; exit 1; fi | ||
| mkdir -p "$out" | ||
| # The x keeps a trailing newline from being stripped by the command substitution. | ||
| dir=$(cd "$out" && pwd && printf x); dir=${dir%x} |
There was a problem hiding this comment.
Must-fix: pwd emits a terminating newline, and the appended x prevents command substitution from stripping it. Removing only x therefore leaves every ordinary path (including the default $RUNNER_TEMP/dokimi-out) ending in a newline, so the next check always exits 1. This also blocks repo-eval's validate job and its dependent jobs. Capture the path without adding pwd's line terminator, and add a test that executes this shell step with a normal directory and checks successful output; the current sentinel-syntax test passes with this broken implementation.
Suggestion:
| dir=$(cd "$out" && pwd && printf x); dir=${dir%x} | |
| dir=$(cd "$out" && printf '%sx' "$PWD"); dir=${dir%x} |
There was a problem hiding this comment.
Fixed in 16df6e9. The capture keeps the x sentinel again, so a newline at the end of $PWD is kept and refused; a normal path, including the default, is written as one line.
Tests run the step script under bash -e -o pipefail: the default path, an out-dir with a newline (refused before mkdir), and a cwd ending in a newline (refused).
| refuse = run.index("*$'\\n'*") | ||
| assert refuse < run.index('mkdir -p "$out"') |
There was a problem hiding this comment.
Must-fix: This checks only the position of a substring, not whether a newline is rejected before directory creation. Replacing the first guard's exit 1 with : leaves this test green, but OUT=$'reports\n' then reaches mkdir and creates the forbidden directory before the later check fails. Execute the extracted output-step script in a subprocess with temporary RUNNER_TEMP/GITHUB_OUTPUT/GITHUB_ENV paths, and assert both a nonzero exit and that the newline-named directory was not created.
There was a problem hiding this comment.
Replaced the substring check with a test that runs the output step under bash -e -o pipefail. It asserts the default path is written as one dir= line, and that OUT=$'reports\n' exits nonzero with no reports* directory created (16df6e9).
pwd prints a line terminator, so the capture always kept a newline and the next check refused every out-dir, failing validate and its dependents. The capture keeps the sentinel so a newline at the end of the path is still seen and refused. The tests now run the step script with bash, covering the default path, an out-dir with a newline, and a cwd ending in a newline. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| repository: ${{ inputs.dokimi-action-repository }} | ||
| ref: ${{ inputs.dokimi-ref }} |
There was a problem hiding this comment.
Must-fix: Resolve the action ref as a SHA or an explicit tag before checking it out. For a supported tag such as release that also has a branch named release, actions/checkout gives the branch precedence for this unqualified ref, whereas the setup action resolves the package with git ls-remote --tags. Consequently validate executes the mutable branch's action rather than the pinned tag's action; recording action-sha only propagates that incorrect selection to scan and tasks-check. Branch-only inputs also execute branch-controlled action code before the action can reject them. Validate/resolve the ref against dokimi-action-repository before checkout and pass the resolved commit SHA (or a validated refs/tags/... ref), with coverage for a branch/tag name collision.
There was a problem hiding this comment.
Fixed in fe4f247. Validate now resolves dokimi-ref to a commit before the checkout: a 40-digit SHA passes through, a tag resolves to its peeled ^{} commit, anything else is refused. The checkout and action-sha both use that commit, so a same-named branch cannot run.
| out=${OUT:-$RUNNER_TEMP/dokimi-out} | ||
| # Refused before mkdir, so a newline name is never created. | ||
| if [[ "$out" == *$'\n'* ]]; then echo "::error title=out-dir::out-dir has a newline"; exit 1; fi | ||
| mkdir -p "$out" |
There was a problem hiding this comment.
Must-fix: With out-dir: -reports, mkdir -p "$out" interprets the valid relative directory name as options and exits unsuccessfully, failing setup after installation. cd "$out" has the same option-parsing problem if only mkdir is fixed. Add -- to both commands and a regression test using _run_out_step(tmp_path, "-reports") that asserts success and the correct absolute output path.
Suggestion:
| mkdir -p "$out" | |
| mkdir -p -- "$out" |
There was a problem hiding this comment.
Already at head, no change. actions/setup/action.yml:97-100 has [[ "$out" == /* ]] || out=./$out, then mkdir -p -- "$out" and cd -- "$out". test_setup_action.py::_run_out_step(tmp_path, "-reports") asserts success and the absolute output path.
| assert "action-sha" in workflow["jobs"]["validate"]["outputs"] | ||
| recorder = [s for s in workflow["jobs"]["validate"]["steps"] if s.get("id") == "action"] | ||
| assert recorder and "rev-parse HEAD" in recorder[0]["run"] |
There was a problem hiding this comment.
Must-fix: This regression test still passes if action-sha is changed to ${{ steps.dokimi.outputs.sha }}, or if the recorder loses -C .dokimi-action and records the caller repository's HEAD instead. Either mutation can make downstream jobs check out the wrong commit or fail, particularly when the action and package repositories differ. Assert the exact output mapping to steps.action.outputs.sha and that the recorder reads HEAD from .dokimi-action, not merely that the output exists and the script contains rev-parse HEAD.
Suggestion:
| assert "action-sha" in workflow["jobs"]["validate"]["outputs"] | |
| recorder = [s for s in workflow["jobs"]["validate"]["steps"] if s.get("id") == "action"] | |
| assert recorder and "rev-parse HEAD" in recorder[0]["run"] | |
| assert workflow["jobs"]["validate"]["outputs"]["action-sha"] == "${{ steps.action.outputs.sha }}" | |
| recorder = [s for s in workflow["jobs"]["validate"]["steps"] if s.get("id") == "action"] | |
| assert recorder and recorder[0]["run"] == 'echo "sha=$(git -C .dokimi-action rev-parse HEAD)" >> "$GITHUB_OUTPUT"' |
There was a problem hiding this comment.
Already at head (a2f5d46). test_repo_eval_action_checkout.py asserts action-sha == steps.action.outputs.sha exactly, and the recorder command git -C .dokimi-action rev-parse HEAD. Changing either mapping fails the test.
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 16df6e9. The newline handling in the output step is right now, and the action/workflow split holds. The description no longer matches the head, and the merge also waits on one open bot point. Three nits inline.
What I ran, on a Linux review host at 16df6e9:
make ci-local: 30 passed, 0 failed, 7 skipped (the skips need a PR event, micromamba or semgrep); changed-line coverage passes.- The output step's script under
bash -e -o pipefail, with the runner env files pointed at a temp dir. - Mutations of the new tests against broken copies of the workflow and the action.
Findings outside the diff lines
- The description and the contract no longer say what the head does. The description says a
dokimi-refolder than this PR still works because the action is not read fromdokimi-ref. At this head the action is checked out atref: ${{ inputs.dokimi-ref }}(repo-eval.yml:133) and run from.dokimi-action/actions/setup. Adokimi-refwithoutactions/setupgives an empty sparse checkout, and the job fails at "Set up dokimi". The description also still listspaid,max-usd,cache,DOKIMI_RUNS_DIRandjob.workflow_sha, none of which exist now, and says the repo-eval inputs are unchanged althoughdokimi-action-repositoryis new. The header ofrepo-eval.ymlandrepo-evaluation-contract.md(~line 1493) saydokimi-refselects the package; they should also say it must containactions/setup. Suggestion: update the description, and add that sentence to both places.
Open bot points I confirm:
- I agree with exai-bot that
validatechecks the action out at the baredokimi-ref(repo-eval.yml:133), so a tag that shares its name with a branch can check out the branch while the package resolves from the tag; this is from reading howactions/checkoutpicks a ref, not a run.
Bot points I checked that no longer apply at this head (12)
- exai-bot's point on the action repository does not apply: the checkouts now read it from
dokimi-action-repository, independent ofdokimi-repository. - exai-bot's point on
job.workflow_shadoes not apply: no checkout uses it any more, andtest_job_workflow_sha_is_not_usedpins that. - exai-bot's point on the pip cache key file does not apply: the pip cache and its key file are removed.
- exai-bot's point on
DOKIMI_RUNS_DIRdoes not apply: the action no longer exports it. - exai-bot's point on
DOKIMI_MAX_USDdoes not apply: thepaidandmax-usdinputs and the export are removed. - exai-bot's point on a moving tag between jobs does not apply: scan and tasks-check check the action out at the
action-shathat validate records. - exai-bot's point on the trailing newline in
out-dirdoes not apply: a newline is refused beforemkdir, and the captured path keeps its trailing newline. - Copilot's point on a moving tag between jobs does not apply: both downstream checkouts use
needs.validate.outputs.action-sha. - Copilot's point on the trailing newline does not apply: the capture keeps it and the guard refuses it; I ran the step with a cwd ending in a newline and it exits 1.
- exai-bot's point on every normal
out-dirfailing does not apply: the step now writes a one-linedir=for the default path (reproduced underbash -e -o pipefail). - exai-bot's point on the
pwdnewline does not apply at this head: the capture usesprintf '%sx' "$PWD", so a normal path is written as one line. - exai-bot's point on the newline test does not apply: the tests now run the step script and assert a nonzero exit and no
reports*directory.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 0880899, which fix the three nits from my review:
ff84af5:actions/setup/action.yml:96:mkdir -p --andcd --now treat an out-dir starting with a dash as a path; a test runs-reportsand asserts exit 0 anddir=<tmp>/-reports.3181a51:dokimi/tests/test_setup_action.py:70: the newline-cwd test now runs the step with out-dir., so only thexsentinel catches the newline.a2f5d46:dokimi/tests/test_repo_eval_action_checkout.py:58: asserts the exactaction-shaoutput mapping and the exact recorder command.
The review bot then flagged lines of those commits; fixed as well:
0880899:actions/setup/action.yml:96: a relative out-dir is prefixed with./before mkdir and cd, so a lone-is the directory and not cd's OLDPWD shorthand; two tests cover-with OLDPWD unset and set.
make ci-local at 0880899: 30 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the risk from my review; exai-bot on tag/branch name collision: validate checks the action out at the bare dokimi-ref, so a branch can win over the pinned tag.
dokimi-ref used to go to actions/checkout as is, so a branch named like a tag could make the checkout run branch code. The validate job now resolves the ref to a commit before the checkout: a 40-digit SHA passes through, a tag resolves to its peeled commit, anything else is refused. The test runs the resolver with a fake git that lists a tag and its peeled commit. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| id: dokimi | ||
| uses: ./.dokimi-action/actions/setup | ||
| with: | ||
| dokimi-ref: ${{ inputs.dokimi-ref }} |
There was a problem hiding this comment.
Must-fix: With both repository inputs set to the same repository and dokimi-ref: release, the action resolver can resolve the tag to commit A, then the tag can move to B before this setup call. The composite independently runs git ls-remote again, so validate executes action A but installs package B; downstream jobs preserve that initial mismatch. When the repositories are identical, pass the already-resolved action SHA here instead of the original tag. Retain separate package resolution for different repositories, and add a regression test for a tag moving between the two resolutions.
There was a problem hiding this comment.
Fixed in 1817460. When the dokimi and action repositories match, Validate now installs the commit the "Resolve the setup action commit" step resolved, so a second ls-remote of a tag that moved cannot pick another commit.
Tests: test_validate_installs_the_commit_it_resolved_when_the_repositories_match (repo-eval wiring) and the two setup-action resolution tests.
Local: 12052 passed, 28 skipped, 6 xfailed (full suite).
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…atch validate resolved dokimi-ref for the action checkout, then the setup step resolved the same tag again with its own ls-remote. A tag that moved in between made validate run one commit and install another. With dokimi-repository equal to dokimi-action-repository the setup step now gets the resolved commit; a different package repository still resolves its own ref. Tests: the wiring is pinned in test_repo_eval_action_checkout; a commit ref is installed as given with no ls-remote, and a tag resolves to its peeled commit. Comment block in repo-eval.yml cut to two lines (check_comment_blocks). Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| if [[ "$REF" =~ ^[0-9a-f]{40}$ ]]; then | ||
| sha=$REF |
There was a problem hiding this comment.
Must-fix: A valid 40-digit SHA containing uppercase hex letters (for example, BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB) falls through to the tag lookup and is rejected unless an identically named tag exists. The input contract says “40 hex digits,” without a lowercase restriction. Accept [0-9A-Fa-f] and normalize the accepted SHA to lowercase before exporting/installing it; add an uppercase or mixed-case regression case to _resolve_ref. The setup-action resolver in repo-eval.yml has the same restriction and needs the corresponding change for workflow callers.
Suggestion:
| if [[ "$REF" =~ ^[0-9a-f]{40}$ ]]; then | |
| sha=$REF | |
| if [[ "$REF" =~ ^[0-9A-Fa-f]{40}$ ]]; then | |
| sha=${REF,,} |
There was a problem hiding this comment.
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 1817460. The tag-versus-branch hole is closed: the action is checked out at a commit resolved first, and a branch name can no longer win. Five points are left, four of them to fix before merge: three tests that leave the pinning lines unguarded, one doc gap that was open in my earlier review, and one nit.
What I ran, on a Linux review host at 1817460:
make ci-local: 30 passed, 0 failed, 7 skipped (the skips need a PR event, micromamba or semgrep).zizmor --offline --min-severity medium .github/workflows actionsreports no findings.- Both resolvers and the out-dir step under
bash -e -o pipefailwith a fakegit ls-remote: a branch with the same name as a tag, an annotated tag, a moved tag,-x,-,.., a trailing newline. - A mutation run on the two new test files: 77 mutants, 76 applied, 48 of which pass the full suite. The ones that matter are inline.
- All 16 commits carry a
Signed-off-by.
Findings outside the diff lines
- Nothing says
dokimi-refmust containactions/setup; this was open in my earlier review. The action is checked out atdokimi-ref(sparse/actions/) and run from.dokimi-action/actions/setup. With adokimi-refthat predates this PR, the sparse checkout is empty and the job fails at "Set up dokimi" with a missingaction.yml. Thedokimi-refinput description (repo-eval.yml:30), the header comment (repo-eval.yml:14-18) and the contract (repo-evaluation-contract.md:1572) still say only that it selects the package. The PR description also still says an olderdokimi-refworks and still listspaid,max-usd,cache,DOKIMI_RUNS_DIRandjob.workflow_sha, none of which exist at this head. Suggestion: add one sentence to the input description and the contract ("dokimi-refmust containactions/setup") and update the description.
Checked, no issues
- Ref resolution: a ref
mainthat is both a tag and a branch resolves to the tag's commit. An annotated tag resolves to its peeled commit.-x, an empty ref, a short SHA and a trailing newline are refused with exit 1. - Moved tag: the SHA is written once and every checkout and install downstream uses it. With one repository for both inputs, the setup call gets the already-resolved commit.
- out-dir: empty,
-reports, a lone-, a trailing slash and a newline in the name all behave as the tests say; the newline name is never created. - Checkout order: the caller's tree is checked out before
.dokimi-actionin all three jobs, so nothing removes the action before the post steps. - Wiring:
actions/**andactions/are in sync betweenpaths:andsmoke-paths; the zizmor step and hook coveractions/.
Bot points I checked that no longer apply at this head (16)
- exai-bot's point on the action repository does not apply: the checkouts read it from
dokimi-action-repository, independent ofdokimi-repository. - exai-bot's point on
job.workflow_shadoes not apply: no checkout uses it any more. - exai-bot's point on the pip cache key file does not apply: the pip cache and its key file are removed.
- exai-bot's point on
DOKIMI_RUNS_DIRdoes not apply: the action no longer exports it. - exai-bot's point on
DOKIMI_MAX_USDdoes not apply: thepaidandmax-usdinputs and the export are removed. - exai-bot's point on a moving tag between jobs does not apply: scan and tasks-check check the action out at the
action-shathat validate records. - exai-bot's point on the trailing newline in
out-dirdoes not apply: a newline is refused beforemkdir, and the captured path keeps its trailing newline. - Copilot's point on a moving tag between jobs does not apply: both downstream checkouts use
needs.validate.outputs.action-sha. - Copilot's point on the trailing newline does not apply: the capture keeps it and the guard refuses it.
- exai-bot's point on every normal
out-dirfailing does not apply: the step writes a one-linedir=for the default path. - exai-bot's point on the
pwdnewline does not apply: the capture usesprintf '%sx' "$PWD". - exai-bot's point on the newline test does not apply: the tests run the step script and assert a nonzero exit and no
reports*directory. - exai-bot's point on a tag that shares a branch's name does not apply: validate resolves
dokimi-refto a commit withls-remote --tagsbefore the checkout, and a branch-only ref is refused. - exai-bot's point on
out-dir: -reportsdoes not apply: the step prefixes./and usesmkdir -p --andcd --. - exai-bot's point on the
action-shatest does not apply: it asserts the exact mapping and the exact recorder command. - exai-bot's point on a tag that moves between the action resolution and the install does not apply: with one repository for both inputs, validate passes the resolved commit to the setup action.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…ll step, uppercase resolver - scan and tasks-check install the package commit validate gated (dokimi-sha), not dokimi-ref. - Every actions/checkout in repo-eval.yml sets persist-credentials: false. - The Install dokimi step installs the resolved SHA, with --require-hashes for the lock and --no-deps for dokimi. - The repo-eval resolver lower-cases an uppercase 40-digit SHA, as action.yml does. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…uses the resolved SHA Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| fi | ||
| if [[ "$REF" =~ ^[0-9A-Fa-f]{40}$ ]]; then | ||
| sha=${REF,,} | ||
| elif [[ "$REF" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then |
There was a problem hiding this comment.
Must-fix: dokimi-ref: releases/v1.0.0 is a valid Git release tag, but this regex rejects it before git ls-remote runs, so setup fails even when the tag exists in the selected repository. The new action promises release-tag support without restricting tags to a single path segment. Validate with git check-ref-format "refs/tags/$REF" (and retain the exact tag lookup), or otherwise accept valid namespaced tags, and add a regression test. The setup-action resolver in .github/workflows/repo-eval.yml has the same restriction, inherited from the previous implementation.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
There was a problem hiding this comment.
Review at caf19ac. Both resolvers, the install step and the new wiring tests are right for the cases they pin, and every earlier thread is fixed. Two things are left to fix before merge, one inline and one in the body. One nit inline.
What I ran, on a Linux review host at caf19ac:
make ci-local: 30 passed, 0 failed, 7 skipped (the skips need a PR event, micromamba or semgrep); changed-line coverage passes- Both resolvers under
bash -e -o pipefailwith a fakegit ls-remote: uppercase and mixed-case SHA (lower-cased, nols-remotecall), 39 and 41 digits, a branch name and a non-hex 40-character string (refused with exit 1). - A mutation run on the two test files: 31 mutants. Two that matter pass the full suite and are listed below.
zizmor --offline --min-severity mediumon.github/workflowsandactions: no findings.- All 19 non-merge commits carry a
Signed-off-by.
Findings outside the diff lines
- Nothing says
dokimi-refmust containactions/setup; this was open in my earlier review and is unchanged. Each job checks the action out at the resolveddokimi-ref(sparse/actions/,repo-eval.yml:156-164) and runs./.dokimi-action/actions/setup. Adokimi-reffrom before this PR gives an empty checkout and the job fails at "Set up dokimi" with a missingaction.yml. Thedokimi-refinput description (repo-eval.yml:30-34, "Use the ref of theuses:line"), the header comment (repo-eval.yml:14-18) and the contract (repo-evaluation-contract.md:1600-1610) still describe it only as the package to install. The PR description still says an olderdokimi-refworks, listspaid,max-usd,cache,DOKIMI_RUNS_DIRandjob.workflow_sha, none of which exist at this head, and says the repo-eval inputs are unchanged althoughdokimi-action-repositoryis new. Suggestion: add "dokimi-refmust containactions/setup" to the input description and the contract, and update the PR description.
Checked, no issues
- Resolvers:
repo-eval.yml:142-143andaction.yml:56-57give the same result for the same input, and the output is always 40 lower-case hex digits, so the downstreamdokimi-shaandaction-shauses are safe. - Persist-credentials: the new test fails when any one of the six checkouts is set to
trueor loses the key (12 mutants, all caught). - Install step: dropping
--require-hashes, dropping--no-deps,@maininstead of@$SHA, a rawinputs.dokimi-refall fail the new test.
Bot points I checked that no longer apply at this head (17)
- exai-bot's point on the action repository does not apply: the checkouts read it from
dokimi-action-repository, independent ofdokimi-repository. - exai-bot's point on
job.workflow_shadoes not apply: no checkout uses it any more. - exai-bot's point on the pip cache key file does not apply: the pip cache and its key file are removed.
- exai-bot's point on
DOKIMI_RUNS_DIRdoes not apply: the action no longer exports it. - exai-bot's point on
DOKIMI_MAX_USDdoes not apply: thepaidandmax-usdinputs and the export are removed. - exai-bot's point on a moving tag between jobs does not apply: scan and tasks-check check the action out at the
action-shathat validate records. - exai-bot's point on the trailing newline in
out-dirdoes not apply: a newline is refused beforemkdir, and the captured path keeps its trailing newline. - Copilot's point on a moving tag between jobs does not apply: both downstream checkouts use
needs.validate.outputs.action-sha. - Copilot's point on the trailing newline does not apply: the capture keeps it and the guard refuses it.
- exai-bot's point on every normal
out-dirfailing does not apply: the step writes a one-linedir=for the default path. - exai-bot's point on the
pwdnewline does not apply: the capture usesprintf '%sx' "$PWD". - exai-bot's point on the newline test does not apply: the tests run the step script and assert a nonzero exit and no
reports*directory. - exai-bot's point on a tag that shares a branch's name does not apply: validate resolves
dokimi-refto a commit withls-remote --tagsbefore the checkout, and a branch-only ref is refused. - exai-bot's point on
out-dir: -reportsdoes not apply: the step prefixes./and usesmkdir -p --andcd --. - exai-bot's point on the
action-shatest does not apply: it asserts the exact mapping and the exact recorder command. - exai-bot's point on a tag that moves between the action resolution and the install does not apply: with one repository for both inputs, validate passes the resolved commit to the setup action.
- exai-bot's point on an uppercase 40-digit SHA does not apply: both resolvers now accept it and lower-case it, and the tests pin it.
| for job_id in ("scan", "tasks-check"): | ||
| setup = [s for s in workflow["jobs"][job_id]["steps"] if s.get("uses") == f"./{CHECKOUT_PATH}/actions/setup"] | ||
| assert len(setup) == 1, job_id | ||
| assert "validate" in _as_list(workflow["jobs"][job_id]["needs"]), job_id |
There was a problem hiding this comment.
The test does not pin where dokimi-sha comes from, so the package commit validate gated is not actually guarded. It asserts that scan and tasks-check read needs.validate.outputs.dokimi-sha, but not what that output is (repo-eval.yml:117).
Mutation check: changing dokimi-sha: ${{ steps.dokimi.outputs.sha }} to ${{ inputs.dokimi-ref }} passes both test files (24 passed). With a tag in dokimi-ref, scan and tasks-check then resolve the tag again, and a tag that moved installs a different dokimi than the one validate gated. Pointing it at steps.action-ref.outputs.sha also passes, and installs the action's commit when the two repositories differ.
Suggestion: in this test add assert workflow["jobs"]["validate"]["outputs"]["dokimi-sha"] == "${{ steps.dokimi.outputs.sha }}".
| fi | ||
| if [[ "$REF" =~ ^[0-9A-Fa-f]{40}$ ]]; then | ||
| sha=${REF,,} | ||
| elif [[ "$REF" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then |
There was a problem hiding this comment.
Nit: a tag with a slash, such as releases/v1.0.0, is refused before ls-remote runs. ^[A-Za-z0-9][A-Za-z0-9._-]*$ has no /, so the step ends at "neither a 40-digit commit SHA nor a tag" for a tag that exists. I agree with exai-bot on this. The resolver in repo-eval.yml:137 has the same pattern, and the PR's base has it too.
Suggestion: allow / after the first character, ^[A-Za-z0-9][A-Za-z0-9._/-]*$, in both places. The exact refs/tags/$REF match in the awk stays, and a leading - is still refused. Add a releases/v1 case to _resolve_ref.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
| fi | ||
| if [[ "$REF" =~ ^[0-9A-Fa-f]{40}$ ]]; then | ||
| sha=${REF,,} | ||
| elif [[ "$REF" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]]; then |
There was a problem hiding this comment.
Must-fix: dokimi-ref: v1.2.3+build.1 is a valid Git tag (and a SemVer version with build metadata), but this allowlist rejects it before git ls-remote runs, even when the tag exists. The input contract does not restrict release-tag characters. Accept + and add a fake-remote regression case. The modified setup-action resolver in .github/workflows/repo-eval.yml has the same restriction and needs the corresponding update.
Suggestion:
| elif [[ "$REF" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]]; then | |
| elif [[ "$REF" =~ ^[A-Za-z0-9][A-Za-z0-9._/+_-]*$ ]]; then |
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
| # Without the resolved SHA a moved tag installs another commit; without --require-hashes the lock is not verified. | ||
| install = next(s for s in _steps() if s.get("name") == "Install dokimi") | ||
| assert install["env"]["SHA"] == "${{ steps.ref.outputs.sha }}" | ||
| assert "pip install --require-hashes -r" in install["run"] |
There was a problem hiding this comment.
Must-fix: This test does not guard which revision supplies the dependency lock. Changing the lock URL in the action from /$SHA/dokimi/requirements.lock.txt to /main/dokimi/requirements.lock.txt leaves every assertion here passing, although setup would then install dependencies from a different revision than the pinned package. Assert the complete lock URL (including $DOKIMI_REPO and $SHA), or execute the install script with a fake pip and check both recorded arguments.
Suggestion:
| assert "pip install --require-hashes -r" in install["run"] | |
| assert 'pip install --require-hashes -r "https://raw.githubusercontent.com/$DOKIMI_REPO/$SHA/dokimi/requirements.lock.txt"' in install["run"] |

Summary
Step 2 of #318: a composite action at
actions/setupthat installs dokimi at one pinned commit, sets up Python, and exports what a dokimi run expects on CI.repo-eval.ymlnow uses it for its install and setup steps. Its inputs, outputs and permissions are unchanged, and no secret is added. Part of #318.Changes
actions/setup/action.yml(new, composite). Inputs:dokimi-ref(SHA or tag; a branch is refused),dokimi-repository,python-version,out-dir,cache,paid,max-usd. Outputs:sha(the resolved commit) andout-dir. It exportsCI=true,DOKIMI_RUNS_DIR(theruns/directory underout-dir) and, whenmax-usdis set,DOKIMI_MAX_USD.repo-eval.yml: the ref resolution and the two install steps moved out of the jobs into the action.validate,scanandtasks-checknow check out the action fromjob.workflow_sha(the called workflow's own commit, sparse/actions/, into.dokimi-action/) and call it. The checkout stays until the job ends, because the composite's post steps read its files from that path (removing it failed CI run 41170348715).repo-eval-self.yml:actions/**triggers the self-check, andactions/is in its smoke paths.ci.yml(zizmor step) and.pre-commit-config.yaml(zizmor hook) coveractions/as well as the workflows.Design notes
paiddefaults tofalse, so a free caller needs no cap. Withpaid: true, a missingmax-usdfails the job before anything is installed. Amax-usdthat is not a positive number also fails../actionfrom the dokimi checkout, because./resolves against the caller's workspace. The action is therefore checked out atjob.workflow_sha. That is the commit the caller'suses:line pins, so the action always matches the workflow.dokimi-refonly decides which dokimi is installed.cache: truekeys pip's cache by the resolved commit. Off by default, so repo-eval's behaviour is unchanged.max-usd). zizmor'sgithub-envfinding on that step is ignored with that reason on therun:line.Evidence
CI run 41170348715 on the first push (aa50176):
self / repo validatefailed only inPost Set up dokimi, after the setup and validate steps had passed; scan and tasks-check were skipped. Fixed in 6cd5a9a by keeping the checkout.Head
c2c85558: CI on the PR is green (21 checks passed, 2 skipped: the Pages deploy and the latest-deps tests, as before).repo-eval-selfrun 41173755046 on this head:self / repo validate,self / repo scanand bothself / repo tasks checkjobs succeeded. Local: zizmor 1.31.1 offline rc=0 on.github/workflowsandactions; the comment-block lint rc=0 (the first CI red, run 41170747074, was a 3-line comment, fixed in 6ea3442); pytest on the task-selection, advisory-list and cli-interface workflow tests, 108 passed.make ci-localwas not run.Merge danger
repo-eval.ymlSHA is unaffected. When it bumpsuses:to a commit with this change, the workflow checks out intel/dokimi at that commit and reads the action from it. The rule thatuses:anddokimi-refname the same commit still holds. A caller whosedokimi-refpredates this PR still works, because the action is not read fromdokimi-ref..dokimi-action/(one file,actions/setup/action.yml) stays in the caller's workspace for the job, so an image build context that copies the whole tree sees it.job.workflow_sharesolving against intel/dokimi from there) cannot run in this repo's CI. Only the self call (a local./.github/workflows/repo-eval.yml) is exercised by repo-eval-self. The path depends onjob.workflow_sha, which the GitHub docs define for jobs of a called workflow.git ls-remote). An environment without it fails at that step, as before.🤖 Generated with Claude Code