Skip to content

docs(app-crm): point two stale approval comments at the real consumers - #14840

Merged
os-sales merged 1 commit into
mainfrom
claude/issue-14516-crm-stale-approval-comments
Sep 3, 2026
Merged

os-sales merged 1 commit into
mainfrom
claude/issue-14516-crm-stale-approval-comments

Conversation

@claude

@claude claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #14516

Two comments in examples/app-crm named a "Discount Approval" flow that this app does not contain. These examples are copy-source for AI authors, so a comment naming an absent flow teaches a false shape. This PR rewrites both comments to name what is actually in the tree. Comment-only — no declaration, expression, or export changed.

Scope

Resolution 1 of the card only, per the triage ruling on the issue.

  • Resolution 2 (add a Discount Approval flow to the CRM) is a feature, and it argues against src/flows/index.ts's stated one-flow scope. Out of scope here.
  • Resolution 3 (delete FinanceApproverPosition as an unreferenced declaration) rests on a false premise and is not acted on. The position is live scaffolding with a real consumer; deleting it would break objectstack.config.ts and the permission-set binding. Fence proof below.

Site 1 — examples/app-crm/src/objects/opportunity.object.ts

Before:

    // Mirror target for the Discount Approval flow's approval nodes
    // (ADR-0019). The approval runtime writes the request status here; it is
    // readonly to users so only the flow drives it.

After:

    // Mirror target for an approval node's `approvalStatusField` (ADR-0019):
    // the approval runtime writes the request status here, so it stays
    // readonly to users. This app declares no approval flow of its own — the
    // CRM keeps one flow on purpose (src/flows/index.ts); the worked approval
    // flows are examples/app-showcase/src/automation/flows/, and the
    // `approvalStatusField` wiring is shown in skills/objectstack-automation/SKILL.md.

Why: examples/app-crm/src/flows/ holds only convert-lead.flow.ts and index.ts, and flows/index.ts states the split deliberately — "Automation breadth (approvals, schedules, connectors, subflows, …) is the showcase's job". The mechanism the field serves is real and is now named: an approval node's approvalStatusField config key (ADR-0019; packages/plugins/plugin-approvals/src/approval-service.ts reads it into mirrorField). The worked approval flows are in examples/app-showcase/src/automation/flows/, and the approvalStatusField: 'approval_status' wiring is spelled out in skills/objectstack-automation/SKILL.md.

Site 2 — examples/app-crm/src/security/sales-positions.ts

Before:

/** Referenced by the Discount Approval second step. */

After:

/**
 * Registered in `objectstack.config.ts` (`positions`) and bound to the
 * `crm_sales_user` permission set by `bind-position-sets.ts`, alongside
 * `sales_rep` and `sales_manager`. This app models no approval flow; the
 * worked approval flows live in `examples/app-showcase/src/automation/flows/`.
 */

Why: there is no "Discount Approval second step" anywhere in the repo. The comment now names the position's actual consumers.

Fence — FinanceApproverPosition is still referenced

git grep -n "FinanceApproverPosition" HEAD -- examples/app-crm on the branch head:

examples/app-crm/objectstack.config.ts:17:  FinanceApproverPosition,
examples/app-crm/objectstack.config.ts:100:  positions: [SalesRepPosition, SalesManagerPosition, FinanceApproverPosition],
examples/app-crm/src/security/index.ts:6:  FinanceApproverPosition,
examples/app-crm/src/security/sales-positions.ts:28:export const FinanceApproverPosition = definePosition({

git grep -n "finance_approver" HEAD -- examples/app-crm/src/security/bind-position-sets.ts:

examples/app-crm/src/security/bind-position-sets.ts:10: * (`sales_rep`, `sales_manager`, `finance_approver`) and a `crm_sales_user`
examples/app-crm/src/security/bind-position-sets.ts:33:  ['finance_approver', 'crm_sales_user'],

Import, registration and binding are all intact — nothing was removed.

Comment-only proof

Two independent measurements, both zero-difference:

  1. Diff line filter. Of the 16 changed lines in git diff BASE..HEAD (BASE 2d40f9146), the count of changed lines that are not comment or blank lines is 0.
  2. Token-stream equality. Both files were tokenised with the repo's own TypeScript (ts.createScanner, skipTrivia: true) at BASE and at HEAD:
opportunity.object.ts: base tokens=605 head tokens=605 identical=true
sales-positions.ts:    base tokens=266 head tokens=266 identical=true
RESULT: comment-only (non-trivia token streams identical)

Verification — at branch head 94d444d87

Command Result
pnpm --workspace-concurrency=2 --filter '@objectstack/example-crm^...' build VERDICT command-exit 0
pnpm --filter @objectstack/example-crm run typecheck (tsc --noEmit) VERDICT command-exit 0
pnpm --filter @objectstack/example-crm exec vitest run --maxWorkers=2 Test Files 5 passed (5) · Tests 45 passed (45)
pnpm check:nul-bytes OK (scanned 8070 text file(s) … no raw ASCII control bytes)

Gate family re-derived from the real diff with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (2 paths vs merge base 2d40f9146) — 15 families, all run:

  • 13 green: check-comment-mask-adoption, check-cross-package-test-inputs, check-keyed-text-bounds, check-position-name-fold-loaders, check-shard-attestation, check-system-context-census, check-undeclared-dep-imports, check:examples-live-imports, check:logger-receiver-detach, check:page-declaration-shape, check:published-files, check:test-source-alias, check:type-source-resolution.
  • 2 NOT MEASURED locally, by each gate's own verdict text, neither a red: check-test-completeness exits 3 saying "There is no local log to hand it, so the local reading for this gate is NOT MEASURED. ⛔ It is not a red"; check:dual-build-cjs-loads exits 3 with "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ … ⛔ This is NOT a pass: nothing was measured" (it needs a full pnpm build; CI builds everything). Neither reads any file in this diff.

ESLint — declared narrowing. pnpm lint (repo-wide eslint . --no-inline-config) was narrowed to the two changed files, with the narrowing measured rather than assumed:

  • Population: eslint's own config resolution accepted both paths (neither ignored), --format json reports 2 files linted, 0 errors, 0 warnings, exit 0.
  • Invariance for untouched files: this repo runs one eslint.config.mjs which, in its own words at line 328, "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file, test or not" — so a two-file comment-only diff cannot move any untouched file's verdict. CI runs the full sweep regardless.

Changeset

None, and skip-changeset is applied: the diff is comment-only inside examples/app-crm, which is "private": true and publishes nothing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

Both comments named a "Discount Approval" flow that examples/app-crm does
not contain (src/flows/ holds only convert-lead.flow.ts, and flows/index.ts
states the one-flow scope deliberately). These examples are copy-source for
AI authors, so a comment naming an absent flow teaches a false shape.

- opportunity.object.ts: approval_status is now described as the mirror
  target for an approval node's `approvalStatusField` (ADR-0019), pointing
  at the showcase's approval flows as the worked example.
- sales-positions.ts: FinanceApproverPosition now names its actual
  consumer, the `crm_sales_user` binding in bind-position-sets.ts plus the
  `positions` registration in objectstack.config.ts.

Comment-only: no declaration, expression or export changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions github-actions Bot added the size/s label Sep 3, 2026
@os-sales
os-sales marked this pull request as ready for review September 3, 2026 05:17
@os-sales
os-sales enabled auto-merge September 3, 2026 05:17
@os-sales
os-sales added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 90718b6 Sep 3, 2026
38 checks passed
@os-sales
os-sales deleted the claude/issue-14516-crm-stale-approval-comments branch September 3, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

examples/app-crm: two comments reference a Discount Approval flow the app does not contain

2 participants