fix(rules): correct PUT/PATCH/POST semantics in adding-tables-with-schemas (mode: generate flip blocked on documentation#650)

[Ethan-Arrowood]

Two things, cleanly separable

The Job 1 correctness fixes below stand on their own and can merge independently. They fix statements that are wrong in the rule shipping today, and they do not depend on anything.

The mode: generate migration is deliberately NOT in this PR — it is blocked on HarperFast/documentation#650 merging. That PR (HarperFast/documentation#650) adds the ## Tables and Their Automatic Endpoints section to reference/rest/overview.md, which is the canonical source this rule would generate from. Until it lands, npm run generate cannot resolve the section and fails hard. This PR therefore leaves mode: synthesized in place and only records what the flip will require, as a checklist for a follow-up PR.

---

Job 1 — correctness fixes (merge-ready)

harper-best-practices/rules/adding-tables-with-schemas.md is mode: synthesized, so its endpoint list was hand-written and had drifted from what the write layer actually does. The most serious one is PUT.

PUT /{TableName}/{id} — was actively dangerous

• Before: "Updates an existing record."
• After: "Creates or replaces the record at {id} (upsert). The stored record ends up matching the request body exactly — properties omitted from the body are removed. Send the complete record; use PATCH to change a subset of properties."

Table.put calls update(..., fullUpdate = true), and the fullUpdate branch of _writeUpdate replaces the record rather than merging it. An agent reading "updates an existing record" would send a partial body and silently destroy every field it left out.

PATCH /{TableName}/{id} — merge depth was unstated

• Before: "Performs a partial update on a record."
• After: "Merges the request body into the existing record, preserving unspecified properties. The merge is shallow — a nested object in the body replaces the stored one wholesale rather than being deep-merged."

Table.patch calls update(..., fullUpdate = false). The old wording invited an agent to assume a deep merge and lose sibling keys inside a nested object.

POST /{TableName}/ — trailing slash is load-bearing, and the new key was undocumented

• Before: "Creates a new record."
• After: "Creates a record and returns 201 with the Harper-assigned primary key in the Location response header (the bare key, not a URL). The trailing slash is required — POST /{TableName} returns 404, and POST /{TableName}/{id} returns 405."

Resource.post only creates on a collection target. The two failure modes have different status codes, which matters for an agent trying to diagnose its own request:

• POST /{TableName} (no slash) is rejected during argument normalization, before any resource is constructed — Resource.ts:747-752 throws 404 with the message "A trailing slash is required to POST to the {TableName} collection".
• POST /{TableName}/{id} falls through to missingMethod and returns 405 with an Allow header.

The rule also gave the agent no way to learn where the generated primary key comes back. Note that Location carries the bare primary key, not a resolved URL (REST.ts:319-320, corroborated by openApi.ts:206-208: "primary key of new record"), so it cannot be fetched directly as a href.

DELETE /{TableName}/ — consequence made explicit

• Before: "Deletes all records or filtered records."
• After: "Deletes every record matching the query parameters. With no query parameters it matches — and deletes — every record in the table. Always pass a filter unless emptying the table is the intent."

Technically the old wording was not false, but it read as a casual aside for an operation that empties a table. An unfiltered collection DELETE is treated as a search target and deletes every match.

Smaller fixes folded in

• GET /{TableName} said "Describes the schema itself" — it describes the resource (table, database, declared attributes). Also noted that it takes no trailing slash, since the slash distinction is what separates it from the collection endpoint directly below it.
• "by its ID" → "by its primary key" on both single-record endpoints, matching the schema vocabulary (@primaryKey) the same rule uses three steps earlier.
• DELETE /{TableName}/{id} now precedes the collection form, so the destructive endpoint reads last and the ordering matches documentation#650's table (less diff churn when this rule is generated from it).

Deliberately unchanged, and one line deliberately NOT added

The existing rest: true warning is correct and stays. rest does not appear anywhere in static/defaultConfig.yaml, and componentLoader.ts:610-611 uses an app's config.yaml verbatim rather than merging it with defaults — so for any app that has a config.yaml (which is the case this rule addresses), omitting rest: true genuinely means no REST endpoints.

I had intended to also add a line pinning the table's HTTP method set to exactly GET/PUT/POST/DELETE/PATCH. Verification against the harper source refuted that, so it is not in this PR — see "Verification" below. Asserting it would have introduced a new false statement while fixing four old ones.

harper-best-practices/AGENTS.md is a compiled concatenation of all rule bodies, so it moves with the rule body. It was reproduced with the generator's own assembly step (assembleAgentsMd from scripts/generation/lib/render.mjs, then oxfmt) — no source resolution, no LLM call, and no mode: generate rule was touched. npm run validate passes, including the AGENTS.md round-trip check.

---

Job 2 — what the mode: generate flip will require (follow-up PR, after documentation#650)

Do not start this until documentation#650 is merged to main in the docs repo.

1. harper-best-practices/rules.manifest.yaml — the only file that needs an edit

Everything else is derived. The adding-tables-with-schemas entry (currently the first rule in the file) changes from:

  - rule: adding-tables-with-schemas
    description: Guidelines for adding tables to a Harper database using GraphQL schemas.
    category: schema
    priority: 1
    order: 1
    mode: synthesized

to:

  - rule: adding-tables-with-schemas
    description: Guidelines for adding tables to a Harper database using GraphQL schemas.
    category: schema
    priority: 1
    order: 1
    mode: generate
    sources:
      - path: reference/v5/rest/overview.md
        section: 'Tables and Their Automatic Endpoints'
        role: primary
      - path: reference/v5/database/schema.md
        section: 'Loading Schemas'
        role: supplemental
    must_cover:
      - '@table'
      - '@export'
      - 'rest: true'
      - 'Location'
      - 'shallow'
    cross_links:
      - defining-relationships
      - extending-tables
      - automatic-apis
      - querying-rest-apis

Notes on each field, because the exact values matter:

• sources[].path is a docs build path, not a repo path. documentation#650 edits reference/rest/overview.md, but sources resolve against <docs-path>/build/, where current-version docs live under reference/v5/. So the path is reference/v5/rest/overview.md. Verified: build/reference/v5/rest/overview.md exists and its heading levels match the source file 1:1.
• sources[].section must match documentation#650's heading text exactly. sliceSection in scripts/generation/lib/sources.mjs normalizes whitespace and lowercases, but does not fuzzy-match. If the heading is renamed during review of #650, this string must be updated in lockstep or generate-rules.mjs exits 1 with Section heading "..." not found. Re-read the merged heading before writing this value.
• sources is required and must be non-empty for mode: generate (validate-generated.mjs Layer 1), and conversely must be omitted while the mode is synthesized — which is why it cannot be added ahead of the flip.
• must_cover is only valid on mode: generate (Layer 1 rejects it on any other mode), and each string must appear verbatim in the generated body (Layer 4). Keep the list short and pin the facts this PR just fixed — Location and shallow are the two that regressed silently before.
• cross_links is already legal in any mode and each entry must be a known rule slug. The current hand-written body links to defining-relationships, extending-tables, automatic-apis, and querying-rest-apis; declare them so the generator can reproduce those links instead of dropping them.

2. Scope decision to make before flipping — this is the real work, not the manifest edit

The ## Tables and Their Automatic Endpoints section from #650 covers only step 4 of this rule (the endpoint list and the rest: true requirement). It does not cover:

• step 1, dedicated .graphql files per table and graphqlSchema.files wildcards
• step 2, node_modules/harper/schema.graphql as the directive reference
• step 3, @relationship (delegated to defining-relationships)
• step 5, "do not @export if you intend to extend the table" (delegated to extending-tables)

A flip that sources only the REST section will silently lose steps 1, 2, and 5. Steps 1–2 partly overlap schema-design-tooling (order 2), which already sources schema.md#Overview / #Type Directives / #Field Directives. So pick one, explicitly:

• (a) add the supplemental schema.md#Loading Schemas source shown above and accept some overlap with schema-design-tooling; or
• (b) narrow this rule to the endpoint surface, move steps 1–2 into schema-design-tooling, and update description accordingly — note that changing description also moves the generated index block in SKILL.md; or
• (c) keep mode: synthesized. This is a legitimate outcome. synthesized exists for rules with no single canonical source, and this rule is a navigation hub across four other rules, not a 1:1 restatement of one docs section. #650 makes (a)/(b) possible; it does not make them correct.

The node_modules/harper/schema.graphql pointer in step 2 has no docs source at all and will be dropped by the generator's "do not invent" rule (scripts/generation/templates/system-prompt.md) under any of (a) or (b). Decide whether that is an acceptable loss.

3. Frontmatter fields the generated rule will carry that this one lacks

Written by buildFrontmatter in scripts/generation/lib/render.mjs — do not hand-write these, the generator owns them:

Field	Value
metadata.sources	the manifest sources normalized to path#section strings
metadata.sourceCommit	docs repo git HEAD at generation time
metadata.inputHash	16-char SHA-256 prefix of the resolved source content

metadata.mode flips synthesized → generate. name and description are already present and are written from the manifest. Layer 3 of the validator asserts all three new fields are absent while the mode is synthesized and that metadata.sources matches the manifest exactly once it is generate — so the manifest edit and the regeneration must land in the same commit.

4. No other file needs an entry

Checked explicitly, so the follow-up does not go hunting:

• scripts/generation/lib/manifest.mjs — SKILLS is keyed per skill directory, not per rule. No entry needed.
• .github/workflows/generate.yaml — iterates every non-synthesized manifest rule automatically. No entry needed.
• harper-best-practices/AGENTS.md — regenerated by the generator (pipeline step 7). Never hand-edited.
• harper-best-practices/SKILL.md — only the block between the <!-- BEGIN GENERATED INDEX --> / <!-- END GENERATED INDEX --> sentinels is generated, and it derives from rule / description / category / priority / order only. It changes only if option (b) above changes description.
• The manifest header comment (rules.manifest.yaml, the # Phase 2 … Phase 4 … block) is a running log of which rules were flipped in which phase and currently ends "All others remain synthesized." Add a Phase 5 line so the log stays accurate.

5. Commands for the follow-up, in order

git -C ../documentation switch main && git -C ../documentation pull   # must include #650
npm --prefix ../documentation ci && npm --prefix ../documentation run build
npm run generate -- --docs-path ../documentation --rule adding-tables-with-schemas
npm run generate -- --docs-path ../documentation        # full run: rebuilds AGENTS.md + SKILL.md index
npm run sync:report -- --docs-path ../documentation --strict
npm run validate
node scripts/generation/validate-generated.mjs --docs-path ../documentation

The docs checkout must be built at main HEAD, or the rule is "born stale": its recorded sourceCommit / inputHash point at an old commit and the next auto-sync PR looks like an unrelated docs change regenerated this rule. --rule skips the AGENTS.md rebuild, which is why the full run follows it. ANTHROPIC_API_KEY must be set in .env — mode: generate makes a live LLM call.

Then review the generated body against this PR's diff: confirm the PUT upsert warning, the PATCH shallow-merge caveat, the POST trailing-slash 404/405 distinction, the Location header, and the unfiltered-DELETE warning all survived the rewrite. Those five facts are the reason this PR exists; a generated body that drops any of them is a regression, and must_cover only guards the substrings listed above.

Be aware that documentation#650's endpoint table does not currently carry the 404-vs-405 distinction for POST, so a generated body sourced only from that section will lose it. Either land that detail in #650 first, or accept the loss knowingly.

---

Verification

Every statement was re-checked against the harper source at c4dd96237 (HarperFast/harper main) rather than against the docs, so this rule and documentation#650 are grounded in the same behavior. Confirmed as written: the PUT upsert/replace path (Table.ts:2263-2266 → Table.ts:1900-1905 → _writeUpdate at Table.ts:2932-2941, where a full update sets recordToStore = recordUpdate with no reference to existingRecord); the shallow PATCH merge (Table.ts:2356-2359 with fullUpdate = false, merging via the one-level spread in tracked.ts:421-443); and the unfiltered collection DELETE (Table.ts:3125-3151, where isSearchTarget is a bare isCollection test with no minimum-condition guard).

Three places where the source did not match the assumptions I started from — all three changed what shipped in this PR:

1. POST /{Table} without a trailing slash returns 404, not 405. My first draft of this fix said 405 for both slash-less forms. Resource.ts:747-752 throws a purpose-built 404 for the no-slash case during argument normalization; only POST /{Table}/{id} reaches missingMethod and returns 405. Corrected before commit.
2. The "method set is exactly GET/PUT/POST/DELETE/PATCH" claim is false, so the line asserting it was dropped. QUERY genuinely works on a plain exported table — Resource.ts:347-351 routes static query to Table.prototype.search (Table.ts:3244) with the request body as the search target. HEAD also works, handled generically in the REST layer (REST.ts:240-242 plus the body strip at REST.ts:352). Only COPY and MOVE actually 405. Separately, allowedMethods() does not mean what it looks like: the OPTIONS handler calls it on the class (REST.ts:251-258), and it inspects statics (Resource.ts:1003-1009), so OPTIONS advertises GET, PUT, POST, DELETE, PATCH, QUERY, MOVE, COPY — over-reporting MOVE/COPY and omitting HEAD. It is not a reliable description of a table's surface.
3. "rest is never on by default" needs a qualifier. True of static/defaultConfig.yaml (no rest key at all), but components/DEFAULT_CONFIG.ts:1-4 sets rest: true as the component fallback used when a component directory has no config file whatsoever (componentLoader.ts:612-613). Since componentLoader.ts:610-611 uses an existing config.yaml verbatim rather than merging it, the rule's existing wording — which is scoped to config.yaml — stays correct and unchanged.

Two smaller nuances, noted rather than folded in to keep the rule terse: @createdTime / @updatedTime attributes survive a PUT (Table.ts:2461-2473), so "properties omitted are removed" holds for user-defined attributes but not framework timestamps; and PUT forces the primary key to the URL id (Table.ts:2484-2487), so a mismatched id in the body cannot create a second record.

Worth feeding back to documentation#650

Two sentences in that PR's new section look imprecise against the source, and it would be better to fix them there than to inherit them when this rule is generated from it:

• "OPTIONS reports the resource's supported methods in an Allow header" — it reports the class's statics, which over-reports MOVE/COPY and omits HEAD (finding 2 above).
• "rest is not enabled by default" — needs the no-config-file carve-out (finding 3 above).

The section's POST row says only "Requires the trailing slash" without a status code, so it is not wrong, but adding the 404-vs-405 distinction there would make it more useful.

Gate

npm run validate passes (format check, build, validate-skills.mjs, validate-generated.mjs), as does the fuller node scripts/generation/validate-generated.mjs --docs-path ../documentation including the source-exists and byte-identical checks.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the documentation in AGENTS.md and adding-tables-with-schemas.md to provide more detailed and accurate descriptions of Harper's automatic REST API endpoints (GET, POST, PUT, PATCH, and DELETE). The review feedback suggests clarifying that for PUT requests, the primary key in the URL overrides any mismatched primary key provided in the request body.

[gemini-code-assist]

Since PUT forces the primary key to the URL {id} (as verified in the Harper source code), any mismatched primary key provided in the request body will be overridden by the URL {id} rather than creating a new record or throwing an error. Consider adding this detail explicitly to clarify the exact behavior of the upsert operation:

- `PUT /{TableName}/{id}`: Creates **or replaces** the record at `{id}` (upsert). The stored record ends up matching the request body exactly — **properties omitted from the body are removed** (the primary key in the URL overrides any mismatched primary key in the body). Send the complete record; use `PATCH` to change a subset of properties.

[Ethan-Arrowood]

Confirmed and fixed in dc4077e. Table.ts:2484-2487 at c4dd96237:

if (primaryKey && recordUpdate[primaryKey] !== id && (fullUpdate || primaryKey in recordUpdate)) {
  // ensure that the primary key is correct, if there is supposed to be one
  recordUpdate[primaryKey] = id;
}

So the URL id wins and a mismatched body key cannot create a second record. That is now stated in the rule, alongside the two @createdTime / @updatedTime exceptions @heskew raised — the previous "matches the request body exactly" was false in all three ways.

_{sent with Claude Opus 5}

[gemini-code-assist]

Since PUT forces the primary key to the URL {id} (as verified in the Harper source code), any mismatched primary key provided in the request body will be overridden by the URL {id} rather than creating a new record or throwing an error. Consider adding this detail explicitly to clarify the exact behavior of the upsert operation:

- `PUT /{TableName}/{id}`: Creates **or replaces** the record at `{id}` (upsert). The stored record ends up matching the request body exactly — **properties omitted from the body are removed** (the primary key in the URL overrides any mismatched primary key in the body). Send the complete record; use `PATCH` to change a subset of properties.

[heskew]

Requesting changes for one current-head documentation-correctness issue.

Prior-review attribution: I agree with @gemini-code-assist’s existing comments that the URL primary key overrides a mismatched primary key in the request body. Codex also confirmed a second exception: Harper stamps @updatedTime and retains or stamps @createdtime during a full PUT. The new statement that the stored record matches the request body exactly is therefore false in multiple supported schemas. Because this rule is agent-facing and this PR exists to correct destructive-write guidance, please qualify that sentence in the canonical rule and regenerate AGENTS.md before merge.

I independently traced the remaining changed claims: PUT is an upsert/full replacement for ordinary user fields, PATCH is shallow, the POST slash/status/Location behavior is accurate, and unfiltered collection DELETE removes all matching rows. No other blocker found.

Validation at exact head dd0304f: npm ci, npm run validate, and git diff --check passed; GitHub checks are green.

🤖 Posted by Codex on behalf of @heskew

[heskew]

Codex finding, complementary to @gemini-code-assist’s primary-key comment: ‘matching the request body exactly’ is also false for framework-managed timestamps. A full PUT stamps @updatedTime and preserves or stamps @createdtime, even when those values are absent from or differ in the body. Please describe this as replacement of ordinary user-defined fields, explicitly note that the URL primary key wins, and carve out framework-managed fields; then regenerate the compiled AGENTS.md.

🤖 Posted by Codex on behalf of @heskew

[Ethan-Arrowood]

You are right, and thanks for tracing it — fixed in dc4077e.

I verified all three exceptions against resources/Table.ts at c4dd96237 rather than take them on faith, and all three hold:

• @updatedTime is re-stamped with txnTime on every write, full or partial (Table.ts:2460-2467) — unconditional, no reference to the body.
• On a full update over an existing entry, @createdTime is restored from that entry (Table.ts:2468-2474, comment: "make sure to retain original created time"), so the body value is discarded. Only a genuinely new entry gets a fresh stamp.
• if (primaryKey && recordUpdate[primaryKey] !== id && (fullUpdate || primaryKey in recordUpdate)) recordUpdate[primaryKey] = id; (Table.ts:2484-2487) — the URL id wins, so a mismatched body key cannot create a second record.

You are also right about the judgment call. The PR body noted the timestamp and primary-key nuances and left them out "to keep the rule terse" — wrong call for this particular sentence, given the rule is agent-facing and the whole point of the PR is correcting destructive-write guidance. An unqualified "exactly" is the same class of false statement I was fixing.

One thing that reinforces it: automatic-apis.md:77 on main already says "PUT replaces the stored record exactly. Three exceptions always apply: a @createdTime attribute keeps the original value, an @updatedTime attribute is re-stamped with the time of the write, and the primary key is forced to match the {id} in the URL." So the old wording did not just overstate — it contradicted a sibling rule. I matched that vocabulary so the two agree.

New wording keeps the hazard in front and the exceptions after:

Creates or replaces the record at {id} (upsert). Properties omitted from the body are removed — send the complete record, or use PATCH to change a subset. Three exceptions survive the replacement: an @updatedTime attribute is re-stamped with the time of the write, a @createdTime attribute keeps its original value, and the primary key is forced to match the {id} in the URL, so a mismatched key in the body cannot create a second record.

AGENTS.md regenerated with the generator's own render step (assembleAgentsMd + oxfmt) — no source resolution, no LLM call. harper-mcp/AGENTS.md came back byte-identical, which is a decent check that the assembly is faithful.

I also merged origin/main, which the branch was 14 commits behind. Worth flagging since it was a latent problem independent of your review: AGENTS.md concatenates every rule body, so assembling it from the stale tree would have reverted rules regenerated since. The merge was conflict-free and the diff against main is now only the intended lines.

_{sent with Claude Opus 5}

[kriszyp]

generated
_{🤖 Reviewed with Codex}

[kriszyp]

Could we qualify “matching the request body exactly”? PUT fully replaces ordinary record properties, but Harper forces the primary key from the URL, preserves the original @createdTime, and re-stamps @updatedTime. For example, a PUT containing only { "name": "new" } will still retain/generate those schema-managed attributes, so omitted properties are not universally removed. Stating these exceptions while retaining the warning about omitted client-controlled fields would make this precise.

[kriszyp]

Good stuff
_{🤖 Reviewed with Codex}

[kriszyp]

Could we qualify “matching the request body exactly”? PUT fully replaces ordinary record properties, but Harper forces the primary key from the URL, preserves the original @createdTime, and re-stamps @updatedTime. For example, a PUT containing only { "name": "new" } will still retain/generate those schema-managed attributes, so omitted properties are not universally removed. Stating these exceptions while retaining the warning about omitted client-controlled fields would make this precise.