Skip to content

[file-diet] Split activation job builder into focused compiler modules#44792

Merged
pelikhan merged 3 commits into
mainfrom
copilot/file-diet-refactor-compiler-activation-job
Jul 11, 2026
Merged

[file-diet] Split activation job builder into focused compiler modules#44792
pelikhan merged 3 commits into
mainfrom
copilot/file-diet-refactor-compiler-activation-job

Conversation

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

pkg/workflow/compiler_activation_job_builder.go had grown into a 1.1k+ line mixed-responsibility file, making activation-job logic hard to navigate and evolve. This change decomposes it into focused modules by concern while preserving existing activation job behavior and public API.

  • Module decomposition (single-responsibility boundaries)

    • compiler_activation_context.go: activation build context initialization, setup/workflow_call bootstrap, engine output wiring.
    • compiler_activation_permissions.go: permission map assembly, app-token gating, script-inferred permission handling.
    • compiler_activation_daily_aic.go: daily AIC guardrail env/token resolution and guardrail step composition.
    • compiler_activation_steps.go: activation step builders (reaction, text output, status comment, lock/version/checkout, safety checks).
    • compiler_activation_outputs.go: activation outputs, feedback/validation orchestration, needs/if wiring, artifact upload, environment assembly.
    • Removed monolith: compiler_activation_job_builder.go.
  • No API surface change

    • buildActivationJob call flow remains unchanged; extracted methods/functions keep the same receiver/signatures where applicable.
    • Shared activationJobBuildContext remains the coordination object across extracted files.
  • Targeted coverage added for extracted helpers

    • New test file: compiler_activation_builder_split_test.go.
    • Adds focused assertions for:
      • engine ID resolution defaults/trimming,
      • daily AIC env rendering (literal vs expression),
      • text-output env line construction,
      • defaulting of comment outputs.
// Example of the refactor shape (same behavior, clearer ownership):
permissions, err := c.buildActivationPermissions(ctx)
if err != nil {
	return nil, fmt.Errorf("failed to build activation permissions: %w", err)
}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor compiler_activation_job_builder.go into focused modules [file-diet] Split activation job builder into focused compiler modules Jul 10, 2026
Copilot AI requested a review from pelikhan July 10, 2026 16:47
@pelikhan pelikhan marked this pull request as ready for review July 10, 2026 18:32
Copilot AI review requested due to automatic review settings July 10, 2026 18:32

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.

Pull request overview

This PR refactors the activation job builder in pkg/workflow by splitting the former monolithic compiler_activation_job_builder.go into smaller, concern-focused compiler modules while keeping the activation job assembly flow and outputs consistent.

Changes:

  • Decomposed activation-job construction into focused modules (context init, permissions, steps, outputs, daily AIC).
  • Removed the previous monolithic activation-job builder file.
  • Added targeted unit tests covering extracted helper behavior (engine ID defaulting, daily AIC env rendering, env line generation, comment output defaults).
Show a summary per file
File Description
pkg/workflow/compiler_activation_context.go Introduces activationJobBuildContext and activation context/setup/bootstrap helpers.
pkg/workflow/compiler_activation_permissions.go Extracts activation permission calculation and app-token minting gates.
pkg/workflow/compiler_activation_daily_aic.go Extracts daily AIC guardrail token/env/step builders.
pkg/workflow/compiler_activation_steps.go Extracts activation job step builders (reaction, oauth token check, checkout/lock/version, status comment, skill install, etc.).
pkg/workflow/compiler_activation_outputs.go Extracts activation outputs wiring, needs/if computation, artifact upload, and environment assembly.
pkg/workflow/compiler_activation_builder_split_test.go Adds unit tests for newly extracted helpers.
pkg/workflow/compiler_activation_job_builder.go Removed (replaced by the extracted modules).

Review details

Tip

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

  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +161 to +166
// Always include the engine-specific skill directory when either inline skills are enabled
// or frontmatter skills are configured.
if isFeatureEnabled(constants.FeatureFlag("inline-agents"), ctx.data) || len(ctx.data.Skills) > 0 {
skillDir := GetEngineSkillDir(engineID)
ctx.steps = append(ctx.steps, fmt.Sprintf(" /tmp/gh-aw/%s\n", skillDir))
}
@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Field Value
Category refactor
Risk Medium
Score 58/100
Action batch_review
Batch compiler-refactors

Score breakdown: Impact 28 · Urgency 12 · Quality 18

Decomposes 1.1k-line activation job builder into focused modules. No API change. CI passing. Good test coverage added. Can be reviewed alongside similar compiler refactors.

Generated by 🔧 PR Triage Agent · 226.8 AIC · ⌖ 5.86 AIC · ⊞ 5.4K ·

@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Triage Report

Field Value
Category refactor
Risk 🟡 Medium
Priority Score 50/100 (Impact: 25, Urgency: 10, Quality: 15)
Recommended Action batch_review — batch: compiler-refactor

Rationale: Decomposes a 1.1k-line activation job builder into focused modules. Large change (2368 lines net delta) but CI passing (43 checks ✅). Medium risk — no behavior change intended, but large refactor warrants careful review. Batch with other compiler changes.

Run §29115410581

Generated by 🔧 PR Triage Agent · 226.8 AIC · ⌖ 5.86 AIC · ⊞ 5.4K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

pr-sous-chef

@copilot please run the pr-finisher skill, address unresolved review comments, refresh the branch if needed, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 10.4 AIC · ⌖ 4.92 AIC · ⊞ 7.5K ·
Comment /souschef to run again

…-empty

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

pr-sous-chef

@copilot please run the pr-finisher skill, address unresolved review comments, refresh the branch if needed, and rerun checks once the branch is up to date....

Fixed in commit fix: include skill dir in artifact upload when SkillReferences is non-empty.

Updated the condition in addActivationArtifactUploadStep to check len(ctx.data.SkillReferences) > 0 alongside the existing len(ctx.data.Skills) > 0 check, so the skill directory is included in the uploaded artifact whenever frontmatter skills are configured via either field — consistent with addActivationSkillInstallSteps and the equivalent guard already present in notify_comment_conclusion_helpers.go.

Copilot AI requested a review from gh-aw-bot July 10, 2026 21:31
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address any remaining review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 12.5 AIC · ⌖ 5.63 AIC · ⊞ 7.5K ·
Comment /souschef to run again

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address any remaining review feedback, and rerun checks once the branch is up to date.

...

All review feedback has already been addressed (the SkillReferences fix was committed as 9bdaa62). Local validation (make fmt, make lint, make test-unit) all pass with no new issues. The branch is clean and up to date. CI must be re-triggered by a maintainer before merge.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

pr-sous-chef

@copilot please run the pr-finisher skill, address any remaining review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 13.9 AIC · ⌖ 4.91 AIC · ⊞ 7.5K ·
Comment /souschef to run again

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

pr-sous-chef

@copilot please run the pr-finisher skill, address any remaining review feedback, and rerun checks once the branch is up to date....

No remaining work — all review feedback was addressed in 9bdaa62, local validation passes (make fmt, make lint, make test-unit), and the branch is up to date. Ready for human CI re-trigger and merge.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Triage

Field Value
Category refactor
Risk medium
Priority Score 48 / 100
Recommended Action batch_review
Batch pr-batch:code-refactor (with #44874)

Score breakdown: Impact 25/50 · Urgency 10/30 · Quality 13/20

Rationale: Decomposes 1100+ line compiler file into focused modules (+1232/-1136 lines, 7 files). Large net-neutral refactor; CI passing. Batch with #44874 for a focused refactor review session.

Generated by 🔧 PR Triage Agent · 151.2 AIC · ⌖ 7.52 AIC · ⊞ 5.4K ·

@pelikhan pelikhan merged commit 4656dee into main Jul 11, 2026
31 checks passed
@pelikhan pelikhan deleted the copilot/file-diet-refactor-compiler-activation-job branch July 11, 2026 08:00
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.8

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.

[file-diet] Refactor: Split compiler_activation_job_builder.go (1135 lines) into focused modules

4 participants