Skip to content

Fix rootless cleanup diagnostics obscuring startup failures - #6935

Closed
lpcox with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-chmod-warning-issue
Closed

Fix rootless cleanup diagnostics obscuring startup failures#6935
lpcox with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-chmod-warning-issue

Conversation

Copilot AI commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reuse the rootless artifact permission-error classifier for direct cleanup chmod failures
  • demote expected EPERM/EACCES cleanup failures to concise debug output
  • retain warnings for unexpected cleanup failures
  • verify cleanup leaves the primary startup error as the last prominent diagnostic

Fixes #6934

Copilot AI and others added 5 commits August 4, 2026 15:23
…6924)

* Initial plan

* fix(api-proxy): preserve explicit provider models

* fix: preserve alias semantics during nested resolution

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

Copilot-Session: 2e281ab2-e1f4-42a2-9798-47863526dc60

* ci: accept unreachable npm CLI advisories

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

Copilot-Session: 2e281ab2-e1f4-42a2-9798-47863526dc60

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Landon Cox <landon.cox@microsoft.com>
Copilot-Session: 2e281ab2-e1f4-42a2-9798-47863526dc60
… auth (#6923)

* Initial plan

* feat: add OTLP workload identity auth

* fix(api-proxy): bind OTLP WIF to one endpoint

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

Copilot-Session: 2e281ab2-e1f4-42a2-9798-47863526dc60

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Landon Cox <landon.cox@microsoft.com>
Copilot-Session: 2e281ab2-e1f4-42a2-9798-47863526dc60
…6918)

Upgrade the gh-aw extension to the latest pre-release (v0.84.3) and
recompile all agentic workflows.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reuse the rootless artifact permission classification for direct cleanup chmod failures.

Add coverage for diagnostic ordering and unexpected failures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox changed the title [WIP] Fix rootless cleanup chmod warning obscuring AWF startup failure Fix rootless cleanup diagnostics obscuring startup failures Aug 4, 2026
@lpcox
lpcox marked this pull request as ready for review August 4, 2026 22:43
Copilot AI balanced review requested due to automatic review settings August 4, 2026 22:43
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Documentation Preview

Documentation build failed for this PR. View logs.

Built from commit 97cdc20

@lpcox

lpcox commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Superseded by a clean PR branch based on the current main branch; the implementation is unchanged.

@lpcox lpcox closed this Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 98.94% 98.97% 📈 +0.03%
Statements 98.83% 98.85% 📈 +0.02%
Functions 99.19% 99.19% ➡️ +0.00%
Branches 95.15% 95.14% 📉 -0.01%
📁 Per-file Coverage Changes (3 files)
File Lines (Before → After) Statements (Before → After)
src/artifact-preservation.ts 96.3% → 96.3% (+0.07%) 96.3% → 96.4% (+0.07%)
src/artifact-permissions.ts 97.4% → 97.9% (+0.48%) 97.4% → 97.9% (+0.48%)
src/log-directory-setup.ts 96.2% → 100.0% (+3.78%) 96.3% → 100.0% (+3.71%)

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

Standalone EPERM/EACCES codes remain misclassified, and unrelated security-sensitive changes exceed the described scope.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Addresses #6934 by demoting expected rootless cleanup permission failures, but also includes substantial unrelated telemetry, model-routing, security, and workflow changes.

Changes:

  • Reuses the artifact permission-error classifier during cleanup and adds regression coverage.
  • Adds GCP workload identity authentication for OTLP exports.
  • Changes model alias precedence and regenerates several workflows.
File summaries
File Description
src/services/api-proxy-env-config.ts Forwards OTLP workload identity credentials.
src/services/api-proxy-env-config.test.ts Tests OTLP credential forwarding.
src/artifact-preservation.ts Demotes benign cleanup permission failures.
src/artifact-preservation-errors.test.ts Tests cleanup diagnostic severity.
src/artifact-permissions.ts Extracts the shared permission-error classifier.
docs/api-proxy-sidecar.md Documents OTLP workload identity federation.
containers/api-proxy/test-helpers/otel-test-utils.js Resets new OTLP environment variables.
containers/api-proxy/server.startup-model-validation.test.js Tests direct model precedence.
containers/api-proxy/otel.js Connects workload identity to OTLP exporters.
containers/api-proxy/otel-workload-identity.test.js Tests workload identity configuration.
containers/api-proxy/otel-workload-identity.js Implements OTLP workload identity.
containers/api-proxy/otel-fanout.test.js Tests endpoint-specific authentication.
containers/api-proxy/otel-exporters.js Supports dynamic export headers.
containers/api-proxy/model-resolver.test.js Tests alias/model name collisions.
containers/api-proxy/model-resolver.js Prioritizes explicit provider models.
containers/api-proxy/key-validation.js Updates model resolution diagnostics.
containers/api-proxy/Dockerfile Packages the workload identity module.
.grype.yaml Adds vulnerability exceptions.
.github/workflows/smoke-docker-sbx.lock.yml Regenerates the Docker sbx workflow.
.github/workflows/smoke-bounded-queries.lock.yml Regenerates bounded-query smoke coverage.
.github/workflows/smoke-bounded-queries-sbx.lock.yml Regenerates sbx bounded-query coverage.
.github/workflows/smoke-bounded-queries-gvisor.lock.yml Regenerates gVisor bounded-query coverage.
.github/workflows/smoke-bounded-agents.lock.yml Regenerates bounded-agent coverage.
.github/workflows/smoke-bounded-agents-gvisor.lock.yml Regenerates gVisor bounded-agent coverage.
.github/workflows/auth-doctor-updater.lock.yml Refreshes action pins and schedule.
.github/skills/agentic-workflows/SKILL.md Adds an observability guidance reference.
.github/aw/actions-lock.json Corrects setup action SHAs.
Review details
  • Files reviewed: 30/84 changed files
  • Comments generated: 4
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@@ -163,6 +163,7 @@ function buildProxyRoutingEnv(networkConfig: NetworkConfig): Record<string, stri
* api-proxy spans are children of the workflow trace.
*/
function buildOtelEnv(): Record<string, string> {
const workloadIdentityConfigured = Boolean(process.env.GH_AW_OTLP_WORKLOAD_IDENTITY?.trim());
Comment on lines +35 to +37
return /(?:^|\n)(?:chown|chmod):.*(?:operation not permitted|permission denied|\bEPERM\b|\bEACCES\b)/i.test(
details.join('\n'),
);
Comment thread .grype.yaml
Comment on lines +77 to +79
# AWF does not pass remote input to npm's brace expansion code. npm 12.0.1,
# the latest official release as of 2026-08-04, still bundles 5.0.7; retain
# this acceptance only until an official npm release bundles >=5.0.9.
Comment thread .grype.yaml
Comment on lines +97 to +100
# No official npm release contains the fix yet: npm 12.0.1, the latest
# release as of 2026-08-04, still bundles ip-address 10.2.0. Hand-patching
# npm's internal dependency tree would replace a verified upstream artifact
# with a locally modified distribution.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⏳ Copilot review left inline comments.

@copilot To proceed:

  1. Ask @copilot to address the review feedback (reply to this comment or the review thread)
  2. Once the fix is pushed, add the ready-for-aw label to trigger agentic CI smoke tests

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.

Rootless cleanup chmod warning obscures the primary AWF startup failure

3 participants