# Review request: the POC path

Branch `feat/rest-write-followups`, unmerged. 591 tests pass, lint and build clean, checked by
exit code. Deployed to the `workato-poc-mcp` Fly app only.

This is a different request from the last nine rounds. Those reviewed the code by subsystem, which
is why they kept finding the next layer down. This one is scoped by **what the demo actually
executes**, and asks for the review to stop at that boundary rather than continue outward.

## Why this scope

The POC is a partnership demo: a Workato Genie answers support questions using TiDB as its
knowledge base and memory. The path from a Workato builder opening the connector to the Genie
answering a question is:

```
Recipe beats, over the REST connector:
  1. save a connection            -> ping
  2. pick a table                 -> listTables
  3. fields render in the recipe  -> describeTable  (feeds object_definitions)

Genie beats, over MCP at /mcp:
  4. the Genie retrieves          -> tidb_tpl_search_known_issues -> runTemplate -> templates.json
  5. the Genie writes back        -> tidb_tpl_record_case (preflight + confirm)
```

Everything on that path is in scope. Nothing else is.

**Corrected 2026-07-31.** An earlier version of this file said the Genie retrieves over
`POST /q/<name>`. That was wrong, and the mistake was mine rather than the demo script's:
`DEMO-SCRIPT.md` says MCP, names the four `tidb_tpl_*` skills, and records a live verification
against `/mcp`. Confirmed again since: `tools/list` on the deployment returns those four tools,
and `tidb_tpl_record_case` carries a `confirm` parameter, which is the preflight wrapper. So beat
6's claim about preflight is true on the transport the demo actually uses.

Both transports publish the same templates, so the REST `/q/*` routes exist and are reachable.
They are simply not what the Genie calls.

Four of the five have never been reviewed. `runTemplate` and `ping` you have named as unreviewed
in every report. `listTables` and `describeTable` appear in probes only incidentally, through the
drift guard and the status-coverage checks, and their behaviour has never been examined. The SQL
the Genie runs has never been read by anyone but its author.

## What to review

**1. `runTemplate`** (`packages/remote/src/rest-handlers.ts`)

The whole safety argument of this POC is that the agent cannot run arbitrary SQL: an operator
registers named statements and the caller supplies values only. That argument rests entirely on
this function and on `translatePlaceholders` in `packages/core/src/tools/`. If a caller can alter
the statement, change which statement runs, or reach a statement not published to them, the claim
we are making to a partner is false.

Specifically worth attacking: whether positional binding holds for every placeholder form the
templates file can express, what happens when a param is present but null or an object or an
array rather than absent, whether `template.kind` is trustworthy for deciding the response shape,
and whether a template that returns nothing is distinguishable from one that failed.

**2. `ping`** (same file)

This is the connection test. Workato will not save a connection whose test fails, so a wrong
answer here blocks the POC at step one, before anything else can be demonstrated.

It deliberately returns 200 with `warm: false` rather than failing when a cold Starter cluster
does not answer in 4 seconds. Worth checking that the race cannot leak a rejection, that the
feature flags describe this database rather than the tier, and that a genuinely broken deployment
is still distinguishable from a cold one by a caller reading the response.

**3. `listTables` and `describeTable`** (same file)

`listTables` drives the table picker and `describeTable` drives the field list in every action.
`describeTable` is the operation the whole product argument depends on: it is what decides which
columns a recipe is allowed to write, and a wrong classification means either a recipe that cannot
insert or one that silently overwrites a value TiDB owns.

Note `listTables` issues one DDL read per table, capped at 200, and returns `truncated` rather
than silently cutting. The 30 second introspection cache applies to `listTables` and not to
`describeTable`.

**4. The four approved queries** (`~/Documents/workato-poc/mcp/templates.json`)

This is a query review, not a code review, and it is the highest-value item here. These four
statements are what the Genie actually executes:

| name                   | what it is for                                             |
| ---------------------- | ---------------------------------------------------------- |
| `search_known_issues`  | hybrid retrieval, the longest statement at ~880 characters |
| `get_account_context`  | the account's history, for beat 3                          |
| `check_repeat_contact` | whether this account has been in touch recently            |
| `record_case`          | the only write the Genie can perform                       |

Worth checking: whether each returns what the demo beat claims, whether the retrieval statement's
ranking is defensible or accidental, whether any of them can be made to return another account's
rows by parameter choice alone, what `record_case` overwrites, and whether the parameter types
survive the JSON round trip.

The demo script that makes claims about these is at `~/Documents/workato-poc/DEMO-SCRIPT.md`, with
the measured numbers each beat asserts.

## Deliberately out of scope

Please do not report these. They are known, they are written down, and opening them now adds
rounds without changing whether the demo works.

- **Response schemas are not compared in either direction by the drift guard.** Stated in the
  commit that added it. Real gap, accepted for now.
- **The OpenAPI document has not been read as a specification**, only checked for name-level
  drift. A parameter can still be documented with a wrong description and pass.
- **`countRows`** is not on the demo path.
- **`connector.rb`** (724 lines, in the POC repo) is what Workato runs, and it has been validated
  by a live import rather than by review. It stays as it is until the listing conversation.
- **The rate limit is per process, not per deployment.** Measured and written at both call sites.
  Not a finding.

## Conventions that still hold

Same as previous rounds. Verify by exit code rather than by reading output. Adversarial probe
tests are welcome and have been the most useful part of this loop; if you write one that
characterises a defect, say so in its name so a passing run is not mistaken for correct behaviour.

Two things previous rounds established that are worth carrying in:

- When you suggest a fix, state its precondition. Round 6's unique-key suggestion was adopted
  without one and reintroduced a silent row skip, which round 7 then had to find.
- A guard that has never been seen to fail has not been tested. Both new guards in this branch
  were landed by breaking them first.

## What I am asking for

A verdict per item, and an explicit statement of whether anything here blocks the demo. If the
answer is that nothing does, please say so plainly, because that is the decision this request
exists to make. Follow-ups that do not block are welcome as a list, and they will be recorded
rather than fixed in this branch.