[Repo Assist] eng(build): enforce TreatWarningsAsErrors across all source projects#527
Conversation
Adds <TreatWarningsAsErrors>true</TreatWarningsAsErrors> to src/Directory.Build.props so compiler warnings in source code fail the build rather than accumulating silently. All source projects currently produce 0 warnings; no behaviour change at HEAD. The one intentional non-exhaustive switch in ExecApprovalsCoordinator (CS8524) is suppressed via <NoWarn>CS8524</NoWarn> in OpenClaw.Shared.csproj because the switch deliberately omits a _ arm so developers notice when a new enum value is added. A comment in the csproj explains the rationale. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex review: needs changes before merge. Latest ClawSweeper review: 2026-05-23 13:09 UTC / May 23, 2026, 9:09 AM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. Source inspection shows PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge Security Review findings
Review detailsBest possible solution: Apply the warning-as-error policy consistently to all intended source projects and keep the CS8524 exception narrowly scoped to the one intentional switch. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows Is this the best way to solve the issue? No. The maintainable version should cover the nested CommandPalette source project and localize the CS8524 exception instead of suppressing that warning throughout Label changes:
Label justifications:
Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 5a87c9921c31. |
|
ClawSweeper PR egg 🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress. Hatch commandComment Hatchability rules:
What is this egg doing here?
|
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary (Task 4 — Engineering Investments)
Adds
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>tosrc/Directory.Build.propsso compiler warnings in source code are treated as build errors rather than accumulating silently over time.Why
The
tests/Directory.Build.propsalready enforcesTreatWarningsAsErrorsfor all test projects. This PR extends the same discipline to production source code undersrc/. Warnings that slip through today tend to become noise that gets ignored, and noise is how subtle bugs enter the codebase.What changed
src/Directory.Build.props<TreatWarningsAsErrors>true</TreatWarningsAsErrors>src/OpenClaw.Shared/OpenClaw.Shared.csproj<NoWarn>$(NoWarn);CS8524</NoWarn>The CS8524 suppression
ExecApprovalsCoordinator.csline 148 contains an intentional non-exhaustive switch expression (no_arm) so the compiler warns when a newExecApprovalPromptOutcomeenum value is added without a corresponding case. The code comment documents this intent. WithTreatWarningsAsErrors, that warning would become a build error, which defeats the purpose (the failure mode would be "won't compile" rather than "developer is alerted to add a case"). The<NoWarn>CS8524</NoWarn>suppression is scoped toOpenClaw.Sharedonly and is documented in the csproj comment.Test Status
All source projects build with 0 errors, 0 warnings after this change:
Shared test suite: 1950 passed, 8 pre-existing Linux/WSL infrastructure failures (unchanged from baseline).