Skip to content

Move model alias/multiplier propagation from step env to activation artifact file#34837

Open
Copilot wants to merge 12 commits into
mainfrom
copilot/update-model-aliases-and-multipliers
Open

Move model alias/multiplier propagation from step env to activation artifact file#34837
Copilot wants to merge 12 commits into
mainfrom
copilot/update-model-aliases-and-multipliers

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

The compiler currently pushes merged model data through step environment variables, which bloats compiled logs and creates churn. This change shifts model multiplier/alias propagation to a file-based flow rooted in the activation artifact, so downstream jobs read the same materialized payload from disk.

  • Activation job now materializes merged model data

    • generate_aw_info.cjs now builds /tmp/gh-aw/model_multipliers.json by merging:
      • built-in setup payload (actions/setup/js/model_multipliers.json)
      • workflow overrides from engine.token-weights (GH_AW_INFO_TOKEN_WEIGHTS)
    • The activation artifact upload list now includes /tmp/gh-aw/model_multipliers.json.
  • Runtime ET/model logic now reads from artifact-backed file

    • effective_tokens.cjs now resolves multipliers in deterministic order:
      1. /tmp/gh-aw/model_multipliers.json (activation artifact payload)
      2. GH_AW_MODEL_MULTIPLIERS (compatibility fallback)
      3. built-in model_multipliers.json in setup scripts
    • This removes dependency on global env injection for normal execution paths.
  • Setup no longer injects large model JSON into job env

    • Removed GH_AW_MODEL_MULTIPLIERS export block from actions/setup/setup.sh.
    • This reduces generated YAML/env noise and log churn across jobs.
  • Readability hardening in JS merge/parse paths

    • Extracted plain-object normalization helpers to avoid repeated inline object-shape checks in merge/parsing paths.
// effective_tokens.cjs: runtime source precedence
const parsedFromMergedFile = parseMultipliersFile("/tmp/gh-aw/model_multipliers.json");
if (parsedFromMergedFile) return parsedFromMergedFile;

if (Object.prototype.hasOwnProperty.call(process.env, "GH_AW_MODEL_MULTIPLIERS")) {
  const parsedFromEnv = parseMultipliersJSON(process.env.GH_AW_MODEL_MULTIPLIERS || "");
  return parsedFromEnv ?? null;
}

return parseMultipliersFile(path.join(__dirname, "model_multipliers.json"));


✨ PR Review Safe Output Test - Run 26431767267

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · opus47 6.5M ·

Copilot AI and others added 3 commits May 26, 2026 01:55
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review May 26, 2026 02:16
Copilot AI review requested due to automatic review settings May 26, 2026 02:16
@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🧪 Test Quality Sentinel completed test quality analysis.

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

PR Code Quality Reviewer completed the code quality review.

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #34837 does not have the 'implementation' label and has only 10 new lines (≤100) in default business logic directories.

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

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

This PR moves model alias/multiplier propagation from step environment variables to a file-based flow rooted in the activation artifact, reducing compiled log/env churn and making downstream jobs read a consistent materialized payload from disk.

Changes:

  • Activation job now writes /tmp/gh-aw/model_multipliers.json (built-in multipliers merged with GH_AW_INFO_TOKEN_WEIGHTS) and uploads it in the activation artifact.
  • Runtime ET logic (effective_tokens.cjs) now resolves multipliers in order: merged file → GH_AW_MODEL_MULTIPLIERS env fallback → built-in model_multipliers.json.
  • Setup no longer exports the large GH_AW_MODEL_MULTIPLIERS JSON into GITHUB_ENV; tests and golden fixtures are updated accordingly.
Show a summary per file
File Description
pkg/workflow/compiler_activation_job_builder.go Upload activation artifact now includes /tmp/gh-aw/model_multipliers.json.
actions/setup/setup.sh Removes exporting GH_AW_MODEL_MULTIPLIERS into job env.
actions/setup/js/generate_aw_info.cjs Writes merged multipliers file into /tmp/gh-aw/ during activation.
actions/setup/js/generate_aw_info.test.cjs Adds coverage for merged multipliers file creation and override merge behavior.
actions/setup/js/effective_tokens.cjs Adds file/env/built-in precedence for multipliers resolution.
actions/setup/js/effective_tokens.test.cjs Updates tests to reflect new precedence (including built-in fallback).
actions/setup/js/parse_mcp_gateway_log.cjs Updates doc comment for ET multiplier data source.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden Golden updated to include the new artifact path.
pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden Golden updated to include the new artifact path.

Copilot's findings

Tip

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

  • Files reviewed: 16/16 changed files
  • Comments generated: 3

Comment on lines +69 to 80
if (Object.prototype.hasOwnProperty.call(process.env, "GH_AW_MODEL_MULTIPLIERS")) {
const raw = process.env.GH_AW_MODEL_MULTIPLIERS;
if (raw && raw.trim()) {
const parsedFromEnv = parseMultipliersJSON(raw);
if (parsedFromEnv) {
_parsedMultipliers = parsedFromEnv;
return _parsedMultipliers;
}
}
_parsedMultipliers = null;
return null;
}
Comment on lines +49 to +53
beforeEach(() => {
_resetCache();
if (fs.existsSync(MERGED_MULTIPLIERS_PATH)) {
fs.unlinkSync(MERGED_MULTIPLIERS_PATH);
}
Comment on lines 51 to 53
* Parses token-usage.jsonl content and returns an aggregated summary.
* Computes effective tokens (ET) per model using the GH_AW_MODEL_MULTIPLIERS env var.
* Computes effective tokens (ET) per model using merged/built-in model multipliers data.
* @param {string} jsonlContent - The token-usage.jsonl file content
@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🧪 Test Quality Sentinel Report

Test Quality Score: 90/100 — Excellent

Analyzed 3 test(s) in 2 JavaScript files: 3 design tests (behavioral contracts), 0 implementation tests, 0 guideline violations.

📊 Metrics & Test Classification (3 tests analyzed)
Metric Value
New/modified tests analyzed 3
✅ Design tests (behavioral contracts) 3 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 2 (67%)
Duplicate test clusters 0
Test inflation detected No
🚨 Coding-guideline violations 0

Test Classification Details

Test File Classification Issues Detected
"falls back to built-in multipliers when env var is not set" actions/setup/js/effective_tokens.test.cjs ✅ Design None — covers missing-env-var edge case
"should write merged model multipliers file" actions/setup/js/generate_aw_info.test.cjs ✅ Design Happy-path only; no write-failure or malformed-input case
"should merge custom token weights into model multipliers file" actions/setup/js/generate_aw_info.test.cjs ✅ Design None — verifies both output file and aw_info.json

Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 0 tests
  • 🟨 JavaScript (*.test.cjs): 3 tests (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All three tests verify observable file-system outputs and behavioral contracts of the new model-alias/multiplier propagation path.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · sonnet46 1.4M ·

Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 90/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%).

Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /zoom-out, /improve-codebase-architecture, and /tdd — commenting with non-blocking suggestions.

📋 Key Themes & Highlights

Key Themes

  • Lookup precedence edge case: When GH_AW_MODEL_MULTIPLIERS is present but empty/invalid, the built-in file tier is silently skipped. This may be intentional but diverges from the documented 3-tier spec and is untested.
  • Scoping inconsistency: parseTokenWeightsFromEnv and writeMergedModelMultipliers are defined inside main() while getPlainObjectOrEmpty lives at module scope — inconsistent placement for large, reusable helpers.
  • Duplicate plain-object guard: isPlainObject in effective_tokens.cjs and getPlainObjectOrEmpty in generate_aw_info.cjs implement identical logic — good candidate for a shared utility.
  • Minor: JSDoc cast in getPlainObjectOrEmpty is missing parentheses around the expression.

Positive Highlights

  • ✅ Solid architectural move — file-based artifact propagation is cleaner than injecting large JSON blobs into job environment variables.
  • ✅ Compatibility fallback retained for GH_AW_MODEL_MULTIPLIERS — smooth migration path for any existing consumers.
  • ✅ Good test additions in generate_aw_info.test.cjs covering the merge/write path and custom override case.
  • isPlainObject and parseMultipliersFile/parseMultipliersJSON extraction improves readability in effective_tokens.cjs.
  • ✅ All golden files updated, confirming the compiler change is reflected end-to-end.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.5M

Comment thread actions/setup/js/effective_tokens.cjs Outdated
return _parsedMultipliers;
}
}
_parsedMultipliers = null;
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.

[/zoom-out] Silent null-return when env var is present but invalid skips the built-in file fallback.

If GH_AW_MODEL_MULTIPLIERS is set to "" or contains invalid JSON, the function returns null without attempting the built-in file. This differs from the documented 3-tier lookup — tier 3 (built-in file) is unreachable whenever the env var key exists, even with an unusable value.

💡 Suggested fix

Continue to the next tier on invalid/empty value:

if (Object.prototype.hasOwnProperty.call(process.env, "GH_AW_MODEL_MULTIPLIERS")) {
  const raw = process.env.GH_AW_MODEL_MULTIPLIERS;
  if (raw && raw.trim()) {
    const parsedFromEnv = parseMultipliersJSON(raw);
    if (parsedFromEnv) {
      _parsedMultipliers = parsedFromEnv;
      return _parsedMultipliers;
    }
    // invalid JSON — fall through to built-in
  }
  // env var present but empty — fall through to built-in
}

Alternatively, document explicitly that a set-but-invalid env var intentionally blocks the built-in tier.


test("returns 1.0 when env var is not set", () => {
test("falls back to built-in multipliers when env var is not set", () => {
_resetCache();
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.

[/tdd] Missing edge-case test: env var present but invalid, merged file absent — should verify which multipliers (built-in or none) are returned.

The test on line 130 renames the old 'returns 1.0 when env var is not set' case but doesn't add a complementary test for when the env var is set but contains invalid JSON (and the merged file doesn't exist).

💡 Suggested test
test("returns null (no built-in fallback) when GH_AW_MODEL_MULTIPLIERS is set to invalid JSON", () => {
  _resetCache();
  process.env.GH_AW_MODEL_MULTIPLIERS = "not-json";
  // merged file already absent (cleaned in beforeEach)
  expect(getModelMultiplier("claude-opus-4.5")).toBe(1.0); // or toBeGreaterThan(1.0) if built-in should still apply
});

This pins the intended behaviour so it can't silently regress.


// Write to /tmp/gh-aw directory to avoid inclusion in PR
fs.mkdirSync(TMP_GH_AW_PATH, { recursive: true });
writeMergedModelMultipliers(core, tokenWeights);
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.

[/improve-codebase-architecture] writeMergedModelMultipliers is called before its declaration (relies on JS function hoisting) and is nested inside main() alongside parseTokenWeightsFromEnv.

Defining large helper functions inside an async top-level function makes the module harder to navigate — readers expect to see them near top-of-file or at module level, not buried after several hundred lines of main(). Both helpers have no closure state and could trivially be lifted to module scope.

💡 Suggested refactor

Move parseTokenWeightsFromEnv and writeMergedModelMultipliers to module scope (outside main), the same way getPlainObjectOrEmpty is already defined at module level. The call in main stays the same; only the function definition moves.

/**
* @param {unknown} value
* @returns {Record<string, unknown>}
*/
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.

[/improve-codebase-architecture] getPlainObjectOrEmpty duplicates the plain-object guard already extracted as isPlainObject in effective_tokens.cjs.

Both files now implement the same value !== null && typeof value === 'object' && !Array.isArray(value) pattern independently. A shared utils.cjs or object_utils.cjs module would prevent this drift.

💡 Suggested approach

Extract to a shared file, e.g. actions/setup/js/object_utils.cjs:

/** `@param` {unknown} v `@returns` {v is Record<string, unknown>} */
function isPlainObject(v) {
  return v !== null && typeof v === "object" && !Array.isArray(v);
}

/** `@param` {unknown} v `@returns` {Record<string, unknown>} */
function toPlainObjectOrEmpty(v) {
  return isPlainObject(v) ? /** `@type` {Record<string, unknown>} */ (v) : {};
}

module.exports = { isPlainObject, toPlainObjectOrEmpty };

Both files can then require the shared helpers.

module.exports = { main };

/**
* @param {unknown} value
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.

[/improve-codebase-architecture] JSDoc cast is missing parentheses — /** @type */ value instead of /** @type */ (value).

The standard JSDoc type-assertion form requires wrapping the expression in parentheses: /** @type {T} */ (expr). Without parens, some type checkers and linters misparse the annotation.

💡 Fix
// Before
return /** `@type` {Record<string, unknown>} */ value;

// After
return /** `@type` {Record<string, unknown>} */ (value);

ghost
ghost previously requested changes May 26, 2026
Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

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

REQUEST_CHANGES — 1 high + 3 medium issues found across the ET/multiplier refactor.

### Issues summary

High

  • Number(mult) in parseMultipliersJSON stores NaN without a Number.isFinite guard; any non-numeric value in a multipliers file silently poisons ET calculations for that model.

Medium

  • getMultipliersData(): when GH_AW_MODEL_MULTIPLIERS is set but empty or invalid, the function returns null and skips the built-in file. The three-tier fallback described in the PR collapses to two for anyone with the env var exported (even empty).
  • getPlainObjectOrEmpty: JSDoc cast /** @type {T} */ value is missing parentheses — the assertion has no effect and the inferred return type remains unknown.
  • writeMergedModelMultipliers: fs.writeFileSync is unguarded; a write failure propagates as an uncaught exception with no core.setFailed diagnostic, unlike the read paths above it.

Overall direction (activation-artifact file over env-var injection) is sound. Fix the Number.isFinite guard and the env-var fallback logic before merging.

🔎 Code quality review by PR Code Quality Reviewer · sonnet46 2.8M

multipliers[model.toLowerCase()] = Number(mult);
if (isPlainObject(parsed.multipliers)) {
for (const [model, mult] of Object.entries(parsed.multipliers)) {
multipliers[model.toLowerCase()] = Number(mult);
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.

Number(mult) stores NaN without validation, silently corrupting multiplier lookups.

Any non-numeric multiplier value (string, object, null) becomes NaN in the map. Downstream getModelMultiplier() returns NaN, causing all ET calculations for that model to silently produce NaN instead of falling back to 1.0.

💡 Suggested fix
const numMult = Number(mult);
if (Number.isFinite(numMult)) {
  multipliers[model.toLowerCase()] = numMult;
}

This is the same guard already applied to token_class_weights entries a few lines above — consistent and cheap.

Comment thread actions/setup/js/effective_tokens.cjs Outdated
}
}
_parsedMultipliers = null;
return null;
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.

When GH_AW_MODEL_MULTIPLIERS is set but empty or invalid, the built-in file fallback is silently skipped.

The hasOwnProperty guard at line 56 treats any presence of the env var — including GH_AW_MODEL_MULTIPLIERS="" or invalid JSON — as an explicit override: it sets _parsedMultipliers = null and returns without reaching the built-in file path. A caller that exports the env var as empty (e.g. from a template) will get null multipliers permanently, even though model_multipliers.json is right there.

💡 Suggested fix

Fall through to the built-in when the env var is present but not parseable:

if (Object.prototype.hasOwnProperty.call(process.env, "GH_AW_MODEL_MULTIPLIERS")) {
  const raw = process.env.GH_AW_MODEL_MULTIPLIERS;
  if (raw && raw.trim()) {
    const parsedFromEnv = parseMultipliersJSON(raw);
    if (parsedFromEnv) {
      _parsedMultipliers = parsedFromEnv;
      return _parsedMultipliers;
    }
  }
  // env var present but empty/invalid — fall through to built-in
}

Only cache null after all three sources have been exhausted.

Comment thread actions/setup/js/generate_aw_info.cjs Outdated
*/
function getPlainObjectOrEmpty(value) {
if (value && typeof value === "object" && !Array.isArray(value)) {
return /** @type {Record<string, unknown>} */ value;
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.

Incorrect JSDoc cast syntax — type assertion has no effect without parentheses.

/** @type {T} */ value is a comment before an expression; it does not cast in JSDoc/TypeScript. Without parentheses, the type annotation is silently ignored and the return type is inferred as unknown, defeating the @returns {Record<string, unknown>} contract.

💡 Suggested fix
return /** `@type` {Record<string, unknown>} */ (value);

The parentheses are required for JSDoc type-casts to be recognized by TypeScript and type-aware editors.

};

const mergedPath = `${TMP_GH_AW_PATH}/model_multipliers.json`;
fs.writeFileSync(mergedPath, JSON.stringify(merged, null, 2));
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.

fs.writeFileSync failure is unhandled and will crash the activation job step with a raw stack trace.

The read paths above have try/catch + core.warning, but the write does not. A disk-full or unexpected permission error will propagate as an uncaught exception from inside main, failing the step with no actionable diagnostic.

💡 Suggested fix
try {
  fs.writeFileSync(mergedPath, JSON.stringify(merged, null, 2));
  core.info(`Generated merged model multipliers at: ${mergedPath}`);
} catch (error) {
  core.setFailed(`Failed to write merged model multipliers: ${String(error)}`);
}

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.

Smoke reply test. Me answer old review thread with tiny footprint.

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · gpt55 4M

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review ALL comments and review comments. Respond with resolution and resolve review comments.

Copilot AI and others added 2 commits May 26, 2026 02:36
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 26, 2026

@copilot review ALL comments and review comments. Respond with resolution and resolve review comments.

Addressed all three review threads in b94fd19:

  • fixed effective_tokens.cjs fallback order so invalid/empty GH_AW_MODEL_MULTIPLIERS now falls through to built-in multipliers,
  • isolated effective_tokens.test.cjs from shared /tmp/gh-aw/model_multipliers.json by using a per-test temp path override,
  • updated the ET precedence doc comment in parse_mcp_gateway_log.cjs to match runtime behavior.

Copilot AI requested a review from pelikhan May 26, 2026 02:43
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot update compiler to stop online the full model info in env variables. Only needed if workflow has custom models

@pelikhan
Copy link
Copy Markdown
Collaborator

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🎬 THE ENDSmoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

⚠️ Smoke Antigravity failed. Antigravity encountered unexpected challenges...

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

The threat detection results could not be parsed.

Review the workflow run logs for details.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Agent Container Tool Check (Corrected)

Tool Status Version
bash 5.2.21
sh available
git 2.54.0
jq 1.7
yq 4.53.2
curl 8.5.0
gh 2.92.0
node 22.22.3
python3 3.14.5
go 1.24.13
java 21.0.11
dotnet 10.0.300

Result: 12/12 tools available ✅ — PASS

🔧 Tool validation by Agent Container Smoke Test · sonnet46 624.3K ·

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Smoke Test Results\n- GitHub MCP Testing: ✅\n- Web Fetch Testing: ✅\n- File Writing Testing: ✅\n- Bash Tool Testing: ✅\n- Build gh-aw: ❌\n\nOverall status: FAIL

Warning

Firewall blocked 1 domain

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

  • localhost

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

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

Smoke Gemini — Powered by Gemini ·

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

The threat detection results could not be parsed.

Review the workflow run logs for details.

Comment Memory

Night builds in silence
Green checks bloom across the graph
Dawn keeps the branch warm

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex · gpt54 11.6M ·

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot the awf config should be constructed from this file, not inlined JSON

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Smoke bot here. Me tap rock. Test smoke go poof. 🪨🔥

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · gpt55 4M ·

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Smoke Copilot 26431767340: FAIL
PRs: Handle Codex response-event logs in threat-detection result parsing; [community] Update community contributions in README
✅ GitHub, mcpscripts, Serena, Playwright, file/bash, discussion, build, artifact, dispatch, PR review, memory, sub-agent, check run
❌ Web Fetch tool unavailable
Author: app/copilot-swe-agent; Assignees: pelikhan, Copilot

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · gpt55 4M ·

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Comment Memory

Tiny tests wake
Bots tap stone keys softly
Green checks greet dawn

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · gpt55 4M ·

Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Smoke review done. Me leave non-blocking notes.

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · gpt55 4M

include-hidden-files: true
path: |
/tmp/gh-aw/aw_info.json
/tmp/gh-aw/model_multipliers.json
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.

Smoke review note: new artifact path show good. Me check line can take review comment.

include-hidden-files: true
path: |
/tmp/gh-aw/aw_info.json
/tmp/gh-aw/model_multipliers.json
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.

Same artifact path here too. Smoke test says locked workflow carries model file.

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Smoke Test: Claude — Run 26431767267

Core (#1–13): ✅✅✅✅✅✅✅✅✅✅✅✅✅
PR review (#14–20): ✅✅✅✅✅✅⚠️ (close skipped — no safe test PR)

Overall: ✅ PASS

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · opus47 6.5M ·

Copy link
Copy Markdown
Contributor

@ghost ghost left a comment

Choose a reason for hiding this comment

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

💥 Automated smoke test review - all systems nominal!

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

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

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · opus47 6.5M

if data.EngineConfig != nil && data.EngineConfig.TokenWeights != nil {
// Embed custom token weights only when custom model multipliers are configured.
// This avoids emitting large model payload env values when workflows only customize
// token-class weights.
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.

Nice guard — gating on len(Multipliers) > 0 keeps the env payload lean when only token-class weights are customized. 👍

// This avoids emitting large model payload env values when workflows only customize
// token-class weights.
if data.EngineConfig != nil && data.EngineConfig.TokenWeights != nil && len(data.EngineConfig.TokenWeights.Multipliers) > 0 {
if tokenWeightsJSON, err := json.Marshal(data.EngineConfig.TokenWeights); err == nil {
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.

Consider whether a nil Multipliers map should be normalized at config-load time so this conditional stays close to the data definition. Optional nit.

Copilot AI and others added 2 commits May 26, 2026 04:29
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 26, 2026

@copilot the awf config should be constructed from this file, not inlined JSON

Implemented in 9389877 (with core change in 7eeda42). AWF config generation now avoids inlining custom model multipliers in JSON and instead merges apiProxy.modelMultipliers from /tmp/gh-aw/model_multipliers.json at runtime when building awf-config.json.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot the python 9389877 to a JavaScript GitHub script file in actions/setup/js, add tests, update generated steps by compiler

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 26, 2026

@copilot the python 9389877 to a JavaScript GitHub script file in actions/setup/js, add tests, update generated steps by compile...

Done in 1bb6eaf. I moved the inline Python model-multiplier merge logic to actions/setup/js/merge_awf_model_multipliers.cjs, updated compiler output to invoke that JS script, added merge_awf_model_multipliers.test.cjs, and recompiled generated workflow steps/golden outputs.

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

pr-sous-chef: @copilot review all comments and address the open review threads, especially the invalid-env fallback and NaN validation.

Generated by 👨‍🍳 PR Sous Chef · gpt54 7M ·

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.

3 participants