Review summary

Change reviewed

PR #349 consolidates reusable-workflow compilation state into internal/compiler/reusable_job.go. The new component carries provenance, inputs, dependency and output bindings, call guards, secret authority, and token-permission warning state through recursive workflow expansion. It also owns the transitions from a sourced workflow job to a JobInstance, and from that instance to the reusable-workflow fields in a serialized plan.

The surrounding stages now delegate to that component:

• internal/compiler/reusable.go passes a reusableExpansion value through nested workflow resolution and accumulates authority state at each call.
• internal/compiler/job_graph_expansion.go delegates topology, matrix description, candidate construction, blocking checks, and dependency binding to sourcedJob methods.
• internal/compiler/plan_builder.go delegates workflow provenance, needs, deferred inputs, and call guards to JobInstance.lowerReusablePlan.
• internal/compiler/compiler.go and internal/compiler/bundle.go store and read secret and token-warning state through the consolidated authority value.
• internal/compiler/compiler_test.go adds an end-to-end nested-workflow test covering provenance, inherited secrets, root token permissions, deferred inputs and outputs, status-only prerequisites, nested call guards, dependencies, and plan encode/decode stability.

Evidence examined

I read the complete seven-file diff (639 additions and 466 deletions), the full new reusable-job component, and the surrounding resolver, graph-expansion, plan-building, warning, and secret-authorization code. I traced the state from the root caller through nested resolve calls, flattenedJob, newCandidate, dependency binding, requiredSecrets, and lowerReusablePlan.

I compared the relocated dependency-binding and plan-lowering helpers with their versions on main. I also checked the existing reusable-workflow tests for nested call guards, deferred inputs, output projection, explicit and inherited secret forwarding, root GITHUB_TOKEN permissions, and nested permission warnings. The new test exercises the combined path at the plan boundary.

GitHub reported Buildkite build #1744 passed, including the public-actions proof and producer/consumer smoke checks. Socket checks also passed. The review build #19611 was still in progress when the review was submitted, and its raw Buildkite job data was unavailable through the local credentials.

I could not run local Go tests or go vet: neither the pinned mise installation nor a Go binary was available in this environment. I did run git diff --check, which passed.

No Linear issue identifier appeared in the branch name, PR title, or PR description.

Findings and verdict

I found no specific correctness issue and posted no inline comments. The refactor preserves the important propagation rules I traced: secret authority remains scoped at each reusable-workflow edge, ignored-permission and narrowed-token flags accumulate across nesting, the outer reusable-call position is retained for warnings, and dependency and plan projections use the same data as before.

I submitted a COMMENT review rather than an approval. Although the implementation appears behavior-preserving, it changes ownership and execution flow in the secret-inheritance and GITHUB_TOKEN permission authority paths, so human review remains appropriate.

Trigger source: automatic.