Repository navigation
feat(knowledge): org-shared Data.shared and web-app viewer decisions (1.12.0) - #23
Conversation
richard-lua
left a comment
There was a problem hiding this comment.
Code review — LOW risk
Documentation-only change: it teaches the knowledge base two lua-cli 3.50.0 APIs (Data.shared(name) and Workflows.viewer.decideApproval/resume) and bumps the version from 1.10.0 to 1.11.0 across plugin.json, both package.json/package-lock.json, marketplace.json, the MCP server stamps, the X-Lua-Client header, and the rebuilt dist/server.js. The version bump is complete and consistent, the new sections are marked 3.50.0+, cross-references between primitives.md, decision-trees.md, web-apps.md, workflows.md and lua-debug.md line up, and no executable plugin logic is touched. The only real risk is the stated merge gate — lua-cli 3.50.0 is not yet on npm — which the author has correctly flagged and left PINNED_MIN_LUA_CLI unchanged for.
No blocking findings. The change is safe to merge once the merge gate (lua-cli 3.50.0 published) is satisfied; the version stamps and docs are internally consistent and the guidance (admin-only ACLs, never *, no secrets/PII in shared collections, builder hands admin requests to the user rather than running them) is sound.
PR Risk Reviewer — automated senior review of c09ce2a · risk: low · confidence: 0.85
… 1.11.0 - primitives.md: Data.shared(name) section (same methods as Data, when to pick it over private Data or a supervision grant, org-admin ACL setup via the admin API, errors, audit, billing, offline behaviour) and a routing-table row - decision-trees.md: data shared by several agents of one org routes to Data.shared - web-apps.md / workflows.md: Workflows.viewer.decideApproval / resume from non-GET routes - lua-debug: SharedDataError branch - version 1.11.0 in plugin.json, package.json, marketplace.json and the MCP server stamps; dist/server.js rebuilt Requires lua-cli 3.50.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c09ce2a to
6d97b4b
Compare
richard-lua
left a comment
There was a problem hiding this comment.
Code review — LOW risk
Documentation-only change teaching the knowledge base two lua-cli 3.50.0 APIs (Data.shared(name) and Workflows.viewer.decideApproval/resume), a SharedDataError branch in lua-debug.md, and a complete 1.11.0 → 1.12.0 version bump across plugin.json, both package.json/package-lock.json, marketplace.json, the MCP server crash stamps, the X-Lua-Client header, and the rebuilt dist/server.js. The version stamps are internally consistent, the new sections are marked 3.50.0+, cross-references between primitives.md, decision-trees.md, web-apps.md, workflows.md and lua-debug.md line up, and no executable plugin logic changes. The guidance is sound (admin-only ACLs, never *, no secrets/PII in shared collections, the builder hands admin requests to the user rather than running them, every Data.shared call fails offline).
No blocking findings. The sole real risk is the stated merge gate — lua-cli 3.50.0 is not yet on npm — which the author flagged and correctly handled by leaving PINNED_MIN_LUA_CLI unchanged. Safe to merge once that gate is satisfied.
PR Risk Reviewer — automated senior review of 6d97b4b · risk: low · confidence: 0.86 · re-review · round 2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
richard-lua
left a comment
There was a problem hiding this comment.
Previously flagged
No blocking findings were raised in rounds 1 or 2; both reviews landed at LOW risk. The one standing item — the merge gate (lua-cli 3.50.0 not yet on npm) — remains correctly handled: PINNED_MIN_LUA_CLI is unchanged and every new section is marked 3.50.0+.
Code review — LOW risk
This revision is a clean continuation: the version bump is now fully consistent at 1.12.0 across plugin.json, marketplace.json, both package.json/package-lock.json, the MCP crash stamps (src/server.mjs), the X-Lua-Client header (src/api-client.mjs), and the rebuilt dist/server.js. The new Data.shared(name) and Workflows.viewer guidance is internally consistent with the error codes echoed in lua-debug.md, the routing rows in primitives.md/decision-trees.md, and the web-app route constraints in web-apps.md (non-GET only, acts as the signed-in viewer, viewer_unavailable elsewhere, step-up/surface-restricted go to the Inbox). Guidance is sound: admin-only ACLs, never *, ≤200 ids per list, no PII/secrets in shared collections, builder hands admin requests to the user, every Data.shared call fails offline. No executable plugin logic changes.
No new findings. Safe to merge once the stated merge gate (lua-cli 3.50.0 published to npm) is satisfied.
PR Risk Reviewer — automated senior review of bd2f6b2 · risk: low · confidence: 0.88 · re-review · round 3
What
Teaches the builder the lua-cli 3.50.0 SDK additions it covers:
Data.shared(name)(lib/knowledge/primitives.md, new section + routing row;decision-trees.md): org-shared collections with the same methods asData; when to recommend it vs privateDatavs a supervision grant (Agents.data(id)); org-admin setup throughPOST/GET/PUT/DELETE /admin/orgs/:orgId/shared-collections({ name, readers, writers }, explicit agent ids, at most 200 per list, never*; the builder hands the request to the user, never runs it); reader vs writer methods; errorsshared_data_forbidden(403, also unknown / other-org name),shared_data_test_session_write_refused(409),shared_data_unavailable_offline(every call in a local test run); audit, embedding billing, no personal data or secrets.Workflows.viewer.decideApproval/resume(web-apps.md§3,workflows.md§6): non-GET web-app routes decide approvals and answer input steps as the signed-in viewer;viewer_unavailableelsewhere; step-up / surface-restricted approvals stay in the Inbox; locally the route runs as the developer.agents/lua-debug.md:SharedDataErrorbranch by code.dist/server.jsrebuilt withnpm run build; CHANGELOG, README row, USER_GUIDE.Not covered:
Agents.secrets(id).putandAgents.versions(id).list({ status, offset })— the plugin does not teach the supervision surface today.PINNED_MIN_LUA_CLIunchanged (3.50.0 is not on npm yet; the new sections are marked 3.50.0+).Merge gate
Do not merge until lua-cli 3.50.0 is published to npm. Everything here describes 3.50.0 APIs.
Checks
npm run lint(each lint script, offline): all passnpm test(plugin): 68 suites / 2072 tests passmcp/lua-platformnpm test: 10 suites / 149 tests pass🤖 Generated with Claude Code