[thomhurst]

Successful external build-output publication deletes the local archive, but BuildSolutionsModule still declared it as an artifact, causing a false missing-output warning.

Move the Redis artifact declaration to a dedicated producer that runs only for distributed builds without an external transport. Test workers wait for that optional producer and restore its accepted artifact reference; their required build dependency remains in place. External publication still uploads once and deletes its archive.

Validation:

• Capturing-logger regression reproduced the original warning for external publication.
• Sharing/archive tests: 16 passed; 2 Linux-only tests skipped on Windows. These cover one external upload, no duplicate Redis upload, the Redis accepted reference/download, and diagnostics for genuinely missing required output.
• Build publication condition/dependency checks: all 6 passed; this also built the build project without executing the build pipeline.
• Changed-file whitespace formatting passed.

Closes #7237

Summary by CodeRabbit

• New Features
  • Distributed builds without a configured sharing transport can now publish and restore shared build output through the Redis fallback.
  • The fallback is limited to eligible pipelines; builds using a configured transport continue to use that transport.
• Bug Fixes
  • Build output artifacts are now associated with the applicable publication path, helping workers retrieve shared output correctly.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-24T13:21:53.247873Z	018ca6a	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 82a021ff-9e6e-4203-9b68-d73fe5d37d78

Commits

Reviewing files that changed from the base of the PR and between 073b9b6 and 018ca6a.

Files selected for processing: 7

• src/ModularPipelines.Build/Helpers/BuildOutputSharing.cs
• src/ModularPipelines.Build/Modules/BuildSolutionsModule.cs
• src/ModularPipelines.Build/Modules/PublishRedisBuildOutputModule.cs
• src/ModularPipelines.Build/Modules/UnitTests/RunUnitTestModule.cs
• src/ModularPipelines.Build/Program.cs
• test/ModularPipelines.Build.UnitTests/BuildOutputPublicationTests.cs
• test/ModularPipelines.UnitTests/Build/BuildOutputSharingTests.cs

Files with no reviewable changes: 1

• src/ModularPipelines.Build/Modules/BuildSolutionsModule.cs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The build-output artifact declaration now belongs to a Redis fallback module that runs only when sharing is enabled without a transport. Workers optionally depend on that module. Tests cover producer selection, artifact lifecycle warnings, and worker dependencies.

Changes: Build-output publication

Layer / File(s)	Summary
Redis fallback artifact producer src/ModularPipelines.Build/Helpers/BuildOutputSharing.cs, src/ModularPipelines.Build/Modules/BuildSolutionsModule.cs, src/ModularPipelines.Build/Modules/PublishRedisBuildOutputModule.cs, test/ModularPipelines.Build.UnitTests/BuildOutputPublicationTests.cs, test/ModularPipelines.UnitTests/Build/BuildOutputSharingTests.cs	BuildOutputSharing exposes whether Redis fallback applies. PublishRedisBuildOutputModule declares the build-output artifact, while BuildSolutionsModule no longer does. Tests cover producer selection, artifact ownership, publication counts, and missing-output diagnostics.
Worker dependency and pipeline registration src/ModularPipelines.Build/Modules/UnitTests/RunUnitTestModule.cs, src/ModularPipelines.Build/Program.cs, test/ModularPipelines.Build.UnitTests/BuildOutputPublicationTests.cs	The pipeline registers the Redis publication module. Workers add an optional dependency on it and restore using its module ID. Tests check the producer and worker dependency declarations.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related issues

• CI: successful GitHub build-output publication emits a false missing-artifact warning — the changes move the artifact declaration to the Redis fallback producer and add logger coverage for transport and fallback lifecycle cases.

Merge Risk: Minimal · up to 018ca

No actionable merge-blocking risk remains in the reviewed publication and worker paths; the change is ready for normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: it fixes a false missing-artifact warning that appeared after build-output publication. The changeset adds a dedicated Redis build-output producer that runs only for d…
Linked Issues check		Issue #7237 coding requirements are met. BuildSolutionsModule no longer declares the archive. PublishRedisBuildOutputModule declares it only for Linux distributed Redis fallback runs and depends o…
Out of Scope Changes check		The changes remain within issue #7237 scope. Source changes coordinate build-output publication, artifact declaration, producer conditions, and worker dependencies. Tests cover the required publicatio…

Full details: Title check

Explanation

The title names the main change: it fixes a false missing-artifact warning that appeared after build-output publication. The changeset adds a dedicated Redis build-output producer that runs only for distributed fallback, removes the artifact declaration from BuildSolutionsModule, and adds tests for the warning and artifact lifecycle. The title is specific and matches the linked issue objective.

Full details: Linked Issues check

Explanation

Issue #7237 coding requirements are met. BuildSolutionsModule no longer declares the archive. PublishRedisBuildOutputModule declares it only for Linux distributed Redis fallback runs and depends on BuildSolutionsModule. Workers retain the required build dependency and add an optional dependency on the Redis producer. The lifecycle tests use RecordingLogger and verify no warning after external publication, one external upload, no Redis upload, Redis reference/download behavior, and the warning and failure for a genuinely missing required output. The run-condition tests cover transport and instance-count cases.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #7237 scope. Source changes coordinate build-output publication, artifact declaration, producer conditions, and worker dependencies. Tests cover the required publication and lifecycle behavior. The change does not globally suppress missing-artifact diagnostics or execute the full build pipeline.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the build-output trail,
The Redis path now has its own detail.
When transport takes the archive away,
No false missing warning joins the day.
Workers wait for fallback when required,
And tests keep each path verified.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Claude Review

The PR moves the Redis-fallback build-output artifact upload off BuildSolutionsModule and onto a new PublishRedisBuildOutputModule. That module depends on BuildSolutionsModule and runs only on Linux, with the ci-master capability, and only when distributed runs have no external transport. RunUnitTestModule now optionally depends on the new module and restores from its artifact. I checked the diff and the current BuildOutputSharing source. I found no actionable defects. I could not run the tests.

Review evidence

src/ModularPipelines.Build/Modules/PublishRedisBuildOutputModule.cs: The new producer module depends on BuildSolutionsModule and carries the ProducesArtifact attribute. It is gated by the UsesRedisBuildOutput run condition, which returns IsEnabled && transport is null. The condition is marked IPlanningSafe, and the body is a no-op.

src/ModularPipelines.Build/Helpers/BuildOutputSharing.cs: UsesRedisFallback is consistent with PublishAsync: with no transport it does nothing, and with a transport it publishes and deletes the local archive. RestoreAsync still downloads from the producer module id that the caller passes in, so the caller must pass the new module's id.

src/ModularPipelines.Build/Modules/UnitTests/RunUnitTestModule.cs: The optional DependsOn on PublishRedisBuildOutputModule makes workers wait for the Redis upload when that module runs. When the module is skipped, the dependency is optional, so workers are not blocked. RestoreAsync now uses the new module's id, which matches the module that owns the artifact.

test/ModularPipelines.Build.UnitTests/BuildOutputPublicationTests.cs: The tests cover the run-condition matrix of instance count and transport, and that only the new module declares the artifact. They also check the dependency attributes. They are reflection-based, so they do not exercise a real pipeline run.

src/ModularPipelines.Build/Modules/BuildSolutionsModule.cs: The ProducesArtifact attribute is removed, so the external-transport path no longer triggers a spurious missing-artifact warning or failure after the archive is deleted.

No actionable findings.

Optional follow-up notes

PublishRedisBuildOutputModule.ExecuteAsync is a no-op. The framework uploads the artifact only because of the attribute, so the design relies on the ProducesArtifact lifecycle running after a module with an empty body. The tests cover the attributes but not that end-to-end behavior, so a pipeline-level test or a CI run on a multi-instance Redis setup would be a useful follow-up.

UsesRedisBuildOutput resolves BuildOutputSharing through GetRequiredService. This assumes the service is registered in every pipeline configuration, including planning. The tests mock the context, so they do not confirm the registration.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Moves the Redis artifact declaration from BuildSolutionsModule to PublishRedisBuildOutputModule, avoiding a false warning after external publication deletes the archive.

• Build-output archives now come from the matching publisher.

No actionable issues were found. Tests were not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    B[BuildSolutionsModule] --> C{Publication choice}
    C -->|External transport| E[Upload once and delete archive]
    C -->|Redis fallback| R[PublishRedisBuildOutputModule uploads archive]
    C -->|Standalone| L[Use local build files]
    E --> T[RunUnitTestModule]
    R --> T
    L --> T

Loading

_{Reviews (1) · Last reviewed commit: "Fix build-output publication artifact li..." · Reviewed by Greptile}

[thomhurst]

Reviewed head 018ca6a3bbf4405310cebd0ae8a0b319fffe2777 and all review surfaces; no blocking findings or inline threads remain.

Disposition of optional Claude notes: an additional pipeline-level Redis test is non-blocking here. Artifact_Lifecycle_Publishes_Only_The_Redis_Fallback already exercises the actual artifact lifecycle upload and accepted-reference/download path, while the build-project tests verify the producer and worker dependency declarations. Declining additional orchestration coverage in this PR. The registration concern is satisfied by the unconditional builder.Services.AddSingleton<BuildOutputSharing>() in Program.cs, before module registration.

CodeRabbit's optional docstring suggestion is non-blocking wording work for internal build tooling and tests; declining it for this focused diagnostic fix. All applicable CI checks and the reviews completed on the current head, including the distributed full pipeline.