feat(cli): bounded always-on diagnostic sink for unexpected failures

[Chris0Jeky]

Closes #2468
Refs #2351

Summary

The CLI's unknown-exception boundary from #2466 kept the full exception only when the harness
startup trace was enabled (TASKDECK_CLI_TEST_TRACE_CORRELATION). In an ordinary operator run
there is no trace, so the exception was retained nowhere at all and the operator got the generic
line plus a "diagnostics were not captured" notice. This adds the always-on sink #2468 asked for.

backend/src/Taskdeck.Cli/CliFailureSink.cs (new, internal):

• Writes exactly one record per unexpected failure to
  <data directory>/diagnostics/cli-failure-<yyyyMMddTHHmmssZ>-<reference>.txt. The data directory
  is the directory of the resolved SQLite data source, via the existing
  CliFirstRunBootstrapper.ResolveDataDirectory (made internal so there is one resolution, not
  two). That helper already falls back to the working directory for a non-file data source such as
  :memory:; the sink inherits that fallback and says so in a comment.
• Record content: UTC timestamp, correlation reference, CLI version (ProductVersion.Value), argv
  passed through SensitiveDataRedactor.Redact, and SensitiveDataRedactor.SummarizeException
  output. Never a raw stack trace, never a raw Exception.Message.
• Bounds, both constants and both tested: MaximumRecordBytes = 8 * 1024 (truncated with an
  explicit marker, on a UTF-8 character boundary) and MaximumRecordCount = 20 (oldest evicted
  first; the timestamp prefix makes ordinal name order chronological).
• Created with FileMode.CreateNew, so a stale file or a planted symlink at the target path makes
  the write fail rather than being appended to or followed (O_CREAT|O_EXCL on POSIX). On non
  Windows the owner-only mode is set at creation via FileStreamOptions.UnixCreateMode, so there
  is no world-readable window.
• Fail-open on every IO error: TryRecord returns false and prints nothing.

CliUnexpectedFailure.Handle gained two optional parameters (CliFailureSink?,
IReadOnlyList<string>?), so existing three-argument callers and tests are unchanged. It tries the
harness trace first exactly as before, then the sink; "captured" is true if either succeeded. The
reference is the trace correlation when a trace is enabled, otherwise 12 lowercase hex characters
from RandomNumberGenerator.

Stderr contract: every existing string constant is unchanged. The only change is that an ordinary
run now also carries a correlation reference in the existing
Error [UNEXPECTED_ERROR]: <generic message> (trace correlation: <id>) format. The reference is
printed only when a sink actually kept the record; when nothing captured it, the output is the
plain generic line plus the unchanged DiagnosticsUnavailableNotice, as before. This keeps the
existing rule that the CLI never advertises a reference to a record it did not keep.

Program.cs builds the sink from the environment before the host is built (so a failure inside the
host build still has a sink), then re-points it at the configuration-resolved connection string
once builder.Configuration knows it, and passes it plus args to the boundary.

CliTestHarness gained an enableStartupTrace constructor flag (default true, so no existing test
changes behaviour) because the harness always sets the trace environment variable and test (a) has
to prove the ordinary no-trace run.

Docs: the CLI paragraph in docs/security/SECURITY_LOGGING_REDACTION.md now describes the real
retention (location, contents, bounds, permissions, fail-open) instead of saying the CLI has no
always-on sink. The CLI row and the R7 note in
docs/security/UNKNOWN_EXCEPTION_SURFACE_INVENTORY.md were updated only where they had become
false: the shifted Program.cs / CliUnexpectedFailure.cs line numbers, the "harness-only" and
"#2468 tracks" statements, and the test counts.

Red evidence

Both required regressions failed against the old code first.

(a) RealCli_WithoutTheHarnessTrace_KeepsOneRedactedRecordUnderTheDataDirectory, run before any
sink existed:

dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1 \
  --filter "FullyQualifiedName~RealCli_WithoutTheHarnessTrace"
Failed!  - Failed: 1, Passed: 0, Skipped: 0, Total: 1
Did not expect result.StdErr "Error [UnexpectedError]: Unexpected processing error. Check server
logs with the correlation ID.\nFull failure diagnostics were not captured: the CLI has no local
diagnostic sink, and the startup trace is not enabled for this run." to contain "Full failure
diagnostics were not captured: ...".

(f) TryRecord_DoesNotTouchAFileAlreadyAtTheTargetPath, run against a first sink implementation
that used FileMode.Create instead of FileMode.CreateNew:

dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1 \
  --filter "FullyQualifiedName~CliFailureSink"
Failed!  - Failed: 1, Passed: 9, Skipped: 0, Total: 10
Taskdeck.Cli.Tests.CliFailureSinkTests.TryRecord_DoesNotTouchAFileAlreadyAtTheTargetPath [FAIL]
Expected captured to be False, but found True.

Switching that single line to FileMode.CreateNew turned it green.

Verification

All commands run from the worktree
C:/Users/jekyt/source/Taskdeck-Beta/.worktrees/codex-2468-cli-failure-sink on Windows 11.

• dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1 --filter "FullyQualifiedName~CliFailureSink|FullyQualifiedName~CliUnexpectedError|FullyQualifiedName~CliStartupTrace"
  Passed: 28, Failed: 0, Skipped: 0, Total: 28.
• dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1
  Passed: 202, Failed: 0, Skipped: 0, Total: 202 (3 m 29 s).
• dotnet build backend/Taskdeck.sln -c Release
  0 Error(s), 12 Warning(s), all pre-existing nullable warnings in test projects untouched by this
  change.
• node scripts/check-docs-governance.mjs - "Docs governance check passed." (exit 0).
• node scripts/check-unknown-exception-boundary.mjs - "Unknown-exception boundary check passed."
  (exit 0).
• git diff --check - clean.

Cases required by the issue and where they live:

• (a) ordinary no-trace run writes one record with the printed reference:
  CliUnexpectedErrorSafetyTests.RealCli_WithoutTheHarnessTrace_KeepsOneRedactedRecordUnderTheDataDirectory
  (real child process via CliTestHarness).
• (b) POSIX 0600: CliFailureSinkTests.Record_IsOwnerReadWriteOnly_OnPosix.
• (c) 20-record cap evicts oldest: CliFailureSinkTests.TryRecord_EvictsTheOldestRecordsAtTheRetentionCap.
• (d) 8 KB truncation with marker: CliFailureSinkTests.TryRecord_TruncatesAnOversizedRecordWithAMarker.
• (e) uncreatable diagnostics directory prints the notice, no exception text, ExitCodes.Failure:
  CliFailureSinkTests.TryRecord_FailsOpenWhenTheDiagnosticsDirectoryCannotBeCreated and
  CliFailureSinkTests.Handle_WhenTheSinkCannotWrite_SaysDiagnosticsWereNotCapturedAndLeaksNothing.
• (f) pre-existing file at the target path is not appended to:
  CliFailureSinkTests.TryRecord_DoesNotTouchAFileAlreadyAtTheTargetPath.

Not verified

• The full-solution required check dotnet test backend/Taskdeck.sln -c Release -m:1 was NOT run.
  Another worker was running dotnet on this machine and RAM is tight (about 2.5 GB free), so it is
  left to ci-required. Only the CLI project and the solution build were run locally.
• The POSIX 0600 assertions are skipped on this Windows box, both in the new
  Record_IsOwnerReadWriteOnly_OnPosix and in the pre-existing
  FailureSink_IsOwnerReadWriteOnly_OnPosix. FileStreamOptions.UnixCreateMode is therefore
  proven only by the Linux CI leg.
• Symlink refusal is argued from FileMode.CreateNew mapping to O_CREAT|O_EXCL, not measured: the
  test plants a regular file, not a symlink, because creating a symlink on Windows needs elevation.
• No frontend, E2E or manual CLI run was performed; nothing outside backend/src/Taskdeck.Cli,
  backend/tests/Taskdeck.Cli.Tests and the two security docs was touched.

Risk notes

• The stderr contract changed in one way: an ordinary operator run now prints
  (trace correlation: <12 hex>) where it previously printed nothing after the generic message.
  Anything parsing that line for an exact match would see the suffix. All existing assertions in
  CliUnexpectedErrorSafetyTests still pass unmodified.
• The sink writes into the CLI data directory on every unexpected failure. Worst case it holds 20
  files of at most 8 KB, so 160 KB. The records are owner-only on POSIX and inherit NTFS ACLs on
  Windows.
• The records deliberately contain more than stderr does: the redacted exception summary keeps file
  paths, SQLite constraint text and provider URLs, because that is what makes a local diagnostic
  useful. Tokens and the argv secrets the redactor knows about are replaced with [redacted], and
  a raw stack trace is never written. The protection for the rest is the file mode, not redaction -
  the same trade-off the pre-existing startup-<correlation>.failure file already makes.
• CliFirstRunBootstrapper.ResolveDataDirectory went from private to internal. No behaviour
  change; it is now called by the sink so the two agree on the data directory.
• Program.cs re-points the sink after configuration resolves. Between managed entry and that
  point the sink uses the environment-derived connection string, which can differ from an
  appsettings-supplied one; in that window the record lands next to the environment's database
  instead. That is the only directory available before configuration exists, and it is why the sink
  is built twice rather than once.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Fix round 1

Blocking finding (HIGH): failure-sink construction sat outside the unknown-exception boundary and
was not fully guarded, so a malformed TASKDECK_CONNECTION_STRING escaped as an unhandled
exception with a raw stack trace. Confirmed, fixed.

The runtime claim the review left unverified is now verified. Data Source=taskdeck.db;Foreign Keys=yes makes SqliteConnectionStringBuilder raise FormatException from Convert.ToBoolean,
which no filter in ExtractDataSource / ResolveDataDirectory catches. Running the real CLI with
that value on the pre-fix head produced:

Unhandled exception. System.FormatException: String 'yes' was not recognized as a valid Boolean.
   at Microsoft.Data.Sqlite.SqliteConnectionStringBuilder..ctor(String connectionString)
   at Taskdeck.Cli.CliFirstRunBootstrapper.ExtractDataSource(String connectionString)
   at Taskdeck.Cli.CliFailureSink.ForConnectionString(String connectionString)
   at Taskdeck.Cli.CliFailureSink.FromEnvironment()
   at Program.<Main>$(String[] args) in .../Program.cs:line 20

with exit code -532462766 instead of ExitCodes.Failure. Default Timeout=abc raises
FormatException from Convert.ToInt32 and Pooling=sometimes the same from Convert.ToBoolean.

Fix: CliFailureSink.ForConnectionString now wraps its whole body in catch (Exception) and falls
back to the same current-directory root ResolveDataDirectory uses for any other unresolvable data
source. That closes FromEnvironment too, since it delegates here, so nothing at Program.cs:20
can throw. Diagnostics still land for the run rather than being lost.

Regressions added:

• CliFailureSinkTests.ForConnectionString_WithAnUnparsableKeywordValue_FallsBackInsteadOfThrowing
  (Theory, three malformed values).
• CliUnexpectedErrorSafetyTests.RealCli_WithAnUnparsableConnectionString_FailsThroughTheBoundary
  runs the real CLI process and asserts ExitCodes.Failure, no "Unhandled exception", no stack
  trace and no token on stderr.

Both failed on the pre-fix head with the output above and pass now.

Verification for this round, from the worktree:

• dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1 -
  Passed 206, Failed 0, Skipped 0.
• git diff --check 61e94f672...HEAD - clean.
• node scripts/check-docs-governance.mjs - passed.

Not verified this round: the full backend/Taskdeck.sln run (unchanged since the previous round;
no source outside Taskdeck.Cli was touched), and the POSIX 0600 creation mode, which only the
Linux CI leg exercises.

The four non-blocking findings (the DiagnosticsUnavailableNotice wording, the stale
CliUnexpectedFailure class comment, evict-before-write, and the unvalidated reference
path-safety check) are not addressed here and are left for tracking.

[Chris0Jeky]

Review (agent half of the gate; Codex credits exhausted, SC-9). Two fresh-context reviewer passes (read-only, Opus 5).

Round 1 at cc1cb67: verdict FIX_FIRST on one HIGH. The sink was constructed at Program.cs:20, outside the top-level unknown-exception try, and the connection-string parse it reaches (SqliteConnectionStringBuilder via ResolveDataDirectory) throws FormatException for values such as Foreign Keys=yes or Default Timeout=abc, which no catch filter in that chain covered. Reproduced end to end before the fix: the real CLI printed Unhandled exception. System.FormatException ... with a stack trace and exited with the runtime abort code instead of ExitCodes.Failure. Fixed at eefc7d8: CliFailureSink.ForConnectionString absorbs every exception and falls back to <cwd>/diagnostics (the same fallback ResolveDataDirectory already uses), with a unit Theory over the three malformed values and a real-process regression (RealCli_WithAnUnparsableConnectionString_FailsThroughTheBoundary) asserting exit 1, no Unhandled exception, no stack frames and no token on stderr. CLI project 206/206 at that head.

Round 2 (scoped to the fix diff) at eefc7d8: verdict SHIP. Confirmed the construction site has no reachable throw path, the fallback branches are themselves catch-all, the record file name is always hex so the reference cannot steer Path.Combine, and no new CRITICAL/HIGH was introduced.

Properties confirmed by round 1: FileMode.CreateNew + FileShare.None with UnixCreateMode applied at creation (no chmod window), every byte in the record comes from SensitiveDataRedactor (no ex.ToString(), stack or raw message), nothing exception-derived reaches stdout or stderr on any path including fail-open, the 8 KB and 20-record bounds are enforced before the write and cannot split UTF-8, the #2466 stderr constants are byte-identical, the harness trace still takes precedence, and the docs match the shipped behaviour.

Non-blocking findings and dispositions:

1. MEDIUM, fixed in 11ced97: DiagnosticsUnavailableNotice said the CLI "has no local diagnostic sink", which is now false on its only remaining trigger (the sink exists but could not write). The text now names that trigger. Every test references the constant, so nothing else moved.
2. LOW, fixed in 11ced97: the CliUnexpectedFailure class summary described only the harness trace; it now describes both sinks.
3. LOW, fixed in 11ced97: the inventory's test-case counts were stale by the fix round's additions.
4. LOW x3, tracked as [CLI][Security] Failure-sink follow-ups from the PR #2573 review: evict after write, path-safe reference, argv retention policy #2577: evict after a successful write rather than before; validate the reference as hex inside TryRecord; decide the argv retention policy (space-separated secret flags and ordinary user content in the record).
5. LOW (round 2): docs/STATUS.md still carries the Sanitize standalone CLI unexpected failures #2466 "retained nowhere" sentence and has no entry for the new sink. STATUS is written by the lane holding its lease; the shipped-fact packet for this merge names both.

Commit 11ced97 changes one string constant, one doc comment and two doc counts; no logic changed, so no further review pass is owed. Merge gate: hosted ci-required green at the final head plus the aging floor.