Skip to content

fix(server): wait for plugin readiness before session.permission.create - #210

Merged
alltomatos merged 7 commits into
devfrom
fix-permission-plugin-ready
Sep 13, 2026
Merged

alltomatos merged 7 commits into
devfrom
fix-permission-plugin-ready

Conversation

@alltomatos

@alltomatos alltomatos commented Sep 13, 2026 •

Copy link
Copy Markdown
Owner

Closes #209
Resolves #189, #193, #163

Summary of Fixes

1. Fix #209 (v2.session.permission.create race)

  • Root cause: Race in application code: AgentPlugin/ConfigAgentPlugin runs in a forked, non-blocking fiber right after location boot. A permission check racing a freshly-created session observes an empty agent registry, falling back to deny-all (effect: "deny" instead of "ask").
  • Fix: Await PluginInternal.Service.ready (bounded, 5s timeout) in session.permission.create HTTP handler before calling permission.ask(...).

2. Fix e2e (windows) load contention (#189)

  • Root cause: 5 parallel Chromium workers on 2-vCPU Windows CI runners caused extreme CPU starvation during Shiki WASM Web Worker compilation and async diff rendering. Elements in specs like session-timeline-file-projection.spec.ts remained 0-height for 8-12s, hitting the 10s expect timeout and triggering cascading retries with heavy video/trace capture that exceeded the 60s teardown limit.
  • Fix: Scaled workers from 5 to 3 on Windows CI in packages/app/playwright.config.ts, and set adaptive headroom (expect.timeout: 25_000, test timeout 90_000) per packages/app/e2e/AGENTS.md guidelines.

3. Fix missing httpapi scenarios from PR #211

4. Fix Windows unit test timeouts and shell compatibility (#193, #163)

  • Root cause 1: Two prompt shell tests ran command: "sleep 0.2" using it.instance instead of the unix wrapper.
  • Fix 1: Switched to unix wrapper in prompt.test.ts matching other shell tests in the file.
  • Root cause 2: Instance bootstrap tests had no timeout budget, hitting the 60s boundary under full Windows turbo test load.
  • Fix 2: Added explicit 120s budget to instance-bootstrap.test.ts.
  • Root cause 3: eventuallyEffect in workspace.test.ts had a 1500ms timeout, too tight for SSE workspace sync under load.
  • Fix 3: Bumped to 5000ms (port from fix(test): widen more subprocess/eventual-consistency timeouts causing flaky unit failures #202).

Verification (All CI Checks 100% Green)

  • typecheck: pass (2m 25s)
  • nix-eval: pass (49s)
  • check-compliance: pass (3s)
  • check-standards: pass (4s)
  • e2e (linux): pass (7m 42s)
  • e2e (windows): pass (11m 43s)
  • unit (linux): pass (19m 2s)
  • unit (windows): pass (29m 36s)

The default agent's permission ruleset (e.g. the *.env -> ask rule) is
registered by AgentPlugin/ConfigAgentPlugin in a forked, non-blocking
fiber right after a location boots (PluginInternal.Service). A
permission check that races a freshly-created session can observe an
empty agent registry and fall back to deny-all instead of the real
ruleset, causing v2.session.permission.create to intermittently return
effect: "deny" instead of "ask" under the real Effect runtime
(--mode effect in the httpapi exercise).

Await PluginInternal.Service.ready (bounded, 5s timeout) before calling
permission.ask(...), mirroring the same pattern already used in
session/prompt.ts and the provider httpapi handler for this class of
race.

Closes #209

Co-Authored-By: Claude Sonnet 5 <[email protected]>
@github-actions

Copy link
Copy Markdown

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

On 2-vCPU Windows CI runners (windows-latest), running 5 parallel Chromium
browsers under load caused extreme CPU starvation during Shiki WASM Web Worker
compilation and diff rendering. As a result, element visibility assertions
in specs like session-timeline-file-projection.spec.ts took 8-12s, hitting
the 10s expect timeout and causing chronic flaky failures (#189).

Scale workers on Windows CI to 3 to alleviate CPU starvation, and increase
expect timeout to 25s (and test timeout to 90s) on Windows to provide adaptive
headroom per packages/app/e2e/AGENTS.md guidelines.
…prompt test timeouts

- PR #211 merged new memory routes (/memory/global, /memory/project/entries,
  /memory/entry, /memory/promote) without scenarios in test:httpapi, causing
  exerciser failure. Added all 4 scenarios.
- Widen tight 10s subprocess timeout to 30s in two prompt shell-loop tests
  observed timing out at ~10,020ms on Windows CI under heavy load (#193).
alltomatos and others added 2 commits September 13, 2026 14:06
…ts on windows

- In instance-bootstrap.test.ts, provide explicit 120s timeout budget for
  instance bootstrap tests that initialize full instances and git repos
  to avoid Windows CI load timeout near 60s (#193).
- In prompt.test.ts, mark 'sleep 0.2' shell tests as unix-only via unix()
  wrapper, matching other shell tests in the same file.
Investigando ao vivo (mobile não mostrava um projeto com sessão
recente): confirmei que um repo git sem commit/remote resolve pra
ID.global de propósito (já coberto por
"should handle git repository with no commits"), e que fromDirectory
já tem a lógica de resgate certa (UPDATE session SET project_id=novoID
WHERE project_id='global' AND directory=<esse diretório exato>) assim
que o repo ganha um commit — sem afetar sessões não relacionadas em
outros diretórios que também estão em "global". Não havia nenhum teste
cobrindo esse caminho específico (só o caso análogo via remote, em
"migrates cached root project data..."); este cobre a promoção a
partir do literal ID.global.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…163)

Port fix from #202:
- Bump eventuallyEffect timeout in workspace.test.ts from 1500ms to 5000ms
  to provide headroom for workspace sync transitions and SSE events under load.
- Bump subprocess timeout in run-process.test.ts from 30s to 60s matching
  the test harness default.
@alltomatos
alltomatos merged commit 6b9ebbd into dev Sep 13, 2026
10 checks passed
@alltomatos
alltomatos deleted the fix-permission-plugin-ready branch September 13, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant