Skip to content

fix(plugins): read canvas and moodle settings from storage - #967

Merged
abdulrafey1 merged 13 commits into
rafe/lms-settings-loaderfrom
rafe/lms-settings-canvas-moodle
Oct 26, 2026
Merged

abdulrafey1 merged 13 commits into
rafe/lms-settings-loaderfrom
rafe/lms-settings-canvas-moodle

Conversation

@abdulrafey1

@abdulrafey1 abdulrafey1 commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

What

Canvas and Moodle tools no longer take an API URL or token from the model. Each tool loads the
calling user's stored settings, so neither LMS's credentials are written into the chat prompt any more.

Changes

  • fix(plugins): Canvas AuthenticationPayload and Moodle Auth are removed from every tool schema.
    CanvasClient and MoodleClient are built from the stored config.
  • fix(plugins): canvas_authenticate and moodle_authenticate become canvas_check_settings and
    moodle_check_settings (categories canvas-settings / moodle-settings). They take no arguments and
    check the stored settings, which is how the model learns whether the LMS is set up.
    moodle_list_courses takes no arguments either.
  • refactor(plugins): every single-request Canvas tool goes through _canvas_call, and every list tool
    through _canvas_list. Each loads the settings, opens the client and maps errors once.
    CanvasClient.request replaces the private _request_dict and the get/post/put/delete
    wrappers.
  • fix(plugins): the chat tool loop logs PluginNotConfiguredError / PluginConfigInvalidError as one
    warning line without a traceback. The model sees the same message, the audit trail still records
    tool.failed, and the failure kind is unchanged.
  • fix(plugins): CanvasConfig and MoodleConfig drop to_lms_credentials_hint, so chat stops
    appending their credentials to the system prompt.
  • fix(plugins): the LMS rules text drops its "configured / not configured" branches. It now says:
    never ask for credentials; settings live on the Settings page; call the LMS's <lms>_check_settings
    tool to learn whether it is set up.
  • test: sparkth.lib.testing gains schema_property_names, tool_input_property_names,
    mock_http_session and seed_plugin_settings, shared by the plugin tests. The Moodle
    stored-settings fixture is opt-in.

How to Test

  1. uv run pytest sparkth/plugins/canvas sparkth/plugins/moodle sparkth/plugins/chat -v.
  2. The Canvas and Moodle tool tests seed a user's stored settings and assert that the request goes to
    the stored URL with the stored key. A Canvas URL with a path and a trailing slash is covered.
  3. Manual: with Canvas not set up, ask the chat to list your Canvas courses. The reply points to
    the Settings page and asks for no credentials.
  4. Manual: with Moodle set up, ask "Do you have my Moodle credentials?" The model calls
    moodle_check_settings and reveals nothing.

Notes

Breaking for MCP clients: the Canvas and Moodle tool schemas no longer accept auth. On /ai/mcp
no user is bound, so these tools fail with NoAuthenticatedUser; external MCP clients are out of
scope for now. Canvas tools also stop sending the token in request bodies.

This description was written with the assistance of an LLM (Claude).

🤖 Generated with Claude Code

abdulrafey1 and others added 3 commits October 22, 2026 00:09
Canvas tools no longer take api_url or api_token from the model. Each tool
loads the calling user's stored Canvas settings and raises
PluginNotConfiguredError when none exist. The chat LMS rules no longer
branch on configured state and Canvas settings never enter the prompt.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Moodle tools no longer take api_url or api_token from the model. Each tool
loads the calling user's stored Moodle settings and raises
PluginNotConfiguredError when none exist.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The module checks the stored settings rather than authenticating with
model-supplied credentials, and it also holds the LMS vocabulary tool.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1
abdulrafey1 added this pull request to stack #971 October 21, 2026 23:51
@abdulrafey1 abdulrafey1 self-assigned this Oct 22, 2026

@hamza-56 hamza-56 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 point:

Comment thread sparkth/plugins/canvas/tools.py Outdated
Comment thread sparkth/plugins/canvas/tools.py
Comment thread sparkth/plugins/canvas/tools.py Outdated
Comment thread sparkth/plugins/chat/assets/lms_rules_system_prompt.txt Outdated
Comment thread sparkth/plugins/moodle/tests/test_tools.py Outdated
Comment thread sparkth/plugins/moodle/tests/test_tools.py Outdated
Comment thread sparkth/plugins/chat/tests/test_turn_composition.py Outdated
abdulrafey1 and others added 9 commits October 26, 2026 16:17
The rules text names the page the plugin settings errors name, "Settings".

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Each Canvas tool loaded the stored settings, opened a client and mapped errors itself. Two
helpers, _canvas_call and _canvas_list, now do that once; the tools only build the path and
payload. CanvasClient.request replaces the private _request_dict and the get/post/put/delete
wrappers only the tools used.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A tool raising PluginNotConfiguredError or PluginConfigInvalidError is an expected user state, so
the chat tool loop logs one warning line without a traceback. The model sees the same message,
the audit trail still records tool.failed, and the failure kind is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
canvas_authenticate and moodle_authenticate take no credentials any more; they check whether the
user's stored settings work. They are now canvas_check_settings and moodle_check_settings, in the
canvas-settings and moodle-settings tool categories.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The rules told the model to call a "settings-check tool", which matched no tool name. They now
name the <lms>_check_settings convention the Canvas and Moodle tools follow.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The loader patch applied to the whole module, so the real-storage test had to patch the real
loader back in. The tool test classes now ask for stored_config by name instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Canvas no longer puts credentials in the prompt, so asserting its token is absent passed whatever
the switch did. The test now checks only that an LMS the user switched off sends none.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
mock_http_session, seed_plugin_settings and tool_input_property_names replace the copies in the
Canvas and Moodle tests. The credential tests now read the schemas of the tools each plugin
registers, which is what the model sees.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1
abdulrafey1 removed this pull request from stack #971 October 26, 2026 13:01
@abdulrafey1
abdulrafey1 added this pull request to stack #999 October 26, 2026 13:02
…-moodle

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@zaira-bibi zaira-bibi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@abdulrafey1
abdulrafey1 merged commit 8e0675f into main Oct 26, 2026
10 checks passed
@abdulrafey1
abdulrafey1 deleted the rafe/lms-settings-canvas-moodle branch October 26, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants