Skip to content

fix(ci): keep upsert-issue self-tests offline - #333

Merged
devantler merged 3 commits into
mainfrom
codex/upsert-offline-tests-332
Sep 30, 2026
Merged

devantler merged 3 commits into
mainfrom
codex/upsert-offline-tests-332

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Catalogue tests currently create and close a shared live issue, so routine CI runs change the tracker and can interfere with each other.

What

Run those issue lifecycle tests offline with read-only access, including the hosted action smoke test. Preserve the existing coverage and expand checks for issue contents, outputs and failures; guard against restoring live credentials or bypassing required CI. This delivers the issue-test portion of the broader catalogue cleanup.

Fixes #332
Part of #259

@devantler devantler added the bug label Sep 30, 2026
@github-code-quality

github-code-quality Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Code Coverage Overview

Languages: Go, C#

Go / code-coverage/test

The overall line coverage in commit 5d3d491 in the codex/upsert-offline... branch remains at 50%, unchanged from commit 8c99de1 in the main branch.

C# / code-coverage/dotnet

The overall line coverage in commit 5d3d491 in the codex/upsert-offline... branch remains at 100%, unchanged from commit 8c99de1 in the main branch.


Updated September 30, 2026 18:55 UTC

@devantler

devantler commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Validation for source head 5d3d4917716ebe548be05a6f298539a2b02562b1:

  • RED: the new CI boundary guard rejects current main because its issue tests invoke the live action. GREEN: the dedicated job is contents-read only, forwards no secret, uses a synthetic fixture token and repository, disables persisted checkout credentials, and blocks the required check on failures.
  • The action's actual shell implementation passes all 26 offline scenarios, preserving the 14 existing cases and adding payload/output, body-file, input-error and API-failure coverage.
  • All 19 CI mutations are rejected for their intended reasons, including write permissions, secret forwarding, persisted credentials, omitted/ignored tests, missing required aggregation and live smoke-test inputs. RED: equivalent paths with a trailing slash bypassed the initial guard. GREEN: normalized paths also reject trailing-slash and dot-segment variants.
  • RED: the existing merge-group isolation check rejected an unconditional candidate-code job. GREEN: the offline job retains the catalogue's merge-group/release scheduling exclusion, with regressions rejecting a missing predicate or unsafe disjunction. Every command in the actual CI coverage-parity job passed locally after this repair.
  • Two independent action mutations fail: suppressing reopen breaks five scenarios, and ignoring body-file breaks three. Neither mutation is in the PR.
  • The hosted-smoke setup, actual action run block and output verification also pass locally using the inputs declared in CI. The real existing smoke issue ([test] upsert-issue smoke test #248) remains closed with its pre-change update timestamp 2026-09-30T16:14:53Z.
  • All 14 required repository baseline commands, ShellCheck, CI coverage parity, test-script wiring, the shell pipeline adoption check and diff whitespace validation passed. GitHub verifies the source signature as valid.

Current-head hosted evaluation: offline behavior job 110021326393 completed successfully with contents-read permission. Its log confirms all 26 behavior cases, all 19 boundary mutations and the actual composite-action invocation with exact caller outputs. A subsequent read of #248 still shows closed and 2026-09-30T16:14:53Z, confirming that the legacy live smoke issue was not changed.

The full current-head CI run 36754204972 completed successfully on attempt 1, and CI - Required Checks reported success. The provider review remains outstanding; this is not yet promotion readiness.

Local review: checked all five changed files, the permission and credential boundary, exact argument/output assertions, synthetic CLI isolation, and the required-check dependency/result wiring. No actionable finding remains. The consumer action's implementation and interface are unchanged; #259 remains open for the other catalogue CI families.

@github-actions

Copy link
Copy Markdown

✅MegaLinter analysis: Success

Descriptor Linter Files Fixed Errors Max errors Warnings Elapsed time
✅ COPYPASTE jscpd yes no no 0.56s
✅ GO revive 2 0 0 8.8s
✅ REPOSITORY betterleaks yes no no 0.63s
✅ REPOSITORY checkov yes no no 17.53s
✅ REPOSITORY git_diff yes no no 0.06s
✅ REPOSITORY osv-scanner yes no no 0.84s
✅ REPOSITORY secretlint yes no no 0.95s
✅ REPOSITORY syft yes no no 2.85s
✅ REPOSITORY trivy yes no no 11.01s
✅ REPOSITORY trivy-sbom yes no no 0.13s
✅ REPOSITORY trufflehog yes no no 4.98s

See detailed reports in MegaLinter artifacts

Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)

  • Documentation: Custom Flavors
  • Command: npx mega-linter-runner@10.1.0 --custom-flavor-setup --custom-flavor-linters COPYPASTE_JSCPD,GO_REVIVE,REPOSITORY_CHECKOV,REPOSITORY_GIT_DIFF,REPOSITORY_BETTERLEAKS,REPOSITORY_OSV_SCANNER,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG

MegaLinter is provided by OX Security
Show us your support by starring ⭐ the repository

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

✅MegaLinter analysis: Success

Descriptor Linter Files Fixed Errors Max errors Warnings Elapsed time
✅ COPYPASTE jscpd yes no no 0.37s
✅ GO revive 2 0 0 7.19s
✅ REPOSITORY betterleaks yes no no 0.58s
✅ REPOSITORY checkov yes no no 12.69s
✅ REPOSITORY git_diff yes no no 0.07s
✅ REPOSITORY osv-scanner yes no no 0.55s
✅ REPOSITORY secretlint yes no no 0.68s
✅ REPOSITORY syft yes no no 1.1s
✅ REPOSITORY trivy yes no no 8.19s
✅ REPOSITORY trivy-sbom yes no no 0.07s
✅ REPOSITORY trufflehog yes no no 3.46s

See detailed reports in MegaLinter artifacts

Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)

  • Documentation: Custom Flavors
  • Command: npx mega-linter-runner@10.1.0 --custom-flavor-setup --custom-flavor-linters COPYPASTE_JSCPD,GO_REVIVE,REPOSITORY_CHECKOV,REPOSITORY_GIT_DIFF,REPOSITORY_BETTERLEAKS,REPOSITORY_OSV_SCANNER,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG

MegaLinter is provided by OX Security
Show us your support by starring ⭐ the repository

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6ea1ca71-a4e5-4d06-bc65-75b647f552ba

📥 Commits

Reviewing files that changed from the base of the PR and between 8c99de1 and 5d3d491.

📒 Files selected for processing (5)
  • .github/tests/test-upsert-issue-ci.sh
  • .github/tests/test-upsert-issue-reopen.sh
  • .github/tests/upsert-issue-smoke.sh
  • .github/workflows/ci.yaml
  • actions/upsert-issue/README.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
🪛 ast-grep (0.45.3)
.github/tests/test-upsert-issue-reopen.sh

[warning] 109-109: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: GH_TOKEN="offline-fixture"
Note: [CWE-798] Use of Hard-coded Credentials.

(hardcoded-password-assignment-bash)

🪛 Checkov (3.3.17)
.github/workflows/ci.yaml

[low] 2543-2544: Base64 High Entropy String

(CKV_SECRET_6)

🔇 Additional comments (4)
.github/tests/test-upsert-issue-reopen.sh (1)

85-151: LGTM!

.github/workflows/ci.yaml (1)

2505-2550: LGTM!

.github/tests/upsert-issue-smoke.sh (1)

1-44: LGTM!

actions/upsert-issue/README.md (1)

64-74: LGTM!


📝 Walkthrough

Walkthrough

The change adds offline tests for the upsert-issue action and its CI wiring. The CI workflow replaces live issue smoke-test jobs with one job that runs behavior tests, checks CI boundaries, and verifies the action against an offline CLI fixture. The required-check aggregation now includes the consolidated job. The README documents the test coverage and commands.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 5d3d4

The change replaces live issue tests with offline behavior and smoke checks included in required CI. No actionable defect remains; merge after normal required checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5d3d4

The change reduces CI authority and removes live tracker mutations while preserving workflow-level failure propagation. No introduced security concern was established. Remaining uncertainty concerns repository-level required-check enforcement and live API authorization, which offline checks do not verify.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Attacker-controlled checked-out PR code still executes in a CI job, but the inspected job exposes repository-read rather than issue-write authority. The change removes its intended path to modifying the shared tracker; no new tenant, service, data-store or deployment authority was established.

Trust Boundaries and Controls

  • observed — The workflow guard rejects recognized live upsert-issue step invocations, elevated permissions, forwarded credentials, persisted checkout credentials, ignored failures and missing required-result wiring. Its mutation cases exercise these checks. These are regression controls over workflow configuration, not a security boundary against a PR author who can also edit the guard.

Resilience and Maintainability Implications

  • inferred — Normal smoke failures cannot silently produce a successful verification: strict shell execution and non-ignored steps stop the job, while verification rejects missing or nonmatching calls and outputs. Prepare resets call state, and repeating action execution without prepare violates the exact three-call assertion. The fixed temporary path and PATH entry have no helper-owned cleanup; cross-job collision is not established for the inspected ubuntu-latest invocation, while shared-directory reuse by other callers remains unverified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy the coding requirements in #332. The offline fixture executes the action run block and covers create, update, reopen, close, no-op, body-input validation, payloads, outputs, labels…
Out of Scope Changes check ✅ Passed The changed test scripts, CI workflow, offline gh fixture, and README testing section directly support #332. The workflow consolidation and required-check update support the dedicated offline job. N…
Title check ✅ Passed The title clearly and concisely summarizes the main change: keeping the upsert-issue CI self-tests offline.
Description check ✅ Passed The description directly explains the problem, the offline testing approach, read-only access, expanded coverage, and CI safeguards described by the changeset.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (2 skipped: 2 unsupported.)

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Resolution of the ancillary warnings in CodeRabbit's review summary, at 5d3d4917716ebe548be05a6f298539a2b02562b1:

  • The hardcoded-credential and entropy warnings refer to the literal offline-fixture sentinel in the isolated test CLI and hosted smoke input. It is not a GitHub credential. The mocked CLI verifies that exact sentinel; the job holds only contents-read permission, and the completed hosted run proves the calls remain in the fixture. Injecting a real secret would defeat this test boundary.
  • The docstring warning is refuted for these private Bash test helpers. Their purpose is documented in the script headers, the behavior helper has an adjacent explanation, and the README documents the runnable test entrypoints. The guard's success/error contract is exercised by all 19 unsafe CI mutations. This change introduces no consumer-callable function or undocumented consumer input; the coverage percentage does not identify a missing user-facing API contract.

No source change is required for these warnings. body_findings=0-resolved@5d3d4917716ebe548be05a6f298539a2b02562b1; the summary's substantive current-head verdict reports no actionable comments, and the full thread read is zero.

Restarting the ordered provider loop after this same-head refutation. CodeRabbit's per-PR summary states that this review used its included allowance and zero included reviews remain. The follow-up review will use the available included Codex lane.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T18:41:12.408623Z 5d3d491 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 5d3d491771

ℹ️ 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".

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Readiness for 5d3d4917716ebe548be05a6f298539a2b02562b1:

  • Programmatically tested: full CI run 36754204972 and its required summary passed on attempt 1. Local validation includes all 14 repository baseline commands, 26 behavior cases, 19 CI boundary controls and independent behavioral regressions.
  • Reviewed: CodeRabbit's current-head summary is GREEN, with its ancillary fixture/documentation warnings explicitly resolved in this record. The subsequent included Codex review is clean at 5d3d491771, after the refutation and authenticated follow-up request. All review objects and comment-form findings were read; unresolved threads are 0 of 0.
  • Tried as a user: the actual hosted composite action ran against the offline CLI and its exact inputs/outputs were verified in job 110021326393. The previous live smoke issue remains closed with its original timestamp.
  • Fresh base: fetched main is 8c99de12af799346b9dda10da842310e581dd33d; the branch is zero commits behind, mergeable, and GitHub reports CLEAN. Every readable required gate passes. The separate gate census retains UNVERIFIED for the opaque code-quality rule; CLEAN is GitHub's authoritative enforcement result under the merge policy.

Promoting the draft on these three readiness conditions and proceeding with the exact-head merge preflight. #259 remains open for its other real child issues.

@devantler
devantler marked this pull request as ready for review September 30, 2026 18:47
@devantler
devantler merged commit ab13baa into main Sep 30, 2026
491 checks passed
@devantler
devantler deleted the codex/upsert-offline-tests-332 branch September 30, 2026 19:08
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Delivered in v2.5.4.

PR #333 merged the reviewed source head 5d3d4917716ebe548be05a6f298539a2b02562b1 as signed squash commit ab13baa07e443e9a0496cea1b166220af641148d. The published tag resolves directly to that merge commit.

  • Five live upsert smoke jobs are replaced by one offline, contents-read job with no forwarded secret or persisted checkout credential. The production action implementation and consumer interface are unchanged.
  • All 26 behavior scenarios and 19 CI boundary controls passed again from an isolated, clean worktree at the merge commit. The actual action run block, caller inputs/outputs, merge-group guard and ShellCheck also passed.
  • The full post-promotion PR CI passed before merge. CodeRabbit's current-head verdict is GREEN; the subsequent included Codex review is clean after the documented warning refutations. The final review census was zero unresolved threads and no unresolved body findings.
  • Main CI, its actual CI - Required Checks gate, Release, CD, CodeQL and Code Quality all completed successfully at the merge commit.
  • The main hosted offline job passed with contents-read permission. Its log confirms the 26 scenarios, 19 controls, actual composite invocation and exact outputs. A subsequent authenticated read still shows the legacy smoke issue [test] upsert-issue smoke test #248 closed with unchanged timestamp 2026-09-30T16:14:53Z.

#332 is closed and verified. #259 remains open for the TODO, auto-merge and final catalogue audit children. The TODO follow-up's implementation refinement records the distinct wrapper, hosted-permission and genuine scanner proof boundaries. Only this session's exact-head review reservation was retired, with removal read back successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run upsert-issue catalogue self-tests without live issue mutations

1 participant