Skip to content

DEV-2201 Correct and reorganize Data Sources developer documentation - #316

Open
hcarneiro wants to merge 7 commits into
masterfrom
fix/DEV-2201
Open

hcarneiro wants to merge 7 commits into
masterfrom
fix/DEV-2201

Conversation

@hcarneiro

@hcarneiro hcarneiro commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Product areas affected

Developer documentation for the Data Sources JavaScript API and data-source security, including generated human/LLM discovery indexes. No CLI runtime, Studio or API implementation changes.

What does this PR do?

Corrects incomplete or misleading Data Sources API signatures, options, result shapes and security guarantees. Splits the reference into focused pages for reads, writes, attachments, subscriptions, native storage and source management, alongside security rules, examples and preview testing. Preserves existing entry URLs and legacy fragments, updates operators/views/joins and regenerates discovery with one SDK catalog owner.

JIRA ticket

DEV-2201

Result

Readers can distinguish online, native-local and administrative calls, handle queued/pending writes separately from server persistence and choose method-specific security tests. Examples no longer advertise JSON-unsafe regex/type patterns, universal atomic commit behavior or unverified CSV import.

Validation of the original documentation refactor on aad1baeb0bcd24a1a549356973910c2726effe75:

  • 191 documentation tests passed; strict discovery passed for 216 pages and 46 libraries.
  • Jekyll build passed. Existing unrelated Liquid warnings remain in Best-practises and authentication docs.
  • 33 preview route variants returned HTTP 200; all 62 original SDK heading fragments were retained without duplicate IDs. Raw Markdown and generated discovery were checked.
  • SDK/helper probes verified transport, result variants, cursor behavior, write options, attachment conversion and lifecycle behavior using mocked transport/native IO. These are implementation checks, not live authorization or device proof.
  • Two independent technical/usability reviews followed by a fresh third-agent adjudication accepted the content and local preview after corrections.

The joined-source follow-up on a29ccee137635a500cab7aed791752fdae7f2125 documents that a joined-source access lookup does not establish that source’s own read permission, access rules or field restrictions. The warning precedes the join examples, both Users snippets are annotated, and security-testing guidance includes a direct-versus-joined comparison. Read-only API inspection confirmed the boundary. All 191 documentation tests and strict discovery generation passed. Two independent contract/reader reviews and a third-agent adjudication approved this exact commit. The local Jekyll build could not be rerun because its gem dependencies are unavailable in the current shell.

The branch preview is https://fix-dev-2201.fliplet-cli.pages.dev/API/fliplet-datasources . The follow-up’s rendered joins, security and testing pages, plus raw joins Markdown, returned HTTP 200 and contain the new boundary guidance.

The local review preview is http://127.0.0.1:4311/API/fliplet-datasources . This address is only available on the author's machine. Its temporary local search helper is not part of this PR.

Checklist

  • Added automated test coverage as appropriate for this change.

Deployment instructions

Draft for reviewing the changes. Do not merge/publish until the remaining runtime checks and release decision are complete. After publication, verify the generated discovery, raw Markdown, legacy fragments and refreshed hosted search index.

Author concerns

Authenticated disposable app-user operations, real upload/storage persistence, server write/readback, security-enabled preview enforcement/restoration and native-device IO remain unverified. Source inspection and mocked probes do not identify the deployed SDK revision or satisfy those runtime gates. The diff is large because long references were split and the full generated documentation index was regenerated.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Deploying fliplet-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: 48c27fa
Status: ✅  Deploy successful!
Preview URL: https://2de121bf.fliplet-cli.pages.dev
Branch Preview URL: https://fix-dev-2201.fliplet-cli.pages.dev

View logs

@tonytlwu tonytlwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review (via /fliplet-pr-review)

Combined DEV-2201 review with Fliplet/fliplet-studio#9021. The focused SDK/security documentation and discovery changes substantially improve the contract, but joins need an explicit security limitation so builders do not infer direct-read protection for related private records. See inline comment.

Acceptance evidence: 191 documentation tests and strict discovery generation passed; rendered security documentation was checked at desktop and mobile sizes. These checks do not demonstrate real-actor authorization. Include joined-source access and field restrictions in the Studio allowed/denied test matrix alongside representative and held-out workflows. The API behavior described in the finding is existing behavior identified by source inspection, not a newly demonstrated production exploit or a request to expand this ticket into an API fix.

Review basis: subject=aad1baeb0bcd24a1a549356973910c2726effe75 standards=fliplet-engineering@0.18.0 revision=d8c727a0419f9d87a6e38a767a4b95b84380cc44+dirty source=fliplet-pr-review digest=sha256:40c49b60a8bd5384


// ... and users who posted them
Users: { options }
Users: { dataSourceId: 789, on: { 'data.AuthorID': 'id' } }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Document the security boundary for joined sources

The Users example needs an explicit caveat: online joined-source access rules and field restrictions are not automatically enforced like direct reads. In the inspected API implementation, the query validates the primary source's select rules; joined sources are preloaded subject to source/token permissions, but their own access-rule validation is not performed equivalently, and the primary rule's include/exclude filtering does not sanitize nested joined data. A source available to the token can therefore return joined rows/fields that its direct-read rules would restrict. Please state this limitation here and link it from security/testing guidance, so a public-content-to-private-users join is not treated as proof of private-data protection. Include direct-versus-joined allowed/denied and returned-field checks in the authorization matrix. This documents an existing API limitation; no API change is requested in this ticket.

hcarneiro and others added 3 commits October 9, 2026 10:40
@hcarneiro
hcarneiro marked this pull request as ready for review October 14, 2026 13:25

@farhantariq12b farhantariq12b left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review (via /fliplet-pr-review)

Review basis: subject=48c27fa8897c19eb6220ff37cf2e5950f870f8fc standards=fliplet-engineering@0.16.0 revision=f8a269a7f374041e55df4fa3d1d5890f3232d043 source=fliplet-pr-review digest=sha256:b54eff56f86bcdda

Combined DEV-2201 review with Fliplet/fliplet-studio#9021.

Read this first: the content is not what is being blocked

@tonytlwu's joined-source finding is fully resolved, and I found no defect in the documentation itself. The change request below is about the PR's state, not its content — please don't read it as the joins fix not landing.

The joined-source boundary is handled better than asked: a dedicated "Security boundary for online joins" section sits at docs/API/datasources/joins.md:13-19, ahead of the examples as requested, stating that the join evaluates only the primary source's select rules, that the source access lookup does not establish the joined source's read permission or column restrictions, and that primary include/exclude does not sanitize nested joined data. Both Users snippets carry the inline caveat, where/attributes are explicitly marked caller-controlled rather than a security boundary, and it cross-links security-rules.md#joined-source-reads and testing-security.md#check-joined-source-responses. The native-local execution difference is called out too.

Summary

Documentation only — no CLI runtime, Studio or API code. Splits the Data Sources SDK reference into focused pages (reads, writes, attachments, subscriptions, native storage, source management) plus security rules, examples and preview testing; corrects signatures, options, result shapes and security guarantees; updates operators, views and joins; and regenerates the human/LLM discovery indexes with one SDK catalog owner. Entry URLs and legacy fragments are preserved.

Regression risk

No runtime path. The risk is reader-facing: the new write-contract tables and the joins warning describe existing platform limits, so a wrong statement becomes a wrong guarantee someone builds on. That is exactly why the joined-source boundary mattered, and it is the right shape of fix.

Scope exceeds the guard (23 files, +5,175/-6,679) but reads far smaller than it measures: .well-known/llms-full.txt alone is +2,962/-3,995 of generated output, and most of the rest is two long references being split. Net deletion. Branch strategy correct — fix/DEV-2201 to master is right for a generic repo — and the branch is current with master with no foreign files; docs/ only.

Findings

0 Critical, 0 Warnings against the documentation, 1 process Warning. Being explicit so the block is not misread as a content defect.

The body says "Draft for reviewing the changes. Do not merge/publish until the remaining runtime checks and release decision are complete" — but the PR is marked ready for review (isDraft: false) with green CI. The stated state and the actual state disagree, and the only thing preventing a merge is prose a reviewer may not read to the end.

Two things would clear this:

  1. Convert to draft, or remove the "do not merge/publish" language so the PR's state matches its intent. This is the whole of the change request — it is one click.
  2. Close the runtime gates in your own Author concerns: authenticated disposable app-user operations, real upload/storage persistence, server write/readback, security-enabled preview enforcement/restoration, and native-device IO. Source inspection and mocked probes do not identify the deployed SDK revision. This is an evidence gap, not a demonstrated defect — and it is the substantive reason not to publish yet, since this PR's job is to state platform guarantees that readers will rely on.

Acceptance criteria

Not independently re-verified against Jira this round — stating that plainly rather than implying otherwise. @tonytlwu's earlier assessment stands.

CI status

Green — Frontmatter + unit tests and Cloudflare Pages both pass. Branch preview resolves.

Tooling notes

fe-update-check reported FAILED git-pull, so this ran on pack 0.16.0 while the prior review ran 0.18.0 — worth knowing if the standards moved between them. fliplet-kb needs authorisation, so the KB steps used the local checkout. No incident corpus on this machine. I did not re-run the Jekyll build or the 191 documentation tests locally; CI covers the frontmatter and unit suites.

Verdict

REQUEST_CHANGES — on the PR's state and the outstanding runtime evidence, not on its content. The documentation work is good and the joins boundary is resolved thoroughly; I am not asking for edits to the pages.

If you would rather carry the evidence gap as a release gate than a review block, convert this to draft and I will drop to a comment.

Automated review — a human still does the formal approve.

This branch has not been deployed

No deployments
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.

3 participants