feat: add job.workflow_* typed accessors to JobContext#4334
feat: add job.workflow_* typed accessors to JobContext#4334
job.workflow_* typed accessors to JobContext#4334Conversation
Adds typed C# accessors for workflow_ref, workflow_sha, workflow_repository, and workflow_file_path to JobContext. Includes DeriveWorkflowRefComponents() to derive repository and file_path from workflow_ref if not sent by server. Not strictly required — the run-service already sends all 4 fields and the existing blanket copy populates them in expressions. This is a code quality improvement for typed access and a fallback safety net. Part of ADR 10024 / c2c-actions#10025
There was a problem hiding this comment.
Pull request overview
Adds typed accessors on JobContext for new job.workflow_* expression context properties and a fallback derivation step so reusable workflows can identify their own source at runtime.
Changes:
- Added
WorkflowRef,WorkflowSha,WorkflowRepository, andWorkflowFilePathtyped accessors onJobContext, plusDeriveWorkflowRefComponents()to parseworkflow_ref. - Updated
ExecutionContext.InitializeJobto deriveworkflow_repositoryandworkflow_file_pathafter hydrating the job context. - Added L0 tests covering get/set/null semantics for the new accessors and verification of hydration/derivation behavior in
ExecutionContext.
Show a summary per file
| File | Description |
|---|---|
| src/Runner.Worker/JobContext.cs | Introduces typed accessors for workflow_* and adds derivation from workflow_ref. |
| src/Runner.Worker/ExecutionContext.cs | Calls workflow-ref derivation after job context hydration. |
| src/Test/L0/Worker/JobContextL0.cs | Adds unit tests for the new accessors and derivation scenarios. |
| src/Test/L0/Worker/ExecutionContextL0.cs | Adds initialization tests validating hydration/derivation behavior across feature-flag scenarios. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 4
| // Derive workflow_repository and workflow_file_path from workflow_ref | ||
| // if the server sent workflow_ref but not the decomposed fields | ||
| jobContext.DeriveWorkflowRefComponents(); | ||
| } |
There was a problem hiding this comment.
InitializeJob currently hydrates the entire job expression context (including the new workflow_* fields) only when Constants.Runner.Features.AddCheckRunIdToJobContext is enabled. The PR description indicates the workflow identity fields are gated by a different feature flag (actions_expose_job_workflow_context), so with the current implementation the runner will ignore server-sent workflow_* values unless the unrelated check-run-id flag is also on. Consider introducing/using a dedicated feature flag for the workflow identity context (or otherwise decoupling workflow identity hydration from the check-run-id flag) so the behavior matches the intended rollout gate.
| // Format: owner/repo/path/to/file.yml@ref | ||
| var atIndex = workflowRef.IndexOf('@'); | ||
| var pathPart = atIndex >= 0 ? workflowRef.Substring(0, atIndex) : workflowRef; | ||
|
|
||
| // Split into owner/repo and file path at the .github/ boundary | ||
| var githubDirIndex = pathPart.IndexOf("/.github/"); | ||
| if (githubDirIndex < 0) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
DeriveWorkflowRefComponents() uses pathPart.IndexOf("/.github/") without specifying StringComparison. For parsing protocol/identifier strings (like workflow refs) the codebase often uses ordinal comparisons to avoid culture-sensitive behavior and improve predictability. Consider using IndexOf("/.github/", StringComparison.Ordinal) (and similarly for other string searches) here.
| // Arrange: Create a job request message with the feature flag enabled | ||
| var variables = new Dictionary<string, VariableValue>() | ||
| { | ||
| [Constants.Runner.Features.AddCheckRunIdToJobContext] = new VariableValue("true"), | ||
| }; | ||
| var jobRequest = new Pipelines.AgentJobRequestMessage(new TaskOrchestrationPlanReference(), new TimelineReference(), Guid.NewGuid(), "some job name", "some job name", null, null, null, variables, new List<MaskHint>(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List<Pipelines.ActionStep>(), null, null, null, null, null); |
There was a problem hiding this comment.
These new workflow-identity tests toggle Constants.Runner.Features.AddCheckRunIdToJobContext. If workflow identity exposure is intended to be gated by a dedicated feature flag (as described in the PR metadata), these tests (and the production gate) should be updated to use the workflow-identity flag instead; otherwise the tests are implicitly asserting that workflow identity is coupled to the check-run-id rollout.
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Worker")] | ||
| public void InitializeJob_WorkflowIdentityDerived_WhenServerSendsAllFields() |
There was a problem hiding this comment.
Test name InitializeJob_WorkflowIdentityDerived_WhenServerSendsAllFields is misleading: the assertions verify that explicit workflow_repository/workflow_file_path values from the server are preserved (i.e., derivation does not overwrite). Consider renaming to reflect the non-overwrite behavior so the intent is clear when the test fails.
| public void InitializeJob_WorkflowIdentityDerived_WhenServerSendsAllFields() | |
| public void InitializeJob_WorkflowIdentityNotOverwritten_WhenServerSendsAllFields() |
Fixes incorrect parsing for repos named .github (e.g. octo-org/.github). The old /.github/ marker would match too early, deriving the wrong repository name. Also treats empty-string fields as unset during derivation.
- Remove AddCheckRunIdToJobContext flag gate from job context hydration. The flag is permanently force-true server-side (acquirejobhandler.go:1658). The server controls what to send via its own feature flags. - Add owner/repo format validation to DeriveWorkflowRefComponents() - Add tests for PR merge refs, tag refs, invalid repo format, and .github repo names - Update check_run_id flag-disabled test to reflect always-copy behavior
Summary
Adds typed C# accessors for the four new
jobexpression context properties, enabling reusable workflows to determine their own source identity at runtime.New properties
job.workflow_refowner/repo/.github/workflows/file.yml@refs/heads/branchjob.workflow_shajob.workflow_repositoryowner/repoof the workflow filejob.workflow_file_path.github/workflows/file.ymlChanges
Runner.Worker/JobContext.cs— 4 string accessors (WorkflowRef,WorkflowSha,WorkflowRepository,WorkflowFilePath) +DeriveWorkflowRefComponents()to derive repository and file path from the compositeworkflow_refRunner.Worker/ExecutionContext.cs— CallsDeriveWorkflowRefComponents()after hydrating job context from serverContextDataTest/L0/Worker/JobContextL0.cs— 20 unit tests covering get/set/null for all properties + 5 derivation scenariosTest/L0/Worker/ExecutionContextL0.cs— 3 integration tests: flag enabled (hydrates + derives), flag disabled (nothing set), server-sends-all-fields (no overwrite)This PR adds:
CheckRunId,Status,Container,Servicesworkflow_refwithout the decomposed fieldsPart of https://github.com/github/c2c-actions/issues/10025