feat(progress): replace initial progress comment with final summary via state file by aaight · Pull Request #411 · mongrel-intelligence/cascade

aaight · 2026-02-18T18:20:40Z

Summary

Adds src/backends/progressState.ts — a file-based state bridge that stores the progress comment ID so the PostComment gadget can update instead of duplicate it
Wires ProgressMonitor to write the state file after posting the initial "🚀 Starting" comment and clear it on stop()
Modifies PostComment core to check the state file before calling addComment, updating the existing progress comment in-place instead of creating a new one
Adds comprehensive unit tests for all new behaviour (state CRUD, PostComment replacement logic, ProgressMonitor state-file integration)

Closes: https://trello.com/c/cdol7mFL/39-planning-agent-final-summary-comment-should-replace-progress-comment

Changes

New file: `src/backends/progressState.ts`

writeProgressCommentId(repoDir, workItemId, commentId) — writes workItemId:commentId to .cascade-progress-comment-id in the repo dir
readProgressCommentId() — reads from process.cwd(), returns { workItemId, commentId } | null
clearProgressCommentId(repoDir?) — deletes state file from repo dir (or cwd)

Modified: `src/backends/progressMonitor.ts`

Adds repoDir?: string to ProgressMonitorConfig
postInitialComment() — calls writeProgressCommentId() after posting initial comment
stop() — calls clearProgressCommentId() to clean up on exit
postProgressToPM() — extracted helper (reduces complexity), calls writeProgressCommentId() when fallback comment is created

Modified: `src/backends/progress.ts`

Forwards repoDir option from ProgressMonitorOptions to ProgressMonitorConfig

Modified: `src/gadgets/pm/core/postComment.ts`

Before addComment, checks readProgressCommentId()
If found and workItemId matches: calls updateComment() then clearProgressCommentId()
If updateComment fails: falls back to addComment and clears state
If no state: existing addComment behaviour (unchanged)

New tests

tests/unit/backends/progressState.test.ts — CRUD operations with real FS using temp dirs
tests/unit/gadgets/pm/core/postComment.test.ts — updated with replacement and fallback scenarios
tests/unit/backends/progress.test.ts — new describe block for state file integration

Test plan

All 2122 tests pass
Lint passes (0 new errors introduced)
TypeScript type check passes

🤖 Generated with Claude Code

chore: release - merge dev into main

…ia state file

nhopeatall · 2026-02-18T18:24:01Z

🔍 Reviewing PR...

nhopeatall

Summary

The implementation is clean and well-tested, but the feature is currently inert — repoDir is never passed by any caller of createProgressMonitor, so the state file is never written and the PostComment replacement logic never activates.

Architecture & Design

[SHOULD_FIX] `repoDir` is not wired through from any caller

The new repoDir parameter on ProgressMonitorOptions is the linchpin of this feature — without it, writeProgressCommentId is never called and the state file is never created. All three call sites that create progress monitors omit it:

src/backends/adapter.ts:593 — createProgressMonitor({...}) — no repoDir (Claude Code backend path). repoDir is in scope as a local variable.
src/agents/base.ts:617 — createProgressMonitor({...}) — no repoDir (llmist backend, Trello agents). repoDir is available as a closure variable.
src/agents/shared/githubAgent.ts:201 — createProgressMonitor({...}) — no repoDir (llmist backend, GitHub agents). These use GitHub comments rather than Trello, so this may be intentionally omitted.

Without wiring repoDir into at least the first two callers, the state file is never written, readProgressCommentId() always returns null, and the PostComment gadget always falls through to addComment — meaning the progress comment is never replaced. The adapter.ts caller is the most critical since the Claude Code backend is where the cross-process state file bridge is most needed.

Code Quality

The refactoring of postProgress into postProgressToPM is a clean extraction that preserves behavior. The new progressState.ts module is well-structured with good edge case handling (colons in comment IDs, malformed files, missing files). Tests are comprehensive and cover the important scenarios including fallback paths.

The design of readProgressCommentId() reading from process.cwd() while writeProgressCommentId() writes to an explicit repoDir is reasonable for cross-process communication (since the subprocess chdirs to repoDir), though it creates an implicit coupling that should be documented.

nhopeatall · 2026-02-18T18:53:17Z

🔍 Reviewing PR...

nhopeatall

Summary

The core feature — replacing the progress comment with the final summary — cannot work in production because repoDir is never passed to createProgressMonitor by any of the three callers, so the state file is never written.

Architecture & Design

[SHOULD_FIX] Write/Read location asymmetry: writeProgressCommentId() writes to an explicit repoDir parameter, but readProgressCommentId() reads from process.cwd(). This works only because the agent lifecycle does process.chdir(repoDir) before execution, but the coupling is implicit. Consider making both functions use the same parameter (or both use process.cwd()) to make the contract explicit and reduce the chance of bugs when the lifecycle changes.

Code Issues

Blocking

All three callers of createProgressMonitor never pass repoDir — src/agents/base.ts:616, src/agents/shared/githubAgent.ts:200, and src/backends/adapter.ts:593 all call createProgressMonitor({...}) without including repoDir. Since ProgressMonitor guards state file writes with if (this.config.repoDir && ...), the state file is never written, and readProgressCommentId() in PostComment always returns null. The entire replacement feature is dead code. All three callers have repoDir available in scope and just need to pass it through.

Should Fix

.cascade-progress-comment-id missing from .gitignore — Once the feature is wired up, this ephemeral state file will sit in the repo working directory. If an agent happens to git add . or similar, it could be committed. Add .cascade-progress-comment-id to .gitignore.

nhopeatall · 2026-02-18T18:54:49Z

+ */
+export function readProgressCommentId(): { workItemId: string; commentId: string } | null {
+	const filePath = join(process.cwd(), STATE_FILE_NAME);
+


[SHOULD_FIX] readProgressCommentId() reads from process.cwd() while writeProgressCommentId() writes to an explicit repoDir parameter. This implicit coupling to the lifecycle's process.chdir(repoDir) is fragile — if any caller reads before chdir or after chdir-back, it silently returns null. Consider accepting an optional repoDir parameter here too (defaulting to process.cwd() for backward compat) to make the contract explicit.

Fixed. readProgressCommentId() now accepts an optional repoDir?: string parameter (defaulting to process.cwd()) — matching the API of both writeProgressCommentId() and clearProgressCommentId(). Also added a new test case for the explicit repoDir path.

nhopeatall · 2026-02-18T18:54:49Z

 			commentId: this.progressCommentId,
 		});
+
+		// Write state file so PostComment gadget can update this comment


[BLOCKING] This guard (if (this.config.repoDir && this.progressCommentId)) means the state file is only written when repoDir is provided. However, none of the three callers of createProgressMonitor pass repoDir:

src/agents/base.ts:616 — has repoDir in scope, does not pass it

src/agents/shared/githubAgent.ts:200 — GitHub agents don't use Trello, so N/A

src/backends/adapter.ts:593 — has repoDir in scope (line 545), does not pass it

Since this.config.repoDir is always undefined, writeProgressCommentId() is never called, the state file is never written, and the PostComment replacement logic is dead code.

Fix: add repoDir to the createProgressMonitor call sites that use Trello (at minimum adapter.ts and base.ts).

Fixed. repoDir is now wired through at both call sites:

adapter.ts:593 — passes repoDir ?? undefined (converts from string | null)

base.ts:616 — the createProgressMonitor callback signature in lifecycle.ts now passes (fileLogger, repoDir), and base.ts forwards it to createProgressMonitor({ repoDir, ... })

The state file will now actually be written when the ProgressMonitor posts the initial comment.

aaight · 2026-02-18T18:54:54Z

👀 Checking this out...

nhopeatall

Review already submitted - see inline comments above.

aaight · 2026-02-18T18:55:16Z

👀 Checking this out...

aaight · 2026-02-18T18:56:10Z

🔍 Reviewing PR...

aaight · 2026-02-18T18:56:24Z

🔍 Reviewing PR...

aaight

Code Review — PR #411: Progress Comment Replacement via State File

The overall approach is sound — using a filesystem state file as a bridge between the ProgressMonitor and the PostComment gadget is elegant, particularly for the Claude Code subprocess backend. The refactoring of postProgressToPM() is a nice readability win, and the test coverage is comprehensive. However, there are two issues that need to be addressed before merging.

🔴 BLOCKING: `repoDir` is never passed at the call sites — feature is dead code

The state file is only written when repoDir is provided in ProgressMonitorConfig, but none of the callers of createProgressMonitor actually pass this option (noted at src/agents/base.ts:616 and src/backends/adapter.ts:593). This means writeProgressCommentId() is never called at runtime, the state file is never created, and readProgressCommentId() in postComment.ts always returns null. The entire replacement flow is unreachable in production.

Fix: add repoDir to the createProgressMonitor call sites that already have it in scope — at minimum adapter.ts and base.ts.

🟡 SHOULD FIX: Asymmetry between `writeProgressCommentId` and `readProgressCommentId`

writeProgressCommentId(repoDir, ...) takes an explicit directory, but readProgressCommentId() hardcodes process.cwd(). This works correctly only when the agent subprocess has already chdir'd to the repo directory before calling the gadget — an implicit coupling that's easy to silently break.

Suggestion: add an optional repoDir?: string parameter to readProgressCommentId() (defaulting to process.cwd()) to make the contract explicit and allow callers to be more defensive.

✅ What's working well

postInitialComment() integration — writing the state file immediately after the "🚀 Starting" comment is posted is correct; the timing guarantees PostComment will always find it.
stop() cleanup — clearing the state file on stop prevents stale IDs from leaking into the next session.
Fallback logic in postComment.ts — the try/catch around updateComment with clearProgressCommentId() in both success and failure branches is defensive and correct.
clearProgressCommentId() API — accepting optional repoDir defaulting to process.cwd() is the right design; readProgressCommentId should follow the same pattern.
Test coverage — the progressState.test.ts suite is thorough: colon-in-commentId edge case (JIRA IDs), empty file, malformed file. The progress.test.ts integration block correctly validates state file writes and clears.
File format parsing — using indexOf(':') + slice to handle JIRA-style IDs with embedded colons in the comment ID is a good touch.
Minor nit: require('node:fs') inside progressState.test.ts tests should use import { readFileSync } from 'node:fs' (the module is already imported at the top) for ESM consistency.

aaight

Code Review

The approach is solid — a file-based state bridge is a clean way to share the progress comment ID between the ProgressMonitor and the PostComment gadget, and it works for both the llmist backend (same process) and Claude Code (subprocess). The file format, error handling, and test coverage are all well done.

However, there are two issues that need to be fixed before this can land.

🔴 BLOCKING: `repoDir` is never passed to `createProgressMonitor` — the feature is dead code

The BLOCKING reviewer comment (#2823925621) is correct and confirmed. adapter.ts:593 and base.ts:617 both call createProgressMonitor(...) without passing repoDir, so this.config.repoDir is always undefined, writeProgressCommentId() is never called, the state file is never written, and postComment's replacement logic never activates.

In src/backends/adapter.ts around line 593–601, repoDir is in scope (declared at line 526, set at line 545) but not forwarded:

const monitor = createProgressMonitor({
    // ...
    trello: cardId ? { cardId } : undefined,
    // ← repoDir missing here
});

Same issue in src/agents/base.ts around line 616–625.

Fix: Pass repoDir to createProgressMonitor at both call sites. Since repoDir is string | null in adapter.ts, convert with repoDir ?? undefined to satisfy the string | undefined type.

🟡 SHOULD_FIX: `readProgressCommentId()` has an asymmetric API (`process.cwd()` vs explicit `repoDir`)

The reviewer comment (#2823925616) is well-founded. writeProgressCommentId takes an explicit repoDir, while readProgressCommentId implicitly reads from process.cwd(). The correctness depends on callers ensuring process.chdir(repoDir) has been called before reading — which in adapter.ts happens at line 614, after createProgressMonitor is called (line 593). This fragility will cause subtle bugs if the ordering ever changes.

The fix is straightforward: add an optional repoDir?: string param to readProgressCommentId() (defaulting to process.cwd()), then update postComment.ts to pass the working directory. Since postComment doesn't currently receive a repoDir, the simplest fix is to keep defaulting to process.cwd() inside readProgressCommentId but document that callers must have already chdir'd — or better, thread repoDir through to the postComment gadget.

Minor observations (no changes needed)

require() in ESM tests (progressState.test.ts lines 39 and 69): require('node:fs').readFileSync(...) and require('node:fs').writeFileSync(...) are used in a few places. These are in a Vitest environment that supports it, but it would be more consistent to use the ES imports already declared at the top of the file.
Split import statements in progressState.ts: existsSync, rmSync, writeFileSync are imported in one statement and readFileSync in a separate import { readFileSync } — these can be merged into one import for cleanliness.
maybeWriteStateFile is only called in the update-failure fallback path — the success path of postProgressToPM does not call it, which is intentional (the state file written by postInitialComment remains current), but a short comment would make this easier to reason about.

The core feature design is right — fixing the two issues above (especially the BLOCKING one) will make this fully functional.

zbigniewsobiecki · 2026-02-18T19:03:37Z

@aaight address code review concerns

aaight · 2026-02-18T19:03:42Z

Request Addressed

What was asked

Address code review concerns on PR #411 (progress comment replacement feature).

Changes Made

repoDir wired to call sites — passed through adapter.ts, base.ts, and lifecycle.ts so the state file is actually written at runtime (was dead code)
readProgressCommentId() API symmetry — added optional repoDir? parameter matching the other two functions
.cascade-progress-comment-id added to .gitignore — prevents accidental commits of ephemeral state file
Minor cleanups — merged split imports, replaced require() with ESM imports in tests, added clarifying comment

Verification

✅ TypeScript type check: 0 errors
✅ Biome lint: 0 errors
✅ Tests: 2123 passed (all green)

- Pass repoDir to createProgressMonitor in adapter.ts and base.ts so the state file is actually written (previously dead code) - Update lifecycle.ts callback signature to forward repoDir - Add optional repoDir param to readProgressCommentId() for API symmetry with writeProgressCommentId() and clearProgressCommentId() - Add .cascade-progress-comment-id to .gitignore - Merge split fs imports in progressState.ts - Replace require('node:fs') with ESM imports in progressState tests - Add clarifying comment on maybeWriteStateFile call path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aaight · 2026-02-18T19:09:33Z

All code review concerns have been addressed in commit e91c930:

🔴 BLOCKING: `repoDir` never passed to `createProgressMonitor`

Fixed. Wired repoDir through at both call sites:

adapter.ts — passes repoDir ?? undefined directly
base.ts — updated the createProgressMonitor callback signature in lifecycle.ts to pass (fileLogger, repoDir), and base.ts forwards it

🟡 SHOULD_FIX: `readProgressCommentId()` API asymmetry

Fixed. Added optional repoDir?: string parameter (defaults to process.cwd()), matching writeProgressCommentId() and clearProgressCommentId(). Added test coverage for the explicit path.

🟡 SHOULD_FIX: `.cascade-progress-comment-id` missing from `.gitignore`

Fixed. Added to .gitignore.

Minor cleanups

Merged split import statements in progressState.ts
Replaced require('node:fs') with existing ESM imports in progressState.test.ts
Added clarifying comment on maybeWriteStateFile call path in progressMonitor.ts

Verification

✅ TypeScript type check passes (0 errors)
✅ Lint passes (0 errors)
✅ All 2123 tests pass

nhopeatall · 2026-02-18T19:12:34Z

🔍 Reviewing PR...

nhopeatall

Summary

Sound implementation of a file-based state bridge for replacing the initial progress comment with the final summary. The approach of using a filesystem state file to coordinate between the ProgressMonitor (which creates the initial comment) and the PostComment gadget (which posts the final summary) is well-suited for cross-process communication with the Claude Code subprocess backend.

CI passes, types check, and tests are comprehensive.

Should Fix

Missing state file write in `postProgressToPM` fallback path

src/backends/progressMonitor.ts:266-273 — The else branch in postProgressToPM (first tick when this.progressCommentId is null) creates a new comment but does NOT call maybeWriteStateFile():

} else {
    // First tick: create the comment and store its ID
    this.progressCommentId = await provider.addComment(cardId, summary);
    this.config.logWriter('INFO', 'Posted progress update to work item', { ... });
    // ← missing: this.maybeWriteStateFile(cardId, this.progressCommentId);
}

Scenario: If postInitialComment() fails (transient API error on startup), then the first tick() will enter this else branch. The comment is created successfully, this.progressCommentId is set (so future updates work), but the state file is never written. The PostComment gadget won't find the state and will create a duplicate comment instead of replacing it.

The fix is simply adding this.maybeWriteStateFile(cardId, this.progressCommentId); at the end of the else block. The update-success path (line 248) correctly does NOT rewrite the state file (same comment ID), and the update-failure fallback path (line 264) correctly DOES rewrite it (new comment ID). This else branch should be consistent with the fallback path.

This is a low-probability scenario (initial comment fails but subsequent tick succeeds) but worth fixing for completeness since the mechanism is explicitly designed for exactly this kind of resilience.

zbigniewsobiecki · 2026-02-18T19:29:08Z

@aaight address code review concerns

aaight · 2026-02-18T19:29:13Z

Request Addressed

What was asked

Address code review concerns on PR #411 (progress comment replacement feature).

Changes Made

Missing state file write in postProgressToPM else branch — Added this.maybeWriteStateFile() call when the first tick creates a comment because postInitialComment() failed. Without this, the PostComment gadget would not find the progress comment ID and would create a duplicate.
New test case — Added test verifying the state file is written from the first tick when the initial comment fails (transient API error scenario).

Verification

✅ TypeScript type check: 0 errors
✅ Biome lint: 0 errors
✅ All 2124 tests pass

…fails The `else` branch in `postProgressToPM` (entered when `postInitialComment()` fails and the first tick creates the comment) was not calling `maybeWriteStateFile()`, leaving the state file unwritten. This meant the PostComment gadget would not find the progress comment ID and would create a duplicate instead of replacing it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aaight · 2026-02-18T19:32:24Z

Addressed the remaining code review concern from @nhopeatall:

🟡 SHOULD_FIX: Missing state file write in `postProgressToPM` fallback path

Fixed in 597fce5. The else branch in postProgressToPM (entered when postInitialComment() fails and the first tick creates the comment instead) now calls this.maybeWriteStateFile() to write the state file. Previously, if the initial comment failed due to a transient API error, the state file was never written from this path, meaning the PostComment gadget would create a duplicate instead of replacing it.

Added a dedicated test case that simulates this scenario: initial comment fails → first tick creates a new comment → state file is written correctly.

Verification

✅ TypeScript type check: 0 errors
✅ Biome lint: 0 errors
✅ All 2124 tests pass

nhopeatall · 2026-02-18T19:35:46Z

✅ Review complete — APPROVED

Clean implementation of a file-based state bridge for replacing the initial progress comment with the final summary. The state file approach correctly handles cross-process communication (llmist in-process + Claude Code subprocess).

One suggestion: wrap clearProgressCommentId() in a try-catch inside stop() to prevent rmSync failures from masking agent results (called from finally blocks).

nhopeatall

Summary

Clean implementation of a file-based state bridge for replacing the initial progress comment with the final summary. The design is sound — using a filesystem state file is the right approach for cross-process communication between the ProgressMonitor (parent process) and PostComment (potentially a Claude Code subprocess). Tests are comprehensive and CI passes.

Should Fix

src/backends/progressMonitor.ts:139 — clearProgressCommentId() in stop() can throw (e.g., rmSync failure on permissions), and stop() is called from finally blocks in both lifecycle.ts:375 and adapter.ts:648. An uncaught exception here would mask the actual agent execution result. Wrap the cleanup in a try-catch:
```
stop(): void {
    if (this.timer) {
        clearInterval(this.timer);
        this.timer = null;
    }
    try {
        clearProgressCommentId(this.config.repoDir);
    } catch {
        // State file cleanup is best-effort
    }
}
```
Alternatively, add a try-catch inside clearProgressCommentId itself, since cleanup operations should generally be fault-tolerant.

zbigniewsobiecki · 2026-02-18T19:39:17Z

@aaight address code review concerns

aaight · 2026-02-18T19:39:22Z

Request Addressed

What was asked

Address code review concerns on PR #411 (progress comment replacement feature).

Changes Made

Wrapped clearProgressCommentId() in try-catch in stop() — rmSync can throw on permissions errors, and since stop() is called from finally blocks in lifecycle.ts and adapter.ts, an uncaught exception would mask the actual agent result. Now best-effort cleanup.

Verification

✅ TypeScript type check: 0 errors
✅ Biome lint: 0 errors
✅ All 49 relevant tests pass

clearProgressCommentId() uses rmSync which can throw on permissions errors. Since stop() is called from finally blocks in lifecycle.ts and adapter.ts, an uncaught exception would mask the actual agent execution result. Wrap in try-catch for best-effort cleanup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aaight · 2026-02-18T19:41:54Z

Addressed the remaining code review concern from @nhopeatall's approved review:

🟡 SHOULD_FIX: Wrap `clearProgressCommentId()` in try-catch in `stop()`

Fixed in 16b9325. clearProgressCommentId() uses rmSync which can throw on permissions errors. Since stop() is called from finally blocks in both lifecycle.ts:375 and adapter.ts:648, an uncaught exception would mask the actual agent execution result. Now wrapped in a try-catch for best-effort cleanup.

Verification

✅ TypeScript type check: 0 errors
✅ Biome lint: 0 errors
✅ All 49 relevant tests pass

nhopeatall · 2026-02-18T19:45:23Z

🔍 Reviewing PR...

nhopeatall

Summary

LGTM — Clean implementation of file-based state bridge for replacing progress comments with final summaries. The approach is well-suited to the cross-process constraint (llmist same-process + Claude Code subprocess) and all edge cases are handled gracefully.

What I verified:

State file parsing correctly handles colons in commentId (first-colon split), empty/malformed files, and missing files
No real race conditions: single Node.js event loop for llmist backend; sequential write-then-read ordering for Claude Code subprocess
postComment.ts relying on process.cwd() for readProgressCommentId() is correct — executeAgentLifecycle does process.chdir(repoDir) before agent execution
Fallback paths are robust: update failure → create new + clear state; state read failure → null → normal addComment
stop() cleanup is best-effort (won't mask agent errors)
githubAgent.ts correctly ignores repoDir (GitHub agents use PR comments, not Trello progress)
All 49 new/modified tests pass; TypeScript and CI checks pass
.gitignore updated for the ephemeral state file
Dependency direction (gadgets/pm/core → backends/progressState) follows existing patterns (gadgets/tmux → backends/claude-code)

github-actions Bot and others added 3 commits February 18, 2026 17:40

Merge pull request #407 from zbigniewsobiecki/dev

007bd5e

chore: release - merge dev into main

Merge pull request #409 from zbigniewsobiecki/dev

e778d23

chore: release - merge dev into main

feat(progress): replace initial progress comment with final summary v…

5df9dc7

…ia state file

nhopeatall reviewed Feb 18, 2026

View reviewed changes

zbigniewsobiecki mentioned this pull request Feb 18, 2026

fix(triggers): retry check-suite-success when CI suites still in-progress #412

Merged

6 tasks

nhopeatall requested changes Feb 18, 2026

View reviewed changes

aaight commented Feb 18, 2026

View reviewed changes

nhopeatall reviewed Feb 18, 2026

View reviewed changes

nhopeatall approved these changes Feb 18, 2026

View reviewed changes

zbigniewsobiecki merged commit 820b9db into dev Feb 18, 2026
4 checks passed

zbigniewsobiecki deleted the feature/progress-comment-replace branch March 16, 2026 16:43

Conversation

aaight commented Feb 18, 2026

Summary

Changes

New file: src/backends/progressState.ts

Modified: src/backends/progressMonitor.ts

Modified: src/backends/progress.ts

Modified: src/gadgets/pm/core/postComment.ts

New tests

Test plan

Uh oh!

nhopeatall commented Feb 18, 2026

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Summary

Architecture & Design

[SHOULD_FIX] repoDir is not wired through from any caller

Code Quality

Uh oh!

nhopeatall commented Feb 18, 2026

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Summary

Architecture & Design

Code Issues

Blocking

Should Fix

Uh oh!

nhopeatall Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

aaight Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

nhopeatall Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

aaight Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

aaight commented Feb 18, 2026

Uh oh!

nhopeatall left a comment

Choose a reason for hiding this comment

Uh oh!

aaight commented Feb 18, 2026

Uh oh!

aaight commented Feb 18, 2026

Uh oh!

aaight commented Feb 18, 2026

Uh oh!

aaight left a comment

Choose a reason for hiding this comment

Code Review — PR #411: Progress Comment Replacement via State File

🔴 BLOCKING: repoDir is never passed at the call sites — feature is dead code

🟡 SHOULD FIX: Asymmetry between writeProgressCommentId and readProgressCommentId

✅ What's working well

Uh oh!

aaight left a comment

Choose a reason for hiding this comment

Code Review

🔴 BLOCKING: repoDir is never passed to createProgressMonitor — the feature is dead code

🟡 SHOULD_FIX: readProgressCommentId() has an asymmetric API (process.cwd() vs explicit repoDir)

Minor observations (no changes needed)

Uh oh!

zbigniewsobiecki commented Feb 18, 2026

Uh oh!

aaight commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Request Addressed

What was asked

Changes Made

Verification

Uh oh!

aaight commented Feb 18, 2026

🔴 BLOCKING: repoDir never passed to createProgressMonitor

🟡 SHOULD_FIX: readProgressCommentId() API asymmetry

🟡 SHOULD_FIX: .cascade-progress-comment-id missing from .gitignore

New file: `src/backends/progressState.ts`

Modified: `src/backends/progressMonitor.ts`

Modified: `src/backends/progress.ts`

Modified: `src/gadgets/pm/core/postComment.ts`

[SHOULD_FIX] `repoDir` is not wired through from any caller

🔴 BLOCKING: `repoDir` is never passed at the call sites — feature is dead code

🟡 SHOULD FIX: Asymmetry between `writeProgressCommentId` and `readProgressCommentId`

🔴 BLOCKING: `repoDir` is never passed to `createProgressMonitor` — the feature is dead code

🟡 SHOULD_FIX: `readProgressCommentId()` has an asymmetric API (`process.cwd()` vs explicit `repoDir`)

aaight commented Feb 18, 2026 •

edited

Loading

🔴 BLOCKING: `repoDir` never passed to `createProgressMonitor`

🟡 SHOULD_FIX: `readProgressCommentId()` API asymmetry

🟡 SHOULD_FIX: `.cascade-progress-comment-id` missing from `.gitignore`

Missing state file write in `postProgressToPM` fallback path

aaight commented Feb 18, 2026 •

edited

Loading

🟡 SHOULD_FIX: Missing state file write in `postProgressToPM` fallback path

nhopeatall commented Feb 18, 2026 •

edited

Loading

aaight commented Feb 18, 2026 •

edited

Loading

🟡 SHOULD_FIX: Wrap `clearProgressCommentId()` in try-catch in `stop()`