Skip to content

Compare AWF wiring in smoke-copilot vs smoke-pi lock workflows #32504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 5 commits into from
Closed

Compare AWF wiring in smoke-copilot vs smoke-pi lock workflows #32504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d443dff
Add AWF config diff report for smoke-copilot vs smoke-pi
Copilot May 16, 2026
f35a4a0
Refine AWF diff report formatting
Copilot May 16, 2026
5b135b0
Clarify lockfile line references in AWF diff report
Copilot May 16, 2026
d0a03d9
Fix pi reflect endpoint to follow provider gateway port
Copilot May 16, 2026
0892d98
Use AWF management reflect endpoint in Pi provider
Copilot May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions actions/setup/js/pi_provider.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ function resolveGatewayUrl(provider) {
return `http://api-proxy:${port}`;
}

/**
* Resolve the AWF /reflect URL.
*
* /reflect is exposed on the api-proxy management port and is not provider-specific.
*
* @param {string} model
* @returns {string}
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 16, 2026

Choose a reason for hiding this comment

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

[/tdd] The JSDoc says "Falls back to the api-proxy management endpoint when provider is unknown" but because of the || "copilot" default on the first line, resolveGatewayUrl is never called with an unknown/null provider — the AWF_API_PROXY_REFLECT_URL fallback branch is unreachable.

The two JSDoc sentences are mutually contradictory. If defaulting to Copilot for no-prefix models is the intent, remove the "falls back to management endpoint" sentence. If AWF_API_PROXY_REFLECT_URL is the intended safety net, the logic should not default to "copilot" before the gateway lookup:

function resolveReflectUrl(model) {
  const provider = extractProviderFromModel(model);
  const gatewayUrl = provider ? resolveGatewayUrl(provider) : null;
  return gatewayUrl ? `${gatewayUrl}/reflect` : AWF_API_PROXY_REFLECT_URL;
}

*/
function resolveReflectUrl(model) {
return AWF_API_PROXY_REFLECT_URL;
}

/**
* Register a Pi provider and any aliases.
*
Expand Down Expand Up @@ -180,6 +192,7 @@ function piProviderExtension(pi) {
pi.on("agent_start", async () => {
const model = getConfiguredModel();
const provider = extractProviderFromModel(model);
const reflectUrl = resolveReflectUrl(model);

if (provider) {
const gatewayUrl = resolveGatewayUrl(provider);
Expand All @@ -195,7 +208,7 @@ function piProviderExtension(pi) {
// Fetch AWF API proxy reflection data before the agent runs to capture initial proxy state.
// This is best-effort: failures are logged but do not affect the agent session.
await fetchAWFReflect({
reflectUrl: AWF_API_PROXY_REFLECT_URL,
reflectUrl,
outputPath: AWF_REFLECT_OUTPUT_PATH,
timeoutMs: AWF_REFLECT_TIMEOUT_MS,
modelsTimeoutMs: AWF_MODELS_URL_TIMEOUT_MS,
Expand All @@ -204,10 +217,13 @@ function piProviderExtension(pi) {
});

pi.on("agent_end", async () => {
const model = getConfiguredModel();
const reflectUrl = resolveReflectUrl(model);

// Fetch AWF API proxy reflection data after the agent finishes for the post-run step summary.
// This is best-effort: failures are logged but do not affect the agent exit code.
await fetchAWFReflect({
reflectUrl: AWF_API_PROXY_REFLECT_URL,
reflectUrl,
outputPath: AWF_REFLECT_OUTPUT_PATH,
timeoutMs: AWF_REFLECT_TIMEOUT_MS,
modelsTimeoutMs: AWF_MODELS_URL_TIMEOUT_MS,
Expand All @@ -220,4 +236,5 @@ module.exports = piProviderExtension;
module.exports.getConfiguredModel = getConfiguredModel;
module.exports.extractProviderFromModel = extractProviderFromModel;
module.exports.resolveGatewayUrl = resolveGatewayUrl;
module.exports.resolveReflectUrl = resolveReflectUrl;
module.exports.registerConfiguredProviders = registerConfiguredProviders;
8 changes: 8 additions & 0 deletions actions/setup/js/pi_provider.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ describe("pi_provider.cjs", () => {
]);
});

it("resolves reflect URL from provider model", () => {
expect(module.resolveReflectUrl("copilot/claude-sonnet-4")).toBe("http://api-proxy:10000/reflect");
expect(module.resolveReflectUrl("openai/gpt-5")).toBe("http://api-proxy:10000/reflect");
expect(module.resolveReflectUrl("custom/provider-model")).toBe("http://api-proxy:10000/reflect");
expect(module.resolveReflectUrl("claude-sonnet-4")).toBe("http://api-proxy:10000/reflect");
});
Comment on lines +61 to +66

it("logs the configured provider using GH_AW_PI_MODEL during agent_start", async () => {
process.env.GH_AW_PI_MODEL = "copilot/claude-sonnet-4";
global.fetch = vi.fn().mockRejectedValue(new Error("network disabled"));
Expand All @@ -74,5 +81,6 @@ describe("pi_provider.cjs", () => {
await handlers.agent_start();

expect(stderrOutput.some(line => line.includes("provider=copilot model=copilot/claude-sonnet-4"))).toBe(true);
expect(global.fetch).toHaveBeenCalledWith("http://api-proxy:10000/reflect", expect.any(Object));
});
});
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 16, 2026

Choose a reason for hiding this comment

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

[/tdd] The agent_start handler now asserts the correct reflect URL is used (line 86), but the symmetric change to agent_end has no equivalent assertion. Both handlers were updated identically — the test coverage for agent_end is missing.

Consider adding:

it("uses provider-specific reflect URL during agent_end", async () => {
  process.env.GH_AW_PI_MODEL = "copilot/claude-sonnet-4";
  global.fetch = vi.fn().mockRejectedValue(new Error("network disabled"));
  const { handlers } = piProviderExtension(mockPi);

  await handlers.agent_end();

  expect(global.fetch).toHaveBeenCalledWith("(apiproxy/redacted) expect.any(Object));
});

Without this, a future regression that breaks agent_end's reflect URL would go undetected.

71 changes: 71 additions & 0 deletions scratchpad/smoke-copilot-vs-pi-awf-config-diff.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# AWF configuration diff: smoke-copilot vs smoke-pi (`*.lock.yml`)

This note compares:

- `/home/runner/work/gh-aw/gh-aw/.github/workflows/smoke-copilot.lock.yml`
- `/home/runner/work/gh-aw/gh-aw/.github/workflows/smoke-pi.lock.yml`

Line references below are from the current lockfile snapshot in this branch and may shift after future workflow recompiles.

## Top-level workflow env

No differences were found in top-level `env:`. Both workflows set the same OTEL/GH_AW telemetry variables.

## AWF execution env differences (agent job)

Copilot-specific env present in `smoke-copilot.lock.yml` but not in `smoke-pi.lock.yml`:

- `GH_AW_MCP_CONFIG` (`smoke-copilot.lock.yml:1801`)
- `GITHUB_MCP_SERVER_TOKEN` (`smoke-copilot.lock.yml:1811`)
- `COPILOT_AGENT_RUNNER_TYPE` (`smoke-copilot.lock.yml:1797`)
- `GITHUB_COPILOT_INTEGRATION_ID` (`smoke-copilot.lock.yml:1809`)
- Also present only for copilot execute path:
- `AWF_REFLECT_ENABLED`
- `COPILOT_API_KEY`
- `COPILOT_MODEL`
- `GITHUB_API_URL`
- `GITHUB_HEAD_REF`
- `GITHUB_REF_NAME`
- `GITHUB_SERVER_URL`
- `XDG_CONFIG_HOME`

Pi-specific env present in `smoke-pi.lock.yml` but not in `smoke-copilot.lock.yml`:

- `GH_AW_PI_MODEL` (`smoke-pi.lock.yml:848`)
- `PI_CODING_AGENT_DIR` (`smoke-pi.lock.yml:860`)

## AWF command/CLI argument differences

`awf` invocation in copilot includes an extra artifact mount not present in pi:

- Copilot has `--mount "${RUNNER_TEMP}/gh-aw/safeoutputs/upload-artifacts:...:rw"` (`smoke-copilot.lock.yml:1793`)
- Pi does not include this mount (`smoke-pi.lock.yml:843`)

Inner engine command executed via `awf` differs significantly:

- Copilot runs `copilot_harness.cjs` + `/usr/local/bin/copilot` with copilot flags:
- `--disable-builtin-mcps`
- `--autopilot`
- `--max-autopilot-continues 2`
- `--allow-all-tools`
- `--allow-all-paths`
- `--no-custom-instructions`
- (`smoke-copilot.lock.yml:1794`)
- Pi runs `pi --print --mode json --no-session --model ... --extension ...`:
- no copilot-equivalent autopilot/allow-all-paths/no-custom-instructions flags
- (`smoke-pi.lock.yml:844`)

## Guard/integrity policy differences around AWF execution

- Copilot path configures `GH_AW_MIN_INTEGRITY: approved` (`smoke-copilot.lock.yml:619`) and `CLI_PROXY_POLICY` with `"min-integrity":"approved"` (`smoke-copilot.lock.yml:1772`)
- Pi path configures `GH_AW_MIN_INTEGRITY: none` (`smoke-pi.lock.yml:480`) and `CLI_PROXY_POLICY` with `"min-integrity":"none"` (`smoke-pi.lock.yml:826`)

## Conclusion (focused on env/CLI mismatch)

The strongest configuration mismatches tied to AWF behavior are:

1. Pi execute env lacks copilot-path variables used for MCP wiring/session integration (`GH_AW_MCP_CONFIG`, `GITHUB_MCP_SERVER_TOKEN`, integration vars).
2. Pi executes a different engine command (`pi ...`) with a different CLI contract than copilot harness flags.
3. Pi runs under a less strict integrity policy (`none` vs `approved`), which changes guard behavior around tool access/proxying.

These are the primary environment/CLI differences that explain why AWF can be correctly wired for copilot while pi behaves differently.