fix(reliability): roll back partial artifact writes on current main by seonghobae · Pull Request #410 · ContextualWisdomLab/clearfolio

seonghobae · 2026-08-12T05:14:17Z

Objective

Preserve filesystem artifact atomicity and path authority on current protected main: a failed multi-file write must not expose partial or replacement-corrupted bytes.

Exact current-head evidence

protected base: 961f44a19c4ced87f8062b3ab9842a9a08d05235;
test-only RED head: 7b49e72281d7ca235b87f00aab3c7e56f8f8cb82;
exact GREEN head: ef4325940708de83b2e6375c5a0c6ad3b5ba63fd;
changed paths: FileSystemArtifactStore.java and its focused regression;
CI, Security Scan, SAST Semgrep, and fuzz: terminal-success;
mergeable: true;
unresolved review threads: zero.

Contract

Failed multi-file writes roll back newly published bytes, preserve the previously committed PDF and exact metadata during replacement failure, attempt cleanup independently, retain suppressed rollback evidence, fail closed on unreadable snapshots or missing document authority, and confine every resolved artifact path to the configured root.

Scope boundary

Standalone filesystem storage only. This does not provide distributed/object-store transactions, durable deletion receipts, generation fencing, Office conversion, or cross-instance coordination.

Merge gate

Keep this exact head unchanged. Auto-merge may act only after all live required checks remain successful, zero valid unresolved findings remain, and a qualifying independent non-author approval is attached to this exact head. Automated evidence and predecessor reviews do not transfer.

coderabbitai · 2026-08-12T05:14:27Z

Important

Review available on request

🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 00a0ce59-7f3f-4722-b264-6b2eb15c57ff

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

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

seonghobae · 2026-08-12T07:16:11Z

@opencode-agent review

Please review the unchanged exact current head 0adb8103412feaca2f6e5fc15dac687c9ef5614a against live protected main and repository gates. Treat checks/model output as evidence only and submit a formal review only for this exact head.

seonghobae · 2026-08-14T08:33:18Z

Exact-current-head read-only review request for ef4325940708de83b2e6375c5a0c6ad3b5ba63fd on protected main 961f44a19c4ced87f8062b3ab9842a9a08d05235. CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; the two-file storage-integrity delta is mergeable with zero unresolved threads. Verify rollback atomicity, preservation of prior committed bytes/metadata, suppressed cleanup evidence, and configured-root confinement. Do not mutate the branch or bypass independent approval.

@opencode-agent review
@cwl-noema-review review
@strix review
@coderabbitai review

coderabbitai · 2026-08-14T08:33:33Z

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 0 minutes and 27 seconds before sending another message.

seonghobae · 2026-08-15T09:31:20Z

@opencode-agent review
@coderabbitai review
@cwl-noema-review review
@strix review

Review exact head ef4325940708de83b2e6375c5a0c6ad3b5ba63fd only. Exact-head CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Validate rollback of newly published bytes, preservation of committed PDF/metadata during replacement failure, independent cleanup attempts, suppressed rollback evidence, and root-confined path authority.

Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge.

coderabbitai · 2026-08-15T09:32:13Z

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="ContextualWisdomLab/clearfolio"
pr_number="410"
expected_head="ef4325940708de83b2e6375c5a0c6ad3b5ba63fd"
expected_base="961f44a19c4ced87f8062b3ab9842a9a08d05235"

echo "== Live pull request metadata =="
gh pr view "$pr_number" --repo "$repo" --json number,state,mergeable,headRefOid,baseRefOid,headRefName,baseRefName,reviews,statusCheckRollup \
  --jq '{number,state,mergeable,headRefOid,baseRefOid,headRefName,baseRefName,reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}],checks:[.statusCheckRollup[]|{name,conclusion,status}]}'

echo "== Exact head and base validation =="
actual_head="$(gh pr view "$pr_number" --repo "$repo" --json headRefOid --jq .headRefOid)"
actual_base="$(gh pr view "$pr_number" --repo "$repo" --json baseRefOid --jq .baseRefOid)"
printf 'expected_head=%s\nactual_head=%s\nexpected_base=%s\nactual_base=%s\n' \
  "$expected_head" "$actual_head" "$expected_base" "$actual_base"

echo "== Local checked-out commit and changed paths =="
git rev-parse HEAD
git cat-file -e "${expected_head}^{commit}"
git cat-file -e "${expected_base}^{commit}"
git diff --name-status "$expected_base" "$expected_head"
git diff --check "$expected_base" "$expected_head"
git merge-base --is-ancestor "$expected_base" "$expected_head" && echo "base_is_ancestor=true"

echo "== Candidate Java files =="
fd -a '^FileSystemArtifactStore\.java$' .
fd -a -i 'FileSystemArtifactStore.*(Test|IT)\.java$' .

echo "== Java structure =="
while IFS= read -r file; do
  ast-grep outline "$file" --items all --type class,method --view expanded
done < <(fd -a '^FileSystemArtifactStore\.java$' .; fd -a -i 'FileSystemArtifactStore.*(Test|IT)\.java$' .)

echo "== Exact diff =="
git diff --find-renames --find-copies --unified=80 "$expected_base" "$expected_head" -- '*.java'

echo "== Live review threads =="
gh api graphql -F owner="ContextualWisdomLab" -F name="clearfolio" -F number="$pr_number" -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes {
          isResolved
          comments(first: 20) { nodes { author { login } body path line } }
        }
      }
    }
  }
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {comments:[.comments.nodes[]|{author:.author.login,path,line,body}]}]'

Length of output: 6637

🏁 Script executed:

#!/bin/bash
set -euo pipefail

store="src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java"
regression="src/test/java/com/clearfolio/viewer/artifact/FileSystemArtifactStorePartialWriteTest.java"
base="961f44a19c4ced87f8062b3ab9842a9a08d05235"
head="ef4325940708de83b2e6375c5a0c6ad3b5ba63fd"

echo "== Structure: production store =="
ast-grep outline "$store" --items all --type class,method --view expanded || true

echo "== Structure: focused regression =="
ast-grep outline "$regression" --items all --type class,method --view expanded || true

echo "== Exact changed-file diff =="
git diff --find-renames --find-copies --unified=100 "$base" "$head" -- "$store" "$regression"

echo "== Production store, numbered source =="
cat -n "$store"

echo "== Focused regression, numbered source =="
cat -n "$regression"

echo "== Related test assertions and failure injection =="
rg -n -C 5 --type java \
  '(FileSystemArtifactStore|rollback|suppressed|partial|replacement|metadata|Path|Files\.(move|write|delete)|assertThrows|assertArrayEquals)' \
  src/test/java/com/clearfolio/viewer/artifact

echo "== Unresolved review threads =="
gh api graphql \
  -F owner="ContextualWisdomLab" \
  -F name="clearfolio" \
  -F number="410" \
  -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes {
          isResolved
          comments(first: 20) {
            nodes { author { login } body path line }
          }
        }
      }
    }
  }
}' \
  --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {comments:[.comments.nodes[]|{author:.author.login,path,line,body}]}]'

Length of output: 125473

@seonghobae I reviewed exact head ef4325940708de83b2e6375c5a0c6ad3b5ba63fd against base 961f44a19c4ced87f8062b3ab9842a9a08d05235.

I found one blocking condition.

Fail-closed snapshot is incomplete. In src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java, Lines 132-134 use Files.exists(path) to decide that a prior file is absent. Files.exists also returns false when the filesystem cannot determine existence, including access-denied cases. The method can then treat an unreadable existing PDF or metadata file as absent. A subsequent write can replace it, and rollback cannot restore bytes that were not snapshotted. This conflicts with the required fail-closed behavior for unreadable snapshots.

The inspected rollback flow otherwise restores both artifact paths independently and retains cleanup failures as suppressed exceptions. The focused regression also covers ordinary partial writes, replacement preservation, independent cleanup attempts, and a directory-based unreadable snapshot case.

The live PR still matches the requested base and head. It is mergeable. The listed completed checks are successful. The unresolved-thread query returned zero threads. I did not approve, merge, or modify the branch.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

seonghobae · 2026-08-15T11:31:43Z

Exact-current-head independent review request for ef4325940708de83b2e6375c5a0c6ad3b5ba63fd.

The two-file filesystem atomicity slice is mergeable; CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Failed multi-file publication restores the exact prior PDF/metadata snapshot or removes newly created partial files, retains suppressed rollback evidence, and rejects missing document authority. Protected main has advanced since construction, so any base update must regenerate all exact-head evidence.

@opencode-agent review
@coderabbitai review
@cwl-noema-review review
@strix review

Review this exact head read-only. Do not mutate the branch, synthesize author approval, weaken protection, or merge.

coderabbitai · 2026-08-15T11:31:50Z

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 2 minutes and 10 seconds before sending another message.

…12-r2

opencode-agent · 2026-08-19T09:45:19Z

Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #410 at head 3c73780b1946dbcef2fff2026a5fe9ade7ef1c66. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

opencode-agent · 2026-08-19T09:45:20Z

Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #410 at head 3c73780b1946dbcef2fff2026a5fe9ade7ef1c66. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

opencode-agent · 2026-08-19T09:45:22Z

Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #410 at head 3c73780b1946dbcef2fff2026a5fe9ade7ef1c66. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

opencode-agent · 2026-08-20T03:32:18Z