-
-
Notifications
You must be signed in to change notification settings - Fork 193
feat(simulator): migrate build_sim to session-aware defaults; concise description #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ion-aware middleware; add tests; update plan doc - Add in-memory SessionStore (set/get/clear/show) - New workflow: session-management (session-set-defaults, session-clear-defaults, session-show-defaults) - Add createSessionAwareTool with requirements preflight and args-over-defaults merge - Comprehensive tests for tools, store, and factory - Update session_management_plan.md with DI checklist, commit protocol, and pre-commit hook note Existing tools unchanged; no remote push.
…ise description docs(session-management): add Tool Description Policy and update plan example test: update build_sim and session-management tests for session-aware preflight and description
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an in-memory SessionStore and a session-aware tool factory, three session-management tools (set/clear/show) with tests, migrates the simulator build tool to use session-aware validation and a public schema, adds docs/plan, and updates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Handler as Session-aware Handler
participant Store as SessionStore
participant Validator as Zod
participant Logic as LogicFunction
participant Exec as Executor
Caller->>Handler: invoke tool with args
Handler->>Store: getAll()
Handler->>Handler: merge args + session defaults
Handler->>Handler: prune exclusivePairs
Handler->>Handler: evaluate requirements (allOf / oneOf)
alt requirements missing
Handler-->>Caller: Error (missing session defaults + guidance)
else requirements satisfied
Handler->>Validator: validate merged params
alt validation error
Handler-->>Caller: Error (parameter validation + hint to set defaults)
else valid
Handler->>Exec: getExecutor()
Handler->>Logic: call logic(params, executor)
Logic-->>Handler: ToolResponse
Handler-->>Caller: ToolResponse
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| return async (rawArgs: Record<string, unknown>): Promise<ToolResponse> => { | ||
| try { | ||
| const merged: Record<string, unknown> = { ...sessionStore.getAll(), ...rawArgs }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow overriding XOR defaults without clearing session
createSessionAwareTool merges saved defaults and call arguments with const merged = { ...sessionStore.getAll(), ...rawArgs };. When session defaults hold one side of a mutually exclusive pair (e.g. workspacePath) and the caller passes the other (projectPath) to override it, both keys survive in merged, so the internal schema rejects the call as having conflicting values. This forces callers to clear or reset session defaults before every override and violates the documented rule that explicit args take precedence. Consider dropping conflicting defaults when an alternate key is provided or otherwise resolving XOR pairs before validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.gitignore(1 hunks)docs/session_management_plan.md(1 hunks)src/mcp/tools/session-management/__tests__/index.test.ts(1 hunks)src/mcp/tools/session-management/__tests__/session_clear_defaults.test.ts(1 hunks)src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts(1 hunks)src/mcp/tools/session-management/__tests__/session_show_defaults.test.ts(1 hunks)src/mcp/tools/session-management/index.ts(1 hunks)src/mcp/tools/session-management/session_clear_defaults.ts(1 hunks)src/mcp/tools/session-management/session_set_defaults.ts(1 hunks)src/mcp/tools/session-management/session_show_defaults.ts(1 hunks)src/mcp/tools/simulator/__tests__/build_sim.test.ts(3 hunks)src/mcp/tools/simulator/build_sim.ts(2 hunks)src/utils/__tests__/session-aware-tool-factory.test.ts(1 hunks)src/utils/__tests__/session-store.test.ts(1 hunks)src/utils/session-store.ts(1 hunks)src/utils/typed-tool-factory.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Use .ts extensions for all internal relative imports (e.g., import { tool } from './tool.ts')
Use .ts extensions in re-exports for internal files (e.g., export { default } from '../shared/tool.ts')
Use .js extension only for external package entry points (e.g., import ... from '@scope/pkg/file.js')
Never import internal project files using .js extensions
Files:
src/mcp/tools/session-management/index.tssrc/utils/__tests__/session-store.test.tssrc/utils/__tests__/session-aware-tool-factory.test.tssrc/mcp/tools/session-management/__tests__/session_clear_defaults.test.tssrc/mcp/tools/simulator/__tests__/build_sim.test.tssrc/mcp/tools/session-management/session_set_defaults.tssrc/mcp/tools/session-management/session_clear_defaults.tssrc/mcp/tools/session-management/__tests__/session_show_defaults.test.tssrc/mcp/tools/session-management/session_show_defaults.tssrc/utils/typed-tool-factory.tssrc/utils/session-store.tssrc/mcp/tools/session-management/__tests__/index.test.tssrc/mcp/tools/session-management/__tests__/session_set_defaults.test.tssrc/mcp/tools/simulator/build_sim.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.ts: Do not use Vitest mocking APIs in tests (no vi.mock, vi.fn, vi.spyOn, etc.)
In tests, import the core ...Logic function from the tool and use createMockExecutor/createMockFileSystemExecutor to simulate outcomes
Files:
src/utils/__tests__/session-store.test.tssrc/utils/__tests__/session-aware-tool-factory.test.tssrc/mcp/tools/session-management/__tests__/session_clear_defaults.test.tssrc/mcp/tools/simulator/__tests__/build_sim.test.tssrc/mcp/tools/session-management/__tests__/session_show_defaults.test.tssrc/mcp/tools/session-management/__tests__/index.test.tssrc/mcp/tools/session-management/__tests__/session_set_defaults.test.ts
🧠 Learnings (1)
📚 Learning: 2025-09-21T19:27:22.795Z
Learnt from: CR
PR: cameroncooke/XcodeBuildMCP#0
File: AGENTS.md:0-0
Timestamp: 2025-09-21T19:27:22.795Z
Learning: Applies to **/*.{test,spec}.ts : In tests, import the core ...Logic function from the tool and use createMockExecutor/createMockFileSystemExecutor to simulate outcomes
Applied to files:
src/utils/__tests__/session-aware-tool-factory.test.tssrc/mcp/tools/simulator/__tests__/build_sim.test.ts
🧬 Code graph analysis (12)
src/utils/__tests__/session-store.test.ts (1)
src/utils/session-store.ts (1)
sessionStore(42-42)
src/utils/__tests__/session-aware-tool-factory.test.ts (3)
src/utils/session-store.ts (1)
sessionStore(42-42)src/utils/typed-tool-factory.ts (1)
createSessionAwareTool(74-128)src/test-utils/mock-executors.ts (1)
createMockExecutor(27-75)
src/mcp/tools/session-management/__tests__/session_clear_defaults.test.ts (2)
src/utils/session-store.ts (1)
sessionStore(42-42)src/mcp/tools/session-management/session_clear_defaults.ts (1)
sessionClearDefaultsLogic(26-30)
src/mcp/tools/simulator/__tests__/build_sim.test.ts (1)
src/utils/session-store.ts (1)
sessionStore(42-42)
src/mcp/tools/session-management/session_set_defaults.ts (2)
src/utils/session-store.ts (2)
sessionStore(42-42)SessionDefaults(3-13)src/utils/typed-tool-factory.ts (1)
createTypedTool(30-61)
src/mcp/tools/session-management/session_clear_defaults.ts (2)
src/utils/session-store.ts (1)
sessionStore(42-42)src/utils/typed-tool-factory.ts (1)
createTypedTool(30-61)
src/mcp/tools/session-management/__tests__/session_show_defaults.test.ts (1)
src/utils/session-store.ts (1)
sessionStore(42-42)
src/mcp/tools/session-management/session_show_defaults.ts (1)
src/utils/session-store.ts (1)
sessionStore(42-42)
src/utils/typed-tool-factory.ts (2)
src/utils/session-store.ts (2)
SessionDefaults(3-13)sessionStore(42-42)src/utils/errors.ts (1)
createErrorResponse(111-122)
src/mcp/tools/session-management/__tests__/index.test.ts (1)
src/mcp/tools/session-management/index.ts (1)
workflow(1-8)
src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts (2)
src/utils/session-store.ts (1)
sessionStore(42-42)src/mcp/tools/session-management/session_set_defaults.ts (1)
sessionSetDefaultsLogic(21-28)
src/mcp/tools/simulator/build_sim.ts (2)
src/utils/typed-tool-factory.ts (1)
createSessionAwareTool(74-128)src/utils/execution/index.ts (1)
getDefaultCommandExecutor(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: code-review
🔇 Additional comments (8)
.gitignore (1)
111-111: LGTM!The
.factoryignore rule is appropriate for preventing unintended tracking of generated or auxiliary artifacts.src/mcp/tools/session-management/__tests__/index.test.ts (1)
1-58: LGTM!The test suite comprehensively validates the workflow metadata structure and values. Import paths correctly use
.tsextensions per coding guidelines.src/utils/__tests__/session-store.test.ts (1)
1-38: LGTM!The test suite effectively covers SessionStore functionality including set/get, merging, selective clearing, and full clearing. Import paths correctly use
.tsextensions per coding guidelines.src/mcp/tools/session-management/session_show_defaults.ts (1)
1-12: LGTM!The implementation is straightforward and correct. Import paths properly use
.tsextensions per coding guidelines.src/mcp/tools/session-management/__tests__/session_show_defaults.test.ts (1)
1-45: LGTM!The test suite thoroughly validates the tool's export structure and handler behavior for both empty and populated defaults. Import paths correctly use
.tsextensions per coding guidelines.src/mcp/tools/session-management/__tests__/session_clear_defaults.test.ts (1)
1-75: LGTM!The test suite comprehensively covers the tool's functionality including selective clearing, full clearing, and validation behavior. Tests correctly import the
...Logicfunction per coding guidelines, and import paths use.tsextensions.src/utils/__tests__/session-aware-tool-factory.test.ts (1)
1-112: LGTM!The test suite comprehensively validates the session-aware tool factory including default merging, argument precedence, and requirement validation. Tests correctly use
createMockExecutorper coding guidelines and import paths use.tsextensions. Based on learnings.src/mcp/tools/session-management/index.ts (1)
1-8: LGTM!The workflow metadata object is well-structured and serves as a clear public descriptor for the session-management toolset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found 2 potential issues that should be addressed.
| }); | ||
|
|
||
| it('should return current defaults when set', async () => { | ||
| sessionStore.setDefaults({ scheme: 'MyScheme', simulatorId: 'SIM-123' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting defaults in this test without clearing them leaves the singleton sessionStore populated after the suite ends. Because sessionStore is shared across all test files, running this spec before others will leak scheme/simulatorId into later tests that assume a clean store. Please add something like afterEach(() => sessionStore.clear()); (or clear at the end of this test) so the store is reset after every spec.
| describe('session-clear-defaults tool', () => { | ||
| beforeEach(() => { | ||
| sessionStore.clear(); | ||
| sessionStore.setDefaults({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beforeEach seeds the singleton sessionStore, but when validation fails (e.g. the "invalid" keys test) the store never gets cleared and stays populated after the suite finishes. That leaked state can break later tests that expect an empty store. Please add an afterEach(() => sessionStore.clear()); (or clear inside the failing test) so each spec leaves the store in a clean state.
…le; improve oneOf error hints; add exclusivePairs pruning; clear([]) no-op; typo fix in description
…ld) and mandate quick manual CLI test before review
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
…ization; hyphenated CLI samples are valid
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/session_management_plan.md(1 hunks)src/mcp/tools/session-management/__tests__/session_clear_defaults.test.ts(1 hunks)src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts(1 hunks)src/mcp/tools/session-management/__tests__/session_show_defaults.test.ts(1 hunks)src/mcp/tools/session-management/session_set_defaults.ts(1 hunks)src/mcp/tools/simulator/build_sim.ts(2 hunks)src/utils/__tests__/session-store.test.ts(1 hunks)src/utils/session-store.ts(1 hunks)src/utils/typed-tool-factory.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mcp/tools/session-management/tests/session_clear_defaults.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.ts: Do not use Vitest mocking APIs in tests (no vi.mock, vi.fn, vi.spyOn, etc.)
In tests, import the core ...Logic function from the tool and use createMockExecutor/createMockFileSystemExecutor to simulate outcomes
Files:
src/mcp/tools/session-management/__tests__/session_set_defaults.test.tssrc/utils/__tests__/session-store.test.tssrc/mcp/tools/session-management/__tests__/session_show_defaults.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Use .ts extensions for all internal relative imports (e.g., import { tool } from './tool.ts')
Use .ts extensions in re-exports for internal files (e.g., export { default } from '../shared/tool.ts')
Use .js extension only for external package entry points (e.g., import ... from '@scope/pkg/file.js')
Never import internal project files using .js extensions
Files:
src/mcp/tools/session-management/__tests__/session_set_defaults.test.tssrc/mcp/tools/simulator/build_sim.tssrc/mcp/tools/session-management/session_set_defaults.tssrc/utils/__tests__/session-store.test.tssrc/utils/typed-tool-factory.tssrc/mcp/tools/session-management/__tests__/session_show_defaults.test.tssrc/utils/session-store.ts
🧬 Code graph analysis (6)
src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts (2)
src/utils/session-store.ts (1)
sessionStore(47-47)src/mcp/tools/session-management/session_set_defaults.ts (1)
sessionSetDefaultsLogic(21-28)
src/mcp/tools/simulator/build_sim.ts (1)
src/utils/typed-tool-factory.ts (1)
createSessionAwareTool(74-152)
src/mcp/tools/session-management/session_set_defaults.ts (2)
src/utils/session-store.ts (2)
sessionStore(47-47)SessionDefaults(3-13)src/utils/typed-tool-factory.ts (1)
createTypedTool(30-61)
src/utils/__tests__/session-store.test.ts (1)
src/utils/session-store.ts (1)
sessionStore(47-47)
src/utils/typed-tool-factory.ts (1)
src/utils/session-store.ts (2)
SessionDefaults(3-13)sessionStore(47-47)
src/mcp/tools/session-management/__tests__/session_show_defaults.test.ts (1)
src/utils/session-store.ts (1)
sessionStore(47-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: code-review
…une merged payload to align with exclusivePairs pruning
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found 1 potential issue that should be addressed.
| type Params = z.infer<typeof schemaObj>; | ||
|
|
||
| export async function sessionSetDefaultsLogic(params: Params): Promise<ToolResponse> { | ||
| sessionStore.setDefaults(params as Partial<SessionDefaults>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
session-set-defaults currently just merges the supplied values into the existing session defaults. If a user previously set workspacePath and later switches to projectPath (or similarly simulatorName to simulatorId), both values stay in the store, so the next call to build_sim hits the internal schema’s XOR constraint and fails with a validation error. That makes the new session defaults workflow break unless the user manually clears the old key first. Please clear the mutually exclusive counterpart before merging—for example, when projectPath is provided remove any stored workspacePath, and likewise for the other exclusive pairs—so that updating defaults doesn’t leave the store in an invalid state.
…ly provides null/undefined; add tests for null/undefined activation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
…ve counterparts before merge to prevent invalid store state; add tests
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found 1 potential issue that should be addressed.
| sessionStore.clear(Array.from(toClear)); | ||
| } | ||
|
|
||
| sessionStore.setDefaults(params as Partial<SessionDefaults>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sessionSetDefaultsLogic clears previously stored counterparts, but if a caller provides both projectPath and workspacePath (or both simulator fields) in the same request, we still write both keys into the session store because params is passed through unchanged. That leaves the defaults in an impossible state, and the next tool invocation will fail the XOR refine in build_sim ("projectPath and workspacePath are mutually exclusive"). Please either reject the request (e.g. add a Zod refine) or drop the conflicting value before calling setDefaults so the store can never contain mutually exclusive entries.
…n-set-defaults via schema refine; fix exclusivePairs pruning to ignore null/undefined and not override session defaults; update tests
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/mcp/tools/session-management/session_set_defaults.ts (1)
34-37: Consider checking for concrete values before clearing counterparts.The
hasOwnPropertychecks returntrueeven when a value is explicitlyundefined. If a user passes{ projectPath: undefined }, line 34 clearsworkspacePathfrom the session even though no concrete value was provided. This might surprise users who expectundefinedto mean "don't change this key."Consider checking that the value is not
nullorundefinedbefore clearing the counterpart:- if (Object.prototype.hasOwnProperty.call(params, 'projectPath')) toClear.add('workspacePath'); - if (Object.prototype.hasOwnProperty.call(params, 'workspacePath')) toClear.add('projectPath'); - if (Object.prototype.hasOwnProperty.call(params, 'simulatorId')) toClear.add('simulatorName'); - if (Object.prototype.hasOwnProperty.call(params, 'simulatorName')) toClear.add('simulatorId'); + if (Object.prototype.hasOwnProperty.call(params, 'projectPath') && params.projectPath != null) toClear.add('workspacePath'); + if (Object.prototype.hasOwnProperty.call(params, 'workspacePath') && params.workspacePath != null) toClear.add('projectPath'); + if (Object.prototype.hasOwnProperty.call(params, 'simulatorId') && params.simulatorId != null) toClear.add('simulatorName'); + if (Object.prototype.hasOwnProperty.call(params, 'simulatorName') && params.simulatorName != null) toClear.add('simulatorId');This aligns with the sanitization logic in
createSessionAwareTool(lines 93-96 of typed-tool-factory.ts), which treatsnull/undefinedas "not provided."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/mcp/tools/session-management/__tests__/session_set_defaults.test.ts(1 hunks)src/mcp/tools/session-management/session_set_defaults.ts(1 hunks)src/utils/__tests__/session-aware-tool-factory.test.ts(1 hunks)src/utils/typed-tool-factory.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/mcp/tools/session-management/tests/session_set_defaults.test.ts
- src/utils/tests/session-aware-tool-factory.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Use .ts extensions for all internal relative imports (e.g., import { tool } from './tool.ts')
Use .ts extensions in re-exports for internal files (e.g., export { default } from '../shared/tool.ts')
Use .js extension only for external package entry points (e.g., import ... from '@scope/pkg/file.js')
Never import internal project files using .js extensions
Files:
src/mcp/tools/session-management/session_set_defaults.tssrc/utils/typed-tool-factory.ts
🧬 Code graph analysis (2)
src/mcp/tools/session-management/session_set_defaults.ts (2)
src/utils/session-store.ts (2)
SessionDefaults(3-13)sessionStore(47-47)src/utils/typed-tool-factory.ts (1)
createTypedTool(30-61)
src/utils/typed-tool-factory.ts (2)
src/utils/session-store.ts (2)
SessionDefaults(3-13)sessionStore(47-47)src/utils/errors.ts (1)
createErrorResponse(111-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: code-review
🔇 Additional comments (2)
src/mcp/tools/session-management/session_set_defaults.ts (1)
51-57: LGTM! Tool definition follows session-aware design.The tool correctly:
- Exposes
baseSchema.shapeas the public schema (without refinements) for user-facing API.- Uses
schemaObj(with mutual exclusivity refinements) for internal validation in the handler.- Provides a clear description appropriate for a session-management tool.
This design prevents users from providing mutually exclusive keys in a single request while keeping the public API simple.
src/utils/typed-tool-factory.ts (1)
74-160: LGTM! Session-aware tool creation is well-designed.The implementation correctly handles:
Sanitization (lines 93-96): Filters
null/undefinedfrom args so they don't override session defaults. This ensures that explicit arguments take precedence while preserving session values when no concrete value is provided.Exclusive pair pruning (lines 101-114): Only activates when the caller provides a concrete value for any key in a pair, then drops conflicting session defaults. This allows users to override XOR pairs without manually clearing the session first.
Requirement validation (lines 116-141): Checks
allOf/oneOfrequirements against the post-pruningmergedobject (not the originalsessionStore), ensuring consistency between requirement enforcement and downstream logic.Error messages: Provide actionable guidance, including hints to use
session-set-defaultswhen validation fails.This design addresses all concerns from past review comments and enables a clean session-aware workflow.
…t factory level; add test; update build_sim tests to expect factory message
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
This PR migrates the simulator build tool to the new session-aware defaults layer and enforces concise tool descriptions.
Changes:
Validation:
Note
Introduce session-aware defaults (store + factory), add session-management tools, and migrate
build_simto minimal public schema with preflight checks and concise description.SessionStore(src/utils/session-store.ts) andcreateSessionAwareToolwith preflight requirements and exclusivePairs (src/utils/typed-tool-factory.ts).session-set-defaults,session-clear-defaults,session-show-defaultswith schemas/handlers and workflow metadata.simulator/build_simtocreateSessionAwareTool; expose minimal publicschema; add requirements; set concise description.build_simtests for new public schema and preflight messaging.docs/session_management_plan.mddetailing design, middleware, migration plan..gitignoreto exclude.factory.Written by Cursor Bugbot for commit de72b23. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation
Chores