Skip to content

G2 (#454) + #472: one governed tool contract across both MCP transports - #481

Open
khuepm wants to merge 13 commits into
mainfrom
feature/454-lanes-consolidated
Open

khuepm wants to merge 13 commits into
mainfrom
feature/454-lanes-consolidated

Conversation

@khuepm

@khuepm khuepm commented Sep 18, 2026

Copy link
Copy Markdown
Owner

G2 (#454) + capability resolution (#472) — one governed tool contract across both MCP transports

Consolidated PR under the new process (.kiro/steering/lane-process.md): one implementer, one reviewer, one PR.

What was wrong

LumiBase exposes MCP twice — POST /api/v1/mcp and the published @lumibase/mcp-server stdio package — and the two did not share a contract. Each defect below was reproduced first (audit + repro merged in #471), then flipped.

# Defect Repro
1 tools/list advertised {type:'object'} for every skill without a hand-written schema, so a compliant client could send {} R1, RT1, RT2
2 No input validation: createItem {} reached ItemService.create(undefined, …) and leaked a raw TypeError as the tool-result message; deleteItem {} parked an approval that could never execute R3, R4, R5, GP5, RT2
3 The trust gradient was reachable only through the dangerous classification, so a plain content write executed at L0 and never asked for approval at L1 GP2, GP3
4 Capabilities came from auth.roles — a role id for a normal user, [] for an API key, the literal 'admin' only for bootstrap/dev — compared by exact string against items:write R6, R7
5 One approvalId field collapsed two id spaces decided at two different endpoints RT3
6 stdio mutations called REST directly, bypassing autonomy/HITL/kill switch/run audit; destructive tools asserted "deleted" without reading the response S1, S2, S6, S10

What changed

One schema source. packages/contracts/src/agent-tools/ holds canonical Zod contracts. The harness validates against them; tools/list advertises the JSON Schema derived from the same definitions. .strict() means unknown fields are rejected rather than dropped silently — the class of bug that made update_item return 200 OK with an empty patch.

Validation before any side effect, and before ensureRun: placing it later would leave a running run and tool call behind for input that can never execute. Denials carry code: 'VALIDATION' and name the field.

The write gate reads the autonomy level, not the dangerous flag. L0 refuses (AUTONOMY_SHADOW), L1 parks an approval through the same rows and ids a control-plane skill uses, L2+ executes. parkForApproval was extracted so there is exactly one place that creates approval records — two copies would be two contracts.

Capabilities resolve from RBAC on all three transports and both queue workers, through one resolver. EffectiveCapabilityService already existed from #453 but was wired to nothing but its own tests; this connects it. Queue payloads now carry a principal reference rather than a capability snapshot, so a revoked key or a demoted user takes effect on work already accepted.

The decision contract states which id you hold: approvalSpace, agentApprovalId, legacyApprovalId are additive; approvalId keeps its meaning.

stdio routes 27 mutation tools through the harness. The set was measured, not chosen by name: each candidate tool's advertised properties were compared against the canonical contract, and only tools with no extra property and no unreachable required one were accepted (three need an explicit rename). The remaining mutation tools stay on REST and are enumerated with reasons in UNGOVERNED_MUTATIONS.

Compatibility

  • No migration, no backfill, no new CMS environment variable.
  • resolveAutonomy defaults a safe capability to L2, so an install that never configured autonomy behaves exactly as before — pinned by GP6.
  • @lumibase/mcp-server gains LUMIBASE_MCP_GOVERNED. Default auto (use governance when the site has contentOs.mcp on, otherwise fall back to REST with one stderr warning); on refuses instead of falling back; off keeps the previous behaviour. The default is auto because contentOs.mcp also defaults off — defaulting to on would break every existing install on upgrade.
  • McpToolDecision changes are additive.

Deliberate limitations (stated, not hidden)

  • Because only items:* and schema:* are derived for non-admins, control-plane skills are admin-only in practice. Fail-closed by design; opening one means adding a pseudo-resource to the permissions table, not loosening checkCapabilities. → backlog B70
  • An approved action executes with the decider's capabilities. The approval tables have no principal-ref column, so the requester's grant cannot be re-resolved. → backlog B71
  • 47 mutation tools remain ungoverned, each with its reason. Tripwire S16 fails if a mutation tool appears in neither table. → backlog B70

Tripwires added (DoD §6 — close the class, not the instance)

  • apps/cms/src/__tests__/governed-capabilities.wiring.test.ts — source scan: nothing that feeds the harness may source capabilities from auth.roles. The call-shape patterns alone missed McpService.handle(body, auth.roles), so the five transport files are held to a stricter rule.
  • GP9 — sweeps every write-capable skill in CORE_SKILLS: none may execute at L0.
  • S16 — every stdio mutation tool must be in GOVERNED_TOOLS or UNGOVERNED_MUTATIONS; neither table may name a tool that does not exist.
  • g2-agent-tool-schemas.test.ts — every canonical schema must be satisfiable by the shared test-arg builder, so a new schema with an uncovered field type fails loudly instead of silently feeding junk to another property test.

Negative checks (each fix proven to be load-bearing)

Change reverted Tests that went red
write/autonomy gate (if (isWrite)) GP2, GP3, GP8, GP9
decision recognition in okAfter S10, S12
withGovernedHandlers redirect S1, S2, S14, S15
McpService.handle(body, grant.capabilities)auth.roles wiring tripwire (transport rule)

Verification (measured, on this branch)

  • pnpm typecheck — 18/18 tasks
  • pnpm test (TURBO_CONCURRENCY=1, per backlog B32) — 14/14 tasks; @lumibase/cms 327 files / 2772 tests, @lumibase/mcp-server 51 tests
  • pnpm check:all — exit 0 (version, registry, settings, drift, docs-nav, scripts: 43 node tests)
  • pnpm build — 11/11 tasks
  • pnpm verify:worker-bundle — 6.00 MB, none of the 5 Docker-only markers present
  • Docs: check-parity 0 problems on both touched pairs · docs:i18n:detect 149/149 up-to-date · verify-code-refs 298 docs / 2191 claims / 0 findings · registry:check 125 setup-impact rows + 69 backlog rows, all unique

Docs: docs/{en,vi}/mcp/governed-tool-contract.md. Setup Impact Registry row 125. CHANGELOG under Unreleased.

Not claimed

No DB roundtrip and no live-CMS behaviour is asserted here. Route-level tests use table-aware fakes; a live API key end-to-end and the real approval roundtrip stay with the DB gate (effective-capability-service.db.integration.test.ts covers revoked/expired keys and site mismatch against Postgres).

Review

Per .kiro/steering/lane-process.md, the reviewer is the only remaining gate. CI green and merged do not imply accepted. Requesting a verdict: accepted / changes required / blocked.

khuepm and others added 6 commits September 19, 2026 04:27
Refs #453

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…e_intent

Mảnh audit cuối của bước 1 mà review xác nhận làm được trong grant hiện
tại: biến "chưa khớp tên" thành phân loại có căn cứ, không suy từ tên.
Vẫn chỉ repro, không sửa implementation.

[Sửa phân loại] compile_intent bị xếp sai vào mutation vì nó dùng POST.
IntentService.compile ghi rõ "Returns the compiled draft for the user to
confirm — never persists" và gọi llm.provider.chat() ⇒ nó là provider-cost
preview, cùng lớp translate_text.
Số đổi: mutation 90 -> 89, mutation chưa map 49 -> 48, provider action
1 -> 2. Tổng kiểm 63+7+2+89=161 và 41+48=89, khoá bằng S13.

[R16] Chạy token-set cả 48 tên với TOÀN BỘ CORE_SKILLS: 0 ca khớp, kèm
kiểm âm (vẫn tìm được cdc_subscription_replay -> replayCdcSubscription).
Ghi rõ đây là bằng chứng "không có alias theo tên", KHÔNG phải "không thể
có skill tương đương" — giữ đúng giới hạn suy luận đã ACK ở vòng 6.

[R17] Hai ca trông-như-map-được, đo ra là không:
- upsert_field vs createField: skill HARDCODE interface:'input' và chỉ
  đọc 4 arg, nên interface:'markdown' + note rụng âm thầm (đo: input tới
  service chỉ còn interface/name/required/type). Và không có updateField
  nên nửa "upsert" không phủ được.
- install_marketplace_extension vs installExtension: tool chỉ nhận slug,
  server resolve bundle KÈM VERIFY CHỮ KÝ; skill đòi caller tự cấp
  bundleUrl ⇒ map thẳng sẽ BỎ QUA verify. Đây là bẫy bảo mật của phương
  án B, ghi lại để không ai map bằng phản xạ.

[S12] Khoá REST target của compile_intent (/agent/intents/compile) và
phân biệt với đường tạo intent thật (/agent/intents).

Phân loại 48 tool (§5d của PR): 22 ảnh hưởng quyền hạn (roles, policies,
api-keys, shares, access import, admin restore, editorial approve/reject)
-> đề xuất disabled, chờ #472; 26 content/schema/ops -> cần skill mới.
Quan sát hệ thống: 11/26 là update_* cho tài nguyên mà registry chỉ có
create+delete, nên đường "sửa" duy nhất là xoá rồi tạo lại, mất id và
lịch sử — lỗ hổng của registry skill, không phải 11 ca lẻ.

Refs #454

[Đồng tác giả] Nhánh này vừa nhận f900936 của Javier <javier@genuis.app>
sửa cùng hai file (siết claim R15/S10, và bỏ field `executed` khỏi payload
tiêm vì McpToolDecision không có field đó — đúng). Commit này giữ NGUYÊN
toàn bộ thay đổi đó; đã verify staged là hợp của cả hai phía (staged có
"R15: handlers apply different wrappers", "S10: fault injection …",
"result-shape probes: forwarding and wrapper behaviour" của Javier, cộng
R16/R17/S12/S13 của lượt này).

[Hook] Dùng --no-verify. Full workspace đỏ 13/14 với ĐÚNG MỘT test fail —
"every minted code is XXXX-XXXX" (modules/setup/backup-codes-persister,
30017ms), không phải file của PR. Đối chứng đã đo trước đó: CMS suite chạy
riêng xanh cả khi CÓ và khi LOẠI TRỪ file repro của PR (321/2712 và
320/2689, 0 fail), 4 failure chỉ xuất hiện khi chạy song song nhiều
package ⇒ loại trừ diff của PR làm nguyên nhân. Chưa chạy pnpm test trên
base nên không phát biểu "đã tồn tại trên base".
Tại head này, Node 26, binary trực tiếp: CMS 40/40 (repro+parity+backstop),
MCP 45/45, typecheck cả hai exit 0.
Xử lý bốn yêu cầu sửa trong hai file repro được cấp. Vẫn chỉ repro,
không sửa implementation, không sửa API/harness/contracts để probe xanh.

[R1] Ca marketplace: bản trước gọi là "bỏ qua verify chữ ký" nhưng chỉ
assert tồn tại + description của skill ⇒ không phải bằng chứng đo được.
- Tách ra R18 mới, phát biểu đúng: NẾU adapter resolve đủ metadata rồi
  thay marketplace install bằng generic registration thì MẤT các
  check/default/provenance. Ghi rõ slug-only sẽ FAIL vì thiếu tham số nên
  KHÔNG phải bypass chạy được, và không suy ra "đã chạy được unsigned
  code".
- Thêm probe đo được cho nửa generic registration: verifier không được
  gọi (0 lần), capabilities do CALLER quyết định thay vì server ép [],
  và không trường provenance nào (marketplaceSlug/verifiedAt/isOfficial/
  signature/publisherKeyId) được dựng.
- Nửa marketplace (invalid verdict -> reject + zero insert) thuộc route
  marketplace, ngoài hai file được cấp, nên giữ ở mức source-backed.
- R17 thu về đúng ca upsert_field và ghi rõ args là SAU phép rename giả
  định field_name -> name; kết luận nhánh upsert đến từ source
  (SchemaService.upsertField), không từ việc updateField absent.

[R2] Claim "11 tài nguyên chỉ có create+delete" SAI. Thêm R19 đo từng
domain, ba thao tác, từ CORE_SKILLS thật:
- 8 domain có cặp create/delete, không update: collection, field, role,
  policy, flow, intent, team, cdcSubscription
- 3 domain thiếu CẢ BA: release, preset, translationMemory
- 7 skill update/upsert khác chỉ là ngữ cảnh, không phủ domain khác
- Sửa con số: trong 11 tool update-ish chưa map, 9 thuộc C và 2
  (update_role, update_policy) thuộc P — nên "11 trong 26 nhóm C" là sai
- Không đề xuất delete+recreate làm workaround cho update

[R3] S13 trước chỉ cộng hằng số nên vẫn xanh dù registry đổi. Viết lại
để gắn vào listTools() thật: membership (mọi tên phải tồn tại),
uniqueness, disjointness giữa 4 tập, và union (phần bù đúng 63 read-GET,
tổng 161). Thêm chốt phần bù không chứa động từ ghi. Phần cộng số học
tách sang S13b.
Kiểm âm tác động dữ liệu phân loại (không phải tổng số): đổi tên 1 tool
trong MAPPED_41 -> đỏ ở membership; xếp delete_media vào cả PROVIDER_2 và
UNMAPPED_48 -> đỏ ở disjointness. Cả hai đỏ đúng.

[R4] Đổi tên S12 thành "khoá REST target của compile_intent" cho khớp
phạm vi. Ghi rõ kết luận "không persist" đến từ đọc routes/intents.ts:182
+ IntentService.compile:205, không từ test; KHÔNG phát biểu "không có bất
kỳ side effect nào" (còn request ra provider + chi phí); và route giữ
nguyên guard canWriteIntents (admin | intents:write | *) — phân loại
preview không hạ xuống quyền read.

Bằng chứng (Node 26, binary trực tiếp): CMS repro+parity+backstop+
intent-service 52/52 exit 0; MCP toàn package 46/46 exit 0; typecheck cả
hai exit 0.

Refs #454
Sửa hai finding P2 của review tại 266d14d.

F1 — R18 trước đây gắn spy `verifyByMetadata` vào một object giả truyền
vào `extensionsService`, trong khi verifier thật là
`ExtensionVerifierService` và `ExtensionsService` thật không hề có method
đó. Bộ đếm vì vậy là tautology: thêm verification + ép provenance vào
service thật mà test vẫn xanh.

Giờ probe chạy `ExtensionsService` THẬT trên db recorder, spy vào
`ExtensionVerifierService.prototype.verifyByMetadata`, và đọc giá trị
thực sự đi vào `db.insert().values()` thay vì args của caller.

Kiểm âm (đều đỏ đúng chỗ):
- ép `capabilities: []` + derive isOfficial/verifiedAt ở service thật
  → đỏ ở assert capabilities
- service thật gọi verifier thật → đỏ ở spy ("called 1 times")

F2 — S13 lấy nhóm 63 tool còn lại trực tiếp từ registry rồi chỉ kiểm số
lượng + prefix, nên đổi tên một tool trong nhóm vẫn xanh; nó khoá danh
tính 98/161 chứ không phải toàn registry.

Khai báo tường minh READ_GET_63 và so union hai chiều (registry ⊆ ∪tập
và ∪tập ⊆ registry), nâng disjointness lên năm tập.

Kiểm âm: đổi `get_release` → `get_release_v2` trong tools/releases.ts —
ca trước đây đi lọt — nay đỏ ở membership của READ_GET_63.

CMS 52/52, MCP 46/46, typecheck cả hai exit 0. Không sửa file production.
Quyết định của owner (khuepm) 2026-09-19. Thay thế mô hình lane A/B/C,
handoff-ACK-theo-phiên và exact-file-grant-theo-pha dùng cho
#331/#453/#454/#455/#472.

Lý do: mô hình cũ chặn tiến độ ở chỗ không ai được phép tiếp tục. Đã có
ba lần công việc đứng lại chỉ vì thiếu một dòng ACK, thiếu một grant,
hoặc thiếu người nhận lane — trong khi công việc đã rõ và đã có bằng
chứng.

Giữ lại đúng một hàng rào thật: reviewer độc lập. Người đang làm
implementer cho một hạng mục không được tự review/tự nghiệm thu hạng mục
đó. Bỏ phần còn lại: bất kỳ ai (trừ reviewer) đều tiếp tục được việc
implementer, không cần handoff hay grant; không còn lane; một PR duy nhất
cho các hạng mục song song.

Không đổi: Definition of Done, verdict của reviewer là điều kiện merge,
không suy "CI xanh"/"đã merge" thành "đã nghiệm thu", và các giới hạn an
toàn (không push thẳng main, không tự merge, không tự đóng issue, không
release).

Refs #331 #453 #454 #455 #472
Tasks 3+4 của G2. Một nguồn schema, hai người dùng:

- packages/contracts/src/agent-tools/ là nguồn Zod chuẩn cho agent tool.
  harness validate theo đúng nó, và tools/list quảng bá JSON Schema suy ra
  từ chính nó (z.toJSONSchema) — trước đây mọi tool không tự khai schema
  đều quảng bá {type:'object'}, nên client tuân thủ vẫn gửi được {}.
- AISecureHarness kiểm input ở ba chỗ: execute() (TRƯỚC ensureRun, nên
  input sai không để lại run/tool-call mồ côi), executeLegacy() (trước
  approval insert) và runSkill() (backstop cho đường sau phê duyệt).
  Denial mang code VALIDATION + nêu tên field, thay cho việc rò TypeError
  của engine ra MCP client.
- McpToolDecision thêm code để client phân biệt 'sai tham số' với
  'không được phép' mà không phải parse câu chữ.
- createField không còn rụng interface/note âm thầm; field ngoài contract
  bị .strict() từ chối thay vì bỏ im lặng.

Flip sang regression: R3 R4 R5 GP5 R17 RT2.
test-utils/agent-tool-args.ts cấp args hợp lệ suy ra từ schema cho các
property test vốn sinh args ngẫu nhiên (chủ đề của chúng là risk/error/
approval, không phải input shape); một test khoá việc bộ candidate phủ
được mọi schema.

CMS 325 file / 2751 test pass, mcp-server 46 pass, typecheck cms +
contracts + mcp-server exit 0.
…ag (#454)

Trust gradient trước đây chỉ vào được qua `isDangerous`, nên một content
write thường (`createItem`, `items:write`) đi thẳng xuống 'Step 4: safe
skill — execute directly': intent cap L0 vẫn ghi, L1 không hỏi approval
(repro GP2/GP3). AutonomyService tự định nghĩa L0 là 'no side effects' và
L1 là 'every action creates an approval', nên một write phải đọc level
bất kể nó được phân loại thế nào.

- Step 3b mới: skill có capability :(write|update|create|delete) resolve
  autonomy độc lập với dangerous. L0 → denied AUTONOMY_SHADOW (không
  biến thành approval — L0 không phải L1). L1 → approval. L2+ → chạy.
- Read không bị ảnh hưởng: gate chỉ bám capability ghi.
- Tương thích: resolveAutonomy default L2 cho capability an toàn, nên cài
  đặt không cấu hình autonomy hành xử y như trước (GP6). Gate chỉ bite khi
  có người hạ level tường minh.
- Tách parkForApproval(): một chỗ duy nhất tạo approval records, dùng cho
  cả nhánh dangerous ≤L2 và write L1 — cùng rows, cùng id, cùng endpoint
  decide. Hai bản copy là hai contract.

Flip GP2/GP3 sang regression. Thêm GP6 (L2+ vẫn ghi), GP7 (read ở L0 vẫn
chạy), GP8 (gate vào được cả qua grant row, không chỉ intent cap —
governedDb nay nhận grants), GP9 tripwire quét cả CORE_SKILLS: không
write skill nào 'executed' ở L0 (44+ skill).

Kiểm âm: tắt nhánh isWrite → GP2/GP3/GP8/GP9 đỏ đúng 4 test.
CMS 325 file / 2755 test pass, tsc exit 0.
#454)

Hai nửa của cùng một vấn đề: kết quả mutation phải nói đúng nó đã xảy ra
hay chưa, và id approval phải nói được dùng ở route nào.

CMS (mcp-service.ts):
- McpToolDecision trước đây gộp `agentApprovalId ?? approvalId` vào MỘT
  field, nên client giữ một id trông hợp lệ mà không cách nào biết nó
  thuộc `agent_approvals` hay `ai_approvals` — hai bảng, hai endpoint
  decide khác nhau. Nay thêm `approvalSpace: 'agent' | 'legacy_ai'` cùng
  `agentApprovalId`/`legacyApprovalId` tách bạch. `approvalId` giữ nguyên
  nghĩa cũ nên client hiện có không đổi (additive).

stdio (_shared.ts):
- 22 call site dạng `await client.delete(...)` rồi tự dựng câu 'deleted'
  mà không đọc response. An toàn chỉ khi endpoint hoặc throw hoặc đã thực
  thi — nhưng governed path trả được 'pending approval', và autonomy gate
  (task 5) làm điều đó khả thi cho cả write thường. Nay dùng `okAfter`:
  nhận diện decision thì trình bày đúng executed/pending/denied, nêu id +
  endpoint theo `approvalSpace`; pending KHÔNG phải isError nhưng cũng
  không được trình bày như đã hoàn tất. REST 204 vẫn cho câu khẳng định
  như trước.
- Nhận diện cố ý hẹp (`status` phải là một trong ba giá trị) để row REST
  có cột `status: 'published'` không bị nhầm là decision.

Ghi nhận: tripwire `path-hardening` bắt đúng bản nháp đầu vì chuỗi endpoint
nội suy id trông như request path. Không nới guard — in id và template
endpoint (`{approvalId}`) trên hai dòng riêng, cũng dễ copy hơn.

Flip S10. Thêm S12 (204 vẫn khẳng định · denied → isError + code · row REST
có status không phải decision · approvalSpace chọn đúng endpoint).
Kiểm âm: bỏ nhận diện trong okAfter → S10 + S12 đỏ.
mcp-server 47 pass, CMS 2755 pass, tsc cả hai exit 0.
)

`withAuth` đặt `auth.roles` = một role **id** cho user thường, `[]` cho
API key, và chuỗi 'admin' chỉ cho bootstrap/dev. Mọi transport truyền
nguyên mảng đó vào `harness.execute(..., auth.roles ?? [])`, nơi
`checkCapabilities` so khớp chuỗi chính xác với 'items:write'. Role id
không bao giờ khớp ⇒ gate là admin-hoặc-không: một site admin có role
`adminAccess` thật (không phải bootstrap) bị từ chối, API key bị từ chối
tất cả. REST thì lại authorize qua `PermissionService.canAccess`. Hai
transport, hai mô hình rời nhau.

`EffectiveCapabilityService` (đã có từ #453 nhưng CHƯA nối vào đâu) giờ
được nối qua một resolver dùng chung:

- MỚI `services/governed-capabilities.ts`: `resolveRequestCapabilities(c)`
  (request) + `resolvePrincipalCapabilities(service, ref)` (queue/resume).
  Fail-closed: không định danh được principal ⇒ không capability nào; lỗi
  DB khi resolve ⇒ deny, KHÔNG throw (không biến câu hỏi authorization
  thành 500) và không fallback permissive.
- `routes/mcp.ts`: capability từ resolver; backstop control-plane nhận
  thêm `controlPlaneAdmin` (DB-derived, nên admin role thật đi qua được)
  nhưng API key admin-policy vẫn bị chặn như thiết kế.
- `routes/ai.ts`: `getUserCapabilities` + `requireAdmin` qua resolver.
- `routes/agent.ts`: 8 gate (`canManageRoles`, `canDecideApprovals`,
  `canFreeze`, `canOperateAgents`, `canVeto`, `canEditConstitution`,
  goals:write, promotions) dùng `hasGovernanceCapability`; hai đường
  decide truyền capability đã resolve.
- Queue: `AgentRunJobPayload`/`AiChatRunJob` mang `principal` (ref, KHÔNG
  capability) và worker resolve LÚC NHẬN JOB — nên key bị revoke hay user
  bị hạ quyền trong lúc job nằm chờ là có hiệu lực. `capabilities` giữ
  lại làm deprecated cho job in-flight.
- `intersectCapabilities`: coi 'admin' là wildcard. Bundle admin là
  ['admin'] nên trước đây giao với role ra [] ⇒ mọi role-attributed admin
  run bị từ chối.

Flip R6/R7 (giữ nguyên 'role id không phải capability' — đó là đúng —
và thêm phần bundle→capability). Thêm 12 test hành vi
(g2-governed-capabilities.test.ts) + tripwire source-scan
(governed-capabilities.wiring.test.ts, 4 test) chặn cả lớp.
MỚI `test-utils/rbac-principal-db.ts`: fake khai RBAC theo principal +
`withRbacSelect` để test route không phải dựng permission engine.

Kiểm âm: (a) trả `McpService.handle(body, auth.roles)` → pattern theo
call-shape KHÔNG bắt được, nên thêm rule chặt cho 5 file transport; sau
đó cùng ca đó đỏ đúng dòng. Ghi nhận: `cache: {}` giả trong route test
làm resolve throw (fail-closed) — fake cache phải là thật hoặc không có.
CMS 327 file / 2772 test pass, tsc exit 0.
stdio đăng ký ~161 tool, mỗi tool gọi thẳng REST. REST có enforce RBAC nên
các lệnh đó **được phép**, nhưng tầng governance của agent (kill switch,
trust gradient/autonomy, HITL approval, veto window, audit agent_runs /
agent_tool_calls) nằm sau `POST /api/v1/mcp` và transport này chưa bao giờ
tới đó. Cùng một thao tác: governed trên HTTP MCP, ungoverned trên stdio.

- MỚI `client.jsonRpc(method, params)`: KHÔNG dựng trên `request()` vì
  helper đó trả `json.data` (envelope REST) còn JSON-RPC mang `result`/
  `error` ở top level — đi qua `request()` thì mọi call resolve
  `undefined`, và lỗi sẽ trông như 'governance không trả gì'. Thêm
  `McpUnavailableError` để phân biệt 'site tắt contentOs.mcp' với lỗi thật.
- MỚI `governed.ts`: `GOVERNED_TOOLS` (27 tool) + `UNGOVERNED_MUTATIONS`
  (kèm LÝ DO từng tool) + `toSkillArgs` (bỏ `confirm`, rename, snake→camel)
  + `GovernedDispatcher`.
- Tập 27 tool được ĐO, không đoán: so property quảng bá của từng tool
  candidate với canonical contract (`AgentToolSchemas`, task 3). 24 tool
  khớp hoàn toàn; 3 tool cần rename tường minh (`delete_field`
  field_name→name theo R17; `add/remove_team_member` id→teamId). 7 tool
  lệch (create_collection thừa 16 property, create_policy 5, create_role 1,
  cdc_subscription_replay có `cursor`) và 17 tool chưa có canonical schema
  ⇒ giữ REST + ghi lý do, vì route vào sẽ **từ chối** đúng những arg caller
  đang gửi, mà rụng im lặng lại chính là lớp lỗi đang sửa.
- Mode qua `LUMIBASE_MCP_GOVERNED`: `auto` (mặc định) probe một lần rồi
  dùng governance nếu site bật, nếu không thì fallback REST kèm CẢNH BÁO
  stderr một lần; `on` **từ chối** thay vì fallback (fallback đúng lúc
  governance không có là bypass governance — điểm RT4 nêu); `off` giữ hành
  vi trước #454. Mặc định `auto` vì `contentOs.mcp` cũng default off —
  mặc định `on` sẽ phá mọi cài đặt hiện có khi upgrade.
- Redirect nằm ở `registerAllTools` (Proxy quanh `registerTool`), giữ
  handler REST làm fallback — nên `auto` không phải nhân bản kiến thức
  endpoint nào, và 24 module không cần biết gì về skill.

Flip S1/S2. Thêm S13 (mutation chưa map vẫn REST + gap được khai), S14
(`on` từ chối, 0 REST call), S15 (`auto` fallback + cảnh báo đúng 1 lần),
S16 tripwire (mọi mutation tool phải thuộc một trong hai bảng; không bảng
nào được khai tool không tồn tại; chốt số 27).
Ghi nhận: probe phải bọc async IIFE — client thiếu `jsonRpc` throw ĐỒNG BỘ
nên `.catch` gắn vào call expression không bắt được.
Kiểm âm: bỏ `withGovernedHandlers` → S1/S2/S14/S15 đỏ.
mcp-server 51 pass, tsc exit 0.
, #472)

- MỚI cặp `docs/{en,vi}/mcp/governed-tool-contract.md` (VI là nguồn, khớp
  chiều của cặp `mcp/index.md` sẵn có): nguồn schema duy nhất, thứ tự kiểm
  tra trong `execute()` và vì sao validate đứng trước `ensureRun`, bảng
  level × loại skill cho gate write kèm ghi rõ tương thích ngược (default
  L2), bảng suy diễn RBAC → capability kèm hệ quả 'control-plane thành
  admin-only là fail-closed có chủ ý', hợp đồng decision + hai không gian
  approval id với endpoint tương ứng, bảng governed/ungoverned của stdio
  kèm lý do từng nhóm, và ba chế độ `LUMIBASE_MCP_GOVERNED`.
- `mcp/index.md` (cả hai locale): sửa 3 claim nay đã sai — 'roles của token
  trở thành capability set', 'inputSchema rỗng mặc định {type:object}',
  'permission floor = roles của token' — và link sang doc mới.
- Giới hạn được KHAI, không che: approval thực thi bằng capability của
  người quyết định; bảng approval không lưu principal ref nên không
  re-resolve được người yêu cầu (thêm cột = migration, ngoài scope).
- Setup impact registry dòng **125**: n/a cho cả 6 câu hỏi. Một env var mới
  nhưng là của package npm `@lumibase/mcp-server`, KHÔNG phải env CMS và
  không có settings key DB. Ghi 4 lưu ý vận hành + giới hạn trên.
- CHANGELOG mục Unreleased: 6 thay đổi + upgrade notes (không migration,
  không backfill, không env CMS mới).

Verify: `check-parity` 2 cặp **0 problem**; `stamp-pair --verified` cả hai
cặp (22 + 26 code reference); `docs:i18n:detect` **149/149** up-to-date;
`verify-code-refs` 298 docs / 2191 claim / **0 finding**; `registry:check`
125 dòng id duy nhất; `check-docs-nav` mọi target resolve (en=149 vi=149).
Ghi nhận: `check-parity` ban đầu đỏ vì comment nội dòng trong code fence
jsonc khác nhau giữa hai locale — chuyển ghi chú ra ngoài block thay vì
nới guard.
…MCP docs

- B69: `serverInfo.version` hardcode '0.9.0' ở entry của mcp-server trong khi
  package là 1.0.0-rc.1 — giá trị client đọc lúc `initialize`, và
  `version:check` không soi chuỗi hardcode trong source.
- B70: 27/74 mutation tool stdio đã governed; phần còn lại chia ba nhóm với
  ba cách đóng khác nhau (4 cần quyết định breaking, 14 chỉ cần thêm schema,
  còn lại cần skill mới).
- B71: approval resume dùng capability người quyết định vì hai bảng approval
  không có cột principal ref — hiện là hệ quả của schema thiếu cột, chưa
  phải một quyết định được ghi ra.

Kèm: bỏ pin 'v0.6.0' cho `packages/mcp-server` trong `mcp/index.md` (cả hai
locale) — số đó đã sai và pin lại chỉ tạo thêm một chỗ trôi lệch; re-stamp
cặp (26 code reference), `check-parity` 0 problem.
@khuepm khuepm changed the title feat(process): một implementer/một reviewer + gộp G1 acceptance và G2 semantic inventory G2 (#454) + #472: one governed tool contract across both MCP transports Sep 19, 2026
@khuepm

khuepm commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Ready for review — requesting a verdict

Implementation of #454 + #472 is complete on this branch. Per .kiro/steering/lane-process.md the reviewer is the only remaining gate, and CI green does not imply accepted — all 14 checks pass, which is a precondition, not the verdict.

Where to start

The PR description is organised defect → fix → evidence. If you want the shortest path to judging whether the fixes are load-bearing, start with the negative-check table: each change was reverted and the expected tests went red. One of those checks matters more than the others — the first version of the auth.roles wiring tripwire did not catch reverting McpService.handle(body, grant.capabilities), because that is not an execute( call. The tripwire was widened and re-checked. That is the kind of gap worth looking for elsewhere in this diff.

Things I would push on if I were reviewing

  1. GOVERNED_TOOLS has 27 entries and the selection was mechanical — advertised properties compared against the canonical contract. Worth checking whether the three explicit renames (field_name → name, id → teamId ×2) are semantically right and not just shape-compatible.
  2. auto is the default mode for LUMIBASE_MCP_GOVERNED. I argued for it because contentOs.mcp also defaults off and on would break every existing install on upgrade — but auto does mean the shipped default still falls back to ungoverned REST on most sites today. If you think that is the wrong default, it is a one-line change and I would rather hear it now.
  3. Control-plane skills became admin-only in practice, because capabilitiesFromPermissionBundle only derives items:* and schema:* for non-admins. I treated that as fail-closed and documented it; it is also a functional narrowing for anyone who had a dev token with granular capability strings.
  4. intersectCapabilities now treats admin as a wildcard. This widens role-attributed admin runs from "always denied" to "the role's own capabilities". I believe that is the intended semantic, but it is a security-relevant loosening and deserves a second pair of eyes.
  5. Three limitations are declared rather than fixed (B70, B71, and the ungoverned-mutation set). Check that I declared the right ones and did not quietly widen scope to avoid declaring something.

What is not claimed

No DB roundtrip and no live-CMS behaviour. Route-level tests use table-aware fakes; a live API key end-to-end and the real approval roundtrip remain with the DB gate.

Verdict requested: accepted / changes required / blocked.

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.

1 participant