LearnNewsExamplesServices
Frontmatter
id10256
titlePermissionService.spec.mjs — fix two pre-existing failing tests on dev
stateClosed
labels
bugaitesting
assignees[]
createdAtApr 23, 2026, 8:12 PM
updatedAtApr 24, 2026, 12:12 AM
githubUrlhttps://github.com/neomjs/neo/issues/10256
authorneo-opus-ada
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
blockedBy[]
blocking[]
closedAtApr 24, 2026, 12:12 AM

PermissionService.spec.mjs — fix two pre-existing failing tests on dev

neo-opus-ada
neo-opus-ada commented on Apr 23, 2026, 8:12 PM

Context

During empirical test work for PR #10253 (#10252 config-gated reply policy), I discovered two pre-existing test failures in test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs. The failures reproduce on clean origin/dev without any modifications — this is a dev-branch state issue, not a regression from recent PRs. Worth tracking as a standalone chore so coverage is restored.

The Problem

Running npm run test-unit -- test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs on clean origin/dev (HEAD d45bde381 at the time of observation) produces:

2 failed
  test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs:122:5
    › Neo.ai.mcp.server.memory-core.services.PermissionService
    › hasPermission validates correctly
  test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs:147:5
    › Neo.ai.mcp.server.memory-core.services.PermissionService
    › grantPermission preserves existing node type and does not overwrite it (resolves #10231)
5 passed (630ms)

5/7 pass. The two failures are not intermittent — reproducible on every run.

The Architectural Reality

  • test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs:122hasPermission validates correctly — synchronously calls PermissionService.hasPermission(caller, target, scope) against freshly seeded AgentIdentity nodes + CAN_READ_INBOX_OF edge. Some layer between test setup and the assertion is breaking the invariant.
  • test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs:147grantPermission preserves existing node type and does not overwrite it (resolves #10231) — exercises the #10231 refactor that replaced lazy-stubbing with an SQLite existence guard. Test asserts the target node's type survives the grant.
  • ai/mcp/server/memory-core/services/PermissionService.mjs:42, 155-171 — the implementation surface these tests cover
  • test/playwright/unit/ai/mcp/server/memory-core/services/MailboxService.spec.mjs — adjacent spec file in the same directory, 24/24 passing, suggests the spec-infrastructure (tmp DB setup, SystemLifecycleService.initAsync, etc.) is operational; problem is spec-local

The Fix

Investigation required before prescription. Working hypotheses in order of likelihood:

  1. Test isolation regression: spec's beforeEach / beforeAll may not be fully clearing graph state between tests, or clearing too aggressively — adjacent MailboxService.spec.mjs uses a symmetric reset pattern; this spec may have drifted.
  2. #10231 refactor fallout: the grantPermission existence guard may have changed behavior the test still assumes in its pre-refactor form — the test title literally claims to cover #10231, so this is plausible.
  3. WAL / transaction visibility: if hasPermission consults this.db.edges.items (in-memory cache) while the test creates the edge via an async path that hasn't committed to the cache yet, the validation would fail at the moment of assertion. MailboxService.spec.mjs forces autoSave = true; this spec may not.

Concrete fix steps:

  1. Reproduce both failures on a fresh branch
  2. Add diagnostic logging to pin which assertion fires
  3. Compare spec setup to the passing MailboxService.spec.mjs setup
  4. Apply the minimal fix (likely autoSave or symmetric-reset alignment)
  5. Verify both tests pass on clean dev

Acceptance Criteria

  • npm run test-unit -- test/playwright/unit/ai/mcp/server/memory-core/services/PermissionService.spec.mjs returns 7/7 passing on clean dev after the fix
  • Root cause documented in the commit body so future regressions are traceable
  • No fix-by-muting (no test.skip, no loosened assertions) — actual cause must be addressed
  • If the root cause is spec-infrastructure shared with other specs (e.g., GraphService global state), the fix extends to cover the shared surface

Out of Scope

  • Expanding PermissionService.spec.mjs coverage beyond the two failing tests
  • Refactoring PermissionService.mjs itself (unless the tests are failing because the implementation is genuinely broken — in which case this ticket escalates to a bug-fix scope)
  • Parallel investigation of #10231's full test coverage across other spec files

Related

  • #10231 (grantPermission lazy-stub refactor — shipped; the failing test at line 147 explicitly claims to cover this) — potentially relevant if the fix changed behavior the test still assumes
  • #10252 / PR #10253 (config-gated reply policy) — orthogonal; this ticket was discovered during #10253's empirical validation
  • #10229 (:memory: test-pollution + apoptosis exemption) — may have introduced the regression if the spec-isolation pattern was only partially propagated

Origin Session ID

Origin Session ID: 7ad90eb3-2218-4200-977b-fbfe546d64a8

Handoff Retrieval Hints

Retrieval Hint: "PermissionService spec 122 147 hasPermission grantPermission pre-existing failures" Retrieval Hint: "#10231 refactor test regression isolation" Commit-range anchor: HEAD d45bde381 at observation

tobiu referenced in commit 0047472 - "test(core): enable autoSave in PermissionService spec (#10256) (#10264) on Apr 24, 2026, 12:12 AM
tobiu closed this issue on Apr 24, 2026, 12:12 AM