Skip to content

[AI-1019] kcap curate apply: write curated guidelines to CLAUDE.md/AGENTS.md - #191

Merged
alexeyzimarev merged 12 commits into
mainfrom
curate-apply
Jun 26, 2026
Merged

alexeyzimarev merged 12 commits into
mainfrom
curate-apply

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Implements the deferred curation write-back (provisional DEV-1678, now AI-1019) — the follow-up to the curation queue (AI-170). Promotions to claude_md previously only recorded intent; nothing wrote local files. This adds kcap curate apply to actually sync them.

What it does

kcap curate apply detects the current repo, fetches its promoted curation guidelines from the server, keeps the claude_md-targeted ones, and writes them into the repo's project-instruction file(s) inside an idempotent managed block.

  • Multi-harness, not CLAUDE.md-locked: writes CLAUDE.md and/or AGENTS.md — updates whichever exist, creates AGENTS.md if neither does. No personal-memory write-back (harness memory systems differ/unknown).
  • No new server endpoint — reuses GET /api/repositories/{hash}/curation?status=promoted.
  • Repo-root via GitRepository.FindRoot (never the cwd-fallback AppConfig.RepoRoot).
  • Managed block: deterministic order, full re-sync (revocations drop lines), exact full-line markers, fail-closed on malformed/multiple markers, single trailing newline; content outside the markers is never touched.
  • Untrusted text normalized (collapse newlines, defang -->) so a promoted guideline can't corrupt the block.
  • Preview + confirm by default; --dry-run; --yes; atomic write preserving the existing file mode (not forced 0600); symlink de-dup; distinct errors for not-in-repo / no-remote / 404 / 401 / unreachable / malformed / IO.

Structure

Pure, independently-tested building blocks in Capacitor.Cli.Core/Curation/ (CuratedTextNormalizer, CuratedBlock, CuratedTargets, CurationApplyPlanner) composed by a thin CurateCommand. New curate verb in the dispatcher + help-curate.txt + usage section. New AOT-registered DTOs.

Tests

TDD throughout; full unit suite green (1823 passing). Covers normalization, render/splice idempotency + all four malformed-marker cases, target resolution, planner actions/diffs, and atomic mode-preserving writes. The live network end-to-end (apply against a server with real promotions) is manual — not runnable in CI without a seeded server.

Companion / follow-ups

  • Server dialog cleanup (drop the dead "personal memory" checkbox; relabel CLAUDE.md option) lands separately in kcap-server, plus a src/cli submodule bump once this merges.
  • Deferred (non-blocking, from final review): a both-files-exist planner test; a NoOp NewContent==null assertion; a null/missing-items DTO test; a code comment on symlink replacement in WriteFileAtomic.

Spec + plan: docs/superpowers/specs/2026-06-26-curate-apply-writeback-design.md and docs/superpowers/plans/2026-06-26-curate-apply-writeback.md in kcap-server.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Jun 26, 2026

Copy link
Copy Markdown

AI-1019

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add kcap curate apply to sync promoted guidelines into CLAUDE.md/AGENTS.md
✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

Description

• Add kcap curate apply to fetch promoted curation and write instruction files.
• Manage an idempotent, fail-closed marker block without touching surrounding content.
• Add unit coverage for normalization, planning, marker safety, and atomic writes.
Diagram

graph TD
  A["kcap (Program)"] --> B["CurateCommand"] --> C{{"GET /api/.../curation"}} --> D["CurationApplyPlanner"] --> E["CuratedBlock"] --> F[("CLAUDE.md / AGENTS.md")]
  B --> G["CuratedTextNormalizer"]

  subgraph Legend
    direction LR
    _cli["CLI / Module"] ~~~ _api{{"HTTP API"}} ~~~ _file[("File")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Write curated output to a dedicated file (e.g., .kcap/curated.md)
  • ➕ Avoids marker parsing and fail-closed edits of hand-maintained files
  • ➕ Simplifies updates/removals (replace whole file)
  • ➖ Doesn’t integrate with existing harness expectations around CLAUDE.md/AGENTS.md
  • ➖ Requires users/tools to discover and include the separate file
2. Patch-based updates (edit only bullets inside the block)
  • ➕ Smaller diffs and potentially preserves intra-block formatting
  • ➕ Could be more tolerant of manual tweaks within the managed section
  • ➖ More complex and error-prone; increases risk of drift from server truth
  • ➖ Conflicts with the stated goal of full re-sync and deterministic ordering

Recommendation: The PR’s full re-sync approach with exact markers, deterministic ordering, and fail-closed validation is the safest strategy for untrusted, server-sourced text being written into user-controlled files. The managed-block model keeps changes bounded, and the preview/confirm + dry-run workflow reduces surprise. The dedicated-file alternative is cleaner but misaligned with existing instruction file conventions.

Files changed (16) +727 / -0

Enhancement (8) +406 / -0
CuratedBlock.csAdd managed-block render/splice with strict marker validation +91/-0

Add managed-block render/splice with strict marker validation

• Introduces rendering of a deterministic, sorted "Curated guidelines" block bracketed by exact start/end markers. Adds splicing logic to append, replace, or remove the block while preserving surrounding content and ensuring exactly one trailing newline. Fails closed by throwing when markers are malformed, duplicated, or out of order.

src/Capacitor.Cli.Core/Curation/CuratedBlock.cs

CuratedGuideline.csDefine curation domain types and apply plan contracts +22/-0

Define curation domain types and apply plan contracts

• Adds CuratedGuideline and CuratedBlockException types. Defines CurateAction plus FilePlan/ApplyPlan records to describe per-file actions, new content, and added/removed bullet diffs.

src/Capacitor.Cli.Core/Curation/CuratedGuideline.cs

CuratedTargets.csResolve instruction-file targets across CLAUDE.md and AGENTS.md +25/-0

Resolve instruction-file targets across CLAUDE.md and AGENTS.md

• Adds logic to decide which file(s) to touch based on which exist and whether there is curated content to write. Creates AGENTS.md when neither file exists, but avoids creating files when the curated set is empty (only removes stale blocks from existing files).

src/Capacitor.Cli.Core/Curation/CuratedTargets.cs

CuratedTextNormalizer.csNormalize and defang promoted guideline text for safe bullet insertion +35/-0

Normalize and defang promoted guideline text for safe bullet insertion

• Collapses multiline/whitespace-heavy promoted text into a single logical line and trims it to null when empty. Defangs the HTML comment terminator using a zero-width space escape so promoted text cannot terminate the managed marker comment.

src/Capacitor.Cli.Core/Curation/CuratedTextNormalizer.cs

CurationApplyPlanner.csCompute create/update/remove/no-op plan with bullet diffs +49/-0

Compute create/update/remove/no-op plan with bullet diffs

• Builds an ApplyPlan for CLAUDE.md/AGENTS.md based on current file contents and the promoted guideline set. Calculates added/removed bullets by comparing extracted block bullet text and produces NewContent via CuratedBlock.Splice, enabling preview and idempotent writes.

src/Capacitor.Cli.Core/Curation/CurationApplyPlanner.cs

Models.csAdd curation apply DTOs and register them for source-gen JSON +15/-0

Add curation apply DTOs and register them for source-gen JSON

• Adds CurationApplyResponse/CurationApplyItem records with snake_case JSON property mappings. Registers both types with the JsonSerializable context so AOT/source-gen deserialization can be used by the CLI.

src/Capacitor.Cli.Core/Models.cs

CurateCommand.csImplement 'kcap curate apply' orchestration, preview/confirm, and atomic writes +152/-0

Implement 'kcap curate apply' orchestration, preview/confirm, and atomic writes

• Implements end-to-end apply: verify repo root via GitRepository.FindRoot, detect remote for repo hash, fetch promoted items, filter claude_md targets, normalize text, and build an apply plan. Provides preview output with added/removed bullets, supports --dry-run and --yes confirmation skip, de-dups symlinked targets by real path, and writes via a temp-file + rename that preserves existing Unix mode.

src/Capacitor.Cli/Commands/CurateCommand.cs

Program.csAdd 'curate' verb dispatch with apply flags +17/-0

Add 'curate' verb dispatch with apply flags

• Extends the CLI dispatcher with a 'curate apply' subcommand, parsing --dry-run and --yes/-y. Emits usage errors for missing or unknown subcommands.

src/Capacitor.Cli/Program.cs

Tests (6) +300 / -0
CurateApplyFileWriteTests.csTest atomic write behavior and Unix mode preservation +40/-0

Test atomic write behavior and Unix mode preservation

• Adds tests ensuring WriteFileAtomic writes content without leaving a .tmp file behind. On non-Windows, verifies the original file’s Unix permissions are preserved across the atomic rewrite.

test/Capacitor.Cli.Tests.Unit/CurateApplyFileWriteTests.cs

CuratedBlockTests.csCover managed-block rendering, splicing, idempotency, and malformed markers +106/-0

Cover managed-block rendering, splicing, idempotency, and malformed markers

• Adds unit tests for deterministic sorting, append/replace/remove behavior, and strict failure modes on malformed/multiple/out-of-order markers. Validates byte-idempotency and the single trailing newline guarantee, plus bullet extraction.

test/Capacitor.Cli.Tests.Unit/CuratedBlockTests.cs

CuratedTargetsTests.csTest CLAUDE.md/AGENTS.md target resolution rules +33/-0

Test CLAUDE.md/AGENTS.md target resolution rules

• Verifies correct target selection across all combinations: both files exist, only one exists, neither exists (create AGENTS.md), and empty curated set (never create new files).

test/Capacitor.Cli.Tests.Unit/CuratedTargetsTests.cs

CuratedTextNormalizerTests.csTest text normalization and HTML comment terminator defanging +23/-0

Test text normalization and HTML comment terminator defanging

• Adds tests for collapsing multiline text into a single line, ensuring '-->' is neutralized, and returning null for empty/whitespace-only input.

test/Capacitor.Cli.Tests.Unit/CuratedTextNormalizerTests.cs

CurationApplyPlannerTests.csTest apply planning for create/update/no-op/remove with diffs +63/-0

Test apply planning for create/update/no-op/remove with diffs

• Covers planner behavior for creating AGENTS.md when needed, computing added/removed bullet diffs on update, detecting no-op when already current, and removing the existing block when the guideline set is empty.

test/Capacitor.Cli.Tests.Unit/CurationApplyPlannerTests.cs

CurationDtoTests.csValidate curation DTO JSON shape and unknown-field tolerance +35/-0

Validate curation DTO JSON shape and unknown-field tolerance

• Adds tests that deserialize the wrapper payload and snake_case fields using the source-generated JSON context. Confirms unknown fields are ignored and empty item arrays parse as expected.

test/Capacitor.Cli.Tests.Unit/CurationDtoTests.cs

Documentation (2) +21 / -0
help-curate.txtDocument 'kcap curate apply' usage and behavior +18/-0

Document 'kcap curate apply' usage and behavior

• Adds a new help topic describing how apply writes promoted guidelines into a managed block in repo-root instruction files. Documents --dry-run and --yes, plus behavioral notes about targets and idempotent resync.

src/Capacitor.Cli.Core/Resources/help-curate.txt

help-usage.txtExpose the new curate verb in global usage output +3/-0

Expose the new curate verb in global usage output

• Adds a Curation section that lists 'curate apply' and its main purpose (writing promoted guidelines into CLAUDE.md/AGENTS.md).

src/Capacitor.Cli.Core/Resources/help-usage.txt

@qodo-code-review

qodo-code-review Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Tickets: AI-1019

Grey Divider


Action required

1. Symlink replaced on write ✓ Resolved 🐞 Bug ≡ Correctness
Description
DistinctByRealPath de-dups symlinked CLAUDE.md/AGENTS.md by resolving the real path, but the write
still targets the original (possibly symlink) path and uses temp-file + rename overwrite. On
symlinked instruction files this can replace the symlink entry with a regular file, breaking the
link permanently.
Code

src/Capacitor.Cli/Commands/CurateCommand.cs[R129-151]

+    /// <summary>Temp-file + rename; preserves an existing file's Unix mode (default mode for new files).</summary>
+    public static string WriteFileAtomic(string path, string content) {
+        var tmp = path + ".tmp";
+        UnixFileMode? mode = null;
+        if (!OperatingSystem.IsWindows() && File.Exists(path)) mode = File.GetUnixFileMode(path);
+
+        File.WriteAllText(tmp, content);
+        if (mode is { } m && !OperatingSystem.IsWindows()) File.SetUnixFileMode(tmp, m);
+        File.Move(tmp, path, overwrite: true);
+        return path;
+    }
+
+    static List<FilePlan> DistinctByRealPath(List<FilePlan> files) {
+        var seen   = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+        var result = new List<FilePlan>();
+        foreach (var f in files) {
+            string real;
+            try { real = File.Exists(f.Path) ? Path.GetFullPath(new FileInfo(f.Path).ResolveLinkTarget(true)?.FullName ?? f.Path) : f.Path; }
+            catch { real = f.Path; }
+            if (seen.Add(real)) result.Add(f);
+        }
+        return result;
+    }
Evidence
The code resolves link targets only to compute a de-dup key, but still writes using the original
FilePlan.Path and performs a rename-overwrite, which is the operation that will replace the
directory entry at that path.

src/Capacitor.Cli/Commands/CurateCommand.cs[85-119]
src/Capacitor.Cli/Commands/CurateCommand.cs[129-151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DistinctByRealPath` computes a canonical “real path” only for de-duplication, but the subsequent write uses `FilePlan.Path` and `WriteFileAtomic` overwrites via `File.Move(tmp, path, overwrite: true)`. If `path` is a symlink, rename-overwrite replaces the symlink itself rather than updating the symlink target.

## Issue Context
This is most likely to happen when users keep both `CLAUDE.md` and `AGENTS.md` and make one a symlink to the other (a pattern the code explicitly tries to handle via de-dup).

## Fix Focus Areas
- src/Capacitor.Cli/Commands/CurateCommand.cs[129-151]

### Suggested approach
- When deduplicating, carry (or compute) a `WritePath` that is the resolved final target when the file is a symlink.
- Ensure `WriteFileAtomic` writes to the resolved target path (so the symlink remains intact).
- Keep the preview path user-friendly (may still display original logical path(s)), but write to the canonical target.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. curate apply undocumented in README 📘 Rule violation ⚙ Maintainability
Description
The PR adds a new user-facing CLI command kcap curate apply (and flags --dry-run, --yes/-y)
but does not document it in README.md. This can mislead users because the public command reference
won’t match the shipped CLI surface.
Code

src/Capacitor.Cli/Program.cs[R311-325]

+    case "curate": {
+        if (args.Length < 2) {
+            Console.Error.WriteLine("Usage: kcap curate apply [--dry-run] [--yes]");
+            return 1;
+        }
+        switch (args[1]) {
+            case "apply": {
+                var dryRun = args.Contains("--dry-run");
+                var yes    = args.Contains("--yes") || args.Contains("-y");
+                return await CurateCommand.HandleApply(baseUrl!, dryRun, yes);
+            }
+            default:
+                Console.Error.WriteLine($"Unknown curate subcommand: {args[1]}");
+                Console.Error.WriteLine("Usage: kcap curate apply [--dry-run] [--yes]");
+                return 1;
Evidence
Rule 3 requires README.md be updated in the same PR for user-facing CLI changes. The diff adds the
curate apply command and help/usage text, but the README CLI commands section contains no mention
of curate/curate apply.

CLAUDE.md: Keep README.md in sync with user-facing CLI surface changes (same PR)
src/Capacitor.Cli/Program.cs[311-325]
src/Capacitor.Cli.Core/Resources/help-usage.txt[51-53]
src/Capacitor.Cli.Core/Resources/help-curate.txt[1-18]
README.md[134-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new user-facing CLI command `kcap curate apply` (with `--dry-run` and `--yes/-y`) was added, but `README.md` was not updated to reflect the new CLI surface.

## Issue Context
The compliance checklist requires README updates in the same PR for user-visible CLI changes, not only updates to `help-*.txt`.

## Fix Focus Areas
- README.md[134-170]
- src/Capacitor.Cli/Program.cs[311-325]
- src/Capacitor.Cli.Core/Resources/help-usage.txt[51-53]
- src/Capacitor.Cli.Core/Resources/help-curate.txt[1-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Uncaught JSON parse errors ✓ Resolved 🐞 Bug ☼ Reliability
Description
HandleApply calls JsonSerializer.Deserialize without catching JsonException, so a
malformed/non-JSON 200 response will crash the command before it can emit the intended “Malformed
response” error. Other parts of the codebase explicitly catch JsonException for network JSON
parsing to avoid this failure mode.
Code

src/Capacitor.Cli/Commands/CurateCommand.cs[R49-55]

+        var json = await resp.Content.ReadAsStringAsync();
+        var dto  = JsonSerializer.Deserialize(json, CapacitorJsonContext.Default.CurationApplyResponse);
+        if (dto is null) {
+            await Console.Error.WriteLineAsync("Malformed response from server (could not parse curation payload).");
+            return 1;
+        }
+        var items = dto.Items ?? [];
Evidence
CurateCommand deserializes without a try/catch. In contrast, existing network JSON parsing code
catches JsonException and returns a controlled error path, demonstrating expected handling for
malformed responses.

src/Capacitor.Cli/Commands/CurateCommand.cs[49-54]
src/Capacitor.Cli.Core/Eval/EvalQuestionCatalogClient.cs[19-49]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`JsonSerializer.Deserialize(...)` can throw `JsonException` on malformed payloads; the current code only checks for a `null` return, so parse failures can terminate the CLI with an unhandled exception.

## Issue Context
The repo already has established patterns for catching `JsonException` around network JSON parsing and turning it into a user-facing error (rather than a crash).

## Fix Focus Areas
- src/Capacitor.Cli/Commands/CurateCommand.cs[49-55]

### Suggested approach
- Wrap deserialization in `try { ... } catch (JsonException ex) { ... }`.
- Print the existing “Malformed response…” message (optionally include `ex.Message`), then return 1.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

4. Removes adjacent blank lines ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
When removing a managed block, RemoveBlock expands the deletion range to include an adjacent blank
line before the start marker (and after the end marker). This mutates formatting outside the marker
lines and can delete user-intended spacing around the managed block.
Code

src/Capacitor.Cli.Core/Curation/CuratedBlock.cs[R71-76]

+    static void RemoveBlock(List<string> lines, (int start, int end) loc) {
+        var (s, e) = loc;
+        // Also drop one immediately-following blank line we may have inserted on append.
+        if (e + 1 < lines.Count && lines[e + 1].Trim().Length == 0) e++;
+        if (s - 1 >= 0 && lines[s - 1].Trim().Length == 0) s--;
+        lines.RemoveRange(s, e - s + 1);
Evidence
The code explicitly decrements s when the line before the start marker is blank, which causes the
deletion range to include a line outside the managed block.

src/Capacitor.Cli.Core/Curation/CuratedBlock.cs[71-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RemoveBlock` removes an empty line before the start marker (`s--`) and after the end marker (`e++`). This makes removal affect content outside the markers, which can be surprising if the surrounding whitespace was intentional.

## Issue Context
The comment only justifies removing the blank line after the block (which may have been inserted during append). The preceding blank line removal is not similarly justified and is harder to distinguish from user formatting.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Curation/CuratedBlock.cs[71-76]

### Suggested approach
- Consider removing only the trailing blank line after the block (the one the tool might have inserted) and leave the pre-marker blank line intact.
- If you want to keep current behavior, document it explicitly (since it contradicts the “outside markers never touched” expectation).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +311 to +325
case "curate": {
if (args.Length < 2) {
Console.Error.WriteLine("Usage: kcap curate apply [--dry-run] [--yes]");
return 1;
}
switch (args[1]) {
case "apply": {
var dryRun = args.Contains("--dry-run");
var yes = args.Contains("--yes") || args.Contains("-y");
return await CurateCommand.HandleApply(baseUrl!, dryRun, yes);
}
default:
Console.Error.WriteLine($"Unknown curate subcommand: {args[1]}");
Console.Error.WriteLine("Usage: kcap curate apply [--dry-run] [--yes]");
return 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. curate apply undocumented in readme 📘 Rule violation ⚙ Maintainability

The PR adds a new user-facing CLI command kcap curate apply (and flags --dry-run, --yes/-y)
but does not document it in README.md. This can mislead users because the public command reference
won’t match the shipped CLI surface.
Agent Prompt
## Issue description
A new user-facing CLI command `kcap curate apply` (with `--dry-run` and `--yes/-y`) was added, but `README.md` was not updated to reflect the new CLI surface.

## Issue Context
The compliance checklist requires README updates in the same PR for user-visible CLI changes, not only updates to `help-*.txt`.

## Fix Focus Areas
- README.md[134-170]
- src/Capacitor.Cli/Program.cs[311-325]
- src/Capacitor.Cli.Core/Resources/help-usage.txt[51-53]
- src/Capacitor.Cli.Core/Resources/help-curate.txt[1-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in da94005a3 — added a 'Curate guidelines' section to README.md documenting kcap curate apply and its --dry-run / --yes/-y flags, matching the existing command-reference style.

Comment thread src/Capacitor.Cli/Commands/CurateCommand.cs
…ad of replacing it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@alexeyzimarev

Copy link
Copy Markdown
Member Author

Follow-up commit 11d583aa9: refined symlink handling. If CLAUDE.md or AGENTS.md is a symlink (either direction), apply now resolves to the real target and writes the original file, leaving the symlink intact — rather than replacing the link with a regular file. A symlinked pair is written once. Added a test that creates CLAUDE.md -> AGENTS.md, applies, and asserts AGENTS.md is updated while CLAUDE.md stays a symlink.

…ves content outside markers untouched

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@alexeyzimarev

Copy link
Copy Markdown
Member Author

Thanks @qodo-code-review — all four findings addressed:

  1. Symlink replaced on write (Action Required) — resolved in 11d583aa9 (Qodo marked it ✓ Resolved): writes go to the resolved real target, symlink preserved.
  2. curate apply undocumented in README (Action Required) — fixed in da94005a3: added a Curate guidelines section to README.md.
  3. Uncaught JSON parse errors (Review Recommended) — fixed in da94005a3: JsonSerializer.Deserialize is now wrapped in try/catch (JsonException) and routed to the same "Malformed response from server" error path (exit 1), matching the existing EvalQuestionCatalogClient pattern.
  4. RemoveBlock removes adjacent blank lines (Advisory) — fixed in da94005a3: RemoveBlock now deletes only the marker block [s..e], leaving blank lines outside the markers untouched (honoring the "content outside the markers is never touched" guarantee). EOF cleanliness is still handled by the end-of-Splice trailing-newline normalization. Added a test (Splice_remove_does_not_eat_blank_lines_outside_markers).

Full unit suite green (1825).

@qodo-code-review

Copy link
Copy Markdown

Thanks @qodo-code-review — all four findings addressed:

  1. Symlink replaced on write (Action Required) — resolved in 11d583aa9 (Qodo marked it ✓ Resolved): writes go to the resolved real target, symlink preserved.
  2. curate apply undocumented in README (Action Required) — fixed in da94005a3: added a Curate guidelines section to README.md.
  3. Uncaught JSON parse errors (Review Recommended) — fixed in da94005a3: JsonSerializer.Deserialize is now wrapped in try/catch (JsonException) and routed to the same "Malformed response from server" error path (exit 1), matching the existing EvalQuestionCatalogClient pattern.
  4. RemoveBlock removes adjacent blank lines (Advisory) — fixed in da94005a3: RemoveBlock now deletes only the marker block [s..e], leaving blank lines outside the markers untouched (honoring the "content outside the markers is never touched" guarantee). EOF cleanliness is still handled by the end-of-Splice trailing-newline normalization. Added a test (Splice_remove_does_not_eat_blank_lines_outside_markers).

Full unit suite green (1825).

Thanks for the update — looks good.

  • curate apply is documented in README.md.
  • The JSON parse failure path is now handled cleanly.
  • RemoveBlock no longer eats surrounding blank lines.
  • The symlink write behavior is preserved by writing to the resolved real target.

I also see the unit suite is green, so I’m satisfied with the current state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant