Skip to content

fix(core): await secrets store close during TaskStore shutdown - #1822

Merged
gsxdsm merged 2 commits into
Runfusion:mainfrom
plarson:fix/await-taskstore-secrets-close
Jun 30, 2026
Merged

fix(core): await secrets store close during TaskStore shutdown#1822
gsxdsm merged 2 commits into
Runfusion:mainfrom
plarson:fix/await-taskstore-secrets-close

Conversation

@plarson

@plarson plarson commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Waits for the task store's secrets database handle to close during shutdown before returning.
  • Updates workflow gate expectations for the current built-in lifecycle nodes.
  • Adds a patch changeset for the published CLI package.

Test Plan

  • corepack pnpm install
  • corepack pnpm check:changesets
  • corepack pnpm lint
  • corepack pnpm build
  • corepack pnpm test

Summary by CodeRabbit

  • Bug Fixes
    • Improved shutdown behavior so task-store cleanup now waits for secrets storage to close before finishing.
    • Reduced the chance of teardown race conditions and incomplete cleanup during app shutdown.

Copilot AI review requested due to automatic review settings June 29, 2026 19:37
@ghost

ghost commented Jun 29, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 2 individual chapters for you:

Title
1 Await secrets store closure during TaskStore shutdown
2 Add changeset for TaskStore shutdown fix
Open in Stage

Chapters generated by Stage for commit af99a35 on Jun 30, 2026 12:15am UTC.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8cd29fc2-6235-4842-ad45-a7097d6c6acb

📥 Commits

Reviewing files that changed from the base of the PR and between 02a33c6 and af99a35.

📒 Files selected for processing (2)
  • .changeset/thin-rocks-post.md
  • packages/core/src/store.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/thin-rocks-post.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/src/store.ts

📝 Walkthrough

Walkthrough

TaskStore.close() is updated to await the secretsCentralCore.close() call (previously fire-and-forget), capturing the instance first, nulling the field, then awaiting with a try/catch warning. A changeset entry records this as a patch fix.

Changes

TaskStore secrets core shutdown fix

Layer / File(s) Summary
Await secrets core close
packages/core/src/store.ts, .changeset/thin-rocks-post.md
TaskStore.close() now captures secretsCentralCore, clears the field, and awaits close() with a try/catch warning on failure. Changeset documents the patch release.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • gsxdsm

Poem

🐇 A secret once closed in a fire-and-forget,
Now awaited with care, no loose thread to fret.
The store shuts its doors in an orderly way,
No race in the teardown to ruin the day.
Hop hop, patch applied — all tidy and set! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: TaskStore now awaits secrets store closure during shutdown.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR makes TaskStore shutdown more deterministic by awaiting the secrets CentralCore close path and adds a patch changeset for the published CLI package.

Changes:

  • Await secretsCentralCore.close() during TaskStore.close() and clear the field before awaiting to keep shutdown idempotent.
  • Log (best-effort) if secrets CentralCore teardown fails instead of silently ignoring it.
  • Add a patch changeset for @runfusion/fusion describing the fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/core/src/store.ts Awaits secrets CentralCore shutdown during TaskStore close to reduce teardown races and improve shutdown determinism.
.changeset/thin-rocks-post.md Adds a patch changeset for @runfusion/fusion documenting the shutdown fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/store.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a shutdown race in TaskStore.close() where secretsCentralCore.close() was called with void (fire-and-forget), allowing the store's teardown to return before the secrets DB handle was fully released. The fix awaits the close and wraps it in a try/catch, consistent with the existing pluginStore teardown pattern.

  • packages/core/src/store.ts: captures secretsCentralCore locally, nulls the field before calling close (prevents re-entrant access), then awaits CentralCore.close() — which is itself async Promise<void> — with an error-handled console.warn fallback.
  • .changeset/thin-rocks-post.md: adds a patch changeset for @runfusion/fusion with the required labeled fields (summary, category, dev).

Confidence Score: 5/5

Safe to merge — the change is a targeted one-line async fix in the shutdown path with no effect on the hot path.

The fix is narrowly scoped to TaskStore.close(), a path only exercised during teardown. CentralCore.close() is already async Promise, so adding await is correct; the local-capture + null-before-close pattern prevents any re-entrant access; and the try/catch with console.warn matches every other handle teardown in the same function. No callers are broken because TaskStore.close() was already declared async.

No files require special attention.

Important Files Changed

Filename Overview
packages/core/src/store.ts Fixes shutdown race by awaiting secretsCentralCore.close() instead of firing and forgetting; pattern matches existing pluginStore teardown and is correctly guarded with null-before-close and try/catch.
.changeset/thin-rocks-post.md New patch changeset; uses required labeled-field format with correct bump type, category, and dev fields.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant TaskStore
    participant SecretsCentralCore

    Caller->>TaskStore: close()
    TaskStore->>TaskStore: flush agent log buffer
    TaskStore->>TaskStore: close _db / _archiveDb
    TaskStore->>TaskStore: capture secretsCentralCore local ref
    TaskStore->>TaskStore: "this.secretsCentralCore = null"
    TaskStore->>SecretsCentralCore: await close()
    alt close() succeeds
        SecretsCentralCore-->>TaskStore: resolved
    else close() throws
        SecretsCentralCore-->>TaskStore: error
        TaskStore->>TaskStore: console.warn (swallow)
    end
    TaskStore->>TaskStore: "secretsStore = null"
    TaskStore->>TaskStore: pluginStore.close() (sync)
    TaskStore-->>Caller: void (teardown complete)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant TaskStore
    participant SecretsCentralCore

    Caller->>TaskStore: close()
    TaskStore->>TaskStore: flush agent log buffer
    TaskStore->>TaskStore: close _db / _archiveDb
    TaskStore->>TaskStore: capture secretsCentralCore local ref
    TaskStore->>TaskStore: "this.secretsCentralCore = null"
    TaskStore->>SecretsCentralCore: await close()
    alt close() succeeds
        SecretsCentralCore-->>TaskStore: resolved
    else close() throws
        SecretsCentralCore-->>TaskStore: error
        TaskStore->>TaskStore: console.warn (swallow)
    end
    TaskStore->>TaskStore: "secretsStore = null"
    TaskStore->>TaskStore: pluginStore.close() (sync)
    TaskStore-->>Caller: void (teardown complete)
Loading

Reviews (5): Last reviewed commit: "test(engine): align workflow gate expect..." | Re-trigger Greptile

Comment thread packages/core/src/store.ts Outdated
Comment thread .changeset/thin-rocks-post.md
@plarson
plarson force-pushed the fix/await-taskstore-secrets-close branch 3 times, most recently from 02a33c6 to c6a34f0 Compare June 29, 2026 20:13
@gsxdsm

gsxdsm commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

@gsxdsm
gsxdsm force-pushed the fix/await-taskstore-secrets-close branch from c6a34f0 to af99a35 Compare June 30, 2026 00:15
@gsxdsm
gsxdsm merged commit 128bc51 into Runfusion:main Jun 30, 2026
6 checks passed
gsxdsm added a commit that referenced this pull request Jun 30, 2026
## Summary
- Mocks `git remote get-url origin` for dashboard PR lifecycle tests via
`execFileSync`, matching the production repository resolver.
- Keeps the missing-repository error covered for cases where no project
remote is resolvable.
- Tightens dashboard PR lifecycle assertions so PR status checks use the
resolved owner/repo.

## Verification
- `corepack pnpm build` — pass
- `corepack pnpm --filter @runfusion/fusion exec vitest run
src/commands/__tests__/dashboard.test.ts --silent=passed-only
--reporter=dot --no-color` — pass, 86 tests
- `corepack pnpm --filter @runfusion/fusion exec vitest run
src/commands/__tests__/dashboard.test.ts -t
"processPullRequestMergeTask|PR-first auto-merge queue"
--silent=passed-only --reporter=dot --no-color` — pass, 7 tests
- Clean rebuilt stale bundled plugin dist, then `corepack pnpm --filter
@runfusion/fusion exec vitest run --changed
24d7881 --passWithNoTests
--silent=passed-only --reporter=dot --no-color` — pass, 75 files / 1186
tests
- `corepack pnpm check:changesets` — pass
- `corepack pnpm lint` — pass
- `corepack pnpm typecheck` — pass

No changeset: test-only fixture coverage; no package runtime behavior
changed.

## CI note
The current Gate failure is the pre-existing engine-core workflow
expectation drift fixed by PR #1822. Lint, Typecheck, Build, CodeRabbit,
and Greptile passed on this PR.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Expanded dashboard and pull request merge coverage to verify
repository detection and merge-status handling across merge lifecycle
paths.
* Added a failure-path assertion for when the app can’t determine the
current repository, ensuring the operation stops and surfaces the
expected error.
* Improved assertions for auto-merge and manual merge flows to confirm
the correct repository lookup occurs in each scenario.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Current Gate status

`Gate` is failing on current `origin/main` before these PR changes. I
reproduced the same engine-core failures directly on `origin/main`
(`task-pipeline-smoke.test.ts` and
`workflow-graph-executor-parity.test.ts`, caused by the new
`completion-summary` / `post-merge-verification` default workflow
nodes). The PR-local review feedback is addressed and focused validation
passes; the remaining Gate failure should be fixed in the upstream
workflow-test drift PR, not copied into this PR branch.
gsxdsm added a commit that referenced this pull request Jun 30, 2026
## Summary
- Adds a fail-closed resolver for explicit review checkout metadata.
- Routes both in-session step reviews and graph step reviews through the
validated checkout when present.
- Keeps existing workspace per-repo review behavior when no explicit
checkout is configured.

## Verification
- `corepack pnpm check:changesets` — pass
- `corepack pnpm lint` — pass
- `corepack pnpm build` — pass
- `corepack pnpm --filter @fusion/engine exec vitest run
src/__tests__/reviewer-workspace.test.ts --reporter=dot --no-color` —
pass, 11 tests

## Note
- Full `corepack pnpm test` on current main is blocked by existing
workflow gate expectation drift covered separately by PR #1822; this
branch avoids duplicating those commits.


<!-- stage-review-badge-begin -->

---

<a href="https://stagereview.app/Runfusion/Fusion/pull/1823">
  <picture>
<source media="(prefers-color-scheme: dark)"
srcset="https://stagereview.app/assets/gh-open-in-stage-dark.svg">
<img src="https://stagereview.app/assets/gh-open-in-stage-light.svg"
alt="Open in Stage">
  </picture>
</a>

<!-- stage-review-badge-end -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Review steps now correctly run in the intended external checkout when
one is provided.
* Read-only reviewers now use the validated review workspace path
instead of defaulting to the task worktree.
* Added regression coverage to ensure review execution uses the expected
checkout location.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Current Gate status

`Gate` is failing on current `origin/main` before these PR changes. I
reproduced the same engine-core failures directly on `origin/main`
(`task-pipeline-smoke.test.ts` and
`workflow-graph-executor-parity.test.ts`, caused by the new
`completion-summary` / `post-merge-verification` default workflow
nodes). The PR-local review feedback is addressed and focused validation
passes; the remaining Gate failure should be fixed in the upstream
workflow-test drift PR, not copied into this PR branch.
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.

3 participants