feat: add Community Apps callback launch action#51
Closed
elibosley wants to merge 5 commits into
Closed
Conversation
- Add a typed communityApps action for Unraid OS to launch Community Apps inside an iframe. - Before, callback payloads only modeled account, license, and OS update actions. - That left CA iframe launches without a shared contract for server context, install targets, or theme/path preferences. - Add CommunityAppsLaunch to the UPC action union with server data and optional install URL customization fields. - Export the new action type through client and server entrypoints and cover server round-trip generation/parsing. - Regenerate package declaration files so consumers can type against the new contract.
- Purpose: keep declaration builds working after the PR upgrades TypeScript from v5 to v6. - Before: \tsconfig.json(8,25): error TS6046: Argument for '--moduleResolution' option must be: 'node', 'classic', 'node16', 'nodenext'. tsconfig.json(9,5): error TS5023: Unknown compiler option 'allowImportingTsExtensions'. tsconfig.json(10,5): error TS5070: Option '--resolveJsonModule' cannot be specified without 'node' module resolution strategy. inferred the common source directory and CI failed with TS5011 under TypeScript 6. - Why that was a problem: the PR branch looked green locally with stale dependencies, but the actual CI environment installs the new compiler and the build stopped before publish. - What this change accomplishes: it makes the source layout explicit so declaration emit remains stable on TypeScript 6. - How it works: set \rootDir\: \src\ in \ to match the package entrypoints and output structure.
📝 WalkthroughWalkthroughAdds a CommunityApps UPC action and installed-apps types, implements hash and validator, provides a client host-bridge (post-me handshake) and typings, re-exports types from client/server entry points, adds tests, updates README, package.json, and tsconfig rootDir. ChangesCommunityAppsLaunch action type addition
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
sequenceDiagram
participant LocalWindow
participant WindowMessenger
participant ParentHandshake
participant IframeWindow
LocalWindow->>WindowMessenger: construct(remoteWindow, remoteOrigin)
WindowMessenger->>ParentHandshake: initialise handshake(messenger)
ParentHandshake->>IframeWindow: send handshake messages
IframeWindow->>ParentHandshake: respond to handshake
ParentHandshake->>LocalWindow: resolve(connection with close())
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
===========================================
- Coverage 100.00% 92.36% -7.64%
===========================================
Files 3 6 +3
Lines 109 131 +22
Branches 30 36 +6
===========================================
+ Hits 109 121 +12
- Misses 0 10 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/types.ts (1)
165-175: 💤 Low valueConsider documenting the install-field semantics and the
locale/themeoverlap.The interface exposes three closely-related install fields (
installUrl,installUrlTemplate,installParam) whose relationship is not obvious to downstream consumers — e.g. whetherinstallUrlandinstallUrlTemplateare mutually exclusive, and howinstallParamfeeds the template. Additionally,locale(andtheme) duplicate fields already present onServerData(locale, L110) /ActivationCodeData(theme), so precedence between the top-level value and the nestedservervalue is ambiguous. Brief JSDoc on these fields would prevent misuse.🤖 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/types.ts` around lines 165 - 175, Add concise JSDoc to the CommunityAppsLaunch interface documenting the install-field semantics and precedence for locale/theme: state that installUrl and installUrlTemplate are mutually exclusive, describe that installUrlTemplate is a string template where installParam is substituted into a well-defined placeholder (e.g., {param}) and how missing installParam is handled, and clarify whether installTarget/path affect installation flow; also document that top-level locale and theme override or fall back to ServerData.locale and ActivationCodeData.theme (specify the chosen precedence: top-level wins, otherwise use nested), and include examples or allowed formats where useful; update the CommunityAppsLaunch definition and mention related types ServerData and ActivationCodeData to make the precedence explicit.
🤖 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.
Nitpick comments:
In `@src/types.ts`:
- Around line 165-175: Add concise JSDoc to the CommunityAppsLaunch interface
documenting the install-field semantics and precedence for locale/theme: state
that installUrl and installUrlTemplate are mutually exclusive, describe that
installUrlTemplate is a string template where installParam is substituted into a
well-defined placeholder (e.g., {param}) and how missing installParam is
handled, and clarify whether installTarget/path affect installation flow; also
document that top-level locale and theme override or fall back to
ServerData.locale and ActivationCodeData.theme (specify the chosen precedence:
top-level wins, otherwise use nested), and include examples or allowed formats
where useful; update the CommunityAppsLaunch definition and mention related
types ServerData and ActivationCodeData to make the precedence explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd45e23e-b475-4941-9026-7c0fb38c77ba
⛔ Files ignored due to path filters (3)
dist/client.d.tsis excluded by!**/dist/**dist/server.d.tsis excluded by!**/dist/**dist/types.d.tsis excluded by!**/dist/**
📒 Files selected for processing (6)
README.mdsrc/__tests__/server.test.tssrc/client.tssrc/server.tssrc/types.tstsconfig.json
- Add an explicit installedApps payload block for Community Apps launches. - Before, the launch action had no contract for installed-state exposure. - That made it unclear how Core should represent optional installed app matching without weakening the privacy boundary. - Model installed app fingerprints as an optional enabled:true block with algorithm, salt, and keys. - Keep the field omitted unless a user opts into local installed-state badges.
- Replace the flat installed app fingerprint list with a compact status map. - Before, callbacks could only say that a hash was present, so CA could not distinguish currently installed apps from previously installed apps. - Variable-length hash strings also risked wasting URL space in hash-based iframe launches. - Add numeric installed app status codes and fixed sha256-128 app fingerprints encoded as 22-character base64url keys. - Export the status enum through client and server entrypoints so Core can build payloads without hardcoding status values.
- Add lookup mode for Community Apps installed app status payloads. - Before, status context had to be carried as a launch-time app map when CA needed installed-state UI. - That exposed more installed-app information than necessary and could grow callback URLs. - Add fixed-length salted app hash helpers and a post-me parent bridge for visible-app status lookups. - Keep batch maps available while making lookup mode the preferred privacy-preserving integration.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/community-apps.ts (1)
27-32: 💤 Low valueMinor: redundant length check.
The regex
/^[A-Za-z0-9_-]{22}$/already constrains the value to exactly 22 characters, so the trailingvalue.length === COMMUNITY_APPS_INSTALLED_APP_HASH_LENGTHcomparison is dead. It's harmless and keeps the length constant referenced, so feel free to leave it as-is.🤖 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/community-apps.ts` around lines 27 - 32, The type guard isCommunityAppsInstalledAppHash redundantly checks length after the regex already enforces exactly 22 chars; remove the trailing value.length === COMMUNITY_APPS_INSTALLED_APP_HASH_LENGTH clause (or alternatively remove the {22} quantifier in the regex if you prefer the constant), leaving a single consistent length check referencing COMMUNITY_APPS_INSTALLED_APP_HASH_LENGTH via one approach to avoid duplication.
🤖 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.
Nitpick comments:
In `@src/community-apps.ts`:
- Around line 27-32: The type guard isCommunityAppsInstalledAppHash redundantly
checks length after the regex already enforces exactly 22 chars; remove the
trailing value.length === COMMUNITY_APPS_INSTALLED_APP_HASH_LENGTH clause (or
alternatively remove the {22} quantifier in the regex if you prefer the
constant), leaving a single consistent length check referencing
COMMUNITY_APPS_INSTALLED_APP_HASH_LENGTH via one approach to avoid duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 527b7e24-a881-4e31-b38c-1f410a0a3325
⛔ Files ignored due to path filters (9)
dist/client.d.tsis excluded by!**/dist/**dist/client.jsis excluded by!**/dist/**dist/community-apps-client.d.tsis excluded by!**/dist/**dist/community-apps.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/server.d.tsis excluded by!**/dist/**dist/server.jsis excluded by!**/dist/**dist/types.d.tsis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
README.mdpackage.jsonsrc/__tests__/server.test.tssrc/client.tssrc/community-apps-client.tssrc/community-apps.tssrc/post-me.d.tssrc/server.tssrc/types.ts
✅ Files skipped from review due to trivial changes (1)
- src/post-me.d.ts
Member
Author
|
Superseded by #52. The new alpha contracts branch includes the full codex/community-apps-launch-action branch plus the remaining Community Apps install/theme/export contract updates. |
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
communityAppslaunch action for Unraid OS iframe launches.installedAppssupport withenabled: true,algorithm: "sha256-128", launch salt, and fixed-length app hash keys.mode: "lookup"so CA can ask the parent for statuses only for visible apps instead of receiving the full installed/history set.CommunityAppsInstalledAppStatus, fixed hash helpers, andcreateCommunityAppsInstalledAppsHostBridgethrough the client entrypoint.rootDirbuild fix needed for the package build on this branch.Privacy contract
CommunityAppsLaunch.installedAppsshould be omitted by default.mode: "lookup"; the CA iframe sends only visible salted app fingerprints over thepost-mebridge and receives compact status codes for those hashes.appsstatus map, but should not be the default for Unraid OS iframe integration.Installed = 1,PreviouslyInstalled = 2.Testing
pnpm test -- --run src/__tests__/server.test.tspnpm buildSummary by CodeRabbit
New Features
Documentation
Tests