fix(desktop): bound Slack Redux node decoder with context

[SebTardif]

What Problem This Solves

Fixes an issue where users running slacrawl sync --source desktop or slacrawl watch would hang forever when Slack Desktop IndexedDB decode spawned Node and that child never returned. Ctrl-C canceled the Go context, but the Node process kept running, so the CLI stayed parked.

Why This Change Was Made

The ingest context now reaches Extract and the Redux decoder. Node is started with CommandContext, and the blob loop stops when that context is done. Decode framing is unchanged. This does not restack the HTTP client timeout (#126) or the repeated-cursor guard (#159).

User Impact

A stuck Slack Desktop decode no longer holds the CLI after cancel. Sync, watch, and doctor can return when the user stops the command.

Evidence

Call chain: slacrawl sync --source desktop / watch -> slackdesktop.Ingest -> Extract -> extractIndexedDBStates -> runReduxDecoder. The decoder used exec.Command("node", ...) and cmd.Output() with no deadline. That spawn has been unbounded since desktop IndexedDB decode landed (2026-03-08, later kept in 0678e7f on 2026-04-27).

Before the patch, cancel did not stop a decoder that only called setTimeout:

$ go test ./internal/slackdesktop -run TestExtractIndexedDBStatesCancelsHungDecoder -timeout 15s -v
--- FAIL: TestExtractIndexedDBStatesCancelsHungDecoder (2.16s)
    redux_decode_test.go:210: extractIndexedDBStates ignored context cancel for a hung node decoder

After the patch, public Extract with a hanging Node on PATH returns when the 400ms context deadline fires:

$ PATH="/tmp/f003-hang-node:$PATH" /tmp/f003proof /tmp/f003-live-desktop
start extract
extract err=context deadline exceeded elapsed=402ms

The same CommandContext contract kills a hung Node child:

isolated start
isolated err=signal: killed ctx=context deadline exceeded elapsed=403ms

Real behavior proof

• Behavior or issue addressed: Slack Desktop Redux decode spawned Node with no context, so a hung child parked sync/watch and survived Ctrl-C.

• Real environment tested: macOS, Go 1.26, Node on PATH. Patched slacrawl at /tmp/oc-pr-slacrawl-F003. Fixture: recognized V8 IndexedDB blob plus a hanging Node wrapper that only calls setTimeout.

• Exact steps or command run after this patch: Built /tmp/f003proof from the patched module and ran Extract against that blob with PATH pointing at the hanging Node wrapper and a 400ms context deadline. Also ran CommandContext against node -e "setTimeout(function () {}, 1e9)".

• Evidence after fix: terminal output from the patched Extract binary and from CommandContext:

  start extract
  extract err=context deadline exceeded elapsed=402ms
  isolated start
  isolated err=signal: killed ctx=context deadline exceeded elapsed=403ms

• Observed result after fix: Extract returned context deadline exceeded in 402ms instead of waiting on Node. The hung Node child was killed (signal: killed) when the context fired.

• What was not tested: A real Slack Desktop profile whose IndexedDB blob hangs inside the stock decoder script, and a live Ctrl-C against that profile.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 30, 2026, 3:15 PM ET / 19:15 UTC.

ClawSweeper review

What this changes

The PR passes command context through Slack Desktop IndexedDB extraction so a hung Node-based Redux decoder is terminated when the caller cancels.

Regression provenance

Possible regression — probable (reviewed change; failure trace). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep this PR open for maintainer merge review: it is a focused, source-aligned repair for a desktop-command cancellation hang, with sufficient production-path terminal proof and no actionable correctness finding.

Priority: P2
Reviewed head: 42e976f802c96f15fa258171d0861b7aadd24639

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Focused production-path proof, source-aligned context propagation, and targeted regression coverage support a high-confidence repair.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed production owner is the IndexedDB extraction loop and its Node decoder; the PR body records an after-fix patched public Extract run against a recognized Redux blob with a hanging Node wrapper, returning the context deadline in 402 ms and showing the child killed.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owner is the IndexedDB extraction loop and its Node decoder; the PR body records an after-fix patched public Extract run against a recognized Redux blob with a hanging Node wrapper, returning the context deadline in 402 ms and showing the child killed.
Evidence reviewed	5 items	Production context path: Both desktop ingestion and doctor inspection pass their command context into Extract, which forwards it to the IndexedDB decoder. Cancellation implementation: The Node decoder now uses exec.CommandContext; if its execution returns after cancellation, the caller receives the context error instead of a per-blob decode failure. Regression coverage: The added test hangs the actual decoder script, cancels the extraction context, and requires context.Canceled within two seconds.
Findings	None	None.
Security	None	None.

How this fits together

Slack Desktop import snapshots local IndexedDB data and invokes Node to decode supported Redux blobs. The extracted workspace data feeds sync, watch, and doctor command flows.

flowchart LR
A[CLI sync watch doctor] --> B[Desktop import]
B --> C[IndexedDB snapshot]
C --> D[Redux blob decoder]
D --> E[Node process]
E --> F[Workspace data]
A --> G[Cancellation context]
G --> D
G --> E

Loading

Before merge

• Complete next step (P2) - No repair lane is needed: the PR is coherent, has no actionable review finding, and has sufficient production-path proof for maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	5 files affected; 67 added, 27 removed	The diff is limited to context propagation, Node-process cancellation, and regression coverage.
Cancellation coverage	1 new hung-decoder regression test	It covers cancellation through IndexedDB extraction rather than only the child-process helper.

Technical review

Best possible solution:

Merge the focused context propagation and cancellation test so a hung local Node decoder cannot keep a desktop command running after cancellation.

Do we have a high-confidence way to reproduce the issue?

Yes. The prior source launched Node without a context on the documented desktop extraction path, and the supplied after-fix terminal trace exercises patched public Extract with a hanging Node wrapper.

Is this the best way to solve the issue?

Yes. Passing the existing command context to the sole Node spawn and stopping the blob loop on cancellation is the narrowest maintainable repair while retaining normal decoding and per-blob error handling.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against dcbc12d44837.

Labels

Label justifications:

• P2: This repairs a bounded desktop-import cancellation hang without evidence of data loss or a broad runtime outage.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is the IndexedDB extraction loop and its Node decoder; the PR body records an after-fix patched public Extract run against a recognized Redux blob with a hanging Node wrapper, returning the context deadline in 402 ms and showing the child killed.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the IndexedDB extraction loop and its Node decoder; the PR body records an after-fix patched public Extract run against a recognized Redux blob with a hanging Node wrapper, returning the context deadline in 402 ms and showing the child killed.

Evidence

What I checked:

• Production context path: Both desktop ingestion and doctor inspection pass their command context into Extract, which forwards it to the IndexedDB decoder. (internal/slackdesktop/desktop.go:332, 42e976f802c9)
• Cancellation implementation: The Node decoder now uses exec.CommandContext; if its execution returns after cancellation, the caller receives the context error instead of a per-blob decode failure. (internal/slackdesktop/redux.go:572, 42e976f802c9)
• Regression coverage: The added test hangs the actual decoder script, cancels the extraction context, and requires context.Canceled within two seconds. (internal/slackdesktop/redux_decode_test.go:185, 42e976f802c9)
• Documented user surface: Desktop mode documents local-cache sync, watch refresh, doctor diagnostics, and Node-based Redux blob decoding. (docs/desktop-mode.md:64, dcbc12d44837)
• Current-main relationship: The verified PR introduction evidence identifies current main dcbc12d as the merge base and this open PR head as the introduced change, so the repair is not already on main. (internal/slackdesktop/redux.go:572, 42e976f802c9)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Aliou Diallo: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-08-29T18:28:53.104Z sha 42e976f :: needs maintainer review before merge. :: none
• reviewed 2026-08-29T23:54:39.236Z sha 42e976f :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T11:02:06.986Z sha 42e976f :: needs maintainer review before merge. :: none

[steipete]

Maintainer triage: recommend LAND. No source repair was needed. The docs/changelog follow-up is on triage/slacrawl-163-20260831; it preserves contributor credit and leaves this fork branch unchanged. No merge performed.

I reproduced the hang through the real built CLI using a synthetic IndexedDB blob and a temporary node executable that records its PID and then stops responding. The harness waits until that child is running, sends SIGINT only to Slacrawl, and checks both process exit and child cleanup. It uses an isolated config, database, HOME, and PATH; no real Slack profile or data is touched.

GOWORK=off go build -o /tmp/slacrawl-main ./cmd/slacrawl # main
GOWORK=off go build -o /tmp/slacrawl-163 ./cmd/slacrawl  # PR head
python3 proof-cancel.py desktop /tmp/slacrawl-main /tmp/slacrawl-163

Commands exercised: sync --source desktop --workspace TTEST --with-media=false, watch, and doctor, each with --config <isolated-config>.

main sync: still hung 2 seconds after SIGINT; required forced cleanup
main watch: still hung 2 seconds after SIGINT; required forced cleanup
main doctor: still hung 2 seconds after SIGINT; required forced cleanup
PR sync: exit=130; after SIGINT=0.892s; child_alive=false
PR watch: exit=130; after SIGINT=0.745s; child_alive=false
PR doctor: exit=130; after SIGINT=0.275s; child_alive=false

GOWORK=off go test -count=1 ./internal/slackdesktop
ok github.com/openclaw/slacrawl/internal/slackdesktop 63.836s

Codex autoreview returned scoped-clean on both the production patch and the docs follow-up (the skill's default P0 scope). Existing GitHub CI is green. The test injects the hung child locally; it does not claim a naturally hung production Slack profile.

[steipete]

Reviewed the integrated desktop cancellation patch and docs/changelog follow-up. The desktop tests passed locally, and the earlier synthetic built-CLI proof confirms SIGINT stops sync, watch, and doctor and cleans up the hung child. Codex autoreview found no actionable findings in its default P0 scope. Thanks @SebTardif.

[steipete]

Landed as c00d998 with the docs/changelog follow-up. Thanks @SebTardif!

Final head 9d2751c passed GOWORK=off go test -count=1 -timeout 3m ./internal/slackdesktop, GOWORK=off go vet ./internal/slackdesktop ./internal/cli, and make smoke. All GitHub checks passed, including CI, Docker, CodeQL, and secret scanning. The earlier synthetic built-CLI proof above confirms cancellation and child cleanup for sync, watch, and doctor. Autoreview found no actionable findings in its default P0 scope.