Skip to content

fix: per-issue conclusion concurrency in issue-duplication-detector#2318

Merged
lpcox merged 5 commits intomainfrom
copilot/update-copilot-token-usage-report
May 1, 2026
Merged

fix: per-issue conclusion concurrency in issue-duplication-detector#2318
lpcox merged 5 commits intomainfrom
copilot/update-copilot-token-usage-report

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 30, 2026

The Issue Duplication Detector had a ~40% cancellation rate when issues were opened in batches. All agent jobs completed successfully — only the final conclusion job was being cancelled.

Root cause: The conclusion job used a single shared concurrency group across all workflow runs. GitHub Actions enforces a hard limit of 1 running + 1 pending job per group; with 7 runs completing simultaneously, each new arrival evicted the current pending job, cascading cancellations through the batch.

Changes

  • issue-duplication-detector.lock.yml — Scope the conclusion job's concurrency group to the triggering issue number:

    # Before (shared across all runs):
    group: "gh-aw-conclusion-issue-duplication-detector"
    
    # After (isolated per issue):
    group: "gh-aw-conclusion-issue-duplication-detector-${{ github.event.issue.number || github.run_id }}"

    github.run_id as fallback handles workflow_dispatch triggers where no issue number exists.

  • postprocess-smoke-workflows.ts — Add an idempotent transformation that re-applies this fix after any future gh aw compile recompilation. Uses flexible [ ]+ whitespace matching to be resilient to YAML indentation changes.

  • postprocess-smoke-workflows.test.ts — 5 new tests covering: pattern matching, transformation output, idempotency via sentinel, preservation of cancel-in-progress: false, and group name prefix integrity.

Copilot AI linked an issue Apr 30, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Update Copilot token usage report for 2026-04-30 fix: per-issue conclusion concurrency in issue-duplication-detector Apr 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 86.08% 86.16% 📈 +0.08%
Statements 86.08% 86.15% 📈 +0.07%
Functions 88.18% 88.18% ➡️ +0.00%
Branches 80.32% 80.36% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 87.4% → 87.7% (+0.29%) 87.0% → 87.3% (+0.27%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI requested a review from lpcox April 30, 2026 13:56
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

AWF creates audit files (squid.conf, docker-compose.redacted.yml,
policy-manifest.json) as root with 0o600 permissions. When gh-aw's
post-job secret scanner runs as the runner user, it gets EACCES
trying to stat/scan these files, causing job failures.

Since audit files already have secrets redacted, change permissions
from 0o700/0o600 to 0o755/0o644 so they're readable without needing
the chmod a+rX cleanup step to have run first.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

Security Review: Audit Artifact Permission Relaxation

Finding (Low Severity): src/docker-manager.ts

The audit directory and its files are now created world-readable from the start (0o755/0o644) instead of owner-only (0o700/0o600).

For the default case this is safe: workDir itself is still created at 0o700 (line 2117), so other OS users cannot traverse into /tmp/awf-<ts>/audit/ anyway — the relaxed permissions on the subdirectory have no practical effect.

The concern is with custom auditDir paths outside workDir: If a caller sets config.auditDir to a location outside the protected workDir (e.g., a shared log directory), docker-compose.redacted.yml will be immediately world-readable. Its safety then depends entirely on redactDockerComposeSecrets() at docker-manager.ts:2086 being complete. That function uses keyword matching on env var names only:

/(?:KEY|TOKEN|SECRET|PASSWORD|CREDENTIALS?|_B64|_PAT|_AUTH|PRIVATE_KEY)/i

Edge-case names (e.g. ANTHROPIC_API, GH_APP_INSTALLATION, COPILOT_OIDC_TOKEN) would not be caught.

Suggested action: No change needed for the common path. Consider adding a code comment at line 2342 noting that the 0o755 mode is only safe when auditDir is nested inside the 0o700-protected workDir, and flag or warn if a custom auditDir is used that doesn't share that protection.


No issues found in the GitHub Actions workflow changes (concurrency group scoping) or the post-processing script additions.

Generated by Security Guard for issue #2318 · ● 116.5K ·

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox lpcox marked this pull request as ready for review April 30, 2026 23:10
@lpcox lpcox requested a review from Mossaka as a code owner April 30, 2026 23:10
Copilot AI review requested due to automatic review settings April 30, 2026 23:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Scopes the issue-duplication-detector workflow’s conclusion job concurrency group to the triggering issue (or run_id fallback) to prevent GitHub Actions concurrency-group eviction/cancellation when issues are opened in batches. Also adds a post-compile postprocessor transform + tests to keep the lock workflow corrected after future gh aw compile runs. This PR additionally changes AWF audit artifact permissions in writeConfigs().

Changes:

  • Update issue-duplication-detector.lock.yml conclusion job concurrency group to be per-issue (issue.number || run_id).
  • Add an idempotent postprocess transform in postprocess-smoke-workflows.ts to re-apply the concurrency fix after compilation.
  • Add unit tests validating the regex/transform behavior and idempotency.
Show a summary per file
File Description
src/docker-manager.ts Changes audit directory + audit artifact file permissions to be world-readable.
scripts/ci/postprocess-smoke-workflows.ts Adds a targeted transform to scope issue-duplication-detector conclusion concurrency per issue.
scripts/ci/postprocess-smoke-workflows.test.ts Adds tests for the new postprocess regex/transform (and idempotency guard).
.github/workflows/issue-duplication-detector.lock.yml Applies the per-issue concurrency group fix in the compiled lock workflow.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/docker-manager.ts:2356

  • The new 0o644 modes for audit artifacts assume the files are always safe to disclose to all local users. However, redactDockerComposeSecrets() only redacts environment variable values based on key name patterns and does not guarantee that other compose fields (or unexpectedly named env vars) cannot contain sensitive values.

Suggestion: keep audit artifact files at 0o600 by default (and rely on cleanup-time chmod when needed for artifact upload), or strengthen the redaction guarantees before making them world-readable.

  fs.writeFileSync(path.join(auditDir, 'squid.conf'), squidConfig, { mode: 0o644 });

  // Save redacted docker-compose.yml (strip env vars that may contain secrets)
  const redactedCompose = redactDockerComposeSecrets(dockerCompose);
  fs.writeFileSync(
    path.join(auditDir, 'docker-compose.redacted.yml'),
    yaml.dump(redactedCompose, { lineWidth: -1 }),
    { mode: 0o644 }
  );
  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Comment thread src/docker-manager.ts Outdated
Comment on lines 2342 to 2345
// World-readable so gh-aw post-job scanners can access audit artifacts
// (files are already secret-redacted, so 0o755 is safe)
fs.mkdirSync(auditDir, { recursive: true, mode: 0o755 });
}
Comment thread src/docker-manager.ts Outdated
Comment on lines +2342 to +2344
// World-readable so gh-aw post-job scanners can access audit artifacts
// (files are already secret-redacted, so 0o755 is safe)
fs.mkdirSync(auditDir, { recursive: true, mode: 0o755 });
Comment on lines +429 to +430
// The regex itself still matches the suffix of the group name, but the
// sentinel guard in the script prevents double-application.
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 1, 2026

@copilot address the review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 1, 2026

@copilot address the review feedback

Addressed all three review points in 9576d83:

  1. Reverted src/docker-manager.ts — restored audit dir/file permissions to 0o700/0o600. The world-readable change was unrelated to the concurrency fix and introduced a security regression; the existing cleanup-time chmod -R a+rX already handles making files accessible when needed.

  2. Fixed misleading test comment — updated lines 429–430 to accurately state that the regex itself does not match the already-updated group (the closing quote is no longer immediately after ...issue-duplication-detector), so both the sentinel guard and the regex independently prevent double-application.

Copilot finished work on behalf of lpcox May 1, 2026 00:11
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Smoke Test Results

✅ GitHub MCP: Retrieved last 2 merged PRs
✅ Playwright: Page title verified (contains "GitHub")
✅ File Writing: Test file created
✅ Bash Tool: File verification successful

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Smoke Test: Copilot BYOK (Offline Mode)

Test Result
1. GitHub MCP ✅ Listed merged PR #2313 successfully
2. GitHub.com HTTP ⚠️ Template vars not substituted (pre-step skipped)
3. File write/read ⚠️ Template vars not substituted (pre-step skipped)
4. BYOK Inference ✅ Responding via api-proxy → api.githubcopilot.com

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

PR by @Copilot, assignees: @lpcox, @CopilotOverall: PARTIAL PASS (BYOK inference confirmed; pre-computed steps did not execute)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

🔥 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity
File write/read

PR: fix: per-issue conclusion concurrency in issue-duplication-detector
Author: @Copilot · Assignees: @lpcox, @Copilot

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #2318 · ● 490.5K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

PR titles:
[docs] docs: sync authentication-architecture.md for 5-provider api-proxy
api-proxy: expose models_fetch_complete in /health, fix port tables, add readiness polling docs
GitHub MCP review: ✅
safeinputs-gh PR query: ❌
Playwright title: ✅
Tavily search: ❌
File write + bash verify: ✅
Discussion comment: ✅
Build AWF: ✅
Overall: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@lpcox lpcox merged commit ae24b65 into main May 1, 2026
60 of 64 checks passed
@lpcox lpcox deleted the copilot/update-copilot-token-usage-report branch May 1, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

📊 Copilot Token Usage Report2026-04-30

3 participants