feat: continue slow writes with standard MCP request state - #386
Closed
kingpanther13 wants to merge 17 commits into
Closed
feat: continue slow writes with standard MCP request state#386kingpanther13 wants to merge 17 commits into
kingpanther13 wants to merge 17 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughThis change replaces public ChangesMRTR slow-write migration
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟠 High · up to This PR adds cross-request continuation and asynchronous deployment handling, but the current implementation can still misattribute package updates and allow an older deployment to overwrite a newer one after a marker expires. Merge should be blocked until those concurrency and correlation risks are fixed; the remaining documentation and schema follow-ups are secondary. Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPRouter
participant RequestState
participant WriteWorker
participant HubState
Client->>MCPRouter: tools/call for eligible slow write
MCPRouter->>RequestState: Validate and reserve request state
RequestState-->>MCPRouter: requestState
MCPRouter-->>Client: input_required with requestState
Client->>MCPRouter: Continue with requestState
MCPRouter->>WriteWorker: Execute bounded write slice
WriteWorker->>HubState: Apply mutation
HubState-->>WriteWorker: Slice result
WriteWorker->>RequestState: Store checkpoint or terminal result
MCPRouter-->>Client: Continuation or aggregated terminal result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
kingpanther13
changed the title
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libraries/mcp-item-backups-lib.groovy (1)
852-857: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAllow the documented schedule-only request.
toolCreateHubBackupacceptsscheduleOnly=truewith a schedule and withoutconfirmat Lines 407-426.required: ["confirm"]at Line 853 rejects that valid shape at the public schema boundary. Makeconfirmconditional on immediate backup creation, or remove the unconditional requirement and keep the existing runtime validation.Based on learnings,
requiredParamsByTool()is derived from each tool definition’sinputSchema.required; this declaration is enforced beforetoolCreateHubBackuphandlesscheduleOnly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/mcp-item-backups-lib.groovy` around lines 852 - 857, Update the input schema for toolCreateHubBackup by removing the unconditional required entry for confirm, so scheduleOnly requests with a schedule can reach the existing runtime validation; retain confirm validation for immediate backup creation within toolCreateHubBackup.Source: Learnings
🧹 Nitpick comments (2)
tests/sdk_conformance_helpers.py (1)
139-151: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCatch
httpx2.RequestNotReadinstead ofException. Inhttpx2==2.10.0,Request.contentraisesRequestNotReadfor an unread streamed body. Handle this specific exception so the observer degrades tohas_request_state=Falsewithout hiding unrelated defects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sdk_conformance_helpers.py` around lines 139 - 151, Update the exception handling in _fields to catch httpx2.RequestNotRead when accessing request.content, preserving has_request_state=False for unread streamed bodies while allowing unrelated exceptions to propagate; retain the existing JSON parsing safeguards.libraries/mcp-app-cloner-lib.groovy (1)
855-855: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a budget-expiry continuation test for staged clone/import. Force a staging remainder, resume with the same
requestState, and assert that clone/import commits once and creates one app.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/mcp-app-cloner-lib.groovy` at line 855, Add a continuation test for staged clone/import near the stageDisabled configuration, forcing the initial operation to leave a staging remainder, then resuming with the same requestState. Assert the resumed flow commits exactly once and results in exactly one created app.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-08-12-mrtr-slow-writes.md`:
- Around line 99-101: Clarify the checklist in the MRTR plan to remove PR `#378`’s
public opToken polling/replay and related journal/documentation/tests, while
retaining MRTR terminal-result replay through the existing requestState for
brief lost-response recovery. Replace the ambiguous “terminal replay” wording so
it does not imply removing this retained behavior.
In `@docs/testing.md`:
- Around line 429-431: Update the SDK conformance documentation to reconcile the
no-skip policy with the write scenario’s skip_install behavior: state that
neither leg may soft-skip when eligible, while explicitly documenting that
skip_install intentionally excludes live write coverage. Ensure the wording
prevents a green skip from being interpreted as SDK/MRTR proof.
In `@libraries/mcp-code-management-lib.groovy`:
- Around line 1511-1514: Remove the caller-controlled __packageRequestId
handling from toolUpdateAppCode and pass correlation data only through
worker-only context. Validate the active package marker, then derive both
stash.requestId and stash.packageRef from that validated marker rather than
public arguments; add success and failure coverage for a public spoof while a
package deploy is active.
In `@libraries/mcp-item-backups-lib.groovy`:
- Line 835: Move the transport-drop warning and its
hub_get_tool_guide(section='slow_ops') reference for hub_create_backup and
hub_restore_backup outside the [[FLAT_TRIM]] markers. Keep the instruction to
verify current hub state before retrying visible in flat mode, while leaving
only advanced optional detail inside the trim sections.
In `@libraries/mcp-self-admin-lib.groovy`:
- Around line 492-509: Update the package deployment reservation flow around
packageDeployInFlight and _packageDeployMarkerTtlMs so an aged startedAt alone
never admits a second deployment while the original worker may still run. Retain
the marker until the worker records a terminal state; if stale recovery is
necessary, implement a worker-renewed lease and verify ownership before each
_updatePackageBody or other deployment side effect, preserving terminal-record
matching through lastSelfDeploy.
In `@src/test/groovy/server/MrtrContinuationSpec.groovy`:
- Around line 811-826: Expose the MRTR helpers in hubitat-mcp-server.groovy so
both test sites, src/test/groovy/server/MrtrContinuationSpec.groovy:811-826 and
src/test/groovy/server/McpWireSchemaConformanceSpec.groovy:212-243, can call
_mrtrBinding, _mrtrSha256, and _mrtrAbandon directly, or provide equivalent
test-accessible APIs. Preserve _mrtrContentionWaitMs with both zero- and
one-argument forms, and retain _mrtrAbandon(stateId, record, claim, reason)
unchanged in behavior.
In `@TOOL_GUIDE.md`:
- Around line 1164-1168: Update the modern-path description around the
resumed-request behavior to say each continuation advances or coordinates one
bounded slice, rather than claiming it always runs the slice. Preserve the
existing preflight, requestState, deadline, and terminal-result semantics for
hub_set_rule, hub_set_native_app, and the other listed tools.
---
Outside diff comments:
In `@libraries/mcp-item-backups-lib.groovy`:
- Around line 852-857: Update the input schema for toolCreateHubBackup by
removing the unconditional required entry for confirm, so scheduleOnly requests
with a schedule can reach the existing runtime validation; retain confirm
validation for immediate backup creation within toolCreateHubBackup.
---
Nitpick comments:
In `@libraries/mcp-app-cloner-lib.groovy`:
- Line 855: Add a continuation test for staged clone/import near the
stageDisabled configuration, forcing the initial operation to leave a staging
remainder, then resuming with the same requestState. Assert the resumed flow
commits exactly once and results in exactly one created app.
In `@tests/sdk_conformance_helpers.py`:
- Around line 139-151: Update the exception handling in _fields to catch
httpx2.RequestNotRead when accessing request.content, preserving
has_request_state=False for unread streamed bodies while allowing unrelated
exceptions to propagate; retain the existing JSON parsing safeguards.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d9d37d0-7b03-4f14-b27e-cf66d323b390
📒 Files selected for processing (61)
.github/scripts/e2e_scope.py.github/scripts/mcp_probe_hub.sh.github/workflows/hub-e2e.ymlAGENTS.mdCLAUDE.mdTOOL_GUIDE.mdci/groovy2x-spock/scaffold/support/HarnessSpec.groovydocs/superpowers/plans/2026-08-12-mrtr-slow-writes.mddocs/superpowers/specs/2026-08-12-mrtr-slow-writes-design.mddocs/testing.mdhubitat-mcp-server.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-dashboards-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-deploy-jobs-lib.groovylibraries/mcp-devices-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-hpm-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-native-rules-lib.groovylibraries/mcp-rooms-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-virtual-devices-lib.groovylibraries/mcp-visual-rules-lib.groovysrc/test/groovy/server/HubInfoFieldContractSpec.groovysrc/test/groovy/server/HubInternalRetrySpec.groovysrc/test/groovy/server/IncludeResolverSpec.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovysrc/test/groovy/server/MrtrContinuationSpec.groovysrc/test/groovy/server/OpTokenReplaySpec.groovysrc/test/groovy/server/RelayBudgetSpec.groovysrc/test/groovy/server/SetRuleSelfGatewaySpec.groovysrc/test/groovy/server/ToolAppDriverCodeSpec.groovysrc/test/groovy/server/ToolDeploymentJobsSpec.groovysrc/test/groovy/server/ToolLibraryCodeSpec.groovysrc/test/groovy/server/ToolManageFilesSpec.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovysrc/test/groovy/server/ToolUpdateMcpSettingsSpec.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovysrc/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovysrc/test/groovy/support/HarnessSpec.groovysrc/test/groovy/support/McpRequestDriver.groovysrc/test/groovy/support/McpRequestDriverSpec.groovysrc/test/groovy/support/ToolSpecBase.groovytests/BAT-rm-native-crud.mdtests/BAT-v2.mdtests/e2e_test.pytests/sandbox_lint.pytests/sdk-conformance-requirements.txttests/sdk_conformance_helpers.pytests/sdk_conformance_test.pytests/test_e2e_test_helpers.pytests/test_sdk_conformance_helpers.pytools/build-bundle.py
💤 Files with no reviewable changes (10)
- tools/build-bundle.py
- src/test/groovy/support/ToolSpecBase.groovy
- src/test/groovy/server/IncludeResolverSpec.groovy
- src/test/groovy/server/HubInfoFieldContractSpec.groovy
- tests/BAT-rm-native-crud.md
- src/test/groovy/server/OpTokenReplaySpec.groovy
- src/test/groovy/server/ToolDeploymentJobsSpec.groovy
- libraries/mcp-dashboards-lib.groovy
- src/test/groovy/server/ToolUpdateMcpSettingsSpec.groovy
- libraries/mcp-deploy-jobs-lib.groovy
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
src/test/groovy/**/*.groovy
📄 CodeRabbit inference engine (AGENTS.md)
Every new MCP tool ships with BOTH a direct-call (unit) test AND a dispatch-envelope (integration) test under
src/test/groovy/(existing CONTRIBUTING.md rule; reaffirmed).
Files:
src/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovysrc/test/groovy/support/McpRequestDriverSpec.groovysrc/test/groovy/server/HubInternalRetrySpec.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovysrc/test/groovy/support/McpRequestDriver.groovysrc/test/groovy/server/SetRuleSelfGatewaySpec.groovysrc/test/groovy/server/ToolAppDriverCodeSpec.groovysrc/test/groovy/server/ToolLibraryCodeSpec.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovysrc/test/groovy/server/ToolManageFilesSpec.groovysrc/test/groovy/support/HarnessSpec.groovysrc/test/groovy/server/RelayBudgetSpec.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovysrc/test/groovy/server/MrtrContinuationSpec.groovy
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**
Files:
src/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovytests/sandbox_lint.pysrc/test/groovy/support/McpRequestDriverSpec.groovysrc/test/groovy/server/HubInternalRetrySpec.groovylibraries/mcp-hpm-lib.groovylibraries/mcp-virtual-devices-lib.groovyAGENTS.mdsrc/test/groovy/server/ToolRmNativeCrudSpec.groovyCLAUDE.mdsrc/test/groovy/support/McpRequestDriver.groovysrc/test/groovy/server/SetRuleSelfGatewaySpec.groovytests/sdk_conformance_helpers.pysrc/test/groovy/server/ToolAppDriverCodeSpec.groovysrc/test/groovy/server/ToolLibraryCodeSpec.groovylibraries/mcp-discovery-lib.groovytests/test_sdk_conformance_helpers.pytests/sdk-conformance-requirements.txtlibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovydocs/testing.mdlibraries/mcp-app-cloner-lib.groovysrc/test/groovy/server/ToolManageFilesSpec.groovyci/groovy2x-spock/scaffold/support/HarnessSpec.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-rooms-lib.groovysrc/test/groovy/support/HarnessSpec.groovylibraries/mcp-files-lib.groovytests/BAT-v2.mdTOOL_GUIDE.mdlibraries/mcp-visual-rules-lib.groovylibraries/mcp-devices-lib.groovydocs/superpowers/plans/2026-08-12-mrtr-slow-writes.mdlibraries/mcp-bundles-lib.groovysrc/test/groovy/server/RelayBudgetSpec.groovylibraries/mcp-code-management-lib.groovydocs/superpowers/specs/2026-08-12-mrtr-slow-writes-design.mdlibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovysrc/test/groovy/server/MrtrContinuationSpec.groovytests/test_e2e_test_helpers.pytests/sdk_conformance_test.py
**/*.{groovy,py}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{groovy,py}: Every new MCP tool MUST set all four annotation hints explicitly:
Every leaf tool AND every gateway MUST have a display-meta entry
Validation errors (caller-recoverable, bad args): throwIllegalArgumentException. Caught byhandleToolsCalland mapped to JSON-RPC-32602. Existing pattern; reaffirmed. A validation throw MUST fire before any side effect — callers may safely correct and retry-32602, so a tool that mutates state and then throwsIllegalArgumentExceptionwould expose that work to a double-run.
Tools that can return long lists MUST support the project's universal cursor convention (cursor/nextCursor), unless the response naturally fits within the 120KBtools/callcap.
Files:
src/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovytests/sandbox_lint.pysrc/test/groovy/support/McpRequestDriverSpec.groovysrc/test/groovy/server/HubInternalRetrySpec.groovylibraries/mcp-hpm-lib.groovylibraries/mcp-virtual-devices-lib.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovysrc/test/groovy/support/McpRequestDriver.groovysrc/test/groovy/server/SetRuleSelfGatewaySpec.groovytests/sdk_conformance_helpers.pysrc/test/groovy/server/ToolAppDriverCodeSpec.groovysrc/test/groovy/server/ToolLibraryCodeSpec.groovylibraries/mcp-discovery-lib.groovytests/test_sdk_conformance_helpers.pylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovylibraries/mcp-app-cloner-lib.groovysrc/test/groovy/server/ToolManageFilesSpec.groovyci/groovy2x-spock/scaffold/support/HarnessSpec.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-rooms-lib.groovysrc/test/groovy/support/HarnessSpec.groovylibraries/mcp-files-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-devices-lib.groovylibraries/mcp-bundles-lib.groovysrc/test/groovy/server/RelayBudgetSpec.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovysrc/test/groovy/server/MrtrContinuationSpec.groovytests/test_e2e_test_helpers.pytests/sdk_conformance_test.py
libraries/*.groovy
📄 CodeRabbit inference engine (AGENTS.md)
libraries/*.groovy: Thelibrary(...)declaration MUST be the first line of the file. Zero file-scope commentary before it.
Use string-literal handler names forsubscribe/schedule(never bare identifiers).
Do NOT movepreferences {},mappings {}, or any file-scope closure into a library
Do NOT cross-#includeone library from another.
libraries/*.groovy: - Thelibrary(...)declaration MUST be the first line of the file. Zero file-scope commentary before it.
- Use string-literal handler names for
subscribe/schedule(never bare identifiers).- Do NOT move
preferences {},mappings {}, or any file-scope closure into a library (root-level DSL / unverified closure binding under#include).
Files:
libraries/mcp-hpm-lib.groovylibraries/mcp-virtual-devices-lib.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-rooms-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-devices-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovy
{hubitat-mcp-server.groovy,hubitat-mcp-rule.groovy,e2e-deadman-watchdog-v2.groovy,libraries/**/*.groovy}
📄 CodeRabbit inference engine (CLAUDE.md)
Eval.*,GroovyShell,Class.forName,Runtime.exec,new Thread,new File/java.io.File— not allowed
Files:
libraries/mcp-hpm-lib.groovylibraries/mcp-virtual-devices-lib.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-rooms-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-devices-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovy
🧠 Learnings (41)
📓 Common learnings
Learnt from: kingpanther13
Repo: kingpanther13/Hubitat-local-MCP-server PR: 378
File: libraries/mcp-deploy-jobs-lib.groovy:555-571
Timestamp: 2026-08-09T12:53:41.770Z
Learning: In `libraries/mcp-deploy-jobs-lib.groovy`, `atomicState.updateMapValue` is used for per-job deployment checkpoints, but its null-value deletion semantics are undocumented. `_deployOpDelete` therefore rebuilds `atomicState.deployJobs` only for terminal deployment jobs, consistent with the existing terminal-job pruning pattern. Lease guards must prevent cancellation or deletion while an active deployment worker holds `sliceLeaseUntil`, because the worker saves a whole-job snapshot.
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Preserve the adopted next-revision MCP protocol behavior, including modern-versus-legacy header-value handling, validation/error mappings, era-gated `resultType`, unconditional server metadata, cache hints, and opt-in Origin enforcement.
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to tests/e2e_test.py : Changes observable to live MCP clients, including tool behavior, dispatch, gateways, transport, and bug fixes, require an updated e2e scenario; keep renamed tools and scripts synchronized.
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Rename non-conforming tools in lockstep and do not provide deprecation aliases.
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to **/*.groovy : Use `[[FLAT_TRIM]]` only for advanced detail that remains available through `hub_get_tool_guide`; keep basic purpose, required parameters, critical formats, and safety warnings visible in flat mode.
Applied to files:
src/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovylibraries/mcp-hpm-lib.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovysrc/test/groovy/server/ToolManageFilesSpec.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-native-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to **/*.groovy : Use an object root for `inputSchema`; use enums for fixed values; include `required` only when parameters are required; declare an `outputSchema` for every new tool and model nullable fields correctly.
Applied to files:
src/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovylibraries/mcp-variables-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-visual-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Use inputSchema objects, enums for fixed values, omit required when no parameters are mandatory, and declare an outputSchema on every new tool definition with nullable fields modeled accurately.
Applied to files:
src/test/groovy/server/UpdateNativeAppSchemaTrimSpec.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovylibraries/mcp-variables-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-visual-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Validation failures must throw IllegalArgumentException before any side effect; runtime operation failures must return a structured [success:false, error, note] result and use isError:true in the MCP envelope.
Applied to files:
src/test/groovy/server/HubInternalRetrySpec.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to **/*.groovy : Validation failures must throw `IllegalArgumentException` before side effects; runtime operation failures must return `[success:false, error:<message>, note:<guidance>]`; tool execution errors must use `isError: true`.
Applied to files:
src/test/groovy/server/HubInternalRetrySpec.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Rename non-conforming tools in lockstep and do not provide deprecation aliases.
Applied to files:
libraries/mcp-hpm-lib.groovy.github/scripts/e2e_scope.pylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovylibraries/mcp-app-cloner-lib.groovysrc/test/groovy/server/ToolManageFilesSpec.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Do not ship deprecation aliases when renaming non-conforming MCP tools; rename clients and server references in lockstep.
Applied to files:
libraries/mcp-hpm-lib.groovy.github/scripts/e2e_scope.pysrc/test/groovy/server/ToolLibraryCodeSpec.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Tool descriptions must begin with a concise summary; write tools must include safety warnings and pre-flight requirements; descriptions must make implicit context explicit without redundant verbosity.
Applied to files:
libraries/mcp-hpm-lib.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-bundles-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to libraries/*.groovy : Library files must begin with the `library(...)` declaration, keep comments inside methods, use string-literal subscribe/schedule handlers, and avoid preferences, mappings, or file-scope closures.
Applied to files:
libraries/mcp-hpm-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Every new MCP tool must explicitly provide read-only, destructive, idempotent, and open-world annotation classifications through the central annotation machinery.
Applied to files:
libraries/mcp-hpm-lib.groovy.github/scripts/e2e_scope.pylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to hubitat-mcp-server.groovy : Keep gateway configuration, executeTool dispatch cases, gateway display metadata, and annotation/permission aggregators in the main app; keep per-tool implementation and metadata in libraries.
Applied to files:
libraries/mcp-hpm-lib.groovysrc/test/groovy/server/ToolAppDriverCodeSpec.groovy.github/scripts/e2e_scope.pylibraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-10T17:02:34.806Z
Learnt from: kingpanther13
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: :0-0
Timestamp: 2026-08-10T17:02:34.806Z
Learning: In `hubitat-mcp-server.groovy`, token-only operation-result recovery is handled by `_isOpTokenPollShape` in `handleToolsCall` before gateway required-parameter enforcement. `requiredParamsByTool()` is derived from each tool definition’s `inputSchema.required`; preserve these required declarations so the catalog accurately advertises required tool arguments and poll-shape detection remains aligned with runtime validation.
Applied to files:
libraries/mcp-virtual-devices-lib.groovysrc/test/groovy/server/ToolRmNativeCrudSpec.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-files-lib.groovytests/BAT-v2.mdTOOL_GUIDE.mdlibraries/mcp-visual-rules-lib.groovylibraries/mcp-devices-lib.groovylibraries/mcp-bundles-lib.groovysrc/test/groovy/server/RelayBudgetSpec.groovylibraries/mcp-code-management-lib.groovydocs/superpowers/specs/2026-08-12-mrtr-slow-writes-design.mdlibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to tests/e2e_test.py : Keep Rule Machine e2e scenarios small and grouped by concern; never soft-skip wire-format assertions on relay 504s.
Applied to files:
.github/workflows/hub-e2e.ymltests/sdk_conformance_helpers.pytests/test_sdk_conformance_helpers.pydocs/testing.mdtests/BAT-v2.mdtests/test_e2e_test_helpers.pytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to tests/e2e_test.py : Add or update e2e coverage for observable tool, dispatch, gateway, transport/protocol, and bug-fix behavior; keep tool names and scenarios synchronized with the server.
Applied to files:
tests/sdk_conformance_helpers.pysrc/test/groovy/server/ToolAppDriverCodeSpec.groovy.github/scripts/e2e_scope.pysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovytests/test_e2e_test_helpers.pytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to tests/e2e_test.py : Changes observable to live MCP clients, including tool behavior, dispatch, gateways, transport, and bug fixes, require an updated e2e scenario; keep renamed tools and scripts synchronized.
Applied to files:
tests/sdk_conformance_helpers.py.github/scripts/e2e_scope.pytests/sdk-conformance-requirements.txtsrc/test/groovy/server/McpWireSchemaConformanceSpec.groovydocs/testing.mdtests/BAT-v2.mdsrc/test/groovy/server/ToolUpdatePackageSpec.groovytests/test_e2e_test_helpers.pytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to src/test/groovy/**/*.groovy : Every new MCP tool requires both a direct-call unit test and a dispatch-envelope integration test.
Applied to files:
src/test/groovy/server/ToolAppDriverCodeSpec.groovy.github/scripts/e2e_scope.pysrc/test/groovy/server/ToolLibraryCodeSpec.groovysrc/test/groovy/server/McpWireSchemaConformanceSpec.groovydocs/testing.mdsrc/test/groovy/server/ToolUpdatePackageSpec.groovysrc/test/groovy/server/MrtrContinuationSpec.groovy
📚 Learning: 2026-08-10T16:55:15.285Z
Learnt from: kingpanther13
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: :0-0
Timestamp: 2026-08-10T16:55:15.285Z
Learning: In `src/test/groovy/server/ToolDeploymentJobsSpec.groovy`, Spock `given:` labels use single-quoted Groovy string literals. Do not introduce unescaped apostrophes into these labels. Reword possessives or escape apostrophes to keep `compileTestGroovy` valid.
Applied to files:
src/test/groovy/server/ToolAppDriverCodeSpec.groovysrc/test/groovy/server/ToolLibraryCodeSpec.groovysrc/test/groovy/server/ToolManageFilesSpec.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to libraries/*.groovy : Per-tool definitions, implementations, domain helpers, classifications, and display metadata belong in the domain library; gateway membership and dispatch cases remain in the main app; libraries must not cross-include one another.
Applied to files:
.github/scripts/e2e_scope.pysrc/test/groovy/server/ToolLibraryCodeSpec.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-code-management-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to libraries/*.groovy : Keep each tool's definitions, implementation, domain helpers, classification metadata, and display metadata in its domain library; do not cross-include libraries.
Applied to files:
.github/scripts/e2e_scope.pysrc/test/groovy/server/ToolLibraryCodeSpec.groovylibraries/mcp-discovery-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-files-lib.groovylibraries/mcp-code-management-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to hubitat-mcp-server.groovy : Every MCP tool name must use the hub_ prefix, followed by verb-noun order and a verb from the approved vocabulary.
Applied to files:
.github/scripts/e2e_scope.pylibraries/mcp-debug-logging-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to hubitat-mcp-server.groovy : Use Hubitat Groovy sandbox-compatible APIs: never use Eval, GroovyShell, Class.forName, Runtime.exec, threads, filesystem Java APIs, getClass(), or log.isDebugEnabled(); use Hubitat File Manager APIs for files.
Applied to files:
.github/scripts/e2e_scope.pylibraries/mcp-debug-logging-lib.groovysrc/test/groovy/server/ToolManageFilesSpec.groovyci/groovy2x-spock/scaffold/support/HarnessSpec.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to hubitat-mcp-rule.groovy : Use only APIs supported by the Hubitat Groovy sandbox, including avoiding reflection, process execution, threads, and Java filesystem access.
Applied to files:
.github/scripts/e2e_scope.pyci/groovy2x-spock/scaffold/support/HarnessSpec.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-visual-rules-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-rule.groovy : The custom MCP rule-engine child app is legacy and closed to new feature work; new rule capabilities belong in the parent app's native Rule Machine tools.
Applied to files:
.github/scripts/e2e_scope.pylibraries/mcp-debug-logging-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovytests/BAT-v2.mdTOOL_GUIDE.mdlibraries/mcp-visual-rules-lib.groovydocs/superpowers/plans/2026-08-12-mrtr-slow-writes.mdlibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Read-only tools must be reachable through a `hub_read_*` gateway or remain flat; read tools must not be unique to `hub_manage_*` gateways.
Applied to files:
libraries/mcp-discovery-lib.groovylibraries/mcp-variables-lib.groovylibraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovyci/groovy2x-spock/scaffold/support/HarnessSpec.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovyTOOL_GUIDE.mdlibraries/mcp-visual-rules-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-09T12:53:41.770Z
Learnt from: kingpanther13
Repo: kingpanther13/Hubitat-local-MCP-server PR: 378
File: libraries/mcp-deploy-jobs-lib.groovy:555-571
Timestamp: 2026-08-09T12:53:41.770Z
Learning: In `libraries/mcp-deploy-jobs-lib.groovy`, `atomicState.updateMapValue` is used for per-job deployment checkpoints, but its null-value deletion semantics are undocumented. `_deployOpDelete` therefore rebuilds `atomicState.deployJobs` only for terminal deployment jobs, consistent with the existing terminal-job pruning pattern. Lease guards must prevent cancellation or deletion while an active deployment worker holds `sliceLeaseUntil`, because the worker saves a whole-job snapshot.
Applied to files:
libraries/mcp-discovery-lib.groovylibraries/mcp-item-backups-lib.groovyTOOL_GUIDE.mddocs/superpowers/plans/2026-08-12-mrtr-slow-writes.mdsrc/test/groovy/server/RelayBudgetSpec.groovylibraries/mcp-code-management-lib.groovydocs/superpowers/specs/2026-08-12-mrtr-slow-writes-design.mdlibraries/mcp-self-admin-lib.groovylibraries/mcp-native-rules-lib.groovysrc/test/groovy/server/ToolUpdatePackageSpec.groovysrc/test/groovy/server/MrtrContinuationSpec.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to hubitat-mcp-server.groovy : Use read_ gateways only for entirely read-only sub-tools and manage_ gateways for gateways containing writes; read-only tools must also be reachable through a read gateway or remain flat top-level tools.
Applied to files:
libraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovylibraries/mcp-app-cloner-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-custom-rules-lib.groovylibraries/mcp-system-lib.groovyTOOL_GUIDE.mdlibraries/mcp-visual-rules-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Every MCP tool must use the `hub_` service prefix, verb-noun ordering, and an approved verb from the vocabulary table; do not add new verbs without strong justification.
Applied to files:
libraries/mcp-debug-logging-lib.groovylibraries/mcp-item-backups-lib.groovylibraries/mcp-system-lib.groovylibraries/mcp-visual-rules-lib.groovylibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Read/write permission enforcement must remain centralized at the `executeTool()` dispatch chokepoint; advanced overrides may only disable tools, never re-enable them.
Applied to files:
libraries/mcp-debug-logging-lib.groovylibraries/mcp-diagnostics-lib.groovyTOOL_GUIDE.mdlibraries/mcp-code-management-lib.groovylibraries/mcp-self-admin-lib.groovysrc/test/groovy/server/MrtrContinuationSpec.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Every leaf tool and gateway must have display metadata with a unique Title-Case title and a one-sentence summary of at most 140 characters ending in a period.
Applied to files:
libraries/mcp-diagnostics-lib.groovylibraries/mcp-custom-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-server.groovy : Preserve the adopted next-revision MCP protocol behavior, including modern-versus-legacy header-value handling, validation/error mappings, era-gated `resultType`, unconditional server metadata, cache hints, and opt-in Origin enforcement.
Applied to files:
src/test/groovy/server/McpWireSchemaConformanceSpec.groovydocs/testing.mddocs/superpowers/plans/2026-08-12-mrtr-slow-writes.mdlibraries/mcp-code-management-lib.groovydocs/superpowers/specs/2026-08-12-mrtr-slow-writes-design.mdlibraries/mcp-native-rules-lib.groovytests/sdk_conformance_test.py
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to hubitat-mcp-rule.groovy : Treat the custom MCP rule-engine child app as legacy and do not add new features there; implement new rule capabilities through native Rule Machine tools in the parent app.
Applied to files:
libraries/mcp-app-cloner-lib.groovylibraries/mcp-custom-rules-lib.groovytests/BAT-v2.mdlibraries/mcp-visual-rules-lib.groovylibraries/mcp-native-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-*.groovy : Hubitat Groovy code must not use blocked JVM features such as Eval, GroovyShell, reflection, Runtime.exec, threads, or direct filesystem APIs; use the hub File Manager API for files.
Applied to files:
src/test/groovy/server/ToolManageFilesSpec.groovyci/groovy2x-spock/scaffold/support/HarnessSpec.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to **/*.groovy : Tools that can return long lists must support `cursor`/`nextCursor` unless their natural response fits under the 120KB cap; use response-format controls when concise and detailed payloads differ.
Applied to files:
libraries/mcp-files-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Tools returning potentially long lists must support cursor/nextCursor pagination unless their natural response fits within the 120KB tools/call limit.
Applied to files:
libraries/mcp-files-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to packageManifest.json : Deliver libraries through bundles[] rather than libraries[]; do not commit generated bundle artifacts.
Applied to files:
libraries/mcp-bundles-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to packageManifest.json : Deliver libraries through the HPM `bundles[]` mechanism, not `libraries[]`; preserve the production bundle basename.
Applied to files:
libraries/mcp-bundles-lib.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Every new MCP tool must explicitly classify readOnlyHint, idempotentHint, and openWorldHint; emit destructiveHint for writes and omit it for reads.
Applied to files:
libraries/mcp-native-rules-lib.groovy
📚 Learning: 2026-08-08T03:29:37.786Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-08T03:29:37.786Z
Learning: Applies to hubitat-mcp-*.groovy : Use `atomicState` for thread-safe persistence, `state` for UI/counters, and compare device IDs as strings using `.toString()`.
Applied to files:
src/test/groovy/server/MrtrContinuationSpec.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Use atomicState for thread-safe persistence, state for UI/counters, and compare device IDs as strings with toString().
Applied to files:
src/test/groovy/server/MrtrContinuationSpec.groovy
📚 Learning: 2026-08-08T03:29:24.112Z
Learnt from: CR
Repo: kingpanther13/Hubitat-local-MCP-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-08T03:29:24.112Z
Learning: Applies to **/*.groovy : Merge opposite state mutations into one set_<noun>_<attribute> tool, merge filter/projection variants with optional parameters, and avoid consolidation when error modes, safety gates, or payload shapes differ fundamentally.
Applied to files:
src/test/groovy/server/MrtrContinuationSpec.groovy
🪛 ast-grep (0.45.1)
tests/sdk_conformance_helpers.py
[warning] 29-29: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(rf"^{re.escape(prefix)}(\d+)$")
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
tests/test_e2e_test_helpers.py
[info] 201-201: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"success": True})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 236-236: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"success": True})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 306-306: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"success": True})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 LanguageTool
tests/sdk-conformance-requirements.txt
[style] ~16-~16: Try using a descriptive adverb here.
Context: ...f # the closure is left to the resolver on purpose: it is server/auth plumbing imported by...
(ON_PURPOSE_DELIBERATELY)
docs/superpowers/specs/2026-08-12-mrtr-slow-writes-design.md
[style] ~229-~229: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ue state from a duplicate fresh call. - Never silently restart after unknown or expir...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Comment on lines
+99
to
+101
| - [ ] Remove public `opToken` inputs and auto-token output fields. | ||
| - [ ] Remove token polling/replay, recent-operation journal, terminal replay, and | ||
| their public documentation/tests. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Distinguish opToken replay from MRTR terminal replay.
Lines [99-101] say to remove “terminal replay,” but the design retains a terminal result briefly under the same requestState so a lost final HTTP response can be replayed. Clarify that this removes PR #378’s public opToken replay, not MRTR terminal-result replay. If implemented literally, clients lose the documented response-loss recovery.
Suggested wording
- Remove token polling/replay, recent-operation journal, terminal replay, and
+ Remove PR `#378`'s public opToken polling/replay, recent-operation journal, and
+ token-based terminal replay; retain MRTR terminal-result replay under requestState.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-12-mrtr-slow-writes.md` around lines 99 - 101,
Clarify the checklist in the MRTR plan to remove PR `#378`’s public opToken
polling/replay and related journal/documentation/tests, while retaining MRTR
terminal-result replay through the existing requestState for brief lost-response
recovery. Replace the ambiguous “terminal replay” wording so it does not imply
removing this retained behavior.
Comment on lines
+429
to
+431
| `tests/sdk_conformance_test.py` connects one official SDK high-level `Client` through its `streamable_http_client` to a real hub `/mcp` endpoint, pinned directly to `mode="2026-07-28"`. It never uses `mode="auto"` (which could fall back) or `mode="legacy"`. Catalog, benign call, resources, published-`outputSchema`, and the MRTR proof all share that modern client. One `Client.call_tool()` drives the Rule Machine write through multiple state-only continuation responses to one terminal `CallToolResult` — no project-owned loop and no low-level session call. | ||
|
|
||
| **⚠️ One scenario WRITES.** The `outputSchema` one flips the advanced `publishOutputSchemas` setting ON, re-lists, calls `hub_get_info`, then restores the prior value in a `finally` (retried, and verified against the catalog; a failed restore is a CRITICAL failure because every later client would see the wrong catalog). That flip is not incidental — it is the whole point. `ClientSession.call_tool` validates `structuredContent` against any advertised `outputSchema` and raises the literal issue-#342 error (*"has an output schema but did not return structured content"*), but with the toggle OFF every cached schema is `None` and that branch never executes. Flipping it ON is what puts a real validator behind the emitted **wire form** (`_wireOutputSchema`, `required` arrays stripped) against a real result. The re-`list_tools()` after the flip is required, not cosmetic: `call_tool` refreshes the schema cache only for a name it has never seen, and the `tools/list` scenario already cached `hub_get_info`'s schema as `None`. Because the step writes, it is skipped under a `skip_install` dispatch (see the `pull_request_target` note below). | ||
| **⚠️ Two scenario families WRITE.** The `outputSchema` one flips the advanced `publishOutputSchemas` setting ON, re-lists, calls `hub_get_info`, then restores the prior value in a `finally` (retried, and verified against the catalog; a failed restore is a CRITICAL failure because every later client would see the wrong catalog). That flip is not incidental — it is the whole point. `Client.call_tool` validates `structuredContent` against any advertised `outputSchema`; with the toggle OFF every cached schema is `None`, so the validator branch never executes. The modern MRTR scenario reads the best-practice guide through the same high-level modern client and carries its published acknowledgment key on fixture writes, so the standalone command works whether the optional BPS gate is ON or OFF without changing that setting. It creates one UUID-named `BAT_E2E_SDK_MRTR_*` rule, makes one six-action edit, and deletes only that exact app in a `finally`; if the create response is lost, cleanup performs a bounded settle/retry and adopts at most one exact random-name match. It refuses ambiguous matches, never performs a prefix sweep, and never lists or deletes backups. Because the step writes, it is skipped under a `skip_install` dispatch (see the `pull_request_target` note below). |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document the intentional skip_install exception to the no-skip rule.
The new SDK section says the write scenario is skipped under skip_install, while the conformance policy says neither leg may skip. State that the no-soft-skip rule applies to eligible runs and that skip_install intentionally excludes live write coverage. Otherwise a green skip can be mistaken for SDK/MRTR proof.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/testing.md` around lines 429 - 431, Update the SDK conformance
documentation to reconcile the no-skip policy with the write scenario’s
skip_install behavior: state that neither leg may soft-skip when eligible, while
explicitly documenting that skip_install intentionally excludes live write
coverage. Ensure the wording prevents a green skip from being interpreted as
SDK/MRTR proof.
Comment on lines
+1511
to
+1514
| if (args.__packageRequestId != null) { | ||
| stash.requestId = args.__packageRequestId.toString() | ||
| stash.packageRef = atomicState.packageDeployInFlight?.ref | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: dispatcher-only injection, worker-bound packageRef validation,
# and regression coverage for stale/newer markers on success and failure.
rg -n -C 8 \
'__packageRequestId|packageDeployInFlight|packageRef|lastSelfDeploy|requestId' \
hubitat-mcp-server.groovy libraries src/test/groovyRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 50395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- dispatcher and public tool paths ---'
rg -n -C 12 \
'def executeTool|executeTool\(|hub_update_app|toolUpdateAppCode|__packageRequestId|runPackageDeploy' \
hubitat-mcp-server.groovy libraries/mcp-code-management-lib.groovy libraries/mcp-self-admin-lib.groovy \
src/test/groovy/server
printf '%s\n' '--- relevant tests ---'
rg -n -C 15 \
'__packageRequestId|packageDeployInFlight|lastSelfDeploy|hub_update_package|hub_update_app' \
src/test/groovy/serverRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 50395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- symbol locations ---'
rg -n \
'^[[:space:]]*(def|Map|private|protected).*?(executeTool|toolUpdateAppCode|hub_update_app|runPackageDeploy)|__packageRequestId' \
hubitat-mcp-server.groovy libraries/mcp-code-management-lib.groovy libraries/mcp-self-admin-lib.groovy \
src/test/groovy/server
printf '%s\n' '--- dispatcher candidates ---'
rg -n -C 25 'executeTool' hubitat-mcp-server.groovy | head -n 500
printf '%s\n' '--- package-specific tests ---'
rg -n -C 12 \
'package deploy|packageDeployInFlight|runPackageDeploy|__packageRequestId|requestId.*lastSelfDeploy|lastSelfDeploy.*requestId' \
src/test/groovy/server --glob '*.groovy' | head -n 1200Repository: kingpanther13/Hubitat-local-MCP-server
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- executeTool dispatch ---'
sed -n '4300,4475p' hubitat-mcp-server.groovy
printf '%s\n' '--- toolUpdateAppCode validation and stash paths ---'
sed -n '1360,1665p' libraries/mcp-code-management-lib.groovy
printf '%s\n' '--- package worker and forwarding path ---'
sed -n '450,590p' libraries/mcp-self-admin-lib.groovy
sed -n '750,785p' libraries/mcp-self-admin-lib.groovy
printf '%s\n' '--- public hub_update_app schema ---'
rg -n -C 35 'name: "hub_update_app"|name: '\''hub_update_app'\''' \
hubitat-mcp-server.groovy libraries --glob '*.groovy' | head -n 300
printf '%s\n' '--- package tests around request identity ---'
sed -n '780,900p' src/test/groovy/server/ToolUpdatePackageSpec.groovyRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- hub_update_app dispatch case ---'
rg -n -C 8 'case "hub_update_app"|toolUpdateAppCode\(args\)' hubitat-mcp-server.groovy
printf '%s\n' '--- gateway forwarding and argument handling ---'
sed -n '3890,4045p' hubitat-mcp-server.groovy
printf '%s\n' '--- exact package tests ---'
rg -n -C 25 \
'requestId|packageDeployInFlight|__packageRequestId|packageRef|stale|newer|worker' \
src/test/groovy/server/ToolUpdatePackageSpec.groovy
printf '%s\n' '--- deterministic source verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
server = Path("hubitat-mcp-server.groovy").read_text()
code = Path("libraries/mcp-code-management-lib.groovy").read_text()
admin = Path("libraries/mcp-self-admin-lib.groovy").read_text()
def function_body(text, signature, next_signature):
start = text.index(signature)
end = text.index(next_signature, start)
return text[start:end]
execute = function_body(server, "def executeTool(toolName, args)", "\ndef handleGateway")
update = function_body(code, "def toolUpdateAppCode(args)", "\ndef toolUpdateDriverCode")
package = function_body(admin, "def toolUpdatePackage(args)", "\n// Scheduled worker")
worker = function_body(admin, "def runPackageDeploy(Map job = [:])", "\n// Deterministic deploy body")
print("executeTool injects __packageRequestId:",
bool(re.search(r"__packageRequestId", execute)))
print("executeTool directly dispatches hub_update_app:",
bool(re.search(r'case ["\']hub_update_app["\']', execute)))
print("toolUpdateAppCode accepts caller-supplied __packageRequestId:",
"__packageRequestId" in update))
print("public package boundary creates __packageRequestId:",
"__packageRequestId" in package))
print("worker forwards __packageRequestId:",
"__packageRequestId" in worker))
print("stash binds packageRef from current marker ref:",
"atomicState.packageDeployInFlight?.ref" in update))
print("worker validates marker requestId before copying marker:",
"current.requestId?.toString() == requestId" in worker))
PYRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 25936
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
server = Path("hubitat-mcp-server.groovy").read_text()
code = Path("libraries/mcp-code-management-lib.groovy").read_text()
admin = Path("libraries/mcp-self-admin-lib.groovy").read_text()
def body(text, start_marker, end_marker):
start = text.index(start_marker)
end = text.index(end_marker, start)
return text[start:end]
execute = body(server, "def executeTool(toolName, args)", "\ndef handleGateway")
update = body(code, "def toolUpdateAppCode(args)", "\ndef toolUpdateDriverCode")
package = body(admin, "def toolUpdatePackage(args)", "\n// Scheduled worker")
worker = body(admin, "def runPackageDeploy(Map job = [:])", "\n// Deterministic deploy body")
checks = {
"executeTool injects __packageRequestId":
bool(re.search(r"__packageRequestId", execute)),
"executeTool dispatches hub_update_app directly":
bool(re.search(r'case\s+["\']hub_update_app["\']\s*:\s*return\s+toolUpdateAppCode\(args\)', execute)),
"toolUpdateAppCode consumes caller-supplied __packageRequestId":
"__packageRequestId" in update,
"public package boundary creates __packageRequestId":
"__packageRequestId" in package,
"package worker forwards __packageRequestId":
"__packageRequestId" in worker,
"stash reads packageRef from current marker":
"atomicState.packageDeployInFlight?.ref" in update,
"worker validates requestId before copying marker":
"current.requestId?.toString() == requestId" in worker,
}
for name, value in checks.items():
print(f"{name}: {value}")
assert checks["executeTool injects __packageRequestId"] is False
assert checks["executeTool dispatches hub_update_app directly"] is True
assert checks["toolUpdateAppCode consumes caller-supplied __packageRequestId"] is True
assert checks["public package boundary creates __packageRequestId"] is True
assert checks["package worker forwards __packageRequestId"] is True
assert checks["stash reads packageRef from current marker"] is True
assert checks["worker validates requestId before copying marker"] is True
PYRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
server = Path("hubitat-mcp-server.groovy").read_text()
code = Path("libraries/mcp-code-management-lib.groovy").read_text()
admin = Path("libraries/mcp-self-admin-lib.groovy").read_text()
def report(label, value):
print(f"{label}: {value}")
assert value
dispatch = re.search(
r'case\s+["\']hub_update_app["\']\s*:\s*return\s+toolUpdateAppCode\(args\)',
server,
)
report("executeTool dispatches hub_update_app with the original args", dispatch is not None)
# The only package-request construction and forwarding sites must be in the package
# boundary/worker code, not in the dispatcher.
package_sites = [(m.start(), m.group(0)) for m in re.finditer(r'__packageRequestId', admin)]
report("package request sites exist only in mcp-self-admin-lib.groovy", len(package_sites) >= 2)
report("dispatcher contains no __packageRequestId injection", "__packageRequestId" not in server)
# The public hub_update_app handler reads the hidden key directly, and does not
# distinguish a dispatcher-created map from a caller-created map.
report("hub_update_app handler reads __packageRequestId", "__packageRequestId" in code)
report("hub_update_app stashes packageRef from the live marker",
"stash.packageRef = atomicState.packageDeployInFlight?.ref" in code)
# The package worker does bind its own scheduled invocation to the live marker.
report("package worker validates requestId before running",
"current.requestId?.toString() == requestId" in admin)
report("package worker forwards __packageRequestId",
"updateArgs.__packageRequestId = args.__packageRequestId.toString()" in admin)
PYRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 589
Reject caller-supplied __packageRequestId.
executeTool forwards the original arguments to hub_update_app, so a public self-update can supply this reserved field. toolUpdateAppCode then records it and reads packageRef from the live marker, which can falsely associate an unrelated update with an active package worker. Pass correlation data through a worker-only context and derive both requestId and packageRef from the validated marker. Add success and failure tests for a public spoof during an active package deploy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libraries/mcp-code-management-lib.groovy` around lines 1511 - 1514, Remove
the caller-controlled __packageRequestId handling from toolUpdateAppCode and
pass correlation data only through worker-only context. Validate the active
package marker, then derive both stash.requestId and stash.packageRef from that
validated marker rather than public arguments; add success and failure coverage
for a public spoof while a package deploy is active.
| description: """Create a full hub-database backup (whole-hub .lzf). REQUIRED before any Write master op (24h validity).[[FLAT_TRIM]] Optionally set the automatic-backup schedule via `schedule` (scheduleOnly=true sets the schedule only). The only write tool needing no prior backup.[[/FLAT_TRIM]] | ||
| [[FLAT_TRIM]] | ||
| A transport drop (relay ceiling / client timeout) can lose the response while the hub still commits this write; pass opToken, and on a drop re-issue the call with the SAME opToken to poll/replay the committed result instead of re-running it -- see hub_get_tool_guide(section='slow_ops'). | ||
| A transport drop can lose the response while the hub still commits this write; verify current hub state before retrying. See hub_get_tool_guide(section='slow_ops'). |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep transport-drop warnings outside [[FLAT_TRIM]].
The new warning for hub_create_backup and hub_restore_backup is inside [[FLAT_TRIM]]. Flat mode can hide the instruction to verify hub state before retrying. A caller can then repeat a backup or restore after the first write already committed. Move the warning and its hub_get_tool_guide reference outside the trim markers.
Proposed fix
-[[FLAT_TRIM]]
-A transport drop can lose the response while the hub still commits this write; verify current hub state before retrying. See hub_get_tool_guide(section='slow_ops').
-[[/FLAT_TRIM]]
+A transport drop can lose the response while the hub still commits this write; verify current hub state before retrying. See hub_get_tool_guide(section='slow_ops').Based on learnings, Use [[FLAT_TRIM]] only for advanced detail that remains available through hub_get_tool_guide; keep basic purpose, required parameters, critical formats, and safety warnings visible in flat mode.
Also applies to: 985-985
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libraries/mcp-item-backups-lib.groovy` at line 835, Move the transport-drop
warning and its hub_get_tool_guide(section='slow_ops') reference for
hub_create_backup and hub_restore_backup outside the [[FLAT_TRIM]] markers. Keep
the instruction to verify current hub state before retrying visible in flat
mode, while leaving only advanced optional detail inside the trim sections.
Source: Learnings
Comment on lines
492
to
+509
| def inFlight = atomicState.packageDeployInFlight | ||
| if (inFlight instanceof Map && inFlight.startedAt != null) { | ||
| long startedAt = inFlight.startedAt as Long | ||
| long elapsed = now() - startedAt | ||
| def lsd = atomicState.lastSelfDeploy | ||
| boolean finished = (lsd instanceof Map && lsd.at != null && (lsd.at as Long) > startedAt) | ||
| if (!finished && elapsed < guardTtlMs) { | ||
| return [ | ||
| success: false, isError: true, | ||
| inFlight: [ref: inFlight.ref, startedAt: startedAt, elapsedMs: elapsed], | ||
| error: "A package deploy (ref '${inFlight.ref}') is already running on this hub -- it started ${elapsed.intdiv(1000L)}s ago and a full repair takes minutes. Nothing was changed.", | ||
| note: "Do NOT re-run the deploy. If your original call carried an opToken, poll by re-issuing it with the same opToken (the token alone is enough). Watch hub_get_info's lastSelfDeploy for the done-signal (a fresh `at`/`ageMs` means the self-app leg ran). See hub_get_tool_guide(section='slow_ops')." | ||
| long activeStartedAt = inFlight.startedAt as Long | ||
| long elapsed = now() - activeStartedAt | ||
| if (elapsed < _packageDeployMarkerTtlMs()) { | ||
| refusal = [ | ||
| success: false, isError: true, status: "duplicate_in_flight", | ||
| inFlight: [ref: inFlight.ref, startedAt: activeStartedAt, elapsedMs: elapsed], | ||
| error: "A package deploy (ref '${inFlight.ref}') is already running on this hub. Nothing was changed.", | ||
| note: "Do not re-run the deploy. Watch hub_get_info.lastSelfDeploy for a record whose requestId matches the original acceptance response." | ||
| ] | ||
| } | ||
| } | ||
| if (refusal == null) { | ||
| atomicState.packageDeployInFlight = [ | ||
| requestId: requestId, ref: ref, startedAt: startedAt, args: storedArgs | ||
| ] | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not replace an active deployment marker based only on age.
If the first worker exceeds _packageDeployMarkerTtlMs(), this code accepts a second deployment while the first worker can still execute _updatePackageBody. The old worker can then apply an older ref after the newer ref and overwrite atomicState.lastSelfDeploy at Lines 552-568. The newer caller can no longer find its matching terminal record.
Keep the deployment reservation until its worker records a terminal state. If stale-worker recovery is required, use a worker-owned lease that the worker renews and verify the lease before every deployment side effect. Do not admit another deployment only because startedAt is old.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libraries/mcp-self-admin-lib.groovy` around lines 492 - 509, Update the
package deployment reservation flow around packageDeployInFlight and
_packageDeployMarkerTtlMs so an aged startedAt alone never admits a second
deployment while the original worker may still run. Retain the marker until the
worker records a terminal state; if stale recovery is necessary, implement a
worker-renewed lease and verify ownership before each _updatePackageBody or
other deployment side effect, preserving terminal-record matching through
lastSelfDeploy.
Comment on lines
+811
to
+826
| Map targetBinding = script._mrtrBinding('hub_call_rule', 'hub_call_rule', | ||
| [ruleId: [921, 922], action: 'stop']) as Map | ||
| atomicStateMap.mrtrRequests = (0..<16).collectEntries { int index -> | ||
| Map b = script._mrtrBinding('hub_call_rule', 'hub_call_rule', | ||
| [ruleId: [1000 + index, 2000 + index], action: 'stop']) as Map | ||
| [("mrtr-seeded-active-${index}".toString()): [ | ||
| schemaVersion: 1, status: 'active', outerTool: 'hub_call_rule', leafTool: 'hub_call_rule', | ||
| argDigest: b.argDigest, | ||
| startedAt: 1234567880000L + index, updatedAt: 1234567880000L + index, | ||
| expiresAt: 1234567990000L, rounds: 0, generation: 0 | ||
| ]] | ||
| } | ||
| def before = atomicStateMap.mrtrRequests.keySet() as Set | ||
|
|
||
| when: | ||
| Map result = script._mrtrReserve('hub_call_rule', 'hub_call_rule', targetBinding) as Map |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Map the MRTR helper declarations and their visibility in the main app.
set -euo pipefail
fd -t f 'hubitat-mcp-server.groovy' --exec ast-grep outline {} --items all --match '_mrtr|_activeWrites|_writeReserveRequest|runMrtrSlice'
fd -t f 'hubitat-mcp-server.groovy' --exec rg -nP '^\s*(private\s+)?\S+\s+(_mrtr\w+|_activeWrites|_writeReserveRequest|runMrtrSlice)\s*\(' {}Repository: kingpanther13/Hubitat-local-MCP-server
Length of output: 3588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- MRTR declarations ---'
sed -n '1845,1885p;1955,2030p;2110,2185p;2560,2580p' hubitat-mcp-server.groovy
echo '--- direct test helper calls ---'
rg -n -C 2 '_mrtrBinding|_mrtrReserve|_mrtrSha256|_mrtrActiveTtlMs|_mrtrContentionWaitMs|_activeWrites|_writeReserveRequest|_mrtrClaim|_mrtrAbandon' \
src/test/groovy/server/MrtrContinuationSpec.groovy \
src/test/groovy/server/McpWireSchemaConformanceSpec.groovy
echo '--- test script construction and metaclass handling ---'
rg -n -C 3 'GroovyShell|parse\\(|metaClass|setProperty|script\\s*=|newInstance' \
src/test/groovy/server/MrtrContinuationSpec.groovy \
src/test/groovy/server/McpWireSchemaConformanceSpec.groovyRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 20874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- test fixture setup ---'
sed -n '1,180p' src/test/groovy/server/MrtrContinuationSpec.groovy
sed -n '1,180p' src/test/groovy/server/McpWireSchemaConformanceSpec.groovy
echo '--- exact declaration modifiers ---'
rg -n -C 1 '(^|[[:space:]])(private[[:space:]]+)?(def|void|boolean|String|Map|List|Set)[[:space:]]+(_mrtrBinding|_mrtrSha256|_mrtrActiveTtlMs|_mrtrContentionWaitMs|_mrtrReserve|_mrtrClaim|_mrtrAbandon|_activeWrites|_writeReserveRequest)[[:space:]]*\\(' hubitat-mcp-server.groovyRepository: kingpanther13/Hubitat-local-MCP-server
Length of output: 15781
Expose the MRTR helpers used by both specs
These specs directly call private methods on hubitat-mcp-server.groovy. Make _mrtrBinding, _mrtrSha256, and _mrtrAbandon non-private, or provide an equivalent test-accessible API. Preserve _mrtrContentionWaitMs with zero- and one-argument forms and _mrtrAbandon(stateId, record, claim, reason).
📍 Affects 2 files
src/test/groovy/server/MrtrContinuationSpec.groovy#L811-L826(this comment)src/test/groovy/server/McpWireSchemaConformanceSpec.groovy#L212-L243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/groovy/server/MrtrContinuationSpec.groovy` around lines 811 - 826,
Expose the MRTR helpers in hubitat-mcp-server.groovy so both test sites,
src/test/groovy/server/MrtrContinuationSpec.groovy:811-826 and
src/test/groovy/server/McpWireSchemaConformanceSpec.groovy:212-243, can call
_mrtrBinding, _mrtrSha256, and _mrtrAbandon directly, or provide equivalent
test-accessible APIs. Preserve _mrtrContentionWaitMs with both zero- and
one-argument forms, and retain _mrtrAbandon(stateId, record, claim, reason)
unchanged in behavior.
Comment on lines
+1164
to
+1168
| The modern path applies to `hub_set_rule`, `hub_set_native_app`, multi-rule stop/start batches through `hub_call_rule`, `hub_clone_native_app`, and `hub_import_native_app`. | ||
|
|
||
| A client-invented `opToken` is an optional extra, never something to ask the user for: 8-128 characters of A-Za-z0-9._-. Its one unique advantage is verbatim-retry dedup: a transport re-send of the same bytes carries the same token and is refused while running or replayed after completion. An untokened re-send gets a new auto token; while a LONG-RUNNING write is in flight the server also fingerprints the stripped arguments and refuses the match as `duplicate_in_flight`. EVERY tool -- reads included -- accepts a client token; a tokened expensive read also replays instead of re-running. Tokens are per-call nonces, and records are written per-entry so different tokens do not interfere. | ||
| The first request is a mutation-free preflight. The server returns `resultType: "input_required"` with an opaque `requestState`; compatible MCP clients automatically repeat the same tool call with that state. Each resumed request runs one bounded slice and gets a fresh relay deadline. The logical call eventually returns one normal `resultType: "complete"` result describing all slices. | ||
|
|
||
| If the response is lost, do NOT re-run the operation and do NOT invent a fresh token. Re-issue the SAME tool call with the SAME `opToken` — the token alone is enough: a flat tool takes `{opToken: "<yours>"}` with no other arguments (e.g. hub_update_package), a gateway member takes `{tool: "<leaf>", opToken: "<yours>"}` (e.g. `{tool: "hub_set_rule", opToken: "<yours>"}` via hub_manage_rule_machine). The server answers from the token record without running anything twice: | ||
| The state is bound to the original leaf tool and exact original arguments. A mismatched, unknown, or expired state executes nothing. A fresh identical call while the original is active is refused as `duplicate_in_flight` and cannot advance or repeat the write. The terminal result remains replayable briefly under the same requestState so losing only the final HTTP response does not rerun the operation. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Describe internal worker handoffs accurately.
Line [1166] says each resumed request runs one bounded slice. For hub_set_rule and hub_set_native_app, the design queues an internal worker and returns input_required before entering the leaf. A continuation leg can therefore coordinate a slice without executing it. Update the guide so it says a request advances or coordinates a bounded slice. This keeps the guide, protocol behavior, and SDK telemetry assertions aligned.
Suggested wording
- Each resumed request runs one bounded slice and gets a fresh relay deadline.
+ Each resumed request advances or coordinates one bounded slice and gets a
+ fresh relay deadline. Native wizard slices may run in the internal worker.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The modern path applies to `hub_set_rule`, `hub_set_native_app`, multi-rule stop/start batches through `hub_call_rule`, `hub_clone_native_app`, and `hub_import_native_app`. | |
| A client-invented `opToken` is an optional extra, never something to ask the user for: 8-128 characters of A-Za-z0-9._-. Its one unique advantage is verbatim-retry dedup: a transport re-send of the same bytes carries the same token and is refused while running or replayed after completion. An untokened re-send gets a new auto token; while a LONG-RUNNING write is in flight the server also fingerprints the stripped arguments and refuses the match as `duplicate_in_flight`. EVERY tool -- reads included -- accepts a client token; a tokened expensive read also replays instead of re-running. Tokens are per-call nonces, and records are written per-entry so different tokens do not interfere. | |
| The first request is a mutation-free preflight. The server returns `resultType: "input_required"` with an opaque `requestState`; compatible MCP clients automatically repeat the same tool call with that state. Each resumed request runs one bounded slice and gets a fresh relay deadline. The logical call eventually returns one normal `resultType: "complete"` result describing all slices. | |
| If the response is lost, do NOT re-run the operation and do NOT invent a fresh token. Re-issue the SAME tool call with the SAME `opToken` — the token alone is enough: a flat tool takes `{opToken: "<yours>"}` with no other arguments (e.g. hub_update_package), a gateway member takes `{tool: "<leaf>", opToken: "<yours>"}` (e.g. `{tool: "hub_set_rule", opToken: "<yours>"}` via hub_manage_rule_machine). The server answers from the token record without running anything twice: | |
| The state is bound to the original leaf tool and exact original arguments. A mismatched, unknown, or expired state executes nothing. A fresh identical call while the original is active is refused as `duplicate_in_flight` and cannot advance or repeat the write. The terminal result remains replayable briefly under the same requestState so losing only the final HTTP response does not rerun the operation. | |
| The modern path applies to `hub_set_rule`, `hub_set_native_app`, multi-rule stop/start batches through `hub_call_rule`, `hub_clone_native_app`, and `hub_import_native_app`. | |
| The first request is a mutation-free preflight. The server returns `resultType: "input_required"` with an opaque `requestState`; compatible MCP clients automatically repeat the same tool call with that state. Each resumed request advances or coordinates one bounded slice and gets a fresh relay deadline. Native wizard slices may run in the internal worker. The logical call eventually returns one normal `resultType: "complete"` result describing all slices. | |
| The state is bound to the original leaf tool and exact original arguments. A mismatched, unknown, or expired state executes nothing. A fresh identical call while the original is active is refused as `duplicate_in_flight` and cannot advance or repeat the write. The terminal result remains replayable briefly under the same requestState so losing only the final HTTP response does not rerun the operation. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@TOOL_GUIDE.md` around lines 1164 - 1168, Update the modern-path description
around the resumed-request behavior to say each continuation advances or
coordinates one bounded slice, rather than claiming it always runs the slice.
Preserve the existing preflight, requestState, deadline, and terminal-result
semantics for hub_set_rule, hub_set_native_app, and the other listed tools.
Merged
14 tasks
Owner
Author
|
Superseded by #388, which combines the full change directly against main. The branch is intentionally preserved. |
kingpanther13
added a commit
that referenced
this pull request
Aug 15, 2026
…R from new mcp spec (#388) ## Summary - Replace custom client-supplied operation tokens and deployment-job recovery with MCP 2026-07-28 `requestState` continuations over Streamable HTTP. - Keep long native Rule Machine/app writes below the cloud relay ceiling by handing claimed work to an internal Hubitat worker, while retaining duplicate prevention and the global concurrent-write cap. - Make the global write cap memory-only overload protection: no per-call durable lease ledger (two hub-DB round trips on every write call removed), same refusal contract and TTL anti-wedge aging, background work still counted through its own durable records. - Serve the reservation machinery's hot atomicState keys from a write-through in-JVM snapshot, eliminating per-call and per-continuation-leg DB read chatter. - Make package deployment asynchronous and request-correlated, with race-safe recovery for an abandoned scheduled-worker marker, and never let a worker throw overwrite an already-persisted deploy outcome for the same request. - Give the SDK conformance proof the same platform-limiter recovery the suite uses (one verified watchdog bounce + one retry), and poll the e2e restore marker through the watchdog so the main app's post-restore recompile no longer manufactures relay 504s. - Carry the newest same-rule edit baseline across worker executions (a JVM mirror beside the manifest, plus a retried file probe), so backup reuse never depends on cross-execution atomicState visibility and a transient read cannot unlink a healthy baseline. - Retain the useful fixes and production `outputSchema` behavior from #378; live E2E intentionally does not exercise the abandoned output-schema feature. - Route regular and official-SDK live MCP coverage through the modern protocol only. ## Type of change - [x] `feat` — new feature or capability - [ ] `fix` — bug fix - [ ] `chore` — maintenance, dependency bump, or housekeeping - [ ] `refactor` — code restructure with no behaviour change - [ ] `docs` — documentation only - [ ] `test` — tests only - [ ] `ci` — CI/CD pipeline change ## Changes - Use standard state-only `input_required` responses and terminal `complete` tool results for slow writes. - Bind request state to the exact tool and canonical arguments, prevent duplicate execution, retain terminal replay, and preserve the configured all-write concurrency limit; numeric-string `appId`s now reach the pre-reservation refusal seam the same way numeric ones do. - Detach indivisible native-app/rule wizard work from relay-bound HTTP requests. - Replace the #378 public `opToken`/deployment-job path with an internal package worker and durable request-specific outcome. - Concurrent-write accounting is in-memory (`WRITE_REQUEST_LEASES` static + TTL sweep); the e2e suite runs with the cap off and one dedicated live test exercises the `too_many_writes_in_flight` refusal under cap 1. - `hub_list_files` rejects a non-string `filter` before any hub I/O; File Manager name filtering is locale-independent. - Same-rule edit baselines reuse reliably across the detached workers' executions; the reuse probe retries once before the discard path may permanently unlink a handle. - Pin the official Python SDK proof to MCP 2026-07-28 and require one high-level `Client.call_tool()` to complete a logical write lasting more than 10 seconds across individually bounded HTTP legs, with capacity-recovery (bounce + retry on a fresh fixture) when the run lands on a limiter-exhausted hub. - Preserve #378's production output-schema definitions and publication behavior while removing all output-schema live E2E scenarios. - Addresses #376. Supersedes #378 and #386. Closes #376. ## Release Notes - Long Rule Machine and native-app writes now continue automatically using the current MCP protocol instead of timing out at the cloud relay boundary. - Write tools respond noticeably faster over the cloud relay: per-call bookkeeping no longer performs hub-database round trips on every write. - Package updates now acknowledge quickly, run in the background, reject duplicate deployment attempts, and publish a request-correlated final outcome. - Concurrent writes remain globally capped to protect the hub from clients that issue large batches or parallel tool calls. ## Testing - 331 Python/doctest tests plus the e2e helper suites (115 focused tests) pass locally. - Sandbox lint and its self-test pass; Python compilation, Ruff, shell syntax checks, bundle build, and diff checks pass. - Groovy 2.4 parsing, Groovy 2.5 Spock, and all four normal/strict flat/gateway unit-test matrices pass in GitHub Actions. - Three fully green full-lane live E2E runs on the test hub, including the official-SDK conformance proof (7/7 scenarios) and the new live write-cap refusal test. The latest run recorded zero relay 504s and zero platform-limiter interventions anywhere in the run (from five bounces and recurring 504s per run at the PR's midpoint), with the full suite completing in ~55 minutes (from ~69). ## Checklist - [x] **Unit tests added for any new MCP tools, regressions, or bug fixes** (required — see [docs/testing.md](docs/testing.md) for the harness + recipes) - [x] **e2e tests added for new tools and/or regression tests added for any bug fix** (see `tests/e2e_test.py`) - [x] Sandbox lint passes: `python tests/sandbox_lint.py` - [ ] `./gradlew test` passes locally (or CI confirms) - [x] Live-hub BAT tests updated if tool behaviour changed (see `tests/BAT-v2.md`) - [x] Documentation updated if user-facing behaviour or tool surface changed - [x] New/renamed MCP tools follow `AGENTS.md` Tool Design Rules (naming, annotations, schema) — no tool was added or renamed; changed contracts follow the same rules. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added automatic continuation and replay for long-running writes, replacing client-supplied operation tokens. * Added filtered, case-insensitive file listings with pagination support. * Added Rule Machine “stopped” status and expanded health details. * Added configurable concurrent-write limits and asynchronous package deployment tracking. * Added reusable rollback backups with clearer scope and status reporting. * **Bug Fixes** * Improved backup cleanup, validation, error reporting, and transport-failure recovery. * Improved handling of missing visual-rule definitions and unset system status. * **Documentation** * Updated backup, continuation, deployment, and SDK conformance guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
input_required/requestStateresults over Streamable HTTP; no Tasks extension, client plugin, new transport, or caller-supplied continuation token is required.tools/callcontinues to its terminal result.maxConcurrentWritesadmission cap across all writes, including mixed modern and older-client traffic, while retaining the existing synchronous fallback for clients that do not use the modern protocol.Type of change
feat— new feature or capabilityfix— bug fixchore— maintenance, dependency bump, or housekeepingrefactor— code restructure with no behaviour changedocs— documentation onlytest— tests onlyci— CI/CD pipeline changeChanges
Part of #376: this addresses the long-running transport-failure portion. The migration helpers merged in #377 remain; durable cross-turn deployment jobs are intentionally not retained.
input_requiredresponses, argument binding, generation claims, terminal replay, bounded contention waits, and schema-valid tool results.maxConcurrentWrites > 1still works and all write tools share the same cap.hub_set_ruleandhub_set_native_appowner work in an internal ephemeral scheduled worker. Scheduler payloads contain only request identity—never tool arguments, credentials, or continuation state—and the public MCP request returns before a slow Hubitat wizard step can hit the cloud cutoff.2026-07-28; there is no legacy/headerless/auto-fallback test mode.mcp==2.0.0and requires one high-levelClient.call_tool()to drive the automatic continuation. The observer records only safe derived transport facts, never URLs, tokens, arguments, or request state.Release Notes
Testing
c68f2c7emanual fast cohort: all 10 workflows green — Unit Tests (all four strict/normal × flat/gateway jobs), Groovy 2.5 Spock, Groovy 2.4 parsing, Python, Ruff, Sandbox Lint, lane gate, lease scripts, self-deploy recovery, and PR Guard.lastSelfDeploy.requestId, exact package refc68f2c7e,success=true, and no error.mcp==2.0.0live proof: 8/8 scenarios; the slow six-action edit completed in 34.770s across six HTTP 200 legs, five automatic continuation rounds, max physical leg 7.634s, terminalcomplete, exact six-row persisted-state readback, and exact UUID cleanup.complete, exact persisted-state readback, and exact UUID cleanup.Checklist
tests/e2e_test.py)python tests/sandbox_lint.py./gradlew testpasses locally (or CI confirms)tests/BAT-v2.md)AGENTS.mdTool Design Rules (naming, annotations, schema)Summary by CodeRabbit
New Features
Changes