Repository navigation
skills: third-party developer review — fix fabricated/stale surface across all 9 skills - #3374
Merged
Merged
Conversation
…DME framing Third-party review of the published skills bundle (installed via `npx skills add objectstack-ai/framework/skills --all` into a scaffolded app) found the references layer leaking monorepo-only surface: - build-skill-references.ts listed transitively-imported src files that are NOT in the published package (only src/**/*.zod.ts ships) — lazy-schema.ts / visibility.ts / public-auth-features.ts pointers 404 in a consumer's node_modules. The generator now traverses through non-shipping files but never lists them. - Add missing load-bearing schemas to the map: ai/knowledge-source + knowledge-document (the real RAG primitives), automation/ time-relative-trigger + data/validation (state-machine rules), ui/dataset, query date-macros. - Give objectstack-formula a references/_index.md like its 8 siblings (it previously pointed at monorepo-relative packages/ paths instead). - Mark the regen note maintainers-only and stop claiming the root barrel re-exports every symbol (it doesn't — e.g. DATE_MACRO_TOKENS lives on @objectstack/spec/data only). - skills/README.md: reposition the bundle as serving any ObjectStack app (not 'the monorepo'), document the install commands and the /skills catalog boundary (#3101), and describe the real skill anatomy (rules/, contracts/, evals/). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
Third-party review found the skill teaching schemas that don't exist in
@objectstack/spec 16.x:
- The 'RAG Pipeline Configuration' section (sources[]/indexes[]/
retrieval{topK,scoreThreshold,reranker}) matched no schema — the spec
explicitly scopes chunking/retrieval/reranking out of the platform.
Rewritten around KnowledgeSourceSchema (object/file/http sources,
contentFields, where, adapter binding, refresh policy), with an
os:check'd example, and honest registration guidance
(IKnowledgeService.registerSource — no defineStack collection).
- Tool section now teaches first-class defineTool metadata (category
enum, JSON-Schema parameters, required label) with the spec's own
READ-ONLY-PROJECTION caveat (hand-authored tool metadata does not
execute in the open edition), separated from the legacy inline
agent tools[] shape.
- structuredOutput: retry:{maxAttempts} → maxRetries (real field);
guardrails pitfall no longer cites nonexistent requireApprovalFor.
- Agent-level table no longer lists temperature/maxTokens (they live
under model); adds surface/planning/memory/permissions.
- Coherence: description and blueprint now lead with skills + tools +
knowledge sources; *.agent.ts flagged platform-internal (ADR-0063 §2)
instead of being taught to third parties against a nonexistent
src/{agents,skills,rag} reference layout.
- Auto-exposed actions + HITL sections labeled Cloud/EE runtime;
actionSkipReason import caveated as cloud-only.
- evals/README.md: replace data-skill boilerplate with AI-domain evals.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
…eality Third-party review findings applied: - P0: formula fields carry their CEL in `expression`, not `formula` (field.zod.ts; the engine reads def.expression) — the wrong key produced zod-stripped, never-evaluated formulas. - Dialect set is cel/cron/template — the js dialect was retired (#3278); procedural JS is the L2 ScriptBody surface, not an expression dialect. - daysFromNow/daysAgo are calendar-day functions (today() ± n at UTC midnight) — the time-of-day claim was inverted; today() documented as reference-timezone calendar day at UTC midnight (ADR-0053). - Removed surface rows that match no schema: View.criteria, Workflow.Task.dueDate, Workflow.criteria, ai/orchestration, ai/devops-agent, ai/agent-action, ai/nlq, ai/mcp.systemPrompt. - os.org context is { id, tier } — dropped os.org.slug/name examples; documented the current_user binding (ADR-0068). - titleFormat marked deprecated (→ nameField, ADR-0079); model-registry template keys corrected to promptTemplate.system/user. - Replaced monorepo-relative links (packages/, content/docs/) with node_modules paths that resolve in a consumer app; dropped ROADMAP/north-star pointers and the nonexistent build-twice CI claim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
Third-party review found the skill's narrative built on the aspirational
o.* AppTranslationBundle pipeline while every shipped consumer (runtime
resolvers, defineStack translations, coverage CLI, os i18n extract,
example apps) runs on objects.* TranslationData:
- Primary authoring path is now TranslationData + defineTranslationBundle
wired via defineStack({ translations }) with an os:check'd example;
AppTranslationBundle demoted to a clearly-labeled Studio/workbench
secondary format with a do-not-register warning.
- CLI truth: os i18n suggest / os i18n coverage don't exist — workflow
now uses os i18n extract (--locales/--out/--fill, emits TS modules,
not JSON) and os i18n check --locales / os lint --i18n-strict.
- Interpolation is {{var}} (both shipped adapters) — single-brace
examples silently no-op'd; ICU section marked EXPERIMENTAL quoting the
schema's own not-enforced annotation.
- getCoverage/suggestTranslations/getAppBundle/loadAppBundle flagged as
optional contract methods with no shipped implementation.
- localesDir loads only flat {locale}.json; fileOrganization has no
runtime consumer — layouts labeled authoring conventions.
- defaultLocale/supportedLocales marked required; cache/lazyLoad marked
unenforced; _options keyed by field name; namespace prefixing claim
dropped; unshipped contracts/i18n-service.ts pointer replaced with the
subpath type import; evals README rewritten for i18n.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
…havior
Third-party review found ~a third of the skill documenting QueryAST
surface the ObjectQL engine and drivers never execute, mostly failing
silently with wrong results. A consistent 'Schema-reserved — NOT
executed by the engine yet' caveat style now separates spec from
runtime:
- P0 cursor pagination: engine never reads cursor (silently returns
page 1 forever) — replaced the recommendation with manual keyset
pagination (where + orderBy + limit).
- joins, window functions (SQL builder even drops the field argument),
HAVING, per-aggregation FILTER (won_deals silently equals
total_deals), $field cross-field comparisons, most full-text options
(only query+fields execute; terms AND-ed), top-level distinct — all
caveated with working alternatives; KPI pattern rewritten as separate
aggregate calls.
- Expand: select + filter only; per-parent limit/offset/orderBy are not
applied — paginate the related object directly.
- SQL vs in-memory aggregation support matrix added (SQL throws on
count_distinct/array_agg/string_agg); fabricated 'groupBy fields MUST
appear in fields[]' rule deleted.
- Documented two features that DO work and were missing: structured
groupBy date bucketing ({ field, dateGranularity, alias }) and
date-macro tokens ({today}, {30_days_ago}) with their real
client-side resolution scope.
- Description stops headlining joins/window functions; evals README
rewritten for the query domain.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
Third-party review found the skill's core payload — the shapes a
developer would copy — substantially fabricated:
- P0: generated CRUD routes live under /api/v1/data/{object} (the /data
prefix was missing everywhere); there is no /bulk and no GET
/aggregate — batch is POST /data/{object}/batch (+ cross-object
POST /api/v1/batch), aggregation goes through POST .../query.
- P0: the custom-endpoint example matched no schema — rewritten to
RestApiEndpointSchema (method/path/handler/category/public/
permissions/requestSchema string refs/handlerStatus) with
ApiEndpointSchema as the declarative alternative.
- API-methods table: upsert/history/restore/purge are enum gates with
no generated route; search is the global /api/v1/search.
- Service-info, health (/api/v1/health + /ready, envelope shape),
auth config (public/permissions/authRequired + real
RateLimitConfigSchema), datasource (defineDatasource, driver ids
postgres/mysql/mongo/sqlite/memory), inter-service
(kernel.getService<IDataEngine>), and error-envelope guidance all
rewritten against the actual Zod sources; five examples now carry
os:check markers and type-check.
- Added the missing realtime-subscription and auth-provider sections
the description promised; documented environment-scoped route
mounting; dropped unresolvable ADR/issue tokens and stale 9.x/4.0.1
version framing; evals README rewritten for the API domain.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
…accepts Third-party review found SKILL.md's security/advanced sections and rules/field-types.md carrying a thick stratum of pre-16 material — generated code would either throw at ObjectSchema.create() or ship silently dead metadata: - 49 field types (not 48); added the missing secret/user/composite/ repeater/record types; password row corrected (generic path stores plaintext at rest — recommend type:'secret', ADR-0100). - referenceFilters (removed #2377/ADR-0049) → structured lookupFilters across SKILL.md and all rules files. - summary fields use summaryOperations {object, field, function}; formula fields use expression + returnType (no 'currency') with record.-prefixed CEL; vector fields use flat dimensions. - enable.feeds/activities default TRUE (opt-out, not opt-in). - tenancy is strict { enabled, tenantField } — the shared/isolated/ hybrid mode table was a retired key (#2763); dropped softDelete/versioning/partitioning/cdc checklist rows (retired keys that throw) and the pruned encryptionConfig/maskingRule sections in favor of type:'secret' + field requiredPermissions (ADR-0066 D3). - Object-level permissions:{read:[roles]} shape doesn't exist — definePermissionSet boolean CRUD bits; RLS policies take singular operation + required object; current_user.roles → current_user.positions (ADR-0090 D3). - defineObject/defineHook are not exported — ObjectSchema.create + Hook objects via defineStack({ hooks }); object extensions via defineObjectExtension registered in objectExtensions. - rules/field-types.md Config columns purged of ~20 nonexistent knobs (silently-stripped dead metadata); hooks reference corrected (onError default abort, after-hooks in-transaction unless async, update(..., { multi: true }), CEL conditions, no aggregate in api.read); broken monorepo links replaced with plain text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
Third-party review found the five load-bearing examples broken against
the schemas the skill itself cites:
- Cron cadence lives in the START NODE's config.schedule ({ type:
'cron', expression } or bare string) — the top-level schedule: key is
silently stripped and the flow never binds to the scheduler.
- update_record/get_record filters are ObjectQL where MAPS ($in/$lt),
not {field,operator,value} triples (that's the UI view-filter
dialect); date values use {TODAY() + N} template tokens — cel
envelopes are not evaluated in CRUD filter values.
- Decision routing is edge-condition driven; unguarded out-edges all
run in parallel — the old pattern executed both branches. Edges now
carry mutually-exclusive cel conditions.
- FlowSchema requires label on every flow and node, and variables is an
array of { name, type, isInput } — all examples now pass
FlowSchema.parse (runtime-validated via defineFlow).
- http_request → http (retired alias), query_record → get_record
(NO_EXECUTOR throw), object: → objectName:; node table regenerated
from the 20-value FlowNodeAction enum (adds notify/map; script row =
registered callables only, inline JS never executes).
- Approval behavior enum completed (quorum/per_group + minApprovals +
per-approver group); waitEventConfig documented as canonical; unset
http timeoutMs = no timeout at all; removed the false claim that bare
field refs in start/edge conditions resolve to null; description
stops advertising the retired workflow metadata type; evals fixed
(expected output now passes its own criterion).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
…act generator Third-party review found a dense cluster of P0 schema errors that made generated UI metadata fail os validate or render unknown components: - Page types are record|home|app|utility|list — record_detail/ app_launcher/utility_bar don't exist; page examples fixed (type: 'record', object: not objectName:) and all definePage/defineDataset examples now carry the required label. - Interface-page rule updated to the revised ADR-0047 model: the page IS the view definition (page-owned columns/sort/filterBy); sourceView is a deprecated fallback. - Report types = tabular|summary|matrix|joined (charts are the embedded chart: config); dashboard widget type is the 18-value ChartTypeSchema taxonomy; widget placement is optional layout (not required position). - Action locations: global → global_nav (+record_related/ record_section); nav divider → separator; requiresCapability → requiresService; component names record:related_list, line_items, object-metric/object-chart, <object-grid> — all verified against the objectui block registry. - DATE_MACRO_TOKENS/isDateMacroToken import from @objectstack/spec/data (not root); monorepo-relative ADR/examples/content pointers replaced with plain text or shipped zod paths; stale dated objectui coverage scan removed. - build-react-blocks-contract.ts no longer marks .default()-carrying props as required (Zod v4 output-io artifact); contract + reference regenerated; provenance now cites the @objectstack/spec/ui module instead of a non-shipping src path; evals cleaned of internal issue refs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
…icated surface Third-party review found the skill's core plugin-development narrative teaching a data:* kernel hook system that does not exist in the codebase — ctx.hook() accepts any string, so handlers registered against data:beforeInsert etc. silently never fire (the worst failure mode). Also fabricated wholesale: six adapter packages, three driver packages, three scaffold templates, and the Part-3 testing API. - data:* excised from four files; replaced with the real kernel events (kernel:ready|bootstrapped|listening|shutdown, app:seeded, metadata:reloaded, external.schema.drift) with payloads read from the actual trigger sites, plus a prominent record-lifecycle-belongs-to- objectstack-data cross-reference. metadata:changed → metadata:reloaded. - Adapters: only @objectstack/hono + @objectstack/plugin-hono-server exist. Drivers: driver-memory/driver-sql (SqlDriver via Knex)/ driver-mongodb/driver-sqlite-wasm; driver-turso marked cloud/EE-only; driver-postgres/-sqlite/-d1 and SQLiteDriver/PostgreSQLDriver removed. - Templates corrected to blank|todo|compliance|content|contracts| procurement; LiteKernel testing rewritten to the real surface (use/bootstrap/getService — no create/api/seed/reset). - defineStack key list regenerated from ObjectStackDefinitionSchema (41 keys; workflows/approvals/ragPipelines/roles/policies/driver/ logging are silently-stripped phantoms — ADR-0019/0020 noted); bootstrap-patterns.md rewritten around DriverPlugin + a valid manifest (id/version/type/name required). - CLI truth: no os studio / os data seed / os meta apply; os diff/ cloud/start/info descriptions corrected; /health + /ready; Console mounts at /_console/ (studio plugin intentionally not default- loaded); HonoServerPlugin registers before AuthPlugin; port 3000. - Service registry: required key is data; REST plugin id com.objectstack.rest.api registers no service; registerService throws on duplicate; fallbacks are createMemory* factories; null-placeholder anti-pattern removed; deps at ^16.0.0-rc.1 mirroring the blank template; evals README rewritten for platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
Descriptions changed in the review wave: query stops headlining schema-reserved joins/window functions; ai leads with skills/tools/ knowledge sources; automation drops the retired workflow type; platform drops the nonexistent data:* hooks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 104 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Replace the two new 'role' usages introduced by the review wave (junction-attribute example → position; phantom-key example → approvals:) and ratchet the baseline down for the three files whose role-word count improved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
os-zhuang
marked this pull request as ready for review
July 21, 2026 00:56
os-zhuang
pushed a commit
that referenced
this pull request
Jul 21, 2026
All ten patterns were dropped verbatim into a scaffolded 16.0.0-rc.1 app
and run through npm run validate — twice: as-published (passes, proving
the gate does NOT catch these) and as-fixed. The fixes target
runtime-fatal issues invisible to validate:
- Both flow patterns lacked the start-node trigger binding
(config.objectName + triggerType: 'record-after-update' …) — they
validated but never fired; approval-flow nodes (auto_approve /
request_approval / update_record) had no config at all, making them
no-ops. Bindings, node configs, status: 'active', and a CEL start
condition added.
- reference: 'user' → 'sys_user' (no 'user' object exists; lookups
silently never resolve).
- notify recipient '{record.assigned_to.email}' → '{record.assigned_to}'
(recipients are user ids; path-walking a string yields empty
recipients and a node error).
- Formula field: bare 'quantity * unit_price' →
'record.quantity * record.unit_price' + returnType (bare identifiers
resolve to null in the CEL scope).
- Cursor-pagination section replaced with keyset pagination (cursor is
schema-reserved, zero engine/driver execution — same finding as
PR #3374).
- Frontmatter description no longer promises realtime; import callout
gains definePermissionSet.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
This branch was successfully deployed
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
2 participants
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.
概述
以第三方开发者的立场对发布的 skills bundle 做了一次完整审核与修复。模拟真实消费路径验证：
npm create objectstack@16.0.0-rc.1→npx skills add objectstack-ai/framework/skills --all→npm install→npm run validate全链路跑通（发布的@objectstack/spec确实携带 216 个src/**/*.zod.ts，skills 的 spec 指针在用户项目中可解析）。随后对 9 个 skill 逐一对照
@objectstack/spec16.0.0-rc.1 的 Zod 源码与运行时（engine / drivers / CLI / rest / core）核实每条载荷性论断，共确认并修复 40+ 个 P0 事实错误（按此生成的代码要么构建即抛错、要么静默产出死元数据）和大量 P1 第三方不可用问题。系统性问题模式
data:*内核钩子（注册后永不触发）、query 的 cursor/joins/窗口函数/HAVING/FILTER 聚合/$field（多数静默返回错误结果）、api 的幻造路由与配置形状、i18n 的o.*管线与不存在的 CLI 子命令、automation 的顶层schedule:（被静默剥离，定时流永不运行）。referenceFilters、encryptionConfig/maskingRule、softDelete/versioning、tenancy 三模式表、record_detail等已删除/重命名的键与枚举。../../docs/adr/*.md、packages/*、examples/*、content/docs/*相对链接在.agents/skills/安装布局下全部失效；references/_index.md列出了不随包发布的非.zod.ts文件。LiteKernel.create/api/seed/reset、defineObject/defineHook、kernel.resolve()、os studio等。主要修改
build-skill-references.ts：只输出符合发布 glob 的指针；补充缺失 schema（knowledge-source、time-relative-trigger、validation、dataset、date-macros）；为 formula 补齐 references/_index.mdbuild-react-blocks-contract.ts：带.default()的 props 不再标记为 required；provenance 改为可解析的模块路径skills/README.md：定位改为面向所有 ObjectStack 应用；写明/skills目录边界（fix(create-objectstack): scope skill installs to the curated skills/ catalog #3101）与真实 skill anatomy验证
check:skill-examples：32 个os:check示例全部通过类型检查（修复前 21 个，新增 11 个带标记的可编译示例）check:skill-refs/check:skill-docs/check:react-blocks：生成文件全部同步pnpm --filter create-objectstack test：18/18 通过（skills 目录边界守护）pnpm --filter @objectstack/formula test：242/242 通过（stdlib 漂移防护）defineFlow()/FlowSchema.parse运行时验证关联
npx skills add objectstack-ai/framework（缺/skills子路径）及虚构的@v5.0.0固定版本语法 —— 见 objectstack-ai/objectos 的同名分支 PR。_index.md一行摘要取自文件首个 JSDoc 导致的误导性描述。🤖 Generated with Claude Code
https://claude.ai/code/session_012Kj2MJEPXoUXC4LiC3XYJc
Generated by Claude Code