Skip to content

fix(coordinator): repair quality gates and signal replay#162

Open
johannesjo wants to merge 2 commits into
mainfrom
task/use-5-sub-agents-to-search-our-code-61dc12
Open

fix(coordinator): repair quality gates and signal replay#162
johannesjo wants to merge 2 commits into
mainfrom
task/use-5-sub-agents-to-search-our-code-61dc12

Conversation

@johannesjo
Copy link
Copy Markdown
Owner

Summary

  • Fix wait_for_signal_done request replay so timed-out request IDs are cached and replayed, including after coordinator deregistration.
  • Make the repo quality gates green by removing verified dead code/exports, breaking store import cycles, fixing OpenSpec custom-themes state, and tightening Semgrep filesystem-safety checks.
  • Expand CI to run check, tests, dead-code lint, architecture lint, security scan, security fixture tests, and OpenSpec validation.

Verification

  • npm run check
  • npm test
  • npm run lint:dead
  • npm run lint:arch
  • HOME=/tmp PATH=/tmp/parallel-code-semgrep-venv/bin:/home/johannes/.codex/tmp/arg0/codex-arg02XkmZT:/home/johannes/.nvm/versions/node/v22.18.0/lib/node_modules/@openai/codex/node_modules/@openai/codex-linux-x64/vendor/x86_64-unknown-linux-musl/codex-path:/home/johannes/.opencode/bin:/home/johannes/.local/bin:/home/johannes/.local/share/pnpm:/home/johannes/Android/Sdk/tools:/home/johannes/Android/Sdk/platform-tools:/home/johannes/.nvm/versions/node/v22.18.0/bin:/home/johannes/bin:/home/johannes/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/snap/bin:/snap/bin:/home/johannes/.local/share/JetBrains/Toolbox/scripts:/home/johannes/android/tools:/home/johannes/.rvm/bin npm run lint:security
  • HOME=/tmp PATH=/tmp/parallel-code-semgrep-venv/bin:/home/johannes/.codex/tmp/arg0/codex-arg02XkmZT:/home/johannes/.nvm/versions/node/v22.18.0/lib/node_modules/@openai/codex/node_modules/@openai/codex-linux-x64/vendor/x86_64-unknown-linux-musl/codex-path:/home/johannes/.opencode/bin:/home/johannes/.local/bin:/home/johannes/.local/share/pnpm:/home/johannes/Android/Sdk/tools:/home/johannes/Android/Sdk/platform-tools:/home/johannes/.nvm/versions/node/v22.18.0/bin:/home/johannes/bin:/home/johannes/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/snap/bin:/snap/bin:/home/johannes/.local/share/JetBrains/Toolbox/scripts:/home/johannes/android/tools:/home/johannes/.rvm/bin npm run test:security-rules
  • npx openspec validate --all --strict

Review

  • Reviewed via 5 sub-agents, then a final 2-agent spot check after follow-up fixes.
  • Addressed review findings around coordinator replay after deregistration, fake-timer cleanup, deterministic OpenSpec install, fail-closed Semgrep fixture behavior, actual Semgrep scan in CI, and untracked helper-file staging.

macOS-only automatic recovery (foreground + window-refocus repaints) plus a
cross-platform manual redraw shortcut (Cmd/Ctrl+Shift+L). Clears the glyph
texture atlas and forces a full refresh; the buffer is intact, only the GPU
glyph cache corrupts.
@brooksc
Copy link
Copy Markdown
Contributor

brooksc commented Jun 5, 2026

Just curious, why are you submitting PRs for your own repo? are you looking for feedback?

@johannesjo
Copy link
Copy Markdown
Owner Author

Thanks for the cleanup. I rechecked the findings with KISS/YAGNI in mind; I think there are two things to address before merge:

  1. wait_for_signal_done is still not fully idempotent for in-flight retries. The client creates one requestId per call and reuses it across network retries (electron/mcp/client.ts:132), but the coordinator only replays completed cache entries (electron/mcp/coordinator.ts:1634). If the first request is abandoned while still waiting, a retry with the same requestId can register a second waiter; the first waiter can consume/cache the signal, then the retry can later time out and overwrite the cache (electron/mcp/coordinator.ts:1681). The simple fix is a small pending-promise map keyed by coordinatorId/requestId, deleted on settle, plus a regression test for concurrent same-requestId waits.

  2. The branch is stale and currently not mergeable (mergeStateStatus: DIRTY). When rebasing, please keep current main .npmrc entries, especially onlyBuiltDependencies and message=chore(release): %s.

One smaller clarification: the replay after coordinator deregistration test exercises the Coordinator object directly, but the real REST path rejects unregistered X-Coordinator-Id headers before reaching waitForSignalDone. I would either remove/reword that claim, or only add API-level support if there is a concrete product need. I would not add a special auth bypass just to satisfy the test.

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.

2 participants