daily-issues-report: switch Copilot SDK driver from Python to Node in workflow runtime#46894
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot investigate why python is not working |
I investigated the failing runs for |
|
@copilot update the compiler to Install the copilot python sdk |
🤖 PR Triage
Score breakdown: impact 32 + urgency 20 + quality 10 Daily issues report is failing repeatedly due to Copilot Python SDK incompatibility. Switching to Node driver directly unblocks the daily report workflow. Draft — needs undraft + CI.
|
…vers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in |
🔍 PR Triage — Run §29791496271
Rationale: Daily issues report continuously failing due to Python SDK in runtime. Switch to Node fixes root cause. Undraft and merge to restore daily reporting.
|
|
@copilot merge main and recompile |
…de-not-available Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #46894 does not have the 'implementation' label and has 81 new lines of code in business logic directories (threshold: 100). |
|
✅ Test Quality Sentinel completed test quality analysis. |
Done. I merged |
There was a problem hiding this comment.
Pull request overview
Switches Daily Issues Report to the Node Copilot SDK driver while adding isolated Python SDK installation support.
Changes:
- Uses the Node SDK driver for the daily report.
- Installs Python SDK dependencies into a dedicated target and configures
PYTHONPATH. - Regenerates affected workflow lock files.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/daily-issues-report.md |
Selects the Node SDK driver. |
.github/workflows/daily-issues-report.lock.yml |
Applies the generated Node installation and invocation. |
.github/workflows/smoke-call-workflow.lock.yml |
Updates generated reusable-workflow inputs, permissions, and secrets. |
pkg/workflow/copilot_engine_installation.go |
Adds targeted Python SDK installation. |
pkg/workflow/copilot_engine_execution.go |
Configures Python SDK runtime discovery. |
pkg/workflow/copilot_engine_test.go |
Updates and extends SDK installation tests. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Medium
| if currentPythonPath, exists := env["PYTHONPATH"]; copilotSDKRuntimeID(workflowData) == "python" && (!exists || currentPythonPath == "") { | ||
| env["PYTHONPATH"] = copilotSDKPythonPathExpression | ||
| } |
| spec.command = workspaceCommandPrefix + fmt.Sprintf( | ||
| `mkdir -p "%[1]s" && python3 -m pip install --disable-pip-version-check --target "%[1]s" github-copilot-sdk==%[2]s`, | ||
| copilotSDKPythonTargetDir, | ||
| version, | ||
| ) |
| id: copilot | ||
| copilot-sdk: true | ||
| driver: .github/drivers/copilot_sdk_driver_sample_python.py | ||
| driver: .github/drivers/copilot_sdk_driver_sample_node.cjs |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — commenting on test coverage gaps; no blocking issues.
📋 Key Themes & Highlights
Key Themes
- Test coverage gaps:
copilotSDKRuntimeIDhas TypeScript and Ruby extension branches that are not exercised by the new tests; the pre-existing PYTHONPATH guard path is also untested. - Correct fix: switching from Python to Node driver is the right call and directly addresses the root cause.
Positive Highlights
- ✅ New
copilotSDKRuntimeIDfunction cleanly centralises runtime detection in one place — good deep-module design. - ✅
PYTHONPATHinjection is properly scoped to Python-only drivers, avoiding unintended env pollution. - ✅ Tests are structured clearly with named sub-tests following Arrange/Act/Assert.
- ✅ Compiled lock file and source workflow are updated together — no drift.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 25.3 AIC · ⌖ 7.37 AIC · ⊞ 6.7K
Comment /matt to run again
Comments that could not be inline-anchored
pkg/workflow/copilot_engine_execution.go:144
[/tdd] The copilotSDKRuntimeID switch covers .py, .ts/.mts, and .rb, but only .py (positive) and .cjs (node fallback) are exercised by the new tests — the TypeScript and Ruby branches have no coverage.
<details>
<summary>💡 Suggested additions to TestCopilotEngineCopilotSDKPythonDriverSetsPythonPath</summary>
Add sub-tests for the remaining extensions so regressions in those branches don't go undetected:
t.Run("typescript driver does not set PYTHONPATH", func(t *tes…
</details>
<details><summary>pkg/workflow/copilot_engine_execution.go:155</summary>
**[/tdd]** The `PYTHONPATH` guard skips setting the variable when it is already non-empty, but there's no test covering that path — making it unclear whether an existing `PYTHONPATH` should be preserved as-is or extended.
<details>
<summary>💡 Suggested test</summary>
Add a sub-test that pre-populates the env map with a non-empty `PYTHONPATH` and asserts the expected outcome (preserve or extend), making the contract explicit and preventing silent regressions.
</details>
@copilot please addr…
</details>
There was a problem hiding this comment.
Review: Python to Node.js SDK driver switch
Clean fix for the chroot Python module availability issue. Two complementary changes:
- Immediate fix: daily-issues-report now uses the Node.js SDK driver, bypassing the Python availability problem entirely.
- Systemic fix for Python users: --target install dir plus PYTHONPATH injection ensures Python SDK packages are accessible inside the chroot runtime, rather than installing to global site-packages.
The copilotSDKRuntimeID helper cleanly centralises runtime detection (command takes precedence over driver extension). Tests are thorough and updated correctly.
No blocking issues.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 35.2 AIC · ⌖ 5.47 AIC · ⊞ 5K
🧪 Test Quality Sentinel Report✅ Test Quality Score: 82/100 — Excellent
📊 Metrics (6 tests)
Verdict
Score breakdown: design coverage (40/40) + edge coverage (15/30) + no duplicate penalty (-5 for 1 cluster) + no inflation (+10) = 82/100. The new References:
|
There was a problem hiding this comment.
REQUEST_CHANGES — 3 blocking issues found
The core driver switch (Python → Node) for daily-issues-report is correct and the logic of the new copilotSDKRuntimeID helper is sound. However there are issues that need fixing before merge.
Blocking themes
-
PYTHONPATH silently dropped (
copilot_engine_execution.go:154): the guard condition!exists || currentPythonPath == ""means any workflow that already hasPYTHONPATHset skips adding the pip--targetdir, breaking Python SDK imports at runtime. -
Permissions escalation in smoke-call-workflow (
smoke-call-workflow.lock.yml:89):pull-requestsandissuespromoted towritewithout documented rationale. Needs explicit justification, especially for a smoke workflow that may be triggered by untrusted PRs. -
Implicit secrets regression (
smoke-call-workflow.lock.yml:98):secrets: inheritreplaced with an explicit 7-secret list — any secret the callee needs that is not in this list will silently arrive empty and cause auth failures. Needs verification against the callee's full secret usage.
Non-blocking: the two path constants (copilotSDKPythonPathExpression / copilotSDKPythonTargetDir) are a maintenance hazard worth addressing but not blocking.
🔎 Code quality review by PR Code Quality Reviewer · 58.4 AIC · ⌖ 4.93 AIC · ⊞ 5.6K
Comment /review to run again
Comments that could not be inline-anchored
pkg/workflow/copilot_engine_execution.go:154
PYTHONPATH is silently dropped if already set: when a caller workflow already has PYTHONPATH set to a non-empty value, the pip --target directory is never appended, causing import github_copilot_sdk to fail at runtime with no clear error.
<details>
<summary>💡 Suggested fix</summary>
Always prepend unconditionally instead of skipping when the var exists:
if copilotSDKRuntimeID(workflowData) == "python" {
existing := env["PYTHONPATH"]
if existing == "" {
env["P…
</details>
<details><summary>.github/workflows/smoke-call-workflow.lock.yml:89</summary>
**Permissions escalated from read to write without justification**: `pull-requests` upgraded from `read` → `write` and `issues: write` added. The comment says these come from the called workflow, but blindly inheriting write permissions creates unnecessary blast radius if the called workflow is compromised or has a bug.
<details>
<summary>💡 What to verify</summary>
Audit what `smoke-workflow-call.lock.yml` actually does at runtime. If it only reads PRs/issues (e.g. to fetch context) and writ…
</details>
<details><summary>.github/workflows/smoke-call-workflow.lock.yml:98</summary>
**Replacing `secrets: inherit` with an explicit list may silently drop required secrets**: any secret the called workflow depends on that is not in this list will arrive as an empty string at runtime, producing auth failures with no clear error.
<details>
<summary>💡 What to verify</summary>
Cross-check this explicit list against every `secrets.` reference in `smoke-workflow-call.lock.yml` to confirm no required secret is omitted. The regression risk is high: `secrets: inherit` was the safe d…
</details>
<details><summary>pkg/workflow/copilot_engine_execution.go:115</summary>
**Two constants encode the same SDK install path with different syntaxes**: `copilotSDKPythonPathExpression` (execution.go) uses Actions expression syntax `${{ github.workspace }}/...`, while `copilotSDKPythonTargetDir` (installation.go) uses shell syntax `${GITHUB_WORKSPACE}/...`. They must stay in sync manually — renaming the path in one place won't update the other.
<details>
<summary>💡 Suggested fix</summary>
Define a single shared base constant for the relative path segment (`.gh-aw/cop…
</details>
|
🎉 This pull request is included in a new release. Release: |
The Daily Issues Report Generator has been failing repeatedly in the Copilot execution path, preventing daily report generation and leaving a persistent failing run in the workflow fleet. The failure came from the Python SDK driver path in the runtime/chroot environment.
Root cause addressed
daily-issues-reportwas configured to use the Python Copilot SDK sample driver, which failed at runtime due to missing Pythoncopilotmodule availability in the execution environment.Workflow engine/runtime change
.github/workflows/daily-issues-report.mdCompiled workflow update
.github/workflows/daily-issues-report.lock.ymlto reflect:@github/copilot-sdk)