feat(profile): infer plugin PI settings and add Remotion trailer - #59
Conversation
Expose best-effort settings_fields from Property Inspectors so agents can author third-party actions, and ship a rebuildable product trailer under trailer/ (outputs ignored).
|
@codex review |
|
@codex review |
CodeQL py/bad-tag-filter flagged the inline-script extractor for missing optional whitespace before the closing tag angle bracket.
|
@codex review |
Satisfy CodeQL py/bad-tag-filter by accepting attributes/junk between </script and the closing angle bracket.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f20fa74cc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return False | ||
| if button.get("path") or button.get("action_type"): | ||
| return False | ||
| return bool(button.get("plugin_uuid")) and bool(button.get("action_uuid")) |
There was a problem hiding this comment.
Preserve built-in plugin actions during validation
When a button uses plugin_uuid/action_uuid, this predicate always sends it through the installed-plugin catalog; if the user has any readable third-party plugin installed, _build_third_party_action treats a missing catalog entry as fatal. That also catches built-in actions that are not installed under the user Plugins directory, such as com.elgato.streamdeck.profile which the bundled designer skill documents for profile switches (streamdeck_assets/skill/streamdeck-designer/SKILL.md:120), so those users can no longer create profile-switch buttons through the explicit fields and instead get plugin_uuid ... is not installed.
Useful? React with 👍 / 👎.
| plugins = manager.list_plugins( | ||
| plugin_id=arguments.get("plugin_id"), | ||
| include_raw_manifest=arguments.get("include_raw_manifest", False), | ||
| ) |
There was a problem hiding this comment.
Pass through the plugin schema opt-out flag
The tool schema exposes include_settings_schema=false to skip Property Inspector parsing, but this handler neither coerces nor forwards that argument, so every streamdeck_read_plugins call still uses ProfileManager.list_plugins' default include_settings_schema=True. On installations with several PI files this defeats the advertised faster manifest-only mode and returns fields callers explicitly opted out of; add this argument to boolean coercion and pass it into list_plugins.
Useful? React with 👍 / 👎.
|
|
||
| # Styles B & C — keys live in inline and linked JS, not the HTML ids. | ||
| js_blobs: list[str] = [match.group(1) for match in INLINE_SCRIPT_PATTERN.finditer(html)] | ||
| for src in SCRIPT_SRC_PATTERN.findall(html)[:PI_MAX_SCRIPT_FILES]: |
There was a problem hiding this comment.
Count only scripts that can be scanned toward the PI cap
When a Property Inspector includes six library or remote <script> tags before its action-specific script, this slice is applied before the library/remote filtering below, so the real script is never examined. Those plugins then report empty settings_fields even though their custom JS declares payload.settings.* keys, leaving callers without the Settings keys or missing-settings warnings; apply the cap after filtering or count only files actually read.
Useful? React with 👍 / 👎.
| HTML_ATTR_PATTERN = re.compile(r'([a-zA-Z_][\w:-]*)\s*=\s*"([^"]*)"') | ||
| DATA_SETTING_PATTERN = re.compile(r'\bdata-setting\s*=\s*"([^"]+)"') | ||
| SCRIPT_SRC_PATTERN = re.compile(r'<script\b[^>]*\bsrc\s*=\s*"([^"]+)"[^>]*>', re.IGNORECASE) |
There was a problem hiding this comment.
Accept single-quoted PI attributes
These regexes only match double-quoted HTML attributes, but valid Property Inspector markup commonly uses single quotes, e.g. <sdpi-textfield setting='entityId'> or <script src='pi.js'>. In that case the parser silently misses literal setting/data-setting fields and linked JS files, so streamdeck_read_plugins returns incomplete or empty settings_fields for otherwise supported PI styles.
Useful? React with 👍 / 👎.
| for blob in js_blobs: | ||
| for key in JS_SETTINGS_DOT_PATTERN.findall(blob): | ||
| if key in PI_JS_NOISE_KEYS: | ||
| continue | ||
| fields.setdefault(key, {"name": key, "source": "javascript"}) |
There was a problem hiding this comment.
Extract keys written through setSettings
For PIs that only persist values with setSettings({token: ...}) and do not also read payload.settings.token, this extraction loop never records token even though the module comment says setSettings payload literals are supported. The result is property_inspector_empty for that common style, so agents do not learn the Settings keys needed to author the action.
Useful? React with 👍 / 👎.
| script_path = cls._resolve_plugin_relative_path(plugin_dir, src) | ||
| if script_path is None or not script_path.is_file(): | ||
| # PI paths are often relative to the PI file, not the plugin root. | ||
| script_path = (pi_path.parent / src.lstrip("/")).resolve() |
There was a problem hiding this comment.
Strip URL suffixes before resolving PI scripts
If a local Property Inspector script uses a cache-busting URL such as <script src="pi.js?v=2">, the browser loads pi.js but this resolver looks for a literal file named pi.js?v=2 and then skips it. Those plugins lose all settings inferred from that linked JS, so remove query/fragment suffixes before applying the plugin-root and PI-relative path checks.
Useful? React with 👍 / 👎.
| f"{missing_required} but they were not provided " | ||
| f"(settings keys given: {sorted(provided)})." | ||
| ) | ||
| elif field_names and not provided: |
There was a problem hiding this comment.
Warn when partial declared settings are omitted
For actions whose PI exposes fields without required metadata, providing only one setting makes provided non-empty and bypasses this warning entirely, even when other declared fields are missing. That contradicts the tool description that missing declared fields are reported, and it leaves partially authored plugin buttons (e.g. entityId supplied but mode omitted) with no warning despite having an incomplete Settings payload.
Useful? React with 👍 / 👎.
Summary
streamdeck_read_plugins/list_pluginswith best-effort Property Inspectorsettings_fieldsso agents know which Settings keys third-party actions expecttrailer/(source tracked;node_modules/out/ignored)Test plan
uv run pytest tests/test_profile_manager.py tests/test_profile_server.py -v