diff --git a/.github/workflows/e2e-lab.yml b/.github/workflows/e2e-lab.yml index 94f0972..20d4a61 100644 --- a/.github/workflows/e2e-lab.yml +++ b/.github/workflows/e2e-lab.yml @@ -60,6 +60,7 @@ jobs: timeout-minutes: 10 outputs: execute_plan: ${{ steps.plan.outputs.execute_plan }} + commit_plan: ${{ steps.plan.outputs.commit_plan }} case_ids: ${{ steps.plan.outputs.case_ids }} # Per-invocation case filters: the computed-update strategy is fixed at # app boot, so hybrid-mode cases run in a second vitest invocation. @@ -128,6 +129,13 @@ jobs: E2E_LAB_CASE_FILTER: ${{ inputs.case_filter }} run: node e2e-lab/scripts/run-plan.mjs + # One job per commit PER ENGINE. Two engines sharing a job was cheaper by one + # bootstrap and wrong in a way that only showed up on reflection: two passes + # against one database means the second engine runs on state the first left, + # and the guarded column is what would have been reading it. Split, each + # engine gets its own containers and its own database from the commit's own + # migrations, and the two run at the same time — the wall clock is one + # engine's, not two. execute: name: Run cases (${{ matrix.plan.name }}) needs: resolve_inputs @@ -213,6 +221,7 @@ jobs: export E2E_LAB_CASE_FILTER="${{ needs.resolve_inputs.outputs.sync_case_filter }}" export E2E_LAB_COMMIT_SHA="${{ matrix.plan.sha }}" export E2E_LAB_GATING="${{ matrix.plan.gating }}" + export E2E_LAB_ENGINE_LIST="${{ matrix.plan.engine }}" pnpm -F @teable/backend-ee exec vitest run \ --config ./vitest-e2e-lab.config.ts \ @@ -305,8 +314,9 @@ jobs: - name: Build comparison table env: E2E_LAB_ARTIFACT_DIR: ${{ github.workspace }}/e2e-lab-artifacts - E2E_LAB_EXECUTE_PLAN: ${{ needs.resolve_inputs.outputs.execute_plan }} + E2E_LAB_EXECUTE_PLAN: ${{ needs.resolve_inputs.outputs.commit_plan }} E2E_LAB_CASE_FILTER: ${{ inputs.case_filter }} + E2E_LAB_ENGINES: '["v1","v2"]' E2E_LAB_COMPARISON_PATH: ${{ github.workspace }}/e2e-lab-report/comparison.json run: node e2e-lab/scripts/build-comparison.mjs @@ -328,7 +338,7 @@ jobs: continue-on-error: true env: E2E_LAB_ARTIFACT_DIR: ${{ github.workspace }}/e2e-lab-artifacts - E2E_LAB_EXECUTE_PLAN: ${{ needs.resolve_inputs.outputs.execute_plan }} + E2E_LAB_EXECUTE_PLAN: ${{ needs.resolve_inputs.outputs.commit_plan }} run: node e2e-lab/scripts/report-teable-track.mjs # The run's card, built from the same comparison.json the acceptance diff --git a/AGENTS.md b/AGENTS.md index e403448..6528aa1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,13 +25,24 @@ picture, [.agents/README.md](.agents/README.md) to add or change a case, and - Observe through the public API. The database is available for building fixtures the API cannot express, and only there — reaching for it inside a checkpoint throws (`framework/fixture-db.ts`). -- Every case guards v2. v1 still answers, so runners prove which engine served - them (`framework/engine.ts`). +- Every case guards v2, and every case is also asked of v1 as a reference. + Runners prove which engine served them (`framework/engine.ts`); a case whose + feature does not exist on v1 declares `skipV1: "why"` instead of failing + there every run. ## Things that look like oversights and are not Ask before "fixing" any of these: +- **Nothing the v1 column reports can fail a run.** v1 is a reference: the lab + guards v2, which is where fixes land. A v1 cell is evidence to follow up, not + a verdict — partly because reaching v1 at all means unstamping each case's + base, which makes a base no real customer has (theirs predate v2). +- **`skipV1` is declared on the case, never inferred from a failure.** Reading + "v1 said it does not support that" out of an error message fails open: a case + that genuinely breaks, whose error happens to read that way, would be skipped + forever and nobody would learn. + - **A `fixed` case reproducing on an old commit is not red.** That is the world before the fix. Only the gating column turns a reproduction into a regression. The table is in `framework/verdict.ts`, one screen. diff --git a/README.md b/README.md index ae6e3e8..38d7cda 100644 --- a/README.md +++ b/README.md @@ -25,13 +25,21 @@ The execution skeleton is teable-perf-lab's, proven in production there: - **Pinning**: refs are resolved to SHAs once, up front; every job checks out the pinned SHA. - **One job per commit**: isolated database built from that commit's own - migrations, all selected cases run serially, one JSON payload per case - written _before_ any assertion throws — the payloads are the source of - truth, and failures carry the server's own error body. + migrations, every selected case run once per engine, one JSON payload per + case per engine written _before_ any assertion throws — the payloads are the + source of truth, and failures carry the server's own error body. - **Fail-closed report**: every planned (case × commit) cell must have exactly one payload. Missing evidence fails the run; it never renders as an empty cell someone might read as green. +**Two engines, one of them guarded.** v2 is where fixes land, so that is the +column the run is judged on. v1 is asked the same cases as a reference — what +does the engine our older customers are still on do with this — and nothing it +reports fails a run; it renders as its own table. Reaching v1 needs more than +an environment switch, and a case whose feature v1 does not have declares +`skipV1` rather than failing there every run. Both are explained in +[docs/operations/e2e-lab.md](docs/operations/e2e-lab.md). + What is this repository's own: the verdict model. Each case declares the bug it reproduces and its believed status (`open` / `fixed`); the run observes (`absent` / `present` / `error`) and the comparison judges. Known-unfixed bugs diff --git a/cases/base-share/a-share-link-whose-database-is-away.case.ts b/cases/base-share/a-share-link-whose-database-is-away.case.ts new file mode 100644 index 0000000..ddc6d82 --- /dev/null +++ b/cases/base-share/a-share-link-whose-database-is-away.case.ts @@ -0,0 +1,25 @@ +import { defineBugCase } from "../../framework/types"; + +// T6926: a space can be bound to a customer's own database, and that binding can +// be switched off - revoked credentials, a retired connection, a migration part +// way. The share link, the view and the permission are all still correct; there +// is simply nowhere to read from. What came back was an unhandled 500. To +// whoever holds the link - usually somebody outside the company, with no account +// and nobody to ask - a 500 says the product is broken and there is nothing to +// do; a 503 naming an unavailable database says the same page will work later. +export default defineBugCase({ + id: "base-share/a-share-link-whose-database-is-away", + title: "A share link whose database is away says so", + runner: "share-view-unready-data-db", + timeoutMs: 180_000, + bug: { + issue: "T6926", + status: "fixed", + sourceCommits: ["bdcca3f24"], + }, + config: { + namePrefix: "e2e-lab-share-unready-db", + rowTitle: "a-row-behind-the-link", + encryptedUrlPlaceholder: "not-a-real-connection-string", + }, +}); diff --git a/cases/base-share/a-share-link-whose-database-is-away.md b/cases/base-share/a-share-link-whose-database-is-away.md new file mode 100644 index 0000000..c86115b --- /dev/null +++ b/cases/base-share/a-share-link-whose-database-is-away.md @@ -0,0 +1,62 @@ +# base-share/a-share-link-whose-database-is-away + +**T6926** — fixed. On the `share-view-unready-data-db` runner. + +## What the user sees + +Someone opens a share link. The space it belongs to is bound to a database whose +connection has been switched off — revoked credentials, a retired connection, a +migration part way through. The page fails with a 500. + +Everything about the share is still correct: the link, the view, the permission. +There is simply nowhere to read the rows from. + +The person holding the link is usually outside the company. They have no +account, no way to see anything else, and nobody to ask. A 500 tells them the +product is broken and there is nothing to do about it. A 503 naming an +unavailable database tells them, and anything watching the endpoint, that the +same page will work later. + +## Why + +Resolving which database a space reads from threw a plain error when the binding +was not usable. Nothing above it recognised that error, so it surfaced as an +unhandled 500 rather than as the outage it describes. + +## What the checkpoint asserts + +The status **and** the code. 503 alone would be indistinguishable from any other +outage, and being distinguishable is the whole of the fix — so the response must +also call itself `database_connection_unavailable`. + +A 200 is called out separately, because a share link that answered normally +while its database was away would be a different and worse problem than the one +this case is about. + +## Why the fixture is written with SQL + +Binding a space to another database is not part of this observation, and a +connection in the switched-off state is not something a request can ask for. +`fixture-db` writes the two rows; the observation stays on the public share +endpoint. + +Before the binding is written, the fixture opens the share link and requires a 200. Without that, a 503 afterwards could just as well mean the share was never +set up — and the case would pass while proving nothing. + +The space is created for this case alone. The binding under test is a property +of a space, and this must not touch the one every other case reads from. + +## The v1 column + +Skipped, for a reason about this harness rather than about the product. The case +makes its own space and base — the binding under test is a property of a space — +and `framework/case-base.ts` unstamps only the base it manages. A base created +inside a runner is born on v2, so a v1 run answers + +``` +POST /table/{tableId}/view/{viewId}/enable-share was requested of v1 +but v2 answered (reason=new_base) +``` + +which is the harness refusing to fabricate a reference column, not an answer +about v1. Any future runner that creates its own base inherits this. diff --git a/cases/base-share/a-shared-forms-picture.case.ts b/cases/base-share/a-shared-forms-picture.case.ts new file mode 100644 index 0000000..0399bff --- /dev/null +++ b/cases/base-share/a-shared-forms-picture.case.ts @@ -0,0 +1,26 @@ +import { defineBugCase } from "../../framework/types"; + +// T6604: where a form's picture lives is stored as a short path, and the address +// a browser can fetch is worked out from it when the form is read. A shared form +// is read through two layers, and both worked it out - the second over the +// first's answer - so what came back was one address with another stuck on the +// front of it, which fetches nothing. The person who opens the link sees a form +// with a broken picture while the same form inside the product looks right, +// because inside it is read through one layer only. +export default defineBugCase({ + id: "base-share/a-shared-forms-picture", + title: "A shared form's picture has one address, not two", + runner: "shared-form-cover-url", + timeoutMs: 180_000, + bug: { + issue: "T6604", + status: "fixed", + sourceCommits: ["573e0b70e"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-form-cover", + rowTitle: "a-row-behind-the-form", + storedPath: "form/e2e-lab-cover-image", + }, +}); diff --git a/cases/base-share/a-shared-forms-picture.md b/cases/base-share/a-shared-forms-picture.md new file mode 100644 index 0000000..b74a27e --- /dev/null +++ b/cases/base-share/a-shared-forms-picture.md @@ -0,0 +1,59 @@ +# base-share/a-shared-forms-picture + +**T6604** — fixed. On the `shared-form-cover-url` runner. + +## What the user sees + +A shared form with a broken picture. The same form inside the product looks +right, so nothing is wrong with the picture or the form — only with what the +share link hands out. + +The person seeing it is usually outside the company, filling the form in, and has +nothing to compare against. + +## Why + +Where a form's picture lives is stored as a short path. The address a browser can +fetch is worked out from that path when the form is read. + +The shared form is read through two layers, and both worked it out — the second +over the first's answer. What came back was one address with another stuck on the +front of it. Inside the product the same view is read through one layer, which is +why it looks right there. + +## What the checkpoint asserts + +That the address was built **once**: the cover and the logo each carry exactly one +`http(s)://`, and each ends at the stored path. + +Counting addresses rather than comparing against an expected string is +deliberate. What the storage prefix is depends on how the instance is deployed, +and pinning it would make this case about configuration instead of about the +doubling. Ending at the stored path is what says the address still points at the +right thing. + +What is counted is the **scheme**, not `http://`. Joining one address onto +another leaves the inner one with a single slash — measured on the fix's parent, +the value is + +``` +http://127.0.0.1:PORT/api/attachments/read/public/http:/127.0.0.1:PORT/api/attachments/read/public/form/… +``` + +— so looking for the double slash finds one address in a string that plainly +holds two. The first version of this case did exactly that and passed on both +sides. + +Both the cover and the logo are set and both are read, because the fix covers +both and either could regress alone. + +## Why the fixture is shaped this way + +The stored value must be a **short path**, and the runner refuses an address: an +address is exactly what the fix passes through untouched, so a fixture holding one +would be green on both sides. + +Before the checkpoint, the form is read from **inside** the product and its +picture must carry the stored path. That is the control — it says the form and the +stored value are fine, so a doubled address afterwards is about the share path and +not about the fixture. diff --git a/cases/base-share/copy-a-base-whose-tables-share-a-key-name.case.ts b/cases/base-share/copy-a-base-whose-tables-share-a-key-name.case.ts new file mode 100644 index 0000000..aeec710 --- /dev/null +++ b/cases/base-share/copy-a-base-whose-tables-share-a-key-name.case.ts @@ -0,0 +1,26 @@ +import { defineBugCase } from "../../framework/types"; + +// T6990: Postgres constraint names are unique per table, not per schema, and +// old bases carry a self-referencing key called fk___id on every table. +// Duplicating a base drops those keys, copies the rows and puts them back - but +// the step that listed them matched on the name and the schema and not on the +// table, so each table's list came back holding the other's rows. The drop ran +// twice for one table, the second found nothing, and the whole copy died on a +// Postgres error naming a constraint that "does not exist". Reported from +// production as an unhandled rejection in the browser, with the base half-made. +export default defineBugCase({ + id: "base-share/copy-a-base-whose-tables-share-a-key-name", + title: "A base whose tables share a key name can still be copied", + runner: "same-named-fk-base-duplicate", + timeoutMs: 300_000, + bug: { + issue: "T6990", + status: "fixed", + sourceCommits: ["b913e5014"], + }, + config: { + baseNamePrefix: "e2e-lab-same-named-fk", + tableNames: ["the-first-table", "the-second-table"], + rowTitle: "a-row-to-copy", + }, +}); diff --git a/cases/base-share/copy-a-base-whose-tables-share-a-key-name.md b/cases/base-share/copy-a-base-whose-tables-share-a-key-name.md new file mode 100644 index 0000000..dcc3e20 --- /dev/null +++ b/cases/base-share/copy-a-base-whose-tables-share-a-key-name.md @@ -0,0 +1,44 @@ +# base-share/copy-a-base-whose-tables-share-a-key-name + +**T6990** — fixed. On the `same-named-fk-base-duplicate` runner. + +## What the user sees + +Duplicating a base fails. The browser reports an unhandled rejection naming a +Postgres error — a constraint that "does not exist" — and the base is left +half-made. Pressing duplicate again does the same thing, and there is nothing in +the base a person could change to get past it. + +## Why + +Postgres constraint names are unique per **table**, not per schema. Two tables +in one base can each own a foreign key called `fk___id`, and old bases do: a +self-referencing key on the row id column, from before the naming changed. + +Duplicating a base drops those keys, copies the rows, and puts them back. The +step that listed the keys to drop matched on the name and the schema and not on +the table that owns them, so each table's list came back carrying the other +table's rows. The drop then ran the same statement twice for one table; the +second found nothing and raised 42704, and the duplicate died there. + +## What the checkpoint asserts + +The duplicate succeeds — a refused request throws inside the checkpoint, which +is the report — **and** the copy holds every table. A duplicate that answered +201 while losing a table would be the same interrupted copy behind a success. + +## Why the fixture is written with SQL + +Nothing a person can do produces `fk___id` any more. It is what an old base has +been carrying since before the naming convention changed, which is also why +nobody hitting this could get out of it from the interface. `fixture-db` is the +only way to build that state; the observation stays on the public duplicate +endpoint. + +The fixture then counts, before the checkpoint, how many tables in the schema +carry the name. With only one there is nothing to collide, and the case would +report on nothing. + +## Only v2 was repaired + +The fix is on the v2 duplicate route's own foreign-key introspection. v1 keeps its untouched legacy helper, so this case says nothing about the older engine either way. diff --git a/cases/field/a-cross-base-conditional-column-keeps-its-base.case.ts b/cases/field/a-cross-base-conditional-column-keeps-its-base.case.ts new file mode 100644 index 0000000..45a1743 --- /dev/null +++ b/cases/field/a-cross-base-conditional-column-keeps-its-base.case.ts @@ -0,0 +1,28 @@ +import { defineBugCase } from "../../framework/types"; + +// T7064: a conditional column reading a table in another base needs to record +// which base that is. It was dropped on the way into storage, so reopening the +// column's settings found a foreign table it could not place and drew it as a +// table the person has no permission to see. The values kept arriving - only +// the settings could no longer describe themselves, which costs the ability to +// change the column at all. +export default defineBugCase({ + id: "field/a-cross-base-conditional-column-keeps-its-base", + title: "A conditional column reading another base still names that base", + runner: "cross-base-conditional-base-id", + timeoutMs: 300_000, + bug: { + issue: "T7064", + status: "fixed", + sourceCommits: ["e552c5e88"], + }, + config: { + namePrefix: "e2e-lab-cross-base-conditional", + matchedCategory: "hardware", + sourceRows: [ + { category: "hardware", amount: 100 }, + { category: "hardware", amount: 50 }, + { category: "software", amount: 70 }, + ], + }, +}); diff --git a/cases/field/a-cross-base-conditional-column-keeps-its-base.md b/cases/field/a-cross-base-conditional-column-keeps-its-base.md new file mode 100644 index 0000000..396fb88 --- /dev/null +++ b/cases/field/a-cross-base-conditional-column-keeps-its-base.md @@ -0,0 +1,44 @@ +# field/a-cross-base-conditional-column-keeps-its-base + +**T7064** — fixed. On the `cross-base-conditional-base-id` runner. + +## What the user sees + +A conditional lookup or conditional total is pointed at a table in a **different +base**. It works: the values arrive and stay correct. Reopen the column's +settings and the foreign table is drawn as a table the person has no permission +to see. + +Nothing is actually inaccessible. But the column can no longer be changed from +that screen, and a save made from it writes the settings back with the base +already missing. + +## Why + +The column stores three things: which table, which column, and — when the table +is not in this base — which base. The third was dropped crossing the mapping +boundary between the two record engines. What was read back named a table with +no base to resolve it in, and "cannot resolve" renders as "no permission". + +## What the checkpoint asserts + +The field list — which is what the settings screen loads — still carries the +foreign base id, on both the conditional lookup (`lookupOptions.baseId`) and the +conditional total (`options.baseId`). Both, because the fix threaded the id +through two column types and one of them could regress alone. + +Outside the checkpoint, the columns are read once and must hold the values from +the other base. A column that never computed would have nothing meaningful to +say about its source either, and this case is about a column that works and +still cannot describe itself. + +The second base is created in the same space as the host's. Across spaces the +product refuses the link outright — "cross-space link is no longer supported" — +so the state this case is about only exists between two bases of one space. + +The engine is asserted on the create response of the cross-base column itself — +the request that puts the state under test in place. + +## Only v2 has these columns + +Conditional lookups and totals are v2 column types, and the dropped base id sat on the mapping boundary between the two engines that this fix moved. diff --git a/cases/filter/a-row-number-filter-typed-into-the-box.case.ts b/cases/filter/a-row-number-filter-typed-into-the-box.case.ts new file mode 100644 index 0000000..111fa70 --- /dev/null +++ b/cases/filter/a-row-number-filter-typed-into-the-box.case.ts @@ -0,0 +1,24 @@ +import { defineBugCase } from "../../framework/types"; + +// T7071: a filter box produces text, and every numeric column took the number +// that way - except the row-number column, whose comparison demanded a real +// number and answered 500 to a string. The page saved the filter and then broke +// on the row count, so the view a person had just built would not open, and +// would not open again on the next visit either. +export default defineBugCase({ + id: "filter/a-row-number-filter-typed-into-the-box", + title: "A row-number filter holding what the filter box typed", + runner: "autonumber-string-filter", + timeoutMs: 180_000, + bug: { + issue: "T7071", + status: "fixed", + sourceCommits: ["d9f5e61c6"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-autonumber-filter", + rowTitles: ["row-a", "row-b", "row-c", "row-d", "row-e"], + threshold: 2, + }, +}); diff --git a/cases/filter/a-row-number-filter-typed-into-the-box.md b/cases/filter/a-row-number-filter-typed-into-the-box.md new file mode 100644 index 0000000..678cc11 --- /dev/null +++ b/cases/filter/a-row-number-filter-typed-into-the-box.md @@ -0,0 +1,41 @@ +# filter/a-row-number-filter-typed-into-the-box + +**T7071** — fixed. On the `autonumber-string-filter` runner. + +## What the user sees + +A view is filtered on the row-number column, "greater than 50". The filter +saves. The page then fails to draw — the row count behind it answers 500 — and +it fails the same way on every later visit, because the filter is stored on the +view and loaded again each time. + +## Why + +A filter box produces text. That is what the grid sends for numeric columns, and +every numeric column accepted it — except the row-number column, whose +comparison required an actual number and refused the string outright. + +## What the checkpoint asserts + +The row count comes back at all, and the rows behind it are the ones the filter +describes. Both, because the count and the listing are separate paths through +the same comparison: a count that answered a plausible number while the listing +disagreed would be a different bug still worth failing on. + +The expected answer is derived from the numbers the product itself assigned, +not written into the case, so the case does not depend on how the row-number +column happens to start counting. + +The fixture rejects a threshold that selects all the rows or none of them. A +filter that changes nothing cannot tell a comparison that ran from one that +never did. + +The row-number column is added **after** the rows, which is how a table gets one +in practice: the column numbers what is already there. + +## The v1 column + +This case is not skipped on v1, and v1 answers it correctly on every commit, +including the ones where v2 refuses. The string the filter box sends was only +ever a problem for the newer engine — worth knowing, because it means customers +still on v1 never saw this. diff --git a/cases/formula/a-column-that-picks-by-case.case.ts b/cases/formula/a-column-that-picks-by-case.case.ts new file mode 100644 index 0000000..2aa6f87 --- /dev/null +++ b/cases/formula/a-column-that-picks-by-case.case.ts @@ -0,0 +1,34 @@ +import { defineBugCase } from "../../framework/types"; + +// T6980: "cost depends on where the cost comes from" - a manually entered figure +// for some rows, a different figure for others, and otherwise whatever is +// linked. The first two answers are numbers; the last is a list of linked +// records, stored as a document rather than as a number. The step merging the +// branches compared only the ones with a case attached, and those agreed, so it +// never looked at what the otherwise branch held. The database was then asked to +// choose between numbers and a document in one expression and refused, killing +// the column and the schema change it was part of. +export default defineBugCase({ + id: "formula/a-column-that-picks-by-case", + title: "A column that picks by case, ending in linked records, can be made", + runner: "switch-mixed-branch-storage", + timeoutMs: 300_000, + bug: { + issue: "T6980", + status: "fixed", + sourceCommits: ["fd0be31ad"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-switch-mixed", + numberBranches: [ + { choice: "Manual", column: "Manual cost", value: 11 }, + { choice: "Current", column: "Current cost", value: 22 }, + ], + otherwiseChoice: "Unmapped", + linkedRows: [ + { name: "price-one", price: 100 }, + { name: "price-two", price: 200 }, + ], + }, +}); diff --git a/cases/formula/a-column-that-picks-by-case.md b/cases/formula/a-column-that-picks-by-case.md new file mode 100644 index 0000000..5d82ff3 --- /dev/null +++ b/cases/formula/a-column-that-picks-by-case.md @@ -0,0 +1,62 @@ +# formula/a-column-that-picks-by-case + +**T6980** — fixed. On the `switch-mixed-branch-storage` runner. + +## What the user sees + +A column that picks its value by case cannot be created. The rule is ordinary — +"cost depends on where the cost comes from": a manually entered figure for some +rows, a different figure for others, and otherwise whatever is linked. The field +editor offers every part of it. Saving fails, and the schema change it was part +of dies with it. + +## Why + +The first two answers are numbers. The last is a list of linked records, which +is stored as a document rather than as a number. + +The step that merges the branches together compared only the branches with a +case attached. Those agreed — both numbers — so it never looked at what the +otherwise branch held. The database was then asked to choose between numbers and +a document in a single expression, and refused outright. + +Nothing in the interface says the last branch is a different kind of thing from +the others. + +## What the checkpoint asserts + +The column can be made, is not immediately marked broken, and reads the right +number on the rows whose case has a number behind it. + +Making the column is inside the checkpoint. Reconciling the branches happens +while it is built, so that is when the refusal happens; built in setup, the same +refusal would score as "this case could not run here" — the one verdict that +hides the bug. + +The rows falling to the otherwise branch are read but not pinned to a value. What +a list of linked records renders as has changed before (see +`lookup/two-records-with-one-name-are-two-records`), and a case that pinned it +would be rewritten by a change that did not touch this behaviour. What matters +here is that the column exists and the numbered cases are right. + +## Why the fixture is shaped this way + +**Two** number branches, and the runner refuses fewer. The branches with a case +attached have to agree with each other — that agreement is exactly what stopped +the merge from looking any further. With one branch there is nothing to agree +with, and the merge may reach the otherwise branch on its own. + +The linked column must hold a **list**, checked before the checkpoint: holding a +single value it would be the same kind of thing as the numbers, and there would +be nothing to reconcile. + +## The v1 column + +v1 reproduces this on **every** column of the acceptance matrix, `develop` +included. The fix is v2-only, so on the older engine a column of this shape still +cannot be made. Reported rather than enforced — the v1 column is a reference and +never gates a run. + +That is the third case here to say the same thing about v1; the others are +`lookup/distinct-choices-in-the-order-they-appear` and +`lookup/two-records-with-one-name-are-two-records`. diff --git a/cases/lookup/an-any-of-these-total-stays-inside-its-link.case.ts b/cases/lookup/an-any-of-these-total-stays-inside-its-link.case.ts new file mode 100644 index 0000000..1b672a1 --- /dev/null +++ b/cases/lookup/an-any-of-these-total-stays-inside-its-link.case.ts @@ -0,0 +1,60 @@ +import { defineBugCase } from "../../framework/types"; + +// T7004: a total over linked rows narrowed to "status is todo OR status is +// doing". Written with OR, the condition escaped the link - the query stopped +// asking "and linked to this row" and totalled every matching row in the other +// table. The number that came out was a real sum of real rows, so nothing +// looked broken; the tell in the report is a project joined to nothing that +// already shows other people's figures. +export default defineBugCase({ + id: "lookup/an-any-of-these-total-stays-inside-its-link", + title: "An any-of-these total counts only the rows this one is linked to", + runner: "or-filtered-rollup-scope", + timeoutMs: 300_000, + bug: { + issue: "T7004", + status: "fixed", + sourceCommits: ["8713707c2"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-or-rollup-scope", + linkedHost: "the-project-with-work", + unlinkedHost: "the-project-joined-to-nothing", + selectedStatuses: ["todo", "doing"], + work: [ + { + name: "mine-todo", + owner: "the-project-with-work", + status: "todo", + amount: 10, + }, + { + name: "mine-doing", + owner: "the-project-with-work", + status: "doing", + amount: 20, + }, + { + name: "mine-done", + owner: "the-project-with-work", + status: "done", + amount: 30, + }, + { + name: "theirs-todo", + owner: "somebody-else", + status: "todo", + amount: 400, + }, + { + name: "theirs-doing", + owner: "somebody-else", + status: "doing", + amount: 500, + }, + ], + settleTimeoutMs: 60_000, + pollIntervalMs: 500, + }, +}); diff --git a/cases/lookup/an-any-of-these-total-stays-inside-its-link.md b/cases/lookup/an-any-of-these-total-stays-inside-its-link.md new file mode 100644 index 0000000..d02707e --- /dev/null +++ b/cases/lookup/an-any-of-these-total-stays-inside-its-link.md @@ -0,0 +1,51 @@ +# lookup/an-any-of-these-total-stays-inside-its-link + +**T7004** — fixed. On the `or-filtered-rollup-scope` runner. + +## What the user sees + +A project row totals the work linked to it, narrowed to "status is todo **or** +status is doing". The figure is too large: it includes work belonging to other +projects, as long as that work matches the condition. + +The number looks fine. It is a real sum of real rows, in the right units, of the +right order of magnitude — there is nothing to notice. What the report actually +leads with is the other symptom: a project created a moment ago, joined to +nothing at all, already showing a figure. + +## Why + +"Any of these" is written as OR. The link scope — "and linked to this row" — +was being combined with the condition in a way that let the OR swallow it, so +the query asked for every matching row in the other table instead of every +matching row _among this row's_. + +## What the checkpoint asserts + +Two things: + +- the row linked to nothing totals nothing, and +- the linked row totals exactly its own selected work. + +The first is the one a person would notice; the second is the one that says the +condition still works. A build that fixed the scope by ignoring the condition +would pass the first and fail the second. + +## Why the fixture is shaped this way + +Three kinds of row in the other table, and the runner refuses to run without all +three: + +- linked to this project and selected by the condition — what should be counted; +- linked to this project and excluded — proves the condition is still applied; +- selected by the condition but belonging to another project — proves the link + is still applied. + +Drop the third and a total that ignored the link entirely would give the right +answer, and the case would be green on both sides of the fix. The other +project's amounts are an order of magnitude larger than this one's, so a total +that escapes is unmistakable in the failure message rather than merely wrong. + +The wait before the checkpoint is on the **linked** row reaching its correct +total — waiting for the computation to finish, not for the bug to show up. The +unlinked row is then read out of that same settled response. diff --git a/cases/lookup/distinct-choices-in-the-order-they-appear.case.ts b/cases/lookup/distinct-choices-in-the-order-they-appear.case.ts new file mode 100644 index 0000000..9dc78fd --- /dev/null +++ b/cases/lookup/distinct-choices-in-the-order-they-appear.case.ts @@ -0,0 +1,33 @@ +import { defineBugCase } from "../../framework/types"; + +// T7044: two wrong answers from one summary. A parent row summarising its +// children's choice column got "Todo" then "Done" back as "Done, Todo" - sorted, +// not in the order of the rows - so it disagreed with the plain list beside it. +// And when both children said "Todo", the count of distinct values answered 2: +// it was counting rows. Neither looks broken; 2 is the number of children, and +// a reordered pair of words reads as an arbitrary choice. +export default defineBugCase({ + id: "lookup/distinct-choices-in-the-order-they-appear", + title: "Distinct choices come back in the order the rows are in", + runner: "select-rollup-unique-and-count", + timeoutMs: 300_000, + bug: { + issue: "T7044", + status: "fixed", + sourceCommits: ["ebd9d7549"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-select-rollup-unique", + parentRowName: "the-parent", + children: [ + { name: "child-first", status: "Todo" }, + { name: "child-second", status: "Done" }, + ], + whenTheRowIsWritten: "beforeTheSummaries", + alsoCheckAfterAnEdit: true, + retarget: { childName: "child-second", status: "Todo" }, + settleTimeoutMs: 60_000, + pollIntervalMs: 500, + }, +}); diff --git a/cases/lookup/distinct-choices-in-the-order-they-appear.md b/cases/lookup/distinct-choices-in-the-order-they-appear.md new file mode 100644 index 0000000..4211bce --- /dev/null +++ b/cases/lookup/distinct-choices-in-the-order-they-appear.md @@ -0,0 +1,75 @@ +# lookup/distinct-choices-in-the-order-they-appear + +**T7044** — fixed. On the `select-rollup-unique-and-count` runner. + +## What the user sees + +A parent row summarises its children's choice column. Two things are wrong at +once: + +- the distinct values come back **sorted** rather than in the order of the rows: + children reading "Todo" then "Done" produce "Done, Todo"; +- when both children read "Todo", the count of distinct values answers **2**. + It is counting rows. + +Neither reads as a fault. A reordered pair of words looks like an arbitrary +choice the product made, and 2 is the number of children, so it is a number +somebody would act on. + +What makes it findable is the other summaries on the same row. Join and compact +over the same column are correct, so the row shows "Todo, Done" and "Done, Todo" +side by side. + +## What the checkpoint asserts + +Both halves, in two phases: + +1. as built — the distinct values are in row order, and there are as many as + there are distinct values; +2. after one child is edited so the two agree — the distinct values collapse to + one, and the count follows. + +The second phase needs a real edit, not a rewrite of the same value: a write that +changes nothing schedules nothing, and the case would be reading the first +computation twice. + +Both phases run even when the first found something, and the failure carries +everything at once. That is not tidiness: on a pre-fix commit the order is +already wrong in phase one, so a checkpoint that stopped there would never reach +the count — the fault that only appears once two children agree — and the second +half of the report would be asserted but never demonstrated. + +Join and compact are read on every check as the **control**. They take the same +path from the same column, so if they disagree with the rows the whole summary is +broken and the failure says so rather than blaming the distinct values. When the +count is wrong, the message also says whether the number it gave equals the +number of linked rows, because that is what "counting rows" looks like and it +saves the next reader the arithmetic. + +## Why the fixture is shaped this way + +The children's choices must not already be in alphabetical order, and the runner +refuses a fixture where they are: sorting "Done" then "Todo" produces "Done, +Todo", which is also the right answer, so a summary that sorted instead of +keeping row order would look correct. + +After the edit at least two children must agree, or counting rows and counting +distinct values give the same number and the second half proves nothing. + +## The v1 column + +v1 reproduces this on **every** column of the acceptance matrix, `develop` +included. The fix is v2-only, so anyone still on the older engine sees both +faults today: the distinct values sorted rather than in row order, and the count +counting rows. + +The v1 column never fails a run — it is a reference, not a gate — so this is +reported rather than enforced. It is also the clearest thing the v1 column has +said so far: not "v1 was affected too", but "v1 still is". + +## Its neighbour + +T7066 (`893d0ce20`) reports the same wrong order, reached differently — through +records created by API rather than by hand, where the first computation comes out +wrong and a later recompute repairs it. Whether this case also settles that one +is a question for a matrix run against its parent, not an assumption. diff --git a/cases/lookup/the-largest-of-a-borrowed-list.case.ts b/cases/lookup/the-largest-of-a-borrowed-list.case.ts new file mode 100644 index 0000000..b92a050 --- /dev/null +++ b/cases/lookup/the-largest-of-a-borrowed-list.case.ts @@ -0,0 +1,34 @@ +import { defineBugCase } from "../../framework/types"; + +// T7099: a conditional total asking for the largest or the smallest over a +// column that is itself a borrowed list. Sum and average had been taught +// to look inside those lists; these four had not, and went straight at the +// stored list, which Postgres refuses outright. The column then never produced +// anything - empty, with no explanation, on a field the interface offered to +// build. Sum on the same source works, which makes it look like the data is +// wrong rather than the function. +export default defineBugCase({ + id: "lookup/the-largest-of-a-borrowed-list", + title: "The largest of a borrowed list is a number, not a refusal", + runner: "jsonb-lookup-aggregate", + timeoutMs: 300_000, + bug: { + issue: "T7099", + status: "fixed", + sourceCommits: ["281f6ae1a"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-jsonb-agg", + matchKey: "the-only-group", + middleRowName: "the-team", + hostRowName: "the-report", + leaves: [ + { name: "leaf-small", amount: 10 }, + { name: "leaf-large", amount: 30 }, + ], + aggregations: ["max", "min"], + settleTimeoutMs: 60_000, + pollIntervalMs: 500, + }, +}); diff --git a/cases/lookup/the-largest-of-a-borrowed-list.md b/cases/lookup/the-largest-of-a-borrowed-list.md new file mode 100644 index 0000000..7551195 --- /dev/null +++ b/cases/lookup/the-largest-of-a-borrowed-list.md @@ -0,0 +1,73 @@ +# lookup/the-largest-of-a-borrowed-list + +**T7099** — fixed. On the `jsonb-lookup-aggregate` runner. + +## What the user sees + +A report row is given a column totalling across the teams it matches, asking for +the **largest** amount. The column cannot be made: the request comes back +refused, with the database's own words about a function that does not exist. +Smallest behaves the same way. Sum and average over the very same source column +build fine, which makes it look like something about this particular field +rather than about the function. + +The field editor offered all four. + +## Why + +The team row borrows every task's amount, so that column holds a list rather +than one value — that is what borrowing across a one-to-many produces. Sum and +average had been taught to look inside such a list before adding up. Largest, +smallest, all-of and any-of had not: they were applied to the stored list +directly, and Postgres has no largest-of-a-list and will not read a list as a +yes/no. The computation failed and the column never produced anything. + +## What the checkpoint asserts + +Asking for the total is itself inside the checkpoint, because asking is what +fails — before the fix the create is refused outright. Building the chain the +column reads from is setup; asking the question is the observation. Kept the +other way round, the same failure would score as "this case could not run here" +rather than as the bug. + +Each requested total then reads its correct answer, **and** the product does not +mark any of those columns broken. Both directions matter: a column that reads +correctly while still flagged as broken, or one flagged fine while empty, are +each half a fix. The failure message carries both the values read and the broken +list, so a red column says which of the two happened. + +## Why the fixture is a chain of three tables + +Two will not do it. The source column has to hold a list, and a column only +becomes a list by borrowing across a one-to-many — so there is a leaf table with +the values, a middle table borrowing them, and a host table totalling across the +middle. A total taken straight off a plain number column takes a different path +and answers correctly on both sides of the fix. + +The fixture checks that the borrowed columns really do hold lists before going +on, because if they held single values the case would be watching the path that +already worked. + +The expected answers are worked out from the list the product actually built, +read back off the middle row — not from the leaf rows the case seeded. Those are +not the same thing, and asserting against the seed would be asserting against +this case's model of the product rather than against the product. The runner +then refuses a fixture whose borrowed amounts are all equal, since largest and +smallest could not be told apart. + +## Why the tickbox half is not here + +The same fix repaired all-of and any-of over borrowed tickboxes, and this case +deliberately does not cover them. An unticked box does not reach a borrowed list +at all: a pair of leaves, one ticked and one not, produces the borrowed list +`[true]` — measured, not assumed. All-of and any-of over that list both answer +true whether they work or not, so a case built on it would be green on every +column. + +Covering that half needs a source list that can hold `false`, which a borrowed +tickbox column does not appear to produce. The runner refuses the boolean +aggregations rather than asking a question it cannot tell the answer to. + +## Only v2 has this column + +Conditional totals are a v2 column type. There is no field on the older engine to ask this of. diff --git a/cases/record/add-a-row-to-a-table-that-joins-people-columns.case.ts b/cases/record/add-a-row-to-a-table-that-joins-people-columns.case.ts new file mode 100644 index 0000000..47e3e1e --- /dev/null +++ b/cases/record/add-a-row-to-a-table-that-joins-people-columns.case.ts @@ -0,0 +1,34 @@ +import { defineBugCase } from "../../framework/types"; + +// T7024: "everyone involved, listed once, separated by commas" over seven people +// columns, written four functions deep. Each layer re-stated the whole of the +// layer inside it, so the statement the database was asked to plan grew with +// every one, reaching megabytes. The row is recomputed inside the write, so +// nothing came back at all: the page spun and the gateway gave up. The table +// could not accept a row - not slowly, at all - and all a person could see was a +// timeout. +export default defineBugCase({ + id: "record/add-a-row-to-a-table-that-joins-people-columns", + title: "A row can be added to a table whose formula joins people columns", + runner: "nested-user-array-join-create", + timeoutMs: 300_000, + bug: { + issue: "T7024", + status: "fixed", + sourceCommits: ["2c57b7bd8"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-user-array-join", + peopleColumns: 7, + peopleColumnPrefix: "Trainer", + sessionRowName: "the-session", + campusValue: "the-campus", + noteRowName: "the-note-being-added", + // A plain separator. The customer's was an ideographic comma; what grows + // the statement is the nesting, not the character, and this repository is + // English-only. + separator: ", ", + writeBudgetMs: 60_000, + }, +}); diff --git a/cases/record/add-a-row-to-a-table-that-joins-people-columns.md b/cases/record/add-a-row-to-a-table-that-joins-people-columns.md new file mode 100644 index 0000000..60ffbb3 --- /dev/null +++ b/cases/record/add-a-row-to-a-table-that-joins-people-columns.md @@ -0,0 +1,69 @@ +# record/add-a-row-to-a-table-that-joins-people-columns + +**T7024** — fixed. On the `nested-user-array-join-create` runner. + +## What the user sees + +Adding a row to the table never finishes. The page spins and the gateway +eventually times out. Every attempt does the same thing. There is nothing else +to see — no error naming a column, no failed field, just a table that will not +take a row. + +## Why + +The table has several people columns and one formula meaning "everyone involved, +listed once, separated by commas": flatten the people columns into one list, drop +the empties, drop the repeats, join what is left. Four functions, each wrapping +the next. + +Each of those layers re-stated the whole of the layer inside it. The statement +the database was asked to plan therefore grew a layer at a time; at seven people +columns it reached megabytes. Because the row is recomputed inside the write, the +write never returned. + +## What the checkpoint asserts + +The formula column can be **made**, the write returns at all, and the table then +lists the row. + +Making the column is inside the checkpoint, not in setup, and that is not +tidiness. What grew a layer at a time is the statement, and planning it is what +fails — so it fails when the column is created as readily as when a row is +added. Measured: on the fix's parent the case never reaches the write, because +creating the column already answers + +``` +Unexpected unit of work error: Error: Client has encountered a connection error +and is not queryable +``` + +which is the message from the customer's own backend log. Built the other way +round, that failure lands in setup and scores as "this case could not run here" +— the one verdict that hides the bug. + +The reported symptom is the write, and the write is still asserted. It is the +second half of the same defect rather than a different one. + +The request carries its own time limit rather than being allowed to hang. A +request that never answers would run out the whole case and be scored as "this +case could not run here" — the one verdict that would hide the bug. Ending the +wait inside the checkpoint makes the silence the report. + +The limit is deliberately generous. This is not a measurement of speed and does +not belong in the performance lab: the difference being asserted is between an +answer and no answer. + +## Why the fixture is shaped this way + +Seven people columns, because that is where the report was filed and because the +statement grew with the count — fewer columns may plan a large statement that +still completes, which would make the case green on both sides. + +The borrowed column from a second table is part of the reported shape: it puts a +second computed column into the same write, which is what the customer's table +had. + +The people columns are all filled with the same person. The growth is in +planning the statement, not in the data, so what the cells contain does not have +to be elaborate — but they are filled rather than empty so the formula has +something real to work on. diff --git a/cases/table/a-duplicated-table-starts-unshared.case.ts b/cases/table/a-duplicated-table-starts-unshared.case.ts new file mode 100644 index 0000000..9d53855 --- /dev/null +++ b/cases/table/a-duplicated-table-starts-unshared.case.ts @@ -0,0 +1,31 @@ +import { defineBugCase } from "../../framework/types"; + +// T6790: duplicating a table carried each view's sharing across with it - the +// switch, the rules, and the password - and only minted a new address. The copy +// was therefore a live public page from the moment it existed, reachable by +// anyone who had ever been given the source's password, with nothing in the +// interface saying so and no prompt asking. Duplicating a base already got this +// right; duplicating a table did not, and the difference had been frozen into a +// test. +export default defineBugCase({ + id: "table/a-duplicated-table-starts-unshared", + title: "A duplicated table does not come out already published", + runner: "duplicate-shared-view", + timeoutMs: 180_000, + bug: { + issue: "T6790", + status: "fixed", + sourceCommits: ["a5f02fd0c"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-duplicate-unshared", + rowTitle: "row-1", + assert: "copyIsNotShared", + shareMeta: { + password: "not-for-the-copy", + allowCopy: true, + includeHiddenField: true, + }, + }, +}); diff --git a/cases/table/a-duplicated-table-starts-unshared.md b/cases/table/a-duplicated-table-starts-unshared.md new file mode 100644 index 0000000..9c8bc8d --- /dev/null +++ b/cases/table/a-duplicated-table-starts-unshared.md @@ -0,0 +1,58 @@ +# table/a-duplicated-table-starts-unshared + +**T6790** — fixed. On the `duplicate-shared-view` runner, `assert: "copyIsNotShared"`. + +## What the user sees + +A table has a view someone shared, with a password on it. The table is +duplicated. The copy is already published: the sharing switch is on, the share +rules including the password came across, and only the address is new. + +Nobody was asked and nothing says so. Every duplicate of that table is another +live public page, openable by anyone who was ever given the source's password. + +## Why + +Duplicating a base already stripped share state from every copied view. +Duplicating a table only re-minted the share id and left the switch and the +rules alone — on both the v2 path and the legacy one. The two duplicate flows +disagreed about the same thing. + +That disagreement was not an accident that went unnoticed: it had been written +into a unit test as the expected behavior, by an earlier fix (`da43a20a2`) that +was only ever about two tables colliding on one share id. + +## What the checkpoint asserts + +Three separate things about every view in the copy, because they are three +separate ways a copy can be reachable: the switch (`enableShare`), the address +(`shareId`), and the rules behind it (`shareMeta`). + +Then, that the **source** still holds its own link, with the same share id it +started with. Without that, "the copy is not shared" could have been satisfied +by a duplicate that unshared everything, which is a different bug. + +A password is set on the source before duplicating, and the fixture refuses to +continue if it did not stick. The password is the part that makes this more than +untidy: an inherited address is a page nobody opened, an inherited password is a +page other people can already open. + +## The v1 column + +v1 reproduces this too, on both pre-fix columns of the acceptance matrix. That +matches the issue's own reading, which named the legacy duplicate path as +spreading the source view row wholesale and overriding only the share id. +Customers on either engine were affected. + +Worth knowing for anyone re-running this: a **local** run of the v1 column on +`9c97d777c` came back green, while CI on the same commit came back red. CI is +the acceptance surface and its answer is the one recorded here, but the two +disagreeing at all is a harness question that is not settled by this case. + +## Its sibling on this runner + +`table/duplicate-with-shared-view` (T6573, `da43a20a2`) asks the older question +— the duplicate must succeed and must not answer on the source's address. Both +run the same setup and the same request; they differ only in what they read off +the copy. Keeping them on one runner is what makes it visible that the second +answer replaced the first. diff --git a/cases/table/y203-duplicate-with-shared-view.case.ts b/cases/table/y203-duplicate-with-shared-view.case.ts index 492cf4d..0e8ab4a 100644 --- a/cases/table/y203-duplicate-with-shared-view.case.ts +++ b/cases/table/y203-duplicate-with-shared-view.case.ts @@ -21,5 +21,6 @@ export default defineBugCase({ baseId: "seed-base", tableNamePrefix: "e2e-lab-shared-view-copy", rowTitle: "row-1", + assert: "copyHasItsOwnLink", }, }); diff --git a/cases/undo/a-second-undo-after-one-that-failed.case.ts b/cases/undo/a-second-undo-after-one-that-failed.case.ts new file mode 100644 index 0000000..1eeb8c1 --- /dev/null +++ b/cases/undo/a-second-undo-after-one-that-failed.case.ts @@ -0,0 +1,28 @@ +import { defineBugCase } from "../../framework/types"; + +// T7038: undo walks backwards through what you did, and the place it has walked +// back to was moved BEFORE the step was carried out and never moved back when +// the step failed. A failed undo therefore still counted as done, so the next +// press skipped over it and reversed the step before - one the person had not +// asked to reverse. The failed undo itself is honest and visible; the second +// press is the part that quietly takes something else away. +export default defineBugCase({ + id: "undo/a-second-undo-after-one-that-failed", + title: + "A second undo after one that failed retries it, and reaches no further", + runner: "undo-cursor-after-a-failed-undo", + timeoutMs: 180_000, + bug: { + issue: "T7038", + status: "fixed", + sourceCommits: ["130d82efd"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-undo-cursor", + rowName: "the-row-nobody-asked-to-delete", + otherRowName: "the-row-that-took-the-value", + originalCode: "code-first", + changedCode: "code-second", + }, +}); diff --git a/cases/undo/a-second-undo-after-one-that-failed.md b/cases/undo/a-second-undo-after-one-that-failed.md new file mode 100644 index 0000000..7370a7d --- /dev/null +++ b/cases/undo/a-second-undo-after-one-that-failed.md @@ -0,0 +1,75 @@ +# undo/a-second-undo-after-one-that-failed + +**T7038** — fixed. On the `undo-cursor-after-a-failed-undo` runner. + +## What the user sees + +Undo cannot carry out a step — for an ordinary reason, and it says so. Press undo +again, and instead of trying that step once more it reverses the step _before_ +it: in this fixture, the creation of the row. A row disappears that nobody asked +to delete. + +The failed undo is not the problem. It is honest and it is visible. The second +press is the one that quietly takes something else away. + +## Why + +Undo moved the place it had walked back to **before** carrying out the step, and +never moved it back when the step failed. A failed undo therefore still counted +as done, and the next press started from behind it. + +## What the checkpoint asserts + +That the second press retries the same step and reaches no further: the row is +still there, and it still holds the value the failed undo could not put back. + +Both halves matter. The row surviving says undo did not reach past the failed +step; the value being unchanged says the step genuinely still has not been +carried out, rather than having quietly succeeded on the second try for some +other reason. + +## What the two presses answer, measured + +| | first press | second press | rows left | +| ---------------------------- | ------------------------------------ | -------------------------- | -------------------------------- | +| `f44a82cf8` (before the fix) | `failed`, "must have a unique value" | `fulfilled` | only the row that took the value | +| `develop` | `failed`, "must have a unique value" | `failed`, the same message | both | + +The first press is identical on both sides — it is honest either way. The whole +difference is the second one. + +## Why the fixture is shaped this way + +A step fails to reverse here because the column does not allow duplicates: the +row's value was changed away from `code-first`, another row has taken +`code-first` since, and putting the old value back would now collide. Nothing is +wrong with the data or with either request. + +The row that takes the value is written on a **different window id**. The undo +stack is keyed by that id, so writing it on the same one would put it on the +history this case walks back through, and the case would be undoing a different +sequence than it describes. + +Before the checkpoint, the fixture requires that the first press really did fail +and that both rows are still present. A first press that succeeded would leave no +failed step for the second to skip, and the case would be reporting on nothing. + +## What this case does not cover + +The report lists six risks. This case covers one: a failed step still counting as +done. The others are two requests undoing at once, two appends racing, undo +racing with append, a crash between writes, and a partly-successful batch. A +single client against one process cannot show any of those, and nothing here +should be read as guarding them. + +## A trap this case fell into first + +Every write here has to carry the window id, because the undo stack is keyed by +it. The generated client takes no per-call headers, so the first version of this +runner passed them where they were quietly ignored — and undo then answered +`{"status":"empty"}`, which the fixture check read as "not fulfilled" and let +through. The case was green on a pre-fix commit while asserting against an empty +stack. + +Both halves are now closed: the writes go through raw axios, and an `empty` +first press is rejected by name as a fixture that never reached the stack. diff --git a/cases/view/a-column-the-view-does-not-place.case.ts b/cases/view/a-column-the-view-does-not-place.case.ts new file mode 100644 index 0000000..50e4630 --- /dev/null +++ b/cases/view/a-column-the-view-does-not-place.case.ts @@ -0,0 +1,26 @@ +import { defineBugCase } from "../../framework/types"; + +// T6545: the notes a view keeps about a column say where it sits and how wide it +// is. Views made long enough ago have entries with a width and no position at all +// - a shape nothing writes any more. Read back, the missing position was passed +// through as missing, so whatever draws the view was handed a column with no +// place among the others. +export default defineBugCase({ + id: "view/a-column-the-view-does-not-place", + title: "A column whose stored notes give it no place still gets one", + runner: "legacy-column-visibility-metadata", + timeoutMs: 180_000, + bug: { + issue: "T6545", + status: "fixed", + sourceCommits: ["fd32044e4"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-legacy-column-place", + rowTitle: "a-row-in-the-table", + legacy: "noPosition", + order: 1, + width: 241, + }, +}); diff --git a/cases/view/a-column-the-view-does-not-place.md b/cases/view/a-column-the-view-does-not-place.md new file mode 100644 index 0000000..57b023f --- /dev/null +++ b/cases/view/a-column-the-view-does-not-place.md @@ -0,0 +1,71 @@ +# view/a-column-the-view-does-not-place + +**T6545** — fixed. On the `legacy-column-visibility-metadata` runner, +`legacy: "noPosition"`. + +## What the user sees + +A table whose views will not load. + +The notes a view keeps about each of its columns say where the column sits and +how wide it is. In views made long enough ago, some entries carry a width and +**no position at all** — a shape nothing writes any more, and not something +anyone did. + +## Why + +Nothing filled the gap in on the way out, and what a view says about a column is +checked there. An entry with no position does not pass that check, so the request +for the table's views fails — every view at once, not one column in one of them. + +Measured on `66919acae`: + +``` +500 {"message":"Invalid View projection","domainCode":"view.invalid_projection", + "issues":[{"code":"invalid_union","errors":[[{"expected":"number", …}]]}]} +``` + +## What the checkpoint asserts + +The views come back at all, then that the entry carries the column's place among +the columns, and the width the stored notes did carry. + +The first of those is what catches this on a pre-fix commit — the request never +returns an entry to inspect. The other two are what says the gap was filled in +rather than papered over. + +The width matters as much as the position here: filling the gap by replacing the +entry would satisfy "it has a position" while throwing away the only thing the +old notes actually said. + +The position is compared against the column's own index among the table's fields +rather than a number written into the case, so the case does not encode a +particular default — only that a column gets the place it should have. + +## Its sibling on this runner + +`view/a-view-that-says-both-things-about-a-column` (T6597) is the other shape of +old notes: an entry carrying both the older visibility key and the current one. +Both shapes fail the same way — the view list refuses with `Invalid View +projection` — and they were fixed three days apart, this one first. What differs +is which part of the entry the check rejects: an unrecognised key there, a +missing number here. + +Same fixture, same observation, two shapes of old data. That is why they share a +runner and differ only in the `legacy` config value. + +## Why the fixture is written with SQL + +Nothing writes either shape any more, which is also why a base carrying one +cannot get out of it from the interface. Before the checkpoint the fixture reads +the stored notes back and requires that they really are the shape this case is +about — for this one, that there is no `order` in them at all. + +## The v1 column + +v1 is red on every column of the acceptance matrix, `develop` included, and for +the same reason as its sibling's: v1 does not fail the request, it answers 200 and +hands the entry back exactly as stored — here, still without a position. + +So on the older engine this data never caused an outage and was never filled in +either. Reported, not enforced. diff --git a/cases/view/a-grid-grouped-by-a-column-you-cannot-read.case.ts b/cases/view/a-grid-grouped-by-a-column-you-cannot-read.case.ts new file mode 100644 index 0000000..95ff832 --- /dev/null +++ b/cases/view/a-grid-grouped-by-a-column-you-cannot-read.case.ts @@ -0,0 +1,30 @@ +import { defineBugCase } from "../../framework/types"; + +// T6944: under the authority matrix a role can withhold a single column, and the +// rest of the table stays readable - that is the point of withholding one column +// rather than the table. But a view remembers what it is grouped by and the page +// sends that grouping with every request for rows, so asked to group by a column +// the reader may not see, the server refused the request outright. What the +// person got was not a view without its grouping but a view with no rows at all +// and a message about a data validation error, naming neither the column nor the +// grouping. An administrator opening the same view sees everything. +export default defineBugCase({ + id: "view/a-grid-grouped-by-a-column-you-cannot-read", + title: "A grid grouped by a column you cannot read still shows its rows", + runner: "group-on-an-unreadable-column", + timeoutMs: 300_000, + bug: { + issue: "T6944", + status: "fixed", + sourceCommits: ["2ae77481c"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-group-unreadable", + rows: [ + { name: "first-deal", stage: "open", cost: 10 }, + { name: "second-deal", stage: "won", cost: 20 }, + { name: "third-deal", stage: "open", cost: 30 }, + ], + }, +}); diff --git a/cases/view/a-grid-grouped-by-a-column-you-cannot-read.md b/cases/view/a-grid-grouped-by-a-column-you-cannot-read.md new file mode 100644 index 0000000..a65bc2c --- /dev/null +++ b/cases/view/a-grid-grouped-by-a-column-you-cannot-read.md @@ -0,0 +1,91 @@ +# view/a-grid-grouped-by-a-column-you-cannot-read + +**T6944** — fixed. On the `group-on-an-unreadable-column` runner. + +## What the user sees + +Someone whose role withholds one column opens a grid that happens to be grouped +by that column. The grid loads **no rows at all**, with a message about a data +validation error. + +Withholding one column is supposed to leave the rest of the table readable — +that is the point of withholding a column rather than the table. What they get +instead is a view that shows them nothing. + +Nothing in the message names the column, and nothing suggests the grouping is +the thing to change. An administrator opening the same view sees everything, +which is the worst possible shape for a support conversation: the person who can +help cannot see the problem. + +## Why + +A view remembers what it is grouped by, and the page sends that grouping with +every request for rows. The server had two minds about a grouping it could not +honour: a grouping it resolved from the view itself was quietly narrowed to +readable columns, while a grouping that arrived on the request was refused. The +page sends the view's own grouping as if the person had typed it, so the refusal +is what ran — and because it is the grid's own request for rows that fails, the +result is no rows rather than no grouping. + +## What the checkpoint asserts + +The grouped request answers, and answers with every row the person is allowed to +see. + +Both halves. A 200 carrying nothing would be the same empty grid with a friendlier +status. + +## Why the fixture is shaped this way + +Before the checkpoint, the same request is made **without** the grouping, and it +must return every row. That is the control: it says this person can read this +table, so a refusal afterwards is about the grouping rather than about them. + +The fixture also requires that the withheld column really is absent from what +comes back. If the role were not withholding it, grouping by it would be an +ordinary request and the case would be reporting on nothing. + +Exactly one column is withheld, and no rows are. This case is about a withheld +**column**; a role that also hid rows would make "every row the person is allowed +to see" a moving target. + +## Which commit this settles, and which it does not + +Not the one the issue id points at. Measured, one commit at a time: + +| commit | the grouped request | +| ---------------------------------------- | ---------------------------------------------------- | +| `12407c409` — before `f70f0d508` (T6944) | 400, "Group references a field that is not readable" | +| `7bc91231d` — before `a4c8c3396` (T6944) | the same 400 | +| `f44a82cf8` — after both T6944 commits | the same 400 | +| `8fd1e28b9` — before `2ae77481c` (T6997) | the same 400 | +| `2ae77481c` | 200, every row | + +So `bug.sourceCommits` names `2ae77481c`, which carries **T6997**'s id — "evaluate +v2 reads over masked values" — while `bug.issue` stays T6944, because T6944 is +what a person reported and this is that person's symptom. + +Both commits carrying the T6944 id leave this path exactly as it was. They are +recorded in `docs/triage-ledger.md` as halves this case does not settle, rather +than claimed here. + +The first attribution in this case was wrong and the acceptance matrix caught it: +the case was written claiming `a4c8c3396`, and the matrix answered red on a +column **after** that commit. Anything a case claims about which commit fixed +what has to come from a column, not from an issue id. + +## The fixture behind it + +The authority matrix, a role that withholds something, and a second signed-in +person holding that role all have to stand up together, and three other reported +bugs need the same three things. That setup lives in +`framework/authority-matrix.ts` rather than in this runner, and is setup-only for +the same reason `framework/fixture-db.ts` is: asking for it inside a checkpoint +throws. The restricted person's own requests are the observation. + +## The v1 column + +Skipped, and for the harness rather than the product: the case builds its own +base, and `framework/case-base.ts` unstamps only the base it manages, so a base +born inside a runner is born on v2. Same limitation as +`base-share/a-share-link-whose-database-is-away`. diff --git a/cases/view/a-view-that-says-both-things-about-a-column.case.ts b/cases/view/a-view-that-says-both-things-about-a-column.case.ts new file mode 100644 index 0000000..d1899b6 --- /dev/null +++ b/cases/view/a-view-that-says-both-things-about-a-column.case.ts @@ -0,0 +1,28 @@ +import { defineBugCase } from "../../framework/types"; + +// T6597: which columns a view shows has been recorded two ways over this +// product's life - an older note saying whether a column is SHOWN, and the +// current one saying whether it is HIDDEN. Views made long enough ago carry both, +// and nothing writes that shape any more; it is simply what is in the table. Read +// back, the two were passed through side by side, and what a view says about a +// column is checked on the way out - so the request for the table's views failed, +// which is every view at once rather than one column in one of them. +export default defineBugCase({ + id: "view/a-view-that-says-both-things-about-a-column", + title: "A view carrying both notes about a column still reads", + runner: "legacy-column-visibility-metadata", + timeoutMs: 180_000, + bug: { + issue: "T6597", + status: "fixed", + sourceCommits: ["bded2fd80"], + }, + config: { + baseId: "seed-base", + tableNamePrefix: "e2e-lab-legacy-column-meta", + rowTitle: "a-row-in-the-table", + legacy: "bothVisibilityNotes", + order: 1, + width: 241, + }, +}); diff --git a/cases/view/a-view-that-says-both-things-about-a-column.md b/cases/view/a-view-that-says-both-things-about-a-column.md new file mode 100644 index 0000000..0b1f176 --- /dev/null +++ b/cases/view/a-view-that-says-both-things-about-a-column.md @@ -0,0 +1,67 @@ +# view/a-view-that-says-both-things-about-a-column + +**T6597** — fixed. On the `legacy-column-visibility-metadata` runner. + +## What the user sees + +A table whose views will not load. + +The base is old enough that one of its views records a column's visibility the +way this product used to. Nothing anyone did caused it; nothing they can do +undoes it. + +## Why + +Which columns a view shows has been recorded two ways over this product's life: +an older note saying whether a column is **shown**, and the current one saying +whether it is **hidden**. Views made long enough ago carry both, and no request +writes that shape any more. + +Read back, the two were passed through side by side. What a view says about a +column is checked on the way out, and an entry carrying a note nobody expects any +more does not pass that check — so the request for the table's views failed. That +is every view at once, not one column in one of them. + +## What the checkpoint asserts + +That the views come back at all, and that the entry has been settled into one +answer: the older note gone, the current one kept, and the rest of the entry +intact. + +Both halves. A response that came back carrying both notes would hand the +contradiction to whatever reads it next, which is where this started. + +## Why the fixture is written with SQL + +Nothing writes that shape any more — which is exactly why the bases carrying it +cannot get out of it from the interface. `fixture-db` writes the stored notes; +the observation stays on the public view-list endpoint. + +Before the checkpoint, the fixture reads the stored notes back and requires the +older key to actually be there. Without it there is nothing unexpected to read +and the case would report on nothing. + +The entry also carries an order and a width, so a "settled" entry can be told +from an emptied one: dropping the whole entry would satisfy "the older note is +gone" without keeping anything the view needs. + +## The v1 column + +v1 is red on every column of the acceptance matrix, `develop` included, and for a +different reason from v2's. It does not fail the request — it answers 200 and +hands back the entry exactly as stored: + +``` +{"order":1,"visible":true,"hidden":false,"width":241} +``` + +So on the older engine this data never caused an outage and was never settled +either; both notes are still passed through today. The two engines fail this case +in opposite directions, which is worth knowing before anyone reads the v1 column +as "v1 was affected too". + +Reported, not enforced — the v1 column is a reference and never gates a run. It +is the fourth case here to find a v2-only fix leaving the older engine as it was; +the others are `lookup/distinct-choices-in-the-order-they-appear`, +`lookup/two-records-with-one-name-are-two-records` and +`formula/a-column-that-picks-by-case`. diff --git a/docs/triage-ledger.md b/docs/triage-ledger.md index 6645870..6240256 100644 --- a/docs/triage-ledger.md +++ b/docs/triage-ledger.md @@ -118,7 +118,10 @@ The shape is gone; the runner is not kept. | `b90f13537` | T3810 | Written and run: a file uploaded into a cell reaches a watching page carrying its temporary address on the fix's parent too (run 32696695384), so both columns look the same. The single-upload path is evidently decorated already; the fix also touches the batch-create and batch-update projections, which need an attachment token to reach and were not tried. | | `384d2dad1` | T3531 | Written in two shapes and run twice, green on both columns each time: filling a two-way link in from the far side already reaches the near side, and clearing it from there already clears it. Many-to-many in run 32697213211, one-many in run 32697577570. | | `2d93fbef4` | T3303 | Written and run: a formula comparing a number column against blank already answers per row on the fix's parent, empty and zero included (run 32698802701). The half that was broken is the v1 generated-column conversion in `sql-conversion.visitor.ts`, which the lab does not exercise - the same file as the T5496 row above. | -| `7829d83c6` | T6925 | Written in two shapes and run twice, green on both columns each time: an overdue column added over existing rows computes on the fix's parent, whether written as a bare yes/no comparison (run 32705428574) or as an IF() returning two words (run 32704974280). The commit's own reproduction goes through the computed backfill a **field conversion** runs - `table.update` - not the pass that fills a newly created column, and that path was not tried. | +| `7829d83c6` | T6925 | Written in **three** shapes now. The first two were green on both columns: an overdue column added over existing rows computes on the fix's parent, whether written as a bare yes/no comparison (run 32705428574) or as an IF() returning two words (run 32704974280). The third went at the actual cause named in the commit - an `IF()` whose branches are a **date** and a word, so the column is typed as text and every branch is trimmed - and it does not express the behaviour either: on `develop` that column is created without error and then computes **nothing at all**, for the row taking the date and for the row taking the word alike, so there is no correct answer for a pre-fix column to differ from. The path the commit's own reproduction uses is still untried: the computed backfill a **field conversion** runs (`table.update`), not the pass that fills a newly created column. | +| `6ee7f96c4` | T6500 | Written and run twice, green on the fix's parent both times. This is the **field-conversion** backfill the T6925 row names as untried, so that path has now been tried: a table with a number column, rows, and a formula column reading it, then the number column converted to text inside the checkpoint, and in the second shape converted back again - which is what the report describes people doing. Neither direction reproduces `operator does not exist: double precision = text`. Something narrower decides whether the stored column and the freshly computed value end up different kinds; a formula that simply echoes the column is not it. The production reports name six computed fields across two tables in one base, so the shape may need a chain rather than one formula. | +| `7b969558a` | T6890 | Sorting, and deliberately not a change to it: nullable sorts stop emitting a leading `(expr IS NULL)` key and use Postgres's own NULLS FIRST/LAST instead. The commit and the issue both say the point is that the order stays identical to v1's - what changes is the length of the sort list and whether an index can serve it. There is nothing for a case to tell apart. Performance lab, if anywhere. | +| `1ea5d6c40` | T6669 | A read that stopped doing work twice: producing `extra.searchHitIndex` re-ran the whole v1 pipeline - bootstrapping v1 metadata, rebuilding the search WHERE, rescanning for ids that were then thrown away - to get a per-page hit index the v2 read could compute from the ids it already had. The hit index itself is the same on both sides. | | `d36e266aa` | T6912 | Written in two shapes and run twice, green on both columns each time. A payroll chain - rate rows rolling up into an employee's highest rate, a payroll line borrowing that rate and the employee's site, a view filtered on the borrowed site - built entirely through ordinary requests opens on the fix's parent (run 32708030924). The same chain with the borrowed total's rule stripped the way the T6911 case strips it also opens (run 32709591507). The commit's own reproduction is a stored column shape neither of those two produce; what distinguishes it is not established. The already-shipped T6911 case was also run against this parent on its own and stayed green (run 32705941080). | | `6c0970d52` | T6509 | Written in two shapes and run twice, green on both columns each time: a link cell pointing at a row whose name is blank, saved a second time unchanged, comes back without an empty name and can be written straight back. First shape run 32825087075; second - the link naming the column it shows, and the unnamed row written as explicitly having no name - run 32825483798. The commit's own reproduction goes through the v2 contract's own record endpoints rather than the public ones, and what the two send differently is not established. | | `d28589d10` | T6734 | Written in two shapes and run twice, green on both columns each time: a date borrowed across a one-to-one link arrives on the fix's parent, both when the borrowing column is added next to a link that already exists (run 32836154719) and when the host's own date column is converted into a borrowed one (run 32836945426). The commit's own reproduction drains the computed queue between each step; what the two do differently is not established. | @@ -138,6 +141,26 @@ The shape is gone; the runner is not kept. | `0548611b2` | T6576 | Not attempted. The commit's own reproduction is skipped under forced v2 - the spec gates it on the v1 path - and the lab forces v2, so the case could not go red. Same reason as the T5496 and T3303 rows. | | `7cb4431e9` | T6502 | Not attempted, same reason: the commit covers the shape with a forced-v1 e2e, and the lab forces v2. | | `057443dd6` | T6719 | Not attempted. The crash needs a preview flag that turns on a different record-query wrapper; the lab does not set it, so grid statistics take the ordinary path and nothing goes red. | +| `f160eea3b` | T7065 | Not taken while the fix is unshipped. A share-view scope bypass on the selection `*-by-id` endpoints, CVSS 8.1: the issue was still at "deployed to staging" when this batch was written, and a case here is a working public reproduction. It is a good case once it ships - the repro is a single request with a share header - so this row is a reminder, not a rejection. See CONTRIBUTING.md. | +| `ae70b638b` | T7104 | The failure is a connection timeout inside a `table.update` schema operation that then dead-letters after three attempts. What the fix changes is how that timeout is settled - rollback rather than an unrepairable failure - and the lab has no way to make a connection time out on request. Same async-runner trap as T6768 and T6853. | +| `8d5c0fe38` | T7067 | Selection aggregation was being answered by v1, where a date column met a cast v1 cannot do. The fix routes it to v2. That makes the pre-fix state "v1 answered", which `assertServedByV2` treats as the case being unable to run (💥) rather than as the bug - so the column that should be red is the one column the harness refuses to read. The observation is real and reachable; expressing it needs a runner allowed to assert that a request was **not** on v2, which does not exist here. | +| `9f5509f48` | T7019 | An incident, not a behaviour. Concurrent replicas UPSERTing the same five-minute query-observation window took transaction locks that held connections until the pool was exhausted; the fix hardens that write. What a case would have to reproduce is contention between replicas, and this harness runs one application against one database - a single writer never conflicts with itself. Belongs in the performance lab if anywhere. | +| `e3bb7671c` | T6988 | Not attempted, on the strength of the fix's own reproduction. The failure needs a client whose local `cmp_` doc was never created while the server snapshot already sits at generation 2 or higher, and the commit's e2e reaches that by stubbing the snapshot loader through a service hook **and** assigning `doc.version = 0` by hand. Neither is available here: the observation seam is a real subscription over the wire, which fetches the snapshot rather than replaying ops from zero. Reproducing it honestly means winning a race - subscribing to an empty doc and having the generation pass 1 before the create op arrives - which is the shape that produces cases green on every column. Worth revisiting if the realtime helper ever exposes the underlying doc. | +| `4b57c03da` | T7070 | Written and run. The fixture builds cleanly - a manyOne link with a column borrowed through it, then `fixture-db` drops the hidden `__fk_` column the link's own settings still name - and adding a row to the other table is refused. But the refusal is `Failed to insert record: column t.__fk_… does not exist`, raised during the insert, while the fix repairs `Failed to propagate dirty records`, raised by the deferred propagate. Two call sites. The commit's own e2e reaches the second by draining an outbox inside the v2 test container; this harness runs the Nest application, where the same small fixture computes inline and never gets there. Tried one-way and two-way links; both fail in the insert. Same trap as T6728. **The insert-path failure is still present on `develop`** - see the note below the table. | +| `4f35a4a64` | T7047 | The observation lives on the v2 contract's own list endpoint - `limit`/`cursor`/`includeTotal` - not on the public record API this lab reads through, and the lab's client does not speak it. What changed behind that endpoint is also performance-shaped: skip `count(*)` unless asked, page by cursor instead of OFFSET. Same reason as the T5268 row: much of the fix introduces the path it repairs, so there is no before to compare against. | +| `a4e2a0a55` | T7105 | Cost, not behaviour. Field masks unioned every readable field into the SQL projection, so a request for 27 columns still selected all 235. What the request answers with does not change - the same fields come back either way - so there is nothing for a case here to tell apart. It is the product-side follow-up to a 503 incident about wide-table polling, and belongs in the performance lab if anywhere. | +| `719079af1` | T6711 | The observation is a schema operation's own terminal classification - whether a leftover `table.import` is marked dead or repaired - which lives in the background runner and never reaches an HTTP response. The lab has no seam on that: the T7070 attempt established by measurement that a small fixture here computes inline and the deferred path is not reached. Same family as T6768 and T6853. | +| `64b6446061` | T6904 | Same seam. A computed task planned against a table whose `provision_state` is still `pending` was dead-lettered as an obsolete plan instead of retried; the fix changes how the worker classifies that. Both the trigger (a table caught mid-provision by a background stage) and the observation (the task's failure classification) are inside the worker. Nothing a request answers differs. | +| `023b657cd` | T6982 | Written and run twice, green on the fix's parent both times. A settings change was made, the job it recorded was rewritten by `fixture-db` into the interrupted shape the commit describes (pending, `metadata_pending`, no `last_error`, old enough to claim) and `table_meta.provision_state` set to `pending`. First shape asked that the table be out of reach and then come back; second asked only that it read within two minutes. **On both `28a55d9ac` and `develop` the table read immediately anyway**, so there is nothing to tell apart. Whatever else is true, a table carrying `provision_state = 'pending'` was not out of reach in this environment - which is the assumption both shapes were built on. What the fix changes is whether the keeper repairs the job or marks it dead, and that lives in the job's own row; the commit's own e2e reads it through Prisma and drives the runner in process. Same seam as T6711 and T6904. | +| `1f33ae31c` | T7061 | Written and run twice, green on the fix's parent both times. The chain is the reported one: a conditional lookup matching on a shared reference, a formula joining what it borrows, and a matching row added on the other side inside the checkpoint, with the fixture starting empty so the arrival is the trigger. First shape was lookup plus one formula; second added three more formula steps after it, because the commit says the fault needs a stage that runs the lookup edge **while the parent plan still has leftover formula steps**. Neither reproduced. Whether a plan splits into stages at all is the planner's decision - `d74a81ab1` explicitly keeps small chains in one stage - and a case cannot ask for a split from outside. Reaching this needs a chain long or wide enough that the planner splits it, which is a size nobody has established from the public API. | +| `38d0e067e` | T7059 | Also in the browser, and further out: Enter posted a comment while an image was still uploading, so the placeholder went out with no url. The fix holds the composer until the upload lands and makes the progress visible. There is no request to observe - the wrong one was sent on purpose. | +| `55c73a01d` | T6893 | A migration, not a repair: it moves the remaining table REST handlers onto the v2 dual path and stops them reading v1 services. The pre-fix state is therefore "v1 answered", which `assertServedByV2` treats as the case being unable to run rather than as the bug - the column that should be red is the one the harness refuses to read. Same reading as T7067. | +| `41e9ae6de` | T6694 | Same shape, same reason: duplicate reads are moved onto v2. Before it, v1 answers. | +| `60f2045cf` | T6895 | The observation lives on an endpoint this commit introduces. A single POST timed out at the gateway on large workbooks, and the fix replaces it with a stream that reports committed rows as they land - so there is no request both sides answer, and the pre-fix side answers nothing at all on the input that makes the difference. The one behavioural half that is not new - other sheets keep importing after a row cap - still reaches it through the stream. Same reading as T5268. | +| `e770dd1ac` | T7057 | Index coverage, not results. Substring search documents and the trigram indexes behind them are narrowed to text-shaped fields, and an all-field search over an uncovered field falls back to the unindexed path rather than answering differently. What a search returns is the same on both sides; what changes is whether an index can serve it. Performance lab, if anywhere - same reading as T6821. | +| `f70f0d508` | T6944 | Neither commit carrying this issue id fixes the path `view/a-grid-grouped-by-a-column-you-cannot-read` observes. That case is red on `12407c409` (before this commit), on `7bc91231d` (after it), and on `f44a82cf8` (after both), and turns green only at `2ae77481c` — which carries T6997. This one narrows a grouping the server resolves from the view itself; the case exercises a grouping that arrives on the request, which is what the grid actually sends. Reaching the other path needs a request carrying no grouping while the view carries one, and the record endpoint the lab reads through does not obviously offer that. | +| `a4c8c3396` | T6944 | Same reading, same measurements: the case is red on `f44a82cf8`, which is after this commit. It aligns the group metadata a view reports with the permissions applied to it, which is what the settings screen reads, not what the grid's request for rows goes through. | +| `6235527b4` | T7027 | Not taken while the fix is unshipped. A folder's `children` still lists the ids of resources the caller may not read, so a permission-filtered response carries names of things the reader was filtered away from; the reported symptom is a console error and an empty folder. The issue was still at "deployed to staging" when this was written, and a `status: open` case here would be a public reproduction of an unshipped disclosure. Same call as T7065. The fixture it needs now exists (`framework/authority-matrix.ts`), so this is a reminder rather than a rejection: it is ready to write the day it ships. | ### The date comparison inside AND or OR @@ -513,3 +536,66 @@ it in prose is how the two drift apart. To see it: ```bash pnpm triage:covered ``` + +### T7070's neighbour, still open + +Rejecting the T7070 case turned up something that is not T7070. On `develop`, +a base holding a link whose hidden `__fk_` column is missing **cannot accept +rows into the table on the other side at all**: the insert itself is refused +with `column t.__fk_… does not exist`, before any propagation runs. Measured on +`8f3f6df16` and on `692c2b4b5`, with both one-way and two-way links. + +T7070 repaired the propagate path for exactly this state. The insert path was +not part of it and answers the same way it did before. Whether that is worth +its own report is a judgment for a person; it is recorded here so the next pass +does not spend the same afternoon rediscovering it. + +### The permission-matrix family is reachable, and nobody has built the fixture yet + +Four uncovered fixes wait behind one piece of setup that does not exist here +yet: `2ae77481c5`/T6997 (v2 reads over masked values), `68b7d74f05`/T7025 +(archiving gated by the matrix for restricted collaborators), `6235527b4c`/T7027 +(references to permission-filtered nodes), and `a4c8c3396b`+`f70f0d5083`/T6944 +(a grid view whose group field the reader cannot see returns no records at all, +with "Group references a field that is not readable"). + +None of them is blocked by the harness. The matrix is driven entirely through +public endpoints — `PATCH /api/base/:baseId/authority-matrix/status` to turn it +on, `GET /api/base/:baseId/authority-matrix`, `PUT +/api/base/:baseId/authority-matrix/:id` to shape a role — and a second signed-in +user comes from `test/utils/axios-instance/new-user`, which runners can import +the same way they import `init-app`. `enterprise/backend-ee/test/authority/` is +the worked example. + +What is missing is a fixture that puts those together: matrix on, a role that +makes one field unreadable, a second user holding that role. That is a bigger +piece of setup than any case here has needed, and it is worth building once +rather than four times. T6944 is the best first customer — its symptom is the +whole view returning nothing, which is unmistakable — and T7025 should wait +either way, since it was still on staging when this was written. + +### One server-side half of `38d0e067e` is waiting for its release + +Alongside the two browser fixes above, that commit tightens two server checks: +deleting your own comment now needs `record|comment` as editing it does, and the +per-record comment count now needs `record|read` so the matrix row scope covers +it. Both are paths that were open and are now closed, which is a case shape this +repository can express and `framework/authority-matrix.ts` can already build. + +It is not written yet for the same reason as T7065 and T7027: the issues were +still at "deployed to staging" when this was read, and a case here would be a +public reproduction of an unenforced permission that has not shipped. Worth +writing the day it does — the count one especially, since a count that ignores +the row scope reports on rows the reader cannot open. + +### Something noticed while failing to reproduce T6925 + +On `develop`, a formula written as `IF({a checkbox}, {a date}, "a word")` is +accepted, is not marked as having an error, and computes nothing — no date on the +row whose checkbox is ticked, and not even the word on the row whose checkbox is +not. Measured while trying the third shape above, on a two-row table. + +That is not T6925 and it is not claimed to be a fault here; a formula mixing a +date branch with a text branch may simply not be a supported thing to write. But +a column that is accepted, is not flagged, and answers nothing is worth somebody +looking at, and the next person to try this shape will hit it immediately. diff --git a/framework/artifacts.ts b/framework/artifacts.ts index c0abea2..33c1ee3 100644 --- a/framework/artifacts.ts +++ b/framework/artifacts.ts @@ -17,6 +17,8 @@ export interface BugArtifactPayload { // The teable-ee revision this observation belongs to. The comparison table // groups payloads by this field, never by artifact directory names. commitSha: string; + // Which engine answered. v2 is the guarded column; v1 is reference only and + // never fails a run (framework/verdict.ts). engine: string; appUrl: string; observed: ObservedOutcome; @@ -52,7 +54,7 @@ const VERDICT_LABEL: Record = { const renderSummary = (payload: BugArtifactPayload): string => { const lines = [ - `### ${payload.caseId} @ ${payload.commitSha.slice(0, 10)}`, + `### ${payload.caseId} @ ${payload.commitSha.slice(0, 10)} (${payload.engine})`, "", `- verdict: ${VERDICT_LABEL[payload.verdict]}`, `- bug: ${payload.bug.issue} (declared ${payload.bug.status})`, @@ -79,11 +81,17 @@ export const writeBugArtifacts = async ( if (!artifactDir) { // Local direction-finding runs may not set an artifact dir; the console // summary below is still worth having. - console.log(`[e2e-lab] ${payload.caseId}: ${payload.verdict}`); + console.log( + `[e2e-lab] ${payload.caseId} (${payload.engine}): ${payload.verdict}`, + ); return; } await mkdir(artifactDir, { recursive: true }); - const stem = sanitizeCaseId(payload.caseId); + // The engine is part of the file name, not only of the payload: two engines + // write for the same case in the same directory, and a shared stem would + // leave one silently overwriting the other — which the fail-closed report + // would then read as a missing cell rather than as a collision. + const stem = `${sanitizeCaseId(payload.caseId)}-${payload.engine}`; await writeFileAtomically( join(artifactDir, `${stem}.json`), `${JSON.stringify(payload, null, 2)}\n`, diff --git a/framework/authority-matrix.ts b/framework/authority-matrix.ts new file mode 100644 index 0000000..6f3d7fa --- /dev/null +++ b/framework/authority-matrix.ts @@ -0,0 +1,209 @@ +import { Role } from "@teable/core"; +import { + axios, + createBase as apiCreateBase, + createSpace as apiCreateSpace, + deleteSpace, + permanentDeleteSpace, + EMAIL_SPACE_INVITATION, + urlBuilder, + USER_ME, +} from "@teable/openapi"; +import { createNewUserAxios } from "../../utils/axios-instance/new-user"; +import { isInsideCheckpoint } from "./checkpoint"; + +/** + * A base with the authority matrix on, and a signed-in person it restricts. + * + * Several reported bugs are only visible to somebody the matrix limits: a + * column they may not read, a row outside their filter, an action their role + * withholds. Every one of them needs the same three things standing up together + * - the matrix enabled on a base, a role that withholds something, and a second + * person holding that role - and none of it is state the ordinary test user can + * observe, because the person who owns a base is not restricted by its matrix. + * + * That setup is bigger than any single case wants to carry, and building it + * four times would be four chances to build it subtly differently. So it lives + * here, once. + * + * SETUP ONLY, like framework/fixture-db.ts and for the same reason: the + * restricted person's own requests are the observation, but standing them up is + * not. Asking for this inside a `bugCheckpoint()` throws. + * + * Everything goes through public endpoints - the same ones the product's own + * settings screens call - so nothing here depends on internals that move. The + * URL strings are literals rather than imports from the enterprise client + * package, because a case runs against teable-ee revisions weeks apart and a + * moved export would break the case everywhere instead of failing honestly on + * the one commit that moved it. + */ + +const UPDATE_AUTHORITY_MATRIX_STATUS = "/base/{baseId}/authority-matrix/status"; +const ADD_AUTHORITY_MATRIX_ROLE = "/base/{baseId}/authority-matrix-role"; +const UPDATE_AUTHORITY_MATRIX_ROLE_USER = + "/base/{baseId}/authority-matrix-role/{authorityMatrixRoleId}/user"; + +// What a role withholds, per table. The shape the product's own role editor +// posts: actions withheld across the table, rows the role can see at all, and +// per-column withholding. +export interface RestrictedTableRule { + tableId: string; + // e.g. ["record|delete"]. Withheld across the whole table. + disabledActions?: string[]; + // Rows the role may see. Omitted means every row. + recordFilter?: { + conjunction: "and" | "or"; + filterSet: { fieldId: string; operator: string; value: unknown }[]; + }; + // Columns the role may not read, write or fill in. + fieldRecordPermission?: { + fieldId: string; + disabledActions: string[]; + }[]; +} + +// The signed-in client, taken from the helper that makes it rather than from a +// bare "axios" import: the type checker stubs this repository's cross-repo +// imports by name, and a package it has no stub for fails the check. +type SignedInClient = Awaited>; + +export interface RestrictedPerson { + // How they got in, carried through so a case can say so in its report. + join: "editor" | "throughTheRoleAlone"; + // Signed in as the restricted person. Their requests are the observation. + axios: SignedInClient; + userId: string; + email: string; + spaceId: string; + baseId: string; + roleId: string; + // Tears down the space, the base and everything in them. + cleanUp: () => Promise; +} + +// One address for the whole lab. The person is identified by it across runs; +// what they are allowed to do is a property of the role in a base, and every +// case builds its own base, so nothing is shared between cases but the name. +const RESTRICTED_EMAIL = "e2e-lab-restricted-reader@example.com"; +const RESTRICTED_PASSWORD = "12345678a"; + +/** + * Stand up a base with the matrix on and a second person restricted by it. + * + * `buildTables` is called with the new base id, as the OWNER, and returns the + * rules for the restricted person's role. Tables have to exist before a role + * can withhold anything in them, which is why it is a callback rather than an + * argument. + */ +export const withRestrictedPerson = async (options: { + namePrefix: string; + runId: string; + buildTables: (baseId: string) => Promise; + // How the person gets into the space. "editor" invites them first, which is + // the ordinary shape: somebody already working in the space, further limited + // by a role. "throughTheRoleAlone" invites nobody - being given the role is + // what joins them, and it joins them as a Viewer. That difference is not + // cosmetic: a Viewer's base role withholds things a role may grant, and bugs + // have lived exactly in the gap between the two. + join?: "editor" | "throughTheRoleAlone"; +}): Promise => { + if (isInsideCheckpoint()) { + throw new Error( + "the authority matrix is fixture, not observation: build it before bugCheckpoint(), " + + "and make only the restricted person's requests inside it", + ); + } + + const suffix = `${options.namePrefix}-${options.runId}`; + let spaceId = ""; + + const cleanUp = async () => { + if (!spaceId) { + return; + } + await deleteSpace(spaceId); + await permanentDeleteSpace(spaceId); + }; + + try { + // The owner's own space and base. It must not be the seed base: turning the + // matrix on changes what every other case reading that base can see. + const space = await apiCreateSpace({ name: suffix }); + spaceId = space.data.id; + const base = await apiCreateBase({ spaceId, name: `${suffix}-base` }); + const baseId = base.data.id; + + // The second person. Signing up is idempotent - the helper signs in when + // the address is taken - so runs share an identity and nothing else. + const personAxios = await createNewUserAxios({ + email: RESTRICTED_EMAIL, + password: RESTRICTED_PASSWORD, + }); + const userId = (await personAxios.get(USER_ME)).data.id as string; + + // Into the space as an ordinary editor, unless the case wants the person to + // arrive through the role alone. Never as an administrator of the matrix: + // an administrator is exempt from it, and this whole fixture exists to + // produce somebody who is not. + if ((options.join ?? "editor") === "editor") { + await axios.post(urlBuilder(EMAIL_SPACE_INVITATION, { spaceId }), { + role: Role.Editor, + emails: [RESTRICTED_EMAIL], + }); + } + + await axios.patch(urlBuilder(UPDATE_AUTHORITY_MATRIX_STATUS, { baseId }), { + enabled: true, + }); + + const tables = await options.buildTables(baseId); + if (tables.length === 0) { + throw new Error( + "a role that withholds nothing restricts nobody - build at least one table rule", + ); + } + + const role = await axios.post( + urlBuilder(ADD_AUTHORITY_MATRIX_ROLE, { baseId }), + { + name: `${suffix}-role`, + enabled: true, + tables: tables.map((rule) => ({ + enabled: true, + tableId: rule.tableId, + disabledActions: rule.disabledActions ?? [], + ...(rule.recordFilter ? { recordFilter: rule.recordFilter } : {}), + fieldRecordPermission: rule.fieldRecordPermission ?? [], + })), + }, + ); + const roleId = (role.data as { id?: string })?.id; + if (!roleId) { + throw new Error( + `adding the role returned no role: ${JSON.stringify(role.data)}`, + ); + } + + await axios.patch( + urlBuilder(UPDATE_AUTHORITY_MATRIX_ROLE_USER, { + baseId, + authorityMatrixRoleId: roleId, + }), + { userIds: [userId] }, + ); + + return { + axios: personAxios, + join: options.join ?? "editor", + userId, + email: RESTRICTED_EMAIL, + spaceId, + baseId, + roleId, + cleanUp, + }; + } catch (error) { + await cleanUp().catch(() => undefined); + throw error; + } +}; diff --git a/framework/runner-registry.ts b/framework/runner-registry.ts index bea4787..9cad3af 100644 --- a/framework/runner-registry.ts +++ b/framework/runner-registry.ts @@ -125,6 +125,19 @@ import { runConditionalRollupNestedOrMatrixCase } from "./runners/conditional-ro import { runRollupLinkIdentityMatrixCase } from "./runners/rollup-link-identity-matrix.runner"; import { runConditionalRollupEditorBrowserCase } from "./runners/conditional-rollup-editor-browser.runner"; import { runLinkPickerTabSelectionBrowserCase } from "./runners/link-picker-tab-selection-browser.runner"; +import { runAutonumberStringFilterCase } from "./runners/autonumber-string-filter.runner"; +import { runCrossBaseConditionalBaseIdCase } from "./runners/cross-base-conditional-base-id.runner"; +import { runGroupOnAnUnreadableColumnCase } from "./runners/group-on-an-unreadable-column.runner"; +import { runJsonbLookupAggregateCase } from "./runners/jsonb-lookup-aggregate.runner"; +import { runLegacyColumnVisibilityMetadataCase } from "./runners/legacy-column-visibility-metadata.runner"; +import { runNestedUserArrayJoinCreateCase } from "./runners/nested-user-array-join-create.runner"; +import { runOrFilteredRollupScopeCase } from "./runners/or-filtered-rollup-scope.runner"; +import { runSameNamedFkBaseDuplicateCase } from "./runners/same-named-fk-base-duplicate.runner"; +import { runSelectRollupUniqueAndCountCase } from "./runners/select-rollup-unique-and-count.runner"; +import { runShareViewUnreadyDataDbCase } from "./runners/share-view-unready-data-db.runner"; +import { runSharedFormCoverUrlCase } from "./runners/shared-form-cover-url.runner"; +import { runSwitchMixedBranchStorageCase } from "./runners/switch-mixed-branch-storage.runner"; +import { runUndoCursorAfterAFailedUndoCase } from "./runners/undo-cursor-after-a-failed-undo.runner"; import type { BugCase, BugCaseFor, @@ -270,6 +283,19 @@ const runners: { [K in BugRunnerKind]: RunnerFn } = { "rollup-link-identity-matrix": runRollupLinkIdentityMatrixCase, "conditional-rollup-editor-browser": runConditionalRollupEditorBrowserCase, "link-picker-tab-selection-browser": runLinkPickerTabSelectionBrowserCase, + "autonumber-string-filter": runAutonumberStringFilterCase, + "cross-base-conditional-base-id": runCrossBaseConditionalBaseIdCase, + "group-on-an-unreadable-column": runGroupOnAnUnreadableColumnCase, + "jsonb-lookup-aggregate": runJsonbLookupAggregateCase, + "legacy-column-visibility-metadata": runLegacyColumnVisibilityMetadataCase, + "nested-user-array-join-create": runNestedUserArrayJoinCreateCase, + "or-filtered-rollup-scope": runOrFilteredRollupScopeCase, + "same-named-fk-base-duplicate": runSameNamedFkBaseDuplicateCase, + "select-rollup-unique-and-count": runSelectRollupUniqueAndCountCase, + "share-view-unready-data-db": runShareViewUnreadyDataDbCase, + "shared-form-cover-url": runSharedFormCoverUrlCase, + "switch-mixed-branch-storage": runSwitchMixedBranchStorageCase, + "undo-cursor-after-a-failed-undo": runUndoCursorAfterAFailedUndoCase, }; export const executeRegisteredRunner = ( diff --git a/framework/runners/autonumber-string-filter.runner.ts b/framework/runners/autonumber-string-filter.runner.ts new file mode 100644 index 0000000..283b3c5 --- /dev/null +++ b/framework/runners/autonumber-string-filter.runner.ts @@ -0,0 +1,162 @@ +import { and, FieldKeyType, FieldType, isGreater } from "@teable/core"; +import { getRecords as apiGetRecords, getRowCount } from "@teable/openapi"; +import { + createField, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { AutonumberStringFilterCaseConfig } from "../types"; + +// A view filtered on the row-number column, "greater than 50" -> checkpoint: +// the row count comes back, and the rows behind it are the ones the filter +// describes. +// +// The number typed into a filter box arrives as text - that is what a text box +// produces, and it is what the grid sends for every numeric column. The +// row-number column was the one place that was not allowed for: the comparison +// demanded an actual number, refused the string, and answered 500. The page +// showed the filter as saved and then broke on the count, so the view a person +// had just built would not open at all. +// +// A saved filter is worse than a failed one: it is loaded again on every visit, +// so the view stays broken until someone works out that the filter is what did +// it. +// +// The count is checked against the rows, not against a number written into the +// case. Comparing two answers from the product catches the failure a hardcoded +// expectation cannot: a count that returns a plausible-looking wrong number +// while the rows disagree with it. + +const TITLE_FIELD = "Title"; +const ROW_NUMBER_FIELD = "No."; + +export const runAutonumberStringFilterCase = async ( + bugCase: BugCaseFor<"autonumber-string-filter">, + context: BugRunContext, +): Promise => { + const config: AutonumberStringFilterCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + let tableId = ""; + + try { + const table = await createTable(baseId, { + name: `${suffix}-rows`, + fields: [{ name: TITLE_FIELD, type: FieldType.SingleLineText }], + records: config.rowTitles.map((title) => ({ + fields: { [TITLE_FIELD]: title }, + })), + }); + tableId = table.id; + + // Added after the rows, which is how a real table gets one: the column + // numbers what is already there. + const rowNumber = await createField(table.id, { + name: ROW_NUMBER_FIELD, + type: FieldType.AutoNumber, + }); + + // The engine assertion, on a read of this table's rows - the same endpoint + // and the same feature the checkpoint's filtered read uses. The response + // is also what the expected answer is derived from, so this is not a + // separate probe. + const listed = await apiGetRecords(tableId, { + fieldKeyType: FieldKeyType.Id, + take: config.rowTitles.length, + }); + const routing = assertServedByV2(listed.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const numbers = listed.data.records.map((record) => + Number(record.fields[rowNumber.id]), + ); + if (numbers.some((value) => !Number.isFinite(value))) { + throw new Error( + `the row-number column did not number every row: ${JSON.stringify(numbers)}`, + ); + } + + const expected = numbers + .filter((value) => value > config.threshold) + .sort((left, right) => left - right); + if (expected.length === 0 || expected.length === numbers.length) { + throw new Error( + `"greater than ${config.threshold}" selects ${expected.length} of ${numbers.length} rows - ` + + "a filter that selects all or none cannot tell a working comparison from a missing one", + ); + } + + // What a filter box sends: the number as text. + const filter = { + conjunction: and.value, + filterSet: [ + { + fieldId: rowNumber.id, + operator: isGreater.value, + value: String(config.threshold), + }, + ], + }; + + const probe = await bugCheckpoint( + "a-row-number-filter-holding-text-counts-and-lists", + async () => { + // Refused before the fix, and a refusal throws here, which is the + // report. + const counted = await getRowCount(tableId, { filter }); + const rowCount = counted.data.rowCount; + + const filtered = await apiGetRecords(tableId, { + fieldKeyType: FieldKeyType.Id, + filter, + }); + const listedNumbers = filtered.data.records + .map((record) => Number(record.fields[rowNumber.id])) + .sort((left, right) => left - right); + + if (JSON.stringify(listedNumbers) !== JSON.stringify(expected)) { + throw new Error( + `"greater than ${config.threshold}" listed rows ${JSON.stringify(listedNumbers)}, ` + + `expected ${JSON.stringify(expected)} out of ${JSON.stringify(numbers)}`, + ); + } + if (rowCount !== expected.length) { + throw new Error( + `the count says ${rowCount} row(s) while the same filter lists ` + + `${JSON.stringify(listedNumbers)}`, + ); + } + return { rowCount, listedNumbers }; + }, + ); + + return { + details: { + tableId, + rowNumberFieldId: rowNumber.id, + threshold: config.threshold, + allNumbers: numbers, + routing, + ...probe, + }, + }; + } finally { + if (tableId) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/cross-base-conditional-base-id.runner.ts b/framework/runners/cross-base-conditional-base-id.runner.ts new file mode 100644 index 0000000..5b94d98 --- /dev/null +++ b/framework/runners/cross-base-conditional-base-id.runner.ts @@ -0,0 +1,266 @@ +import { FieldKeyType, FieldType } from "@teable/core"; +import { + axios, + getRecords as apiGetRecords, + CREATE_FIELD, + GET_FIELD_LIST, + urlBuilder, +} from "@teable/openapi"; +import { + createBase, + createTable, + deleteBase, + permanentDeleteBase, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { CrossBaseConditionalBaseIdCaseConfig } from "../types"; + +// A conditional column reading a table in ANOTHER base -> open its settings +// again -> checkpoint: the settings still name the base it reads. +// +// A conditional lookup or total needs three things: which table, which column, +// and - when the table is not in this base - which base. The first two were +// stored and the third was dropped, so reopening the column's settings found +// the foreign table with no base to look it up in and drew it as a table the +// person has no permission to see. +// +// Nothing else went wrong. The values kept arriving, because the computation +// had already resolved the table; only the settings could not describe +// themselves any more. What that costs is the ability to change the column: a +// person who opens it sees a permission problem that does not exist, and any +// save from that screen writes back settings with the base already missing. +// +// The values are read outside the checkpoint, before the settings are, and +// that order is deliberate: a column that never computed would also have +// nothing sensible to say about its source, and this case is about a column +// that works. + +const CATEGORY_FIELD = "Category"; +const AMOUNT_FIELD = "Amount"; +const HOST_MATCH_FIELD = "CategoryMatch"; +const LOOKUP_FIELD = "Amounts over there"; +const ROLLUP_FIELD = "Total over there"; + +interface FieldSummary { + id: string; + name: string; + options?: Record; + lookupOptions?: Record; +} + +export const runCrossBaseConditionalBaseIdCase = async ( + bugCase: BugCaseFor<"cross-base-conditional-base-id">, + context: BugRunContext, +): Promise => { + const config: CrossBaseConditionalBaseIdCaseConfig = bugCase.config; + const hostBaseId = globalThis.testConfig.baseId; + const suffix = `${config.namePrefix}-${context.runId}`; + let foreignBaseId = ""; + let hostTableId = ""; + + try { + // A second base beside the host's, in the SAME space. Across spaces the + // product refuses the link outright ("cross-space link is no longer + // supported"), so the state this case is about only exists inside one. + const foreignBase = await createBase({ + spaceId: globalThis.testConfig.spaceId, + name: `${suffix}-other`, + }); + foreignBaseId = foreignBase.id; + + const foreign = await createTable(foreignBase.id, { + name: `${suffix}-source`, + fields: [ + { name: CATEGORY_FIELD, type: FieldType.SingleLineText }, + { name: AMOUNT_FIELD, type: FieldType.Number }, + ], + records: config.sourceRows.map((row) => ({ + fields: { [CATEGORY_FIELD]: row.category, [AMOUNT_FIELD]: row.amount }, + })), + }); + const foreignCategoryId = foreign.fields.find( + (field: { name: string }) => field.name === CATEGORY_FIELD, + )?.id as string; + const foreignAmountId = foreign.fields.find( + (field: { name: string }) => field.name === AMOUNT_FIELD, + )?.id as string; + + const host = await createTable(hostBaseId, { + name: `${suffix}-host`, + fields: [{ name: HOST_MATCH_FIELD, type: FieldType.SingleLineText }], + records: [{ fields: { [HOST_MATCH_FIELD]: config.matchedCategory } }], + }); + hostTableId = host.id; + const hostMatchId = host.fields.find( + (field: { name: string }) => field.name === HOST_MATCH_FIELD, + )?.id as string; + if (!foreignCategoryId || !foreignAmountId || !hostMatchId) { + throw new Error("the fixture tables are not in place"); + } + + const matchFilter = { + conjunction: "and", + filterSet: [ + { + fieldId: foreignCategoryId, + operator: "is", + value: { type: "field", fieldId: hostMatchId }, + }, + ], + }; + + const createFieldRaw = (body: unknown) => + axios.post(urlBuilder(CREATE_FIELD, { tableId: host.id }), body, { + validateStatus: () => true, + }); + + // The column that reads across the base boundary. Its own create response + // carries the routing headers, so the engine is asserted on the request + // that puts the state under test in place rather than on a probe beside it. + const lookupResponse = await createFieldRaw({ + name: LOOKUP_FIELD, + type: FieldType.Number, + isLookup: true, + isConditionalLookup: true, + lookupOptions: { + baseId: foreignBase.id, + foreignTableId: foreign.id, + lookupFieldId: foreignAmountId, + filter: matchFilter, + }, + }); + if (lookupResponse.status !== 201) { + throw new Error( + `the cross-base conditional lookup was refused (${lookupResponse.status}): ${JSON.stringify(lookupResponse.data)}`, + ); + } + const routing = assertServedByV2(lookupResponse.headers, { + operation: "POST /table/{tableId}/field", + feature: "createField", + }); + const lookupField = lookupResponse.data as FieldSummary; + + const rollupResponse = await createFieldRaw({ + name: ROLLUP_FIELD, + type: FieldType.ConditionalRollup, + options: { + baseId: foreignBase.id, + foreignTableId: foreign.id, + lookupFieldId: foreignAmountId, + expression: "sum({values})", + filter: matchFilter, + }, + }); + if (rollupResponse.status !== 201) { + throw new Error( + `the cross-base conditional rollup was refused (${rollupResponse.status}): ${JSON.stringify(rollupResponse.data)}`, + ); + } + const rollupField = rollupResponse.data as FieldSummary; + + // Fixture verification, outside the checkpoint: the columns really do read + // across the boundary. A column that computed nothing would have no source + // worth asking about. + const expectedValues = config.sourceRows + .filter((row) => row.category === config.matchedCategory) + .map((row) => row.amount); + if (expectedValues.length === 0) { + throw new Error( + `no source row carries "${config.matchedCategory}" - the columns would read empty either way`, + ); + } + const rows = await apiGetRecords(host.id, { + fieldKeyType: FieldKeyType.Id, + take: 1, + }); + const cell = rows.data.records[0]?.fields[lookupField.id]; + const total = rows.data.records[0]?.fields[rollupField.id]; + if (JSON.stringify(cell) !== JSON.stringify(expectedValues)) { + throw new Error( + `the cross-base column reads ${JSON.stringify(cell)}, expected ${JSON.stringify(expectedValues)} - the fixture did not compute`, + ); + } + const expectedTotal = expectedValues.reduce((sum, value) => sum + value, 0); + if (Number(total) !== expectedTotal) { + throw new Error( + `the cross-base total reads ${JSON.stringify(total)}, expected ${expectedTotal} - the fixture did not compute`, + ); + } + + const probe = await bugCheckpoint( + "a-cross-base-conditional-column-still-names-its-base", + async () => { + // What the settings screen loads when it is reopened. + const listed = await axios.get( + urlBuilder(GET_FIELD_LIST, { tableId: host.id }), + ); + const readBack = (fieldId: string, name: string) => { + const field = listed.data.find( + (candidate) => candidate.id === fieldId, + ); + if (!field) { + throw new Error(`the ${name} column is gone from the table`); + } + return field; + }; + + const lookupBack = readBack(lookupField.id, "conditional lookup"); + const rollupBack = readBack(rollupField.id, "conditional rollup"); + const lookupBaseId = lookupBack.lookupOptions?.baseId; + const rollupBaseId = rollupBack.options?.baseId; + + if (lookupBaseId !== foreignBase.id) { + throw new Error( + `the conditional lookup came back naming base ${JSON.stringify(lookupBaseId)}, ` + + `expected ${foreignBase.id}. Its whole settings read: ${JSON.stringify(lookupBack.lookupOptions)}`, + ); + } + if (rollupBaseId !== foreignBase.id) { + throw new Error( + `the conditional rollup came back naming base ${JSON.stringify(rollupBaseId)}, ` + + `expected ${foreignBase.id}. Its whole settings read: ${JSON.stringify(rollupBack.options)}`, + ); + } + return { lookupBaseId, rollupBaseId }; + }, + ); + + return { + details: { + hostTableId: host.id, + foreignBaseId: foreignBase.id, + foreignTableId: foreign.id, + routing, + ...probe, + }, + }; + } finally { + if (hostTableId) { + try { + await permanentDeleteTable(hostBaseId, hostTableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${hostTableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + if (foreignBaseId) { + try { + await deleteBase(foreignBaseId); + await permanentDeleteBase(foreignBaseId); + } catch (error) { + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (base ${foreignBaseId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/duplicate-shared-view.runner.ts b/framework/runners/duplicate-shared-view.runner.ts index 6e151cc..4059df9 100644 --- a/framework/runners/duplicate-shared-view.runner.ts +++ b/framework/runners/duplicate-shared-view.runner.ts @@ -3,6 +3,7 @@ import { axios, enableShareView as apiEnableShareView, getViewList as apiGetViewList, + updateViewShareMeta as apiUpdateViewShareMeta, DUPLICATE_TABLE, urlBuilder, } from "@teable/openapi"; @@ -26,6 +27,14 @@ import type { DuplicateSharedViewCaseConfig } from "../types"; // carried the same share id would be worse than the 500 - two tables answering // on one public address, where turning off sharing on either takes down a page // the other one is serving - so the ids are compared. +// +// The second question this runner asks (`assert: "copyIsNotShared"`) is what +// the copy should carry INSTEAD, and it is not "a link of its own": nothing. +// Duplicating a table is not a decision to publish one, and a copy that comes +// out already shared - under the source's password and edit rules, with no +// prompt and nothing in the interface saying so - publishes a table nobody +// chose to publish. The source's own link is checked too, because "the copy is +// not shared" must not have been reached by unsharing both. const NAME_FIELD = "Name"; @@ -60,6 +69,22 @@ export const runDuplicateSharedViewCase = async ( ); } + // The share rules a person set on the source. They matter to the second + // question: a copy that inherits these is reachable with the source's + // password by anyone who ever had it. + if (config.shareMeta) { + await apiUpdateViewShareMeta(table.id, viewId, config.shareMeta); + const sourceViews = await apiGetViewList(table.id); + const sourceView = sourceViews.data.find( + (view: { id: string }) => view.id === viewId, + ) as { shareMeta?: Record } | undefined; + if (!sourceView?.shareMeta?.password) { + throw new Error( + `the share rules did not stick on the source view: ${JSON.stringify(sourceView?.shareMeta)}`, + ); + } + } + // Raw axios with the status open: before the fix this request is refused, // and the generated client drops the response - routing headers included - // the moment it is. @@ -96,14 +121,54 @@ export const runDuplicateSharedViewCase = async ( feature: "duplicateTable", }); - // The copy's own share credential. Reusing the source's would put two - // tables on one public address - a success that is worse than the - // failure it replaced. const copiedViews = await apiGetViewList(copyId); const copiedShareIds = copiedViews.data.map( (view: { id: string; shareId?: string | null }) => view.shareId ?? null, ); + + if (config.assert === "copyIsNotShared") { + // Nothing published. Each of the three is a separate way the copy can + // be reachable: the switch, the address, and the rules behind it. + for (const view of copiedViews.data as { + id: string; + name?: string; + enableShare?: boolean | null; + shareId?: string | null; + shareMeta?: Record | null; + }[]) { + if (view.enableShare || view.shareId || view.shareMeta) { + throw new Error( + `the copied view ${view.name ?? view.id} came out shared: ` + + JSON.stringify({ + enableShare: view.enableShare, + shareId: view.shareId, + shareMeta: view.shareMeta, + }), + ); + } + } + + // And the source keeps its own link - otherwise "the copy is not + // shared" could have been reached by unsharing everything. + const sourceViews = await apiGetViewList(table.id); + const sourceView = sourceViews.data.find( + (view: { id: string }) => view.id === viewId, + ) as { enableShare?: boolean; shareId?: string } | undefined; + if ( + !sourceView?.enableShare || + sourceView.shareId !== sourceShareId + ) { + throw new Error( + `duplicating took the source's own link with it: ${JSON.stringify(sourceView)}`, + ); + } + return { routing, copiedShareIds }; + } + + // The copy's own share credential. Reusing the source's would put two + // tables on one public address - a success that is worse than the + // failure it replaced. if (copiedShareIds.includes(sourceShareId)) { throw new Error( `the copied table's views carry ${JSON.stringify(copiedShareIds)}, which includes the source's ` + diff --git a/framework/runners/group-on-an-unreadable-column.runner.ts b/framework/runners/group-on-an-unreadable-column.runner.ts new file mode 100644 index 0000000..bafaba2 --- /dev/null +++ b/framework/runners/group-on-an-unreadable-column.runner.ts @@ -0,0 +1,210 @@ +import { FieldKeyType, FieldType, SortFunc } from "@teable/core"; +import { GET_RECORDS_URL, urlBuilder } from "@teable/openapi"; +import { createTable, permanentDeleteTable } from "../../../utils/init-app"; +import { withRestrictedPerson } from "../authority-matrix"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { GroupOnAnUnreadableColumnCaseConfig } from "../types"; + +// A grid grouped by a column the person opening it may not read -> open it -> +// checkpoint: the rows come back. +// +// Under the authority matrix a role can withhold a single column. The rest of +// the table is still theirs to read - that is the whole point of withholding +// one column rather than the table. +// +// But a view remembers what it is grouped by, and the page sends that grouping +// with every request for rows. Asked to group by a column the reader may not +// see, the server refused the request outright, so what the person got was not +// a view without its grouping - it was a view with NO ROWS AT ALL and a message +// about a data validation error. Nothing in it names the column, and nothing +// suggests the grouping is the thing to change. An administrator opening the +// same view sees everything, which is the worst possible shape for a support +// conversation. +// +// The same request without the grouping is read first, outside the checkpoint. +// That is the control: it says the person can read this table, so a refusal +// afterwards is about the grouping and not about them. + +const NAME_FIELD = "Name"; +const OPEN_FIELD = "Stage"; +const WITHHELD_FIELD = "Owner cost"; + +export const runGroupOnAnUnreadableColumnCase = async ( + bugCase: BugCaseFor<"group-on-an-unreadable-column">, + context: BugRunContext, +): Promise => { + const config: GroupOnAnUnreadableColumnCaseConfig = bugCase.config; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + let person: Awaited> | undefined; + let tableId = ""; + let withheldFieldId = ""; + let openFieldId = ""; + + if (config.rows.length < 2) { + throw new Error( + "at least two rows, or a request that returns nothing looks the same as one that returns everything", + ); + } + + try { + person = await withRestrictedPerson({ + namePrefix: config.tableNamePrefix, + runId: context.runId, + buildTables: async (baseId) => { + const table = await createTable(baseId, { + name: suffix, + fields: [ + { + name: NAME_FIELD, + type: FieldType.SingleLineText, + isPrimary: true, + }, + { name: OPEN_FIELD, type: FieldType.SingleLineText }, + { name: WITHHELD_FIELD, type: FieldType.Number }, + ], + records: config.rows.map((row) => ({ + fields: { + [NAME_FIELD]: row.name, + [OPEN_FIELD]: row.stage, + [WITHHELD_FIELD]: row.cost, + }, + })), + }); + tableId = table.id; + withheldFieldId = table.fields.find( + (field: { name: string }) => field.name === WITHHELD_FIELD, + )?.id as string; + openFieldId = table.fields.find( + (field: { name: string }) => field.name === OPEN_FIELD, + )?.id as string; + if (!withheldFieldId || !openFieldId) { + throw new Error("the table is not in place"); + } + + // One column withheld, and only one. Everything else stays readable, so + // the person can open the table at all. + return [ + { + tableId: table.id, + fieldRecordPermission: [ + { + fieldId: withheldFieldId, + disabledActions: [ + "record|read", + "record|update", + "record|create", + ], + }, + ], + }, + ]; + }, + }); + + const readAs = async (groupBy?: unknown) => + person!.axios.get(urlBuilder(GET_RECORDS_URL, { tableId }), { + params: { + fieldKeyType: FieldKeyType.Id, + take: config.rows.length, + ...(groupBy ? { groupBy: JSON.stringify(groupBy) } : {}), + }, + validateStatus: () => true, + }); + + // Fixture verification, outside the checkpoint. Two things have to be true + // before the grouped request means anything: the person can read the table, + // and the withheld column really is withheld from them. Without the second, + // grouping by it would be an ordinary request and the case would report on + // nothing. + const plain = await readAs(); + if (plain.status !== 200) { + throw new Error( + `the restricted person cannot read the table at all (${plain.status}): ${JSON.stringify(plain.data)}`, + ); + } + const plainRows = + (plain.data as { records?: { fields: Record }[] }) + ?.records ?? []; + if (plainRows.length !== config.rows.length) { + throw new Error( + `the restricted person sees ${plainRows.length} of ${config.rows.length} rows - ` + + "this case is about a withheld column, not withheld rows", + ); + } + if (plainRows.some((row) => row.fields[withheldFieldId] !== undefined)) { + throw new Error( + `the withheld column came back to the restricted person: ${JSON.stringify(plainRows[0]?.fields)} - ` + + "the role is not withholding it, so grouping by it is an ordinary request", + ); + } + const routing = assertServedByV2(plain.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const probe = await bugCheckpoint( + "a-grid-grouped-by-a-column-you-cannot-read-still-shows-its-rows", + async () => { + const grouped = await readAs([ + { fieldId: withheldFieldId, order: SortFunc.Asc }, + ]); + const body = + typeof grouped.data === "string" + ? grouped.data + : JSON.stringify(grouped.data ?? ""); + + if (grouped.status !== 200) { + throw new Error( + `opening the grid grouped by a column the person may not read answered ${grouped.status}, ` + + `so the whole view has no rows rather than no grouping: ${body}`, + ); + } + const rows = + (grouped.data as { records?: { id: string }[] })?.records ?? []; + if (rows.length !== config.rows.length) { + throw new Error( + `the grouped request answered 200 but returned ${rows.length} of ${config.rows.length} rows: ${body}`, + ); + } + return { rows: rows.length }; + }, + ); + + return { + details: { + baseId: person.baseId, + tableId, + withheldFieldId, + roleId: person.roleId, + routing, + ...probe, + }, + }; + } finally { + if (tableId && person) { + try { + await permanentDeleteTable(person.baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + if (person) { + try { + await person.cleanUp(); + } catch (error) { + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (space ${person.spaceId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/jsonb-lookup-aggregate.runner.ts b/framework/runners/jsonb-lookup-aggregate.runner.ts new file mode 100644 index 0000000..867945f --- /dev/null +++ b/framework/runners/jsonb-lookup-aggregate.runner.ts @@ -0,0 +1,332 @@ +import { FieldKeyType, FieldType, Relationship } from "@teable/core"; +import { + getFields as apiGetFields, + getRecords as apiGetRecords, +} from "@teable/openapi"; +import { + createField, + createRecords, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { JsonbLookupAggregateCaseConfig } from "../types"; + +// A conditional total asking for the LARGEST or the SMALLEST - over a column +// that is itself a borrowed list -> checkpoint: the totals read, and +// they read the right answers. +// +// Chains like this are ordinary. A team row borrows every task's amount from +// the task table, so that column holds a list rather than one value. A report +// row then matches its teams and asks for the largest amount across them. Sum +// and average had been taught to look inside those lists; largest, smallest, +// all-of and any-of had not, and went straight at the stored list. Postgres +// refuses that outright - there is no largest of a list - and the column never +// produced anything. +// +// Only the number half is asked here. The tickbox half of the same fix cannot +// be told apart through a borrowed list: an unticked box does not reach that +// list at all, measured as [true] for a pair of leaves ticked and unticked, so +// all-of and any-of return the same answer whether they work or not. +// +// What the user is left with is a column that stays empty with no explanation, +// on a field the interface offered to build. Sum on the same source works, +// which makes it look like the data is wrong rather than the function. +// +// The chain is three tables because two will not do it: the source column has +// to be a borrowed list, and a column only becomes a list by borrowing across a +// one-to-many. A total straight off a plain number column takes a different +// path and works on both sides of the fix. + +const NAME_FIELD = "Name"; +const AMOUNT_FIELD = "Amount"; +const LEAF_LINK_FIELD = "Leaves"; +const AMOUNT_LOOKUP_FIELD = "Amounts borrowed"; +const MATCH_FIELD = "MatchKey"; + +const sleep = (ms: number) => + new Promise((resolveSleep) => { + setTimeout(resolveSleep, ms); + }); + +export const runJsonbLookupAggregateCase = async ( + bugCase: BugCaseFor<"jsonb-lookup-aggregate">, + context: BugRunContext, +): Promise => { + const config: JsonbLookupAggregateCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + const createdTableIds: string[] = []; + + if (config.leaves.length < 2) { + throw new Error( + "at least two leaf rows, or the borrowed column holds one value and the aggregation has nothing to choose between", + ); + } + + try { + // The far end: the rows carrying the actual values. + const leaf = await createTable(baseId, { + name: `${suffix}-leaf`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { name: AMOUNT_FIELD, type: FieldType.Number }, + ], + records: config.leaves.map((row) => ({ + fields: { + [NAME_FIELD]: row.name, + [AMOUNT_FIELD]: row.amount, + }, + })), + }); + createdTableIds.unshift(leaf.id); + const leafAmountId = leaf.fields.find( + (field: { name: string }) => field.name === AMOUNT_FIELD, + )?.id as string; + + // The middle: one row borrowing every leaf value, so its borrowed columns + // hold lists rather than single values. + const middle = await createTable(baseId, { + name: `${suffix}-middle`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { name: MATCH_FIELD, type: FieldType.SingleLineText }, + ], + records: [], + }); + createdTableIds.unshift(middle.id); + const middleMatchId = middle.fields.find( + (field: { name: string }) => field.name === MATCH_FIELD, + )?.id as string; + const leafLink = await createField(middle.id, { + name: LEAF_LINK_FIELD, + type: FieldType.Link, + options: { + relationship: Relationship.OneMany, + foreignTableId: leaf.id, + }, + }); + await createRecords(middle.id, { + fieldKeyType: FieldKeyType.Id, + typecast: false, + records: [ + { + fields: { + [middle.fields[0].id]: config.middleRowName, + [middleMatchId]: config.matchKey, + [leafLink.id]: leaf.records.map((record: { id: string }) => ({ + id: record.id, + })), + }, + }, + ], + }); + const amountLookup = await createField(middle.id, { + name: AMOUNT_LOOKUP_FIELD, + type: FieldType.Number, + isLookup: true, + lookupOptions: { + foreignTableId: leaf.id, + linkFieldId: leafLink.id, + lookupFieldId: leafAmountId, + }, + }); + + // Fixture verification, outside the checkpoint: the borrowed columns really + // do hold lists. If they held one value each, the aggregations would take + // the ordinary path and answer correctly on both sides of the fix. + for (const borrowed of [amountLookup]) { + if ( + !(borrowed as { isMultipleCellValue?: boolean }).isMultipleCellValue + ) { + throw new Error( + `the borrowed column ${borrowed.name} does not hold a list - the fixture is not in place`, + ); + } + } + + // What the expected answers are worked out FROM: the lists the product + // actually built, read back off the middle row rather than assumed from the + // leaf rows. The two are not the same - a borrowed tickbox column does not + // necessarily carry an entry for every leaf - and a case that asserted + // against the leaves would be asserting against its own model of the + // product instead of against the product. + const middleRows = await apiGetRecords(middle.id, { + fieldKeyType: FieldKeyType.Id, + take: 1, + }); + const borrowedAmounts = middleRows.data.records[0]?.fields[ + amountLookup.id + ] as number[] | undefined; + if (!Array.isArray(borrowedAmounts) || borrowedAmounts.length < 2) { + throw new Error( + `the borrowed amounts read ${JSON.stringify(borrowedAmounts)} - the aggregation needs a list to choose between`, + ); + } + const expected: Record = { + max: Math.max(...borrowedAmounts), + min: Math.min(...borrowedAmounts), + }; + if (expected.max === expected.min) { + throw new Error( + `the borrowed amounts are all ${expected.max} - largest and smallest cannot be told apart`, + ); + } + + // The near end: a row matching the middle row and totalling across it. + const host = await createTable(baseId, { + name: `${suffix}-host`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { name: MATCH_FIELD, type: FieldType.SingleLineText }, + ], + records: [ + { + fields: { + [NAME_FIELD]: config.hostRowName, + [MATCH_FIELD]: config.matchKey, + }, + }, + ], + }); + createdTableIds.unshift(host.id); + const hostMatchId = host.fields.find( + (field: { name: string }) => field.name === MATCH_FIELD, + )?.id as string; + + const matchFilter = { + conjunction: "and", + filterSet: [ + { + fieldId: middleMatchId, + operator: "is", + value: { type: "field", fieldId: hostMatchId }, + }, + ], + }; + + const readHost = async () => { + const response = await apiGetRecords(host.id, { + fieldKeyType: FieldKeyType.Name, + take: 1, + }); + return { + headers: response.headers, + fields: response.data.records[0]?.fields ?? {}, + }; + }; + + // The engine assertion, on the read that derives the expected answers and + // on the same endpoint and feature the checkpoint reads through. Outside + // the checkpoint, so a v1 answer is the case failing to run rather than the + // bug. + const routing = assertServedByV2(middleRows.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const probe = await bugCheckpoint( + "the-largest-of-a-borrowed-list-is-a-number", + async () => { + // Asking for the total is INSIDE the checkpoint, because asking is what + // fails: before the fix the create refuses outright, with the database + // saying there is no largest of a list. Building the chain that column + // reads from is setup; asking the question is the observation. + const totals: { name: string; expression: string; expected: number }[] = + []; + for (const which of config.aggregations) { + const name = `${which} of the borrowed list`; + await createField(host.id, { + name, + type: FieldType.ConditionalRollup, + options: { + foreignTableId: middle.id, + lookupFieldId: amountLookup.id, + expression: `${which}({values})`, + filter: matchFilter, + }, + }); + totals.push({ + name, + expression: `${which}({values})`, + expected: expected[which], + }); + } + + // Waiting for the answers to arrive, not for the bug to appear: the + // loop leaves as soon as every total reads what it should. + const deadline = Date.now() + config.settleTimeoutMs; + let settled = await readHost(); + for (;;) { + const done = totals.every( + (total) => settled.fields[total.name] === total.expected, + ); + if (done || Date.now() >= deadline) { + break; + } + await sleep(config.pollIntervalMs); + settled = await readHost(); + } + + const observed = totals.map((total) => ({ + total: total.expression, + read: settled.fields[total.name] ?? null, + expected: total.expected, + })); + + // The columns' own state as well: a column the product marks broken is + // the honest half of this, and it says the failure is the function + // rather than the data. + const hostFields = await apiGetFields(host.id); + const broken = hostFields.data + .filter( + (field: { name: string; hasError?: boolean }) => + field.hasError && + totals.some((total) => total.name === field.name), + ) + .map((field: { name: string }) => field.name); + + const wrong = observed.filter((item) => item.read !== item.expected); + if (wrong.length > 0) { + throw new Error( + `the totals over a borrowed list read ${JSON.stringify(observed)}` + + (broken.length > 0 + ? `; the product marks these columns broken: ${JSON.stringify(broken)}` + : "; the product does not mark any of them broken"), + ); + } + if (broken.length > 0) { + throw new Error( + `the totals read correctly but the product marks ${JSON.stringify(broken)} broken`, + ); + } + return { observed }; + }, + ); + + return { + details: { + leafTableId: leaf.id, + middleTableId: middle.id, + hostTableId: host.id, + routing, + ...probe, + }, + }; + } finally { + for (const tableId of createdTableIds) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/legacy-column-visibility-metadata.runner.ts b/framework/runners/legacy-column-visibility-metadata.runner.ts new file mode 100644 index 0000000..40ceaf8 --- /dev/null +++ b/framework/runners/legacy-column-visibility-metadata.runner.ts @@ -0,0 +1,197 @@ +import { FieldType } from "@teable/core"; +import { axios, GET_VIEW_LIST, urlBuilder } from "@teable/openapi"; +import { createTable, permanentDeleteTable } from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import { fixtureDb } from "../fixture-db"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { LegacyColumnVisibilityMetadataCaseConfig } from "../types"; + +// A view whose stored notes about a column are of a shape nothing writes any +// more -> open the table -> checkpoint: the view comes back, and the entry is +// settled. +// +// Two shapes, on one runner because the fixture and the observation are the +// same: write notes no request produces, then read the views. +// +// Which columns a view shows has been recorded two ways over the life of this +// product: an older note saying whether a column is SHOWN, and the current one +// saying whether it is HIDDEN. Views made long enough ago carry both, and no +// request writes that shape any more - it is simply what is in the table. +// +// Read back, the two were passed through side by side. What a view says about a +// column is checked on the way out, and an entry carrying a note nobody expects +// any more does not pass that check: the request for the table's views failed, +// which is every view at once rather than one column in one of them. +// +// So the checkpoint asks for the views at all, and then asks that the entry has +// been settled into one answer - the older note gone, the current one kept. A +// request that came back carrying both would be the same contradiction handed +// to whatever reads it next. + +const NAME_FIELD = "Name"; +const OTHER_FIELD = "Other"; + +export const runLegacyColumnVisibilityMetadataCase = async ( + bugCase: BugCaseFor<"legacy-column-visibility-metadata">, + context: BugRunContext, +): Promise => { + const config: LegacyColumnVisibilityMetadataCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + let tableId = ""; + + try { + const table = await createTable(baseId, { + name: suffix, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { name: OTHER_FIELD, type: FieldType.SingleLineText }, + ], + records: [{ fields: { [NAME_FIELD]: config.rowTitle } }], + }); + tableId = table.id; + const viewId = table.views?.[0]?.id; + const columnId = table.fields.find( + (field: { name: string }) => field.name === OTHER_FIELD, + )?.id as string; + if (!viewId || !columnId) { + throw new Error("the table has no view or no second column"); + } + + const readViews = async () => + axios.get(urlBuilder(GET_VIEW_LIST, { tableId }), { + validateStatus: () => true, + }); + + const before = await readViews(); + if (before.status !== 200) { + throw new Error( + `the views do not read before anything is done to them (${before.status}): ${JSON.stringify(before.data)}`, + ); + } + const routing = assertServedByV2(before.headers, { + operation: "GET /table/{tableId}/view", + feature: "getViews", + }); + + // What a view made long enough ago carries. Written with SQL because + // nothing writes either shape any more. + const db = fixtureDb(context.app); + const columnIndex = table.fields.findIndex( + (field: { id: string }) => field.id === columnId, + ); + const legacy = + config.legacy === "bothVisibilityNotes" + ? { + [columnId]: { + order: config.order, + visible: true, + hidden: false, + width: config.width, + }, + } + : // No position at all - the other shape old views carry. + { [columnId]: { width: config.width } }; + await db.execute( + `UPDATE "view" SET "column_meta" = $1 WHERE "id" = $2`, + JSON.stringify(legacy), + viewId, + ); + + // Fixture verification, outside the checkpoint: the older note really is in + // the table. Without it there is nothing unexpected to read back and the + // case would report on nothing. + const stored = await db.query<{ columnMeta: string }[]>( + `SELECT "column_meta" AS "columnMeta" FROM "view" WHERE "id" = $1`, + viewId, + ); + const storedText = String(stored[0]?.columnMeta ?? ""); + const missingMark = + config.legacy === "bothVisibilityNotes" ? '"visible"' : '"order"'; + const present = storedText.includes(missingMark); + if (config.legacy === "bothVisibilityNotes" ? !present : present) { + throw new Error( + `the stored notes are not the shape this case is about (${config.legacy}): ${storedText} - ` + + "the fixture is not in place", + ); + } + + const probe = await bugCheckpoint( + "a-view-with-old-notes-about-a-column-still-reads", + async () => { + const listed = await readViews(); + const body = + typeof listed.data === "string" + ? listed.data + : JSON.stringify(listed.data ?? ""); + if (listed.status !== 200) { + throw new Error( + `asking for the table's views answered ${listed.status} - that is every view at once, ` + + `not one column in one of them: ${body}`, + ); + } + + const view = ( + listed.data as { id: string; columnMeta?: Record }[] + ).find((candidate) => candidate.id === viewId); + const entry = view?.columnMeta?.[columnId] as + | Record + | undefined; + if (!entry) { + throw new Error( + `the view came back with nothing about the column: ${body}`, + ); + } + if (config.legacy === "bothVisibilityNotes") { + if ("visible" in entry) { + throw new Error( + `the view still says both things about the column: ${JSON.stringify(entry)} - ` + + "whatever reads this next is handed the contradiction", + ); + } + if (entry.hidden !== false) { + throw new Error( + `the view came back saying the column is ${JSON.stringify(entry.hidden)}, expected false: ` + + JSON.stringify(entry), + ); + } + } else { + // The entry has to come back with a position. Where a column sits is + // not optional to whatever draws the view, and the stored notes do + // not say. + if (entry.order !== columnIndex) { + throw new Error( + `the view came back with the column at ${JSON.stringify(entry.order)}, expected ` + + `${columnIndex} - its place among the columns: ${JSON.stringify(entry)}`, + ); + } + } + // Either way, what the notes did carry survives. + if (entry.width !== config.width) { + throw new Error( + `the width was ${JSON.stringify(entry.width)}, expected ${config.width}: ${JSON.stringify(entry)}`, + ); + } + return { entry }; + }, + ); + + return { + details: { tableId, viewId, columnId, routing, ...probe }, + }; + } finally { + if (tableId) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/nested-user-array-join-create.runner.ts b/framework/runners/nested-user-array-join-create.runner.ts new file mode 100644 index 0000000..c939ed7 --- /dev/null +++ b/framework/runners/nested-user-array-join-create.runner.ts @@ -0,0 +1,248 @@ +import { FieldKeyType, FieldType, Relationship } from "@teable/core"; +import { + axios, + getRecords as apiGetRecords, + CREATE_RECORD, + urlBuilder, +} from "@teable/openapi"; +import { + createField, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { NestedUserArrayJoinCreateCaseConfig } from "../types"; + +// A table with several people columns and one formula joining them together, +// wrapped four functions deep -> add a row -> checkpoint: the row is added. +// +// "Everyone involved, listed once, separated by commas" is what that formula +// says: flatten the people columns into one list, drop the empties, drop the +// repeats, join what is left. Each of those four steps re-stated the whole of +// the step inside it, so the statement the database was asked to plan grew with +// every layer. At seven people columns it reached megabytes. +// +// The row is recomputed inside the write, so nothing came back at all: the page +// spun and the gateway eventually gave up. The table could not accept a row - +// not slowly, at all - and the only thing a person could see was a timeout. +// +// So the checkpoint's question is simply whether the write returns. It carries +// its own time limit rather than letting the request hang, because a request +// that never answers would end the case as "could not run" instead of as the +// bug it is. + +const NAME_FIELD = "Name"; +const CAMPUS_FIELD = "Campus"; +const LINK_FIELD = "Session"; +const CAMPUS_LOOKUP_FIELD = "Campus, borrowed"; +const JOINED_FIELD = "Everyone involved"; + +export const runNestedUserArrayJoinCreateCase = async ( + bugCase: BugCaseFor<"nested-user-array-join-create">, + context: BugRunContext, +): Promise => { + const config: NestedUserArrayJoinCreateCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + const createdTableIds: string[] = []; + const person = { + id: globalThis.testConfig.userId, + title: globalThis.testConfig.userName, + }; + + if (config.peopleColumns < 2) { + throw new Error( + "at least two people columns, or there is nothing to flatten together", + ); + } + + try { + // The other table, and the column borrowed from it. The borrowed column is + // part of the reported shape: it is what puts a second computed column in + // the same write. + const sessions = await createTable(baseId, { + name: `${suffix}-sessions`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { name: CAMPUS_FIELD, type: FieldType.LongText }, + ], + records: [ + { + fields: { + [NAME_FIELD]: config.sessionRowName, + [CAMPUS_FIELD]: config.campusValue, + }, + }, + ], + }); + createdTableIds.unshift(sessions.id); + const campusFieldId = sessions.fields.find( + (field: { name: string }) => field.name === CAMPUS_FIELD, + )?.id as string; + + const notes = await createTable(baseId, { + name: `${suffix}-notes`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + ], + records: [], + }); + createdTableIds.unshift(notes.id); + const notesNameId = notes.fields[0].id; + + const peopleFieldIds: string[] = []; + for (let index = 0; index < config.peopleColumns; index += 1) { + const field = await createField(notes.id, { + name: `${config.peopleColumnPrefix} ${index + 1}`, + type: FieldType.User, + options: { isMultiple: false, shouldNotify: false }, + }); + peopleFieldIds.push(field.id); + } + + const link = await createField(notes.id, { + name: LINK_FIELD, + type: FieldType.Link, + options: { + relationship: Relationship.OneOne, + foreignTableId: sessions.id, + }, + }); + await createField(notes.id, { + name: CAMPUS_LOOKUP_FIELD, + type: FieldType.LongText, + isLookup: true, + lookupOptions: { + foreignTableId: sessions.id, + linkFieldId: link.id, + lookupFieldId: campusFieldId, + }, + }); + + const flattenArgs = peopleFieldIds + .map((fieldId) => `{${fieldId}}`) + .join(", "); + const expression = `ARRAY_JOIN(ARRAY_UNIQUE(ARRAY_COMPACT(ARRAY_FLATTEN(${flattenArgs}))), "${config.separator}")`; + + // Fixture verification, outside the checkpoint: the table reads before + // anything is written to it, and the engine assertion rides on that read. + const before = await apiGetRecords(notes.id, { + fieldKeyType: FieldKeyType.Id, + take: 1, + }); + if (before.data.records.length !== 0) { + throw new Error("the table was expected to be empty before the write"); + } + const routing = assertServedByV2(before.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const probe = await bugCheckpoint( + "a-row-can-be-added-to-a-table-whose-formula-joins-people-columns", + async () => { + // The formula is made HERE, not in setup. What grew a layer at a time + // is the statement, and planning it is what fails - so it fails when the + // column is made as readily as when a row is added. Building it outside + // would score that first failure as "this case could not run here", + // which is the one verdict that hides the bug. + await createField(notes.id, { + name: JOINED_FIELD, + type: FieldType.Formula, + options: { expression }, + }); + + const startedAt = Date.now(); + // Raw axios with its own time limit. A request that never answers would + // run out the whole case and be reported as "could not run"; this way + // the wait ends here, inside the checkpoint, which is the report. + const response = await axios + .post( + urlBuilder(CREATE_RECORD, { tableId: notes.id }), + { + fieldKeyType: FieldKeyType.Id, + typecast: false, + records: [ + { + fields: Object.fromEntries([ + [notesNameId, config.noteRowName], + ...peopleFieldIds.map((fieldId) => [fieldId, person]), + ]), + }, + ], + }, + { + validateStatus: () => true, + timeout: config.writeBudgetMs, + }, + ) + .catch((error: { code?: string; message?: string }) => { + throw new Error( + `adding a row did not answer within ${config.writeBudgetMs}ms ` + + `(${error.code ?? "no code"}: ${error.message ?? "no message"}) - ` + + `the table cannot accept a row at all`, + ); + }); + const elapsedMs = Date.now() - startedAt; + + if (response.status < 200 || response.status >= 300) { + throw new Error( + `adding a row answered ${response.status} after ${elapsedMs}ms: ` + + (typeof response.data === "string" + ? response.data + : JSON.stringify(response.data)), + ); + } + const recordId = (response.data as { records?: { id?: string }[] }) + ?.records?.[0]?.id; + if (!recordId) { + throw new Error( + `adding a row returned no row after ${elapsedMs}ms: ${JSON.stringify(response.data)}`, + ); + } + + // And the table reads afterwards, with the row in it. + const after = await apiGetRecords(notes.id, { + fieldKeyType: FieldKeyType.Id, + take: 5, + }); + if (after.data.records.length !== 1) { + throw new Error( + `the write answered but the table lists ${after.data.records.length} rows`, + ); + } + return { + recordId, + elapsedMs, + joined: after.data.records[0]?.fields ?? {}, + }; + }, + ); + + return { + details: { + sessionsTableId: sessions.id, + notesTableId: notes.id, + peopleColumns: config.peopleColumns, + routing, + recordId: probe.recordId, + writeMs: probe.elapsedMs, + }, + }; + } finally { + for (const tableId of createdTableIds) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/or-filtered-rollup-scope.runner.ts b/framework/runners/or-filtered-rollup-scope.runner.ts new file mode 100644 index 0000000..685cd72 --- /dev/null +++ b/framework/runners/or-filtered-rollup-scope.runner.ts @@ -0,0 +1,277 @@ +import { Colors, FieldKeyType, FieldType, Relationship } from "@teable/core"; +import { + createRecords as apiCreateRecords, + getRecords as apiGetRecords, +} from "@teable/openapi"; +import { + createField, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { OrFilteredRollupScopeCaseConfig } from "../types"; + +// A total over linked rows, narrowed to "status is todo OR status is doing" -> +// checkpoint: each row totals only the rows it is actually linked to. +// +// "Open work on this project", "unpaid invoices for this customer" - the two- +// or-more-values condition is how a summary says "any of these". Written with +// OR, the condition escaped the link: the query stopped asking "and linked to +// this row" and totalled every matching row in the other table. +// +// The number that comes out is plausible - it is a real sum of real rows - so +// nothing looks broken. The tell is the row that is linked to nothing at all +// and still shows a figure, which is what the report leads with: a project +// created a minute ago, joined to nothing, already showing other people's +// numbers. +// +// The fixture therefore carries three kinds of foreign row, and the case is +// worthless without all three: rows this host is linked to that the condition +// selects, rows it is linked to that the condition excludes, and rows the +// condition selects that belong to somebody else. Drop the third and a total +// that ignored the link would give the right answer anyway. + +const NAME_FIELD = "Name"; +const STATUS_FIELD = "Status"; +const AMOUNT_FIELD = "Amount"; +const LINK_FIELD = "Work"; +const ROLLUP_FIELD = "Open work"; + +const sleep = (ms: number) => + new Promise((resolveSleep) => { + setTimeout(resolveSleep, ms); + }); + +export const runOrFilteredRollupScopeCase = async ( + bugCase: BugCaseFor<"or-filtered-rollup-scope">, + context: BugRunContext, +): Promise => { + const config: OrFilteredRollupScopeCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + let workTableId = ""; + let hostTableId = ""; + + const selected = new Set(config.selectedStatuses); + const mine = config.work.filter((row) => row.owner === config.linkedHost); + const minesSelected = mine.filter((row) => selected.has(row.status)); + const mineExcluded = mine.filter((row) => !selected.has(row.status)); + const othersSelected = config.work.filter( + (row) => row.owner !== config.linkedHost && selected.has(row.status), + ); + if ( + minesSelected.length === 0 || + mineExcluded.length === 0 || + othersSelected.length === 0 + ) { + throw new Error( + "the fixture needs all three kinds of row - linked and selected, linked and excluded, " + + "and selected but belonging to another host. Without the third, a total that ignored " + + "the link would still read correctly", + ); + } + if (config.selectedStatuses.length < 2) { + throw new Error( + "at least two statuses, or the condition has nothing to OR together and this is a different bug", + ); + } + const expectedLinkedTotal = minesSelected.reduce( + (sum, row) => sum + row.amount, + 0, + ); + const statuses = [...new Set(config.work.map((row) => row.status))]; + + try { + const workTable = await createTable(baseId, { + name: `${suffix}-work`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { + name: STATUS_FIELD, + type: FieldType.SingleSelect, + options: { + choices: statuses.map((name) => ({ name, color: Colors.Blue })), + }, + }, + { name: AMOUNT_FIELD, type: FieldType.Number }, + ], + records: config.work.map((row) => ({ + fields: { + [NAME_FIELD]: row.name, + [STATUS_FIELD]: row.status, + [AMOUNT_FIELD]: row.amount, + }, + })), + }); + workTableId = workTable.id; + const statusFieldId = workTable.fields.find( + (field: { name: string }) => field.name === STATUS_FIELD, + )?.id; + const amountFieldId = workTable.fields.find( + (field: { name: string }) => field.name === AMOUNT_FIELD, + )?.id; + if (!statusFieldId || !amountFieldId) { + throw new Error(`the work table ${workTableId} is not in place`); + } + const workIdByName = new Map( + workTable.records.map( + (record: { id: string; fields: Record }) => [ + String(record.fields[NAME_FIELD]), + record.id, + ], + ), + ); + + const hostTable = await createTable(baseId, { + name: `${suffix}-host`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + ], + records: [], + }); + hostTableId = hostTable.id; + const linkField = await createField(hostTableId, { + name: LINK_FIELD, + type: FieldType.Link, + options: { + foreignTableId: workTableId, + relationship: Relationship.OneMany, + }, + }); + + // One host joined to its own rows, and one joined to nothing - the row the + // report is about, created and never linked. + await apiCreateRecords(hostTableId, { + fieldKeyType: FieldKeyType.Name, + typecast: false, + records: [ + { + fields: { + [NAME_FIELD]: config.linkedHost, + [LINK_FIELD]: mine.map((row) => ({ + id: workIdByName.get(row.name) as string, + })), + }, + }, + { fields: { [NAME_FIELD]: config.unlinkedHost } }, + ], + }); + + const rollupField = await createField(hostTableId, { + name: ROLLUP_FIELD, + type: FieldType.Rollup, + options: { expression: "sum({values})" }, + lookupOptions: { + foreignTableId: workTableId, + linkFieldId: linkField.id, + lookupFieldId: amountFieldId, + filter: { + conjunction: "or", + filterSet: config.selectedStatuses.map((status) => ({ + fieldId: statusFieldId, + operator: "is", + value: status, + })), + }, + }, + }); + + const readHosts = async () => { + const response = await apiGetRecords(hostTableId, { + fieldKeyType: FieldKeyType.Name, + take: 10, + }); + const byName = new Map( + response.data.records.map((record) => [ + String(record.fields[NAME_FIELD]), + record.fields[ROLLUP_FIELD] ?? null, + ]), + ); + return { headers: response.headers, byName }; + }; + + // Settling before the checkpoint, on the LINKED host only. Its total is the + // one a working build has to reach, so waiting for it is waiting for the + // computation to finish rather than for the bug to appear - the unlinked + // host is then read from that same settled state. + const deadline = Date.now() + config.settleTimeoutMs; + let settled = await readHosts(); + for (;;) { + if ( + Number(settled.byName.get(config.linkedHost)) === expectedLinkedTotal + ) { + break; + } + if (Date.now() >= deadline) { + break; + } + await sleep(config.pollIntervalMs); + settled = await readHosts(); + } + + const routing = assertServedByV2(settled.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const probe = await bugCheckpoint( + "an-any-of-these-total-counts-only-what-this-row-is-linked-to", + async () => { + const linkedTotal = settled.byName.get(config.linkedHost) ?? null; + const unlinkedTotal = settled.byName.get(config.unlinkedHost) ?? null; + + // The row linked to nothing. Any figure here came from somebody else's + // rows, which is the symptom the report opens with. + if (unlinkedTotal !== null && Number(unlinkedTotal) !== 0) { + throw new Error( + `"${config.unlinkedHost}" is linked to nothing and totals ${JSON.stringify(unlinkedTotal)}. ` + + `The other table holds ${JSON.stringify( + config.work.map( + (row) => `${row.name}/${row.status}/${row.amount}`, + ), + )}`, + ); + } + + if (Number(linkedTotal) !== expectedLinkedTotal) { + throw new Error( + `"${config.linkedHost}" totals ${JSON.stringify(linkedTotal)}, expected ${expectedLinkedTotal} ` + + `from its own ${JSON.stringify(minesSelected.map((row) => row.name))}. ` + + `Linked but excluded: ${JSON.stringify(mineExcluded.map((row) => row.name))}; ` + + `selected but somebody else's: ${JSON.stringify(othersSelected.map((row) => row.name))}`, + ); + } + return { linkedTotal, unlinkedTotal }; + }, + ); + + return { + details: { + workTableId, + hostTableId, + rollupFieldId: rollupField.id, + expectedLinkedTotal, + routing, + ...probe, + }, + }; + } finally { + for (const tableId of [hostTableId, workTableId]) { + if (!tableId) { + continue; + } + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/same-named-fk-base-duplicate.runner.ts b/framework/runners/same-named-fk-base-duplicate.runner.ts new file mode 100644 index 0000000..0339769 --- /dev/null +++ b/framework/runners/same-named-fk-base-duplicate.runner.ts @@ -0,0 +1,193 @@ +import { FieldType } from "@teable/core"; +import { + axios, + createBase as apiCreateBase, + getTableList as apiGetTableList, + permanentDeleteBase, + DUPLICATE_BASE, + urlBuilder, +} from "@teable/openapi"; +import { createTable } from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import { fixtureDb } from "../fixture-db"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { SameNamedFkBaseDuplicateCaseConfig } from "../types"; + +// A base whose tables each carry a foreign key under the SAME name -> +// duplicate the base with its rows -> checkpoint: the copy is made, with every +// table in it. +// +// Postgres constraint names are unique per table, not per schema, so two +// tables in one base can each own a constraint called `fk___id` - and legacy +// bases do, from a self-referencing key on the row id column that no current +// code writes. Duplicating a base drops those keys, copies the rows, and puts +// the keys back. +// +// The step that listed them matched on the name and the schema and not on the +// table, so each table's list came back holding the other table's rows too. +// The drop phase then issued the same DROP twice for one table, the second one +// found nothing, and the whole duplicate died on a Postgres error naming a +// constraint that "does not exist" (42704) - reported from production as an +// unhandled rejection in the browser, with the base half-made. +// +// The keys are written with SQL because nothing a person can do produces them +// any more; they are what an old base has been carrying since before the +// naming changed. That is the same reason nobody could get out of this from +// the interface. + +const NAME_FIELD = "Name"; +const LEGACY_FK_NAME = "fk___id"; + +export const runSameNamedFkBaseDuplicateCase = async ( + bugCase: BugCaseFor<"same-named-fk-base-duplicate">, + context: BugRunContext, +): Promise => { + const config: SameNamedFkBaseDuplicateCaseConfig = bugCase.config; + const spaceId = globalThis.testConfig.spaceId; + const suffix = `${config.baseNamePrefix}-${context.runId}`; + let sourceBaseId = ""; + let copyId = ""; + + if (config.tableNames.length < 2) { + throw new Error( + "two tables at least - one table cannot collide with itself, and the collision is the bug", + ); + } + + try { + const source = await apiCreateBase({ spaceId, name: `${suffix}-source` }); + sourceBaseId = source.data.id; + + const tables = []; + for (const name of config.tableNames) { + tables.push( + await createTable(sourceBaseId, { + name, + fields: [ + { + name: NAME_FIELD, + type: FieldType.SingleLineText, + isPrimary: true, + }, + ], + records: [{ fields: { [NAME_FIELD]: config.rowTitle } }], + }), + ); + } + + // The state an old base carries: a self-referencing key on the row id + // column, under a name that was never made unique per schema. + const db = fixtureDb(context.app); + const placed: { schema: string; table: string }[] = []; + for (const table of tables) { + const physical = await db.physicalTable(table.id); + await db.execute( + `ALTER TABLE "${physical.schema}"."${physical.table}" ` + + `ADD CONSTRAINT "${LEGACY_FK_NAME}" FOREIGN KEY ("__id") ` + + `REFERENCES "${physical.schema}"."${physical.table}" ("__id") ON DELETE SET NULL`, + ); + placed.push(physical); + } + + // Fixture verification, outside the checkpoint: two tables really do hold + // one name between them. With only one, there is nothing to collide and + // the case would report on nothing. + const schema = placed[0]?.schema; + const holders = await db.query<{ count: number }[]>( + `SELECT COUNT(*)::int AS count + FROM pg_constraint con + JOIN pg_class rel ON rel.oid = con.conrelid + JOIN pg_namespace nsp ON nsp.oid = rel.relnamespace + WHERE con.contype = 'f' AND nsp.nspname = $1 AND con.conname = $2`, + schema, + LEGACY_FK_NAME, + ); + const holderCount = holders[0]?.count ?? 0; + if (holderCount !== tables.length) { + throw new Error( + `${holderCount} table(s) in ${schema} carry a key named ${LEGACY_FK_NAME}, expected ${tables.length} - the fixture is not in place`, + ); + } + + const probe = await bugCheckpoint( + "a-base-whose-tables-share-a-key-name-can-be-copied", + async () => { + // Raw axios with the status open. Before the fix this request is + // refused, and the generated client throws a bare "Internal Server + // Error" and drops the response - which is exactly the part worth + // reading, because a 500 that turned out to be something else would + // make this case red for the wrong reason. + const duplicated = await axios.post( + urlBuilder(DUPLICATE_BASE, {}), + { + fromBaseId: sourceBaseId, + spaceId, + name: `${suffix}-copy`, + withRecords: true, + }, + { validateStatus: () => true }, + ); + if (duplicated.status < 200 || duplicated.status >= 300) { + throw new Error( + `duplicating the base answered ${duplicated.status}: ` + + (typeof duplicated.data === "string" + ? duplicated.data + : JSON.stringify(duplicated.data)), + ); + } + copyId = (duplicated.data as { id?: string })?.id ?? ""; + if (!copyId) { + throw new Error( + `duplicating the base produced no copy: ${JSON.stringify(duplicated.data)}`, + ); + } + const routing = assertServedByV2(duplicated.headers, { + operation: "POST /base/duplicate", + feature: "duplicateBase", + }); + + // And the copy is whole. A duplicate that answered 201 while losing a + // table would be the same interrupted copy behind a success. + const copied = await apiGetTableList(copyId); + const copiedNames = copied.data.map( + (table: { name: string }) => table.name, + ); + for (const name of config.tableNames) { + if (!copiedNames.includes(name)) { + throw new Error( + `the copy is missing the table ${JSON.stringify(name)} - it holds ${JSON.stringify(copiedNames)}`, + ); + } + } + return { routing, copiedNames }; + }, + ); + + return { + details: { + sourceBaseId, + copyId, + schema, + constraintName: LEGACY_FK_NAME, + ...probe, + }, + }; + } finally { + for (const id of [copyId, sourceBaseId]) { + if (!id) { + continue; + } + try { + await permanentDeleteBase(id); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (base ${id}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/select-rollup-unique-and-count.runner.ts b/framework/runners/select-rollup-unique-and-count.runner.ts new file mode 100644 index 0000000..1b85a00 --- /dev/null +++ b/framework/runners/select-rollup-unique-and-count.runner.ts @@ -0,0 +1,385 @@ +import { Colors, FieldKeyType, FieldType, Relationship } from "@teable/core"; +import { + createRecords as apiCreateRecords, + getRecords as apiGetRecords, + updateRecord as apiUpdateRecord, +} from "@teable/openapi"; +import { + createField, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { SelectRollupUniqueAndCountCaseConfig } from "../types"; + +// A parent row summarising a choice column across the children it is linked to +// -> checkpoint: the distinct values come back in the order they first appear, +// and the count is of distinct values. +// +// Two wrong answers from one column type. "Todo" then "Done" came back as +// "Done, Todo" - sorted, not in the order of the rows - so the summary +// disagreed with the list beside it, which was right. And when both children +// said "Todo", the count of distinct values answered 2: it was counting rows. +// +// Neither looks broken. A reordered list of two words reads as an arbitrary +// choice rather than a fault, and 2 is the number of children, so it is a +// number somebody can believe. What makes them findable at all is the other +// summaries over the same column - join and compact - which are correct, so the +// row shows "Todo, Done" and "Done, Todo" side by side. +// +// Those two ride along as the control. They take the same path from the same +// source, so if they are wrong too, this is not the distinct-values bug. + +const NAME_FIELD = "Name"; +const STATUS_FIELD = "Status"; +const LINK_FIELD = "Children"; +const JOIN_FIELD = "Joined"; +const COMPACT_FIELD = "Compacted"; +const UNIQUE_FIELD = "Distinct, in order"; +const COUNT_FIELD = "How many distinct"; + +const sleep = (ms: number) => + new Promise((resolveSleep) => { + setTimeout(resolveSleep, ms); + }); + +const firstAppearanceUnique = (values: string[]) => [...new Set(values)]; + +export const runSelectRollupUniqueAndCountCase = async ( + bugCase: BugCaseFor<"select-rollup-unique-and-count">, + context: BugRunContext, +): Promise => { + const config: SelectRollupUniqueAndCountCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + const createdTableIds: string[] = []; + + const initialStatuses = config.children.map((child) => child.status); + const initialUnique = firstAppearanceUnique(initialStatuses); + if (initialUnique.length < 2) { + throw new Error( + "the children need at least two different choices, or there is no order to get wrong", + ); + } + if ( + JSON.stringify(initialUnique) === JSON.stringify([...initialUnique].sort()) + ) { + throw new Error( + `the children's choices ${JSON.stringify(initialUnique)} are already in alphabetical order - ` + + "a summary that sorted them instead of keeping the row order would look correct", + ); + } + + const afterStatuses = config.children.map((child) => + child.name === config.retarget.childName + ? config.retarget.status + : child.status, + ); + const afterUnique = firstAppearanceUnique(afterStatuses); + if (afterUnique.length >= afterStatuses.length) { + throw new Error( + `after the edit the children hold ${JSON.stringify(afterStatuses)}, all different - ` + + "counting rows and counting distinct values would give the same answer", + ); + } + if ( + !config.children.some((child) => child.name === config.retarget.childName) + ) { + throw new Error( + `there is no child called ${JSON.stringify(config.retarget.childName)} to edit`, + ); + } + + const choices = [...new Set([...initialStatuses, config.retarget.status])]; + + try { + const children = await createTable(baseId, { + name: `${suffix}-children`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { + name: STATUS_FIELD, + type: FieldType.SingleSelect, + options: { + choices: choices.map((name) => ({ name, color: Colors.Blue })), + }, + }, + ], + records: config.children.map((child) => ({ + fields: { [NAME_FIELD]: child.name, [STATUS_FIELD]: child.status }, + })), + }); + createdTableIds.unshift(children.id); + const statusFieldId = children.fields.find( + (field: { name: string }) => field.name === STATUS_FIELD, + )?.id as string; + const childIdByName = new Map( + children.records.map( + (record: { id: string; fields: Record }) => [ + String(record.fields[NAME_FIELD]), + record.id, + ], + ), + ); + + const parent = await createTable(baseId, { + name: `${suffix}-parent`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + ], + records: [], + }); + createdTableIds.unshift(parent.id); + const linkField = await createField(parent.id, { + name: LINK_FIELD, + type: FieldType.Link, + options: { + relationship: Relationship.OneMany, + foreignTableId: children.id, + }, + }); + + // The link is written in the children's declared order, which is the order + // the summary is supposed to keep. + const writeTheRow = async () => + apiCreateRecords(parent.id, { + fieldKeyType: FieldKeyType.Name, + typecast: false, + records: [ + { + fields: { + [NAME_FIELD]: config.parentRowName, + [LINK_FIELD]: config.children.map((child) => ({ + id: childIdByName.get(child.name) as string, + })), + }, + }, + ], + }); + + // Which comes first, the row or the summaries. Adding a summary to a table + // that already holds rows fills it in as one job; writing a row into a table + // whose summaries already exist works them out as part of the write. Those + // are different paths and they have been wrong separately. + if (config.whenTheRowIsWritten === "beforeTheSummaries") { + await writeTheRow(); + } + + const summary = async (name: string, expression: string) => + createField(parent.id, { + name, + type: FieldType.Rollup, + options: { expression }, + lookupOptions: { + foreignTableId: children.id, + linkFieldId: linkField.id, + lookupFieldId: statusFieldId, + }, + }); + await summary(JOIN_FIELD, "array_join({values})"); + await summary(COMPACT_FIELD, "array_compact({values})"); + await summary(UNIQUE_FIELD, "array_unique({values})"); + await summary(COUNT_FIELD, "count({values})"); + + if (config.whenTheRowIsWritten === "afterTheSummaries") { + // The row first, then the links, as two writes. That is what a script + // does - create the parent, then attach the children - and it is the + // sequence the report follows. Writing both at once is a different path + // and is answered correctly on both sides of this fix. + const created = await apiCreateRecords(parent.id, { + fieldKeyType: FieldKeyType.Name, + typecast: false, + records: [{ fields: { [NAME_FIELD]: config.parentRowName } }], + }); + const parentRowId = created.data.records[0]?.id; + if (!parentRowId) { + throw new Error("the parent row was not created"); + } + await apiUpdateRecord(parent.id, parentRowId, { + fieldKeyType: FieldKeyType.Name, + record: { + fields: { + [LINK_FIELD]: config.children.map((child) => ({ + id: childIdByName.get(child.name) as string, + })), + }, + }, + }); + } + + const readParent = async () => { + const response = await apiGetRecords(parent.id, { + fieldKeyType: FieldKeyType.Name, + take: 1, + }); + return { + headers: response.headers, + fields: response.data.records[0]?.fields ?? {}, + }; + }; + + // Settling on the CONTROL, which is correct on both sides of the fix: + // waiting for the computation to finish rather than for the bug. + const settleOnControl = async (expectedJoined: string[]) => { + const deadline = Date.now() + config.settleTimeoutMs; + let seen = await readParent(); + for (;;) { + const joined = seen.fields[JOIN_FIELD]; + const asList = Array.isArray(joined) + ? joined.map(String) + : String(joined ?? "") + .split(",") + .map((part) => part.trim()) + .filter(Boolean); + if ( + JSON.stringify(asList) === JSON.stringify(expectedJoined) || + Date.now() >= deadline + ) { + return seen; + } + await sleep(config.pollIntervalMs); + seen = await readParent(); + } + }; + + const settled = await settleOnControl(initialStatuses); + const routing = assertServedByV2(settled.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const asList = (value: unknown) => + Array.isArray(value) + ? value.map(String) + : String(value ?? "") + .split(",") + .map((part) => part.trim()) + .filter(Boolean); + + const probe = await bugCheckpoint( + "distinct-choices-come-back-in-the-order-they-appear", + async () => { + const check = ( + seen: Record, + expectedUnique: string[], + when: string, + ) => { + const scene = { + when, + joined: seen[JOIN_FIELD] ?? null, + compacted: seen[COMPACT_FIELD] ?? null, + distinct: seen[UNIQUE_FIELD] ?? null, + howManyDistinct: seen[COUNT_FIELD] ?? null, + }; + + // The control first. Join and compact take the same path from the + // same column; if they are wrong, this is not the distinct-values bug. + const joined = asList(seen[JOIN_FIELD]); + const compacted = asList(seen[COMPACT_FIELD]); + const rowOrder = + when === "at first" ? initialStatuses : afterStatuses; + if ( + JSON.stringify(joined) !== JSON.stringify(rowOrder) || + JSON.stringify(compacted) !== JSON.stringify(rowOrder) + ) { + throw new Error( + `the summaries that are not under test disagree with the rows ${JSON.stringify(rowOrder)} ` + + `${when}: ${JSON.stringify(scene)}. The whole summary is wrong, not the distinct values`, + ); + } + + // Every wrong answer at once, rather than the first. The two halves + // of this report are separate faults in the same column, and a + // failure that stopped at the order would leave the count untested on + // exactly the commits where it is broken. + const problems: string[] = []; + + const distinct = asList(seen[UNIQUE_FIELD]); + if (JSON.stringify(distinct) !== JSON.stringify(expectedUnique)) { + problems.push( + `the distinct choices come back as ${JSON.stringify(distinct)}, ` + + `expected ${JSON.stringify(expectedUnique)} - the order the rows are in`, + ); + } + const howMany = Number(seen[COUNT_FIELD]); + if (howMany !== expectedUnique.length) { + problems.push( + `the count of distinct choices reads ${JSON.stringify(seen[COUNT_FIELD])}, ` + + `expected ${expectedUnique.length}` + + (howMany === rowOrder.length + ? " - which is the number of linked rows, so it is counting rows" + : ""), + ); + } + return { scene, problems }; + }; + + // Both phases run even if the first found something. The two halves of + // this report are separate faults in one column, and stopping at the + // order would leave the count undemonstrated on exactly the commits + // where it is broken - the order is wrong there first, and the count + // only becomes wrong once two children agree. + const first = check(settled.fields, initialUnique, "at first"); + + if (!config.alsoCheckAfterAnEdit) { + if (first.problems.length > 0) { + throw new Error( + `${first.problems.join("; ")}. The row read ${JSON.stringify(first.scene)}`, + ); + } + return { first: first.scene, second: null }; + } + + // The second half of the report: make two children agree, so the count + // of distinct values and the count of rows stop being the same number. + await apiUpdateRecord( + children.id, + childIdByName.get(config.retarget.childName) as string, + { + fieldKeyType: FieldKeyType.Id, + record: { fields: { [statusFieldId]: config.retarget.status } }, + }, + ); + const after = await settleOnControl(afterStatuses); + const second = check(after.fields, afterUnique, "after the edit"); + + const problems = [ + ...first.problems.map((problem) => `at first, ${problem}`), + ...second.problems.map((problem) => `after the edit, ${problem}`), + ]; + if (problems.length > 0) { + throw new Error( + `${problems.join("; ")}. The row read ${JSON.stringify(first.scene)} ` + + `and then ${JSON.stringify(second.scene)}`, + ); + } + + return { first: first.scene, second: second.scene }; + }, + ); + + return { + details: { + childrenTableId: children.id, + parentTableId: parent.id, + routing, + ...probe, + }, + }; + } finally { + for (const tableId of createdTableIds) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/share-view-unready-data-db.runner.ts b/framework/runners/share-view-unready-data-db.runner.ts new file mode 100644 index 0000000..f099c80 --- /dev/null +++ b/framework/runners/share-view-unready-data-db.runner.ts @@ -0,0 +1,210 @@ +import { FieldType } from "@teable/core"; +import { + axios, + enableShareView as apiEnableShareView, + createBase as apiCreateBase, + createSpace as apiCreateSpace, + deleteSpace, + permanentDeleteSpace, + SHARE_VIEW_GET, + urlBuilder, +} from "@teable/openapi"; +import { createTable } from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import { fixtureDb } from "../fixture-db"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { ShareViewUnreadyDataDbCaseConfig } from "../types"; + +// A shared view in a space whose own database is not available -> open the +// share link -> checkpoint: the page is told the database is unavailable, not +// that something went wrong. +// +// Spaces can be bound to a customer's own database. That binding can be turned +// off - revoked credentials, a connection retired, a migration part way - and +// the space then has nowhere to read from. Everything about the share is still +// correct: the link, the view, the permission. +// +// What came back was an unhandled 500. To whoever holds the link - typically +// somebody outside the company, with no account and no way to ask anyone - a +// 500 says the product is broken and there is nothing to do but try again. A +// 503 naming an unavailable database says the same page will work later, and it +// says the same thing to whatever is watching the endpoint. +// +// So the assertion is the status AND the code. A 503 that arrived without +// saying why would be indistinguishable from any other outage, and the point of +// the fix is that this one is distinguishable. +// +// The binding is written with SQL because the API to bind a space to another +// database is not part of this observation, and a disabled connection is not +// something a request can ask for. + +const NAME_FIELD = "Name"; +const UNAVAILABLE_CODE = "database_connection_unavailable"; + +export const runShareViewUnreadyDataDbCase = async ( + bugCase: BugCaseFor<"share-view-unready-data-db">, + context: BugRunContext, +): Promise => { + const config: ShareViewUnreadyDataDbCaseConfig = bugCase.config; + const suffix = `${config.namePrefix}-${context.runId}`; + let spaceId = ""; + let connectionId = ""; + const db = fixtureDb(context.app); + + try { + // Its own space: the binding under test is a property of a space, and this + // must not touch the one every other case reads from. + const space = await apiCreateSpace({ name: suffix }); + spaceId = space.data.id; + const base = await apiCreateBase({ spaceId, name: `${suffix}-base` }); + const table = await createTable(base.data.id, { + name: `${suffix}-table`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + ], + records: [{ fields: { [NAME_FIELD]: config.rowTitle } }], + }); + const viewId = table.views?.[0]?.id; + if (!viewId) { + throw new Error(`the table ${table.id} has no view to share`); + } + + const shared = await apiEnableShareView({ tableId: table.id, viewId }); + const shareId = shared.data?.shareId; + if (!shareId) { + throw new Error( + `sharing the view returned no link: ${JSON.stringify(shared.data)}`, + ); + } + const routing = assertServedByV2(shared.headers, { + operation: "POST /table/{tableId}/view/{viewId}/enable-share", + feature: "enableViewShare", + }); + + // Fixture verification, outside the checkpoint: the link works while the + // space still reads from the ordinary place. Without this, a 503 later + // could just as well mean the share was never set up. + const beforeBinding = await axios.get( + urlBuilder(SHARE_VIEW_GET, { shareId }), + { validateStatus: () => true }, + ); + if (beforeBinding.status !== 200) { + throw new Error( + `the share link answers ${beforeBinding.status} before the space is bound anywhere: ` + + JSON.stringify(beforeBinding.data), + ); + } + + // The state: the space is bound to a database whose connection is switched + // off. Nothing about the share changes. + connectionId = `e2elab${context.runId}` + .replace(/[^a-zA-Z0-9]/g, "") + .slice(0, 24); + await db.execute( + `INSERT INTO "data_db_connection" + ("id", "encrypted_url", "url_fingerprint", "internal_schema", "status", "created_by", "created_time") + VALUES ($1, $2, $3, $4, 'disabled', 'e2e-lab', NOW())`, + connectionId, + config.encryptedUrlPlaceholder, + `e2e-lab-${context.runId}`, + "__teable_internal", + ); + await db.execute( + `INSERT INTO "space_data_db_binding" + ("id", "space_id", "data_db_connection_id", "mode", "state", "created_by", "created_time") + VALUES ($1, $2, $3, 'byodb', 'ready', 'e2e-lab', NOW())`, + `${connectionId}b`, + spaceId, + connectionId, + ); + + const bound = await db.query<{ count: number }[]>( + `SELECT COUNT(*)::int AS count FROM "space_data_db_binding" WHERE "space_id" = $1`, + spaceId, + ); + if ((bound[0]?.count ?? 0) !== 1) { + throw new Error( + `the space is bound to ${bound[0]?.count ?? 0} databases - the fixture is not in place`, + ); + } + + const probe = await bugCheckpoint( + "a-share-link-whose-database-is-away-says-so", + async () => { + const response = await axios.get( + urlBuilder(SHARE_VIEW_GET, { shareId }), + { validateStatus: () => true }, + ); + const body = + typeof response.data === "string" + ? response.data + : JSON.stringify(response.data ?? ""); + const code = (response.data as { code?: string })?.code; + + if (response.status === 200) { + throw new Error( + `the share link answered 200 while the space's database is switched off: ${body}`, + ); + } + if (response.status !== 503) { + throw new Error( + `the share link answered ${response.status}, expected 503 - to whoever holds this link, ` + + `anything else says the product is broken rather than that the page will work later. ` + + `The response was ${body}`, + ); + } + if (code !== UNAVAILABLE_CODE) { + throw new Error( + `the share link answered 503 but called it ${JSON.stringify(code)}, expected ` + + `${JSON.stringify(UNAVAILABLE_CODE)} - a 503 that does not say why is any other outage. ` + + `The response was ${body}`, + ); + } + return { status: response.status, code }; + }, + ); + + return { + details: { + spaceId, + tableId: table.id, + shareId, + routing, + ...probe, + }, + }; + } finally { + if (connectionId) { + try { + await db.execute( + `DELETE FROM "space_data_db_binding" WHERE "space_id" = $1`, + spaceId, + ); + await db.execute( + `DELETE FROM "data_db_connection" WHERE "id" = $1`, + connectionId, + ); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (binding ${connectionId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + if (spaceId) { + try { + await deleteSpace(spaceId); + await permanentDeleteSpace(spaceId); + } catch (error) { + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (space ${spaceId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/shared-form-cover-url.runner.ts b/framework/runners/shared-form-cover-url.runner.ts new file mode 100644 index 0000000..df3d6b0 --- /dev/null +++ b/framework/runners/shared-form-cover-url.runner.ts @@ -0,0 +1,181 @@ +import { ViewType } from "@teable/core"; +import { + axios, + createView as apiCreateView, + enableShareView as apiEnableShareView, + SHARE_VIEW_GET, + urlBuilder, + VIEW_OPTION, +} from "@teable/openapi"; +import { createTable, permanentDeleteTable } from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { SharedFormCoverUrlCaseConfig } from "../types"; + +// A form with a picture at the top, shared -> open the link -> checkpoint: the +// address of the picture is an address. +// +// Where a form's picture lives is stored as a short path, and the address a +// browser can fetch is worked out from it when the form is read. The shared +// form is read through two layers, and both worked it out - the second one over +// the first one's answer. What came back was one address with another stuck on +// the front of it, which fetches nothing. +// +// So the person who opens the shared link sees a form with a broken picture, +// while the same form inside the product looks right - it is only read through +// one layer there. Nothing is wrong with the picture or the form. +// +// The address is checked for being built once, not for being any particular +// string: what the storage prefix is depends on how the instance is deployed, +// and pinning it would make this case about configuration. + +const NAME_FIELD = "Name"; + +export const runSharedFormCoverUrlCase = async ( + bugCase: BugCaseFor<"shared-form-cover-url">, + context: BugRunContext, +): Promise => { + const config: SharedFormCoverUrlCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + let tableId = ""; + + if (/^https?:\/\//i.test(config.storedPath)) { + throw new Error( + "the stored path must be a short path, not an address - an address is what the fix passes through untouched", + ); + } + + try { + const table = await createTable(baseId, { + name: suffix, + fields: [{ name: NAME_FIELD, type: "singleLineText" as never }], + records: [{ fields: { [NAME_FIELD]: config.rowTitle } }], + }); + tableId = table.id; + + const form = await apiCreateView(tableId, { + name: `${suffix}-form`, + type: ViewType.Form, + }); + const viewId = form.data.id; + + // Where the picture lives, as it is stored: a short path. + const options = await axios.patch( + urlBuilder(VIEW_OPTION, { tableId, viewId }), + { options: { coverUrl: config.storedPath, logoUrl: config.storedPath } }, + { validateStatus: () => true }, + ); + if (options.status < 200 || options.status >= 300) { + throw new Error( + `setting the form's picture answered ${options.status}: ${JSON.stringify(options.data)}`, + ); + } + + const shared = await apiEnableShareView({ tableId, viewId }); + const shareId = shared.data?.shareId; + if (!shareId) { + throw new Error( + `sharing the form returned no link: ${JSON.stringify(shared.data)}`, + ); + } + const routing = assertServedByV2(shared.headers, { + operation: "POST /table/{tableId}/view/{viewId}/enable-share", + feature: "enableViewShare", + }); + + // Fixture verification, outside the checkpoint: read from inside the + // product, the address is built once. That is the control - it says the + // picture and the form are fine, and it is the same view the shared link + // serves. + const inside = await axios.get( + urlBuilder("/table/{tableId}/view/{viewId}", { tableId, viewId }), + { validateStatus: () => true }, + ); + const insideCover = ( + inside.data as { options?: { coverUrl?: string } } | undefined + )?.options?.coverUrl; + if (!insideCover || !insideCover.includes(config.storedPath)) { + throw new Error( + `inside the product the form's picture reads ${JSON.stringify(insideCover)}, ` + + `which does not carry ${JSON.stringify(config.storedPath)} - the fixture is not in place`, + ); + } + + const probe = await bugCheckpoint( + "a-shared-forms-picture-has-one-address", + async () => { + const opened = await axios.get( + urlBuilder(SHARE_VIEW_GET, { shareId }), + { validateStatus: () => true }, + ); + const body = + typeof opened.data === "string" + ? opened.data + : JSON.stringify(opened.data ?? ""); + if (opened.status !== 200) { + throw new Error( + `opening the shared form answered ${opened.status}: ${body}`, + ); + } + + const view = ( + opened.data as { + view?: { options?: { coverUrl?: string; logoUrl?: string } }; + } + )?.view; + const seen = { + coverUrl: view?.options?.coverUrl, + logoUrl: view?.options?.logoUrl, + }; + + for (const [which, value] of Object.entries(seen)) { + if (!value) { + throw new Error( + `the shared form carries no ${which}: ${JSON.stringify(seen)}`, + ); + } + // Built once. Two addresses in one string is the whole fault, and + // counting them says so without pinning what the address is. + // + // The SCHEME is what gets counted, not "http://": joining one address + // onto another leaves the inner one with a single slash - the measured + // value is ".../public/http:/127.0.0.1/..." - so looking for the + // double slash finds one address in a string that plainly holds two. + const addresses = value.match(/https?:/gi)?.length ?? 0; + if (addresses !== 1) { + throw new Error( + `the shared form's ${which} carries ${addresses} addresses, expected one: ` + + `${JSON.stringify(value)} - the address was worked out twice, once over the other`, + ); + } + if (!value.endsWith(config.storedPath)) { + throw new Error( + `the shared form's ${which} does not end at the stored path ` + + `${JSON.stringify(config.storedPath)}: ${JSON.stringify(value)}`, + ); + } + } + return { seen }; + }, + ); + + return { + details: { tableId, viewId, shareId, routing, ...probe }, + }; + } finally { + if (tableId) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/switch-mixed-branch-storage.runner.ts b/framework/runners/switch-mixed-branch-storage.runner.ts new file mode 100644 index 0000000..a4243d6 --- /dev/null +++ b/framework/runners/switch-mixed-branch-storage.runner.ts @@ -0,0 +1,264 @@ +import { Colors, FieldKeyType, FieldType, Relationship } from "@teable/core"; +import { + createRecords as apiCreateRecords, + getFields as apiGetFields, + getRecords as apiGetRecords, +} from "@teable/openapi"; +import { + createField, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { SwitchMixedBranchStorageCaseConfig } from "../types"; + +// A column that picks its value by case - this number for one kind of row, that +// number for another, and otherwise the linked records -> checkpoint: the column +// can be made, and it reads what each case says. +// +// Written out, the rule is "cost depends on where the cost comes from": a +// manually entered figure for some rows, a different figure for others, and for +// everything else whatever is linked. The first two answers are numbers. The +// last is a list of linked records, which is stored as a document rather than as +// a number. +// +// The step that merges the branches together compared only the ones with a +// case attached, and those agreed - both numbers - so it never looked at what +// the otherwise branch held. The database was then asked to choose between +// numbers and a document in one expression and refused outright, which killed +// the whole column: it could not be created, and the schema change it was part +// of died with it. +// +// The interface offers all of this. Nothing about the formula is unusual, and +// nothing says the last branch is a different kind of thing from the others. + +const NAME_FIELD = "Name"; +const PRICE_FIELD = "Price"; +const BASIS_FIELD = "Cost basis"; +const LINK_FIELD = "Prices"; +const SWITCH_FIELD = "Cost"; + +export const runSwitchMixedBranchStorageCase = async ( + bugCase: BugCaseFor<"switch-mixed-branch-storage">, + context: BugRunContext, +): Promise => { + const config: SwitchMixedBranchStorageCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + const createdTableIds: string[] = []; + + if (config.numberBranches.length < 2) { + throw new Error( + "two number branches at least - the branches with a case attached have to agree with each other, " + + "or the step that merges them would have looked at the otherwise branch anyway", + ); + } + const basisChoices = [ + ...config.numberBranches.map((branch) => branch.choice), + config.otherwiseChoice, + ]; + if (new Set(basisChoices).size !== basisChoices.length) { + throw new Error( + `the cases are not distinct: ${JSON.stringify(basisChoices)}`, + ); + } + + try { + // The linked table. Its rows are what the otherwise branch reads, and a + // many-valued link means that branch holds a list rather than one value - + // which is what makes it a different kind of thing from the numbers. + const prices = await createTable(baseId, { + name: `${suffix}-prices`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { name: PRICE_FIELD, type: FieldType.Number }, + ], + records: config.linkedRows.map((row) => ({ + fields: { [NAME_FIELD]: row.name, [PRICE_FIELD]: row.price }, + })), + }); + createdTableIds.unshift(prices.id); + const priceRowIds = prices.records.map( + (record: { id: string }) => record.id, + ); + + const services = await createTable(baseId, { + name: `${suffix}-services`, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + { + name: BASIS_FIELD, + type: FieldType.SingleSelect, + options: { + choices: basisChoices.map((name) => ({ name, color: Colors.Blue })), + }, + }, + ...config.numberBranches.map((branch) => ({ + name: branch.column, + type: FieldType.Number, + })), + ], + records: [], + }); + createdTableIds.unshift(services.id); + const fieldId = (name: string) => { + const found = services.fields.find( + (field: { name: string }) => field.name === name, + )?.id; + if (!found) { + throw new Error(`the services table has no ${name} column`); + } + return found as string; + }; + const basisId = fieldId(BASIS_FIELD); + const numberIds = config.numberBranches.map((branch) => + fieldId(branch.column), + ); + + const link = await createField(services.id, { + name: LINK_FIELD, + type: FieldType.Link, + options: { + // Many-to-many so every row can hold the same list. One-to-many gives + // each linked record a single parent, and the fixture needs several + // rows - one per case - all reading a list. + relationship: Relationship.ManyMany, + foreignTableId: prices.id, + }, + }); + + // One row per case, each linked to the priced rows so the otherwise branch + // has something to read. + const rows = [ + ...config.numberBranches.map((branch, index) => ({ + name: `row-${branch.choice}`, + basis: branch.choice, + expected: branch.value, + index, + })), + { + name: `row-${config.otherwiseChoice}`, + basis: config.otherwiseChoice, + expected: null, + index: -1, + }, + ]; + await apiCreateRecords(services.id, { + fieldKeyType: FieldKeyType.Id, + typecast: false, + records: rows.map((row) => ({ + fields: { + [services.fields[0].id]: row.name, + [basisId]: row.basis, + ...Object.fromEntries( + config.numberBranches.map((branch, index) => [ + numberIds[index], + branch.value, + ]), + ), + [link.id]: priceRowIds.map((id: string) => ({ id })), + }, + })), + }); + + // Fixture verification, outside the checkpoint: the linked column really + // holds a list. Holding one value, it would be the same kind of thing as + // the numbers and there would be nothing to reconcile. + const seeded = await apiGetRecords(services.id, { + fieldKeyType: FieldKeyType.Id, + take: rows.length, + }); + const linkCell = seeded.data.records[0]?.fields[link.id]; + if (!Array.isArray(linkCell) || linkCell.length < 2) { + throw new Error( + `the linked column holds ${JSON.stringify(linkCell)} - the otherwise branch needs a list`, + ); + } + const routing = assertServedByV2(seeded.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const cases = config.numberBranches + .map((branch, index) => `"${branch.choice}", {${numberIds[index]}}`) + .join(", "); + const expression = `SWITCH({${basisId}}, ${cases}, {${link.id}})`; + + const probe = await bugCheckpoint( + "a-column-that-picks-by-case-can-be-made", + async () => { + // The column is made HERE. Reconciling the branches happens while it is + // built, so the refusal happens then - building it in setup would score + // that as "this case could not run here" instead of as the bug. + const made = await createField(services.id, { + name: SWITCH_FIELD, + type: FieldType.Formula, + options: { expression }, + }); + + const listed = await apiGetFields(services.id); + const back = listed.data.find( + (field: { id: string }) => field.id === made.id, + ) as { hasError?: boolean } | undefined; + if (back?.hasError) { + throw new Error( + `the column was created and immediately marked broken: ${expression}`, + ); + } + + // And it reads what each case says, at least where the answer is a + // number. A column that exists and computes nothing is the same outage + // one step later. + const after = await apiGetRecords(services.id, { + fieldKeyType: FieldKeyType.Id, + take: rows.length, + }); + const byName = new Map( + after.data.records.map((record) => [ + String(record.fields[services.fields[0].id]), + record.fields[made.id] ?? null, + ]), + ); + const scene = Object.fromEntries(byName); + for (const row of rows) { + if (row.expected === null) { + continue; + } + if (Number(byName.get(row.name)) !== row.expected) { + throw new Error( + `the row whose case is ${JSON.stringify(row.basis)} reads ` + + `${JSON.stringify(byName.get(row.name))}, expected ${row.expected}. ` + + `The column reads ${JSON.stringify(scene)}`, + ); + } + } + return { fieldId: made.id, scene }; + }, + ); + + return { + details: { + pricesTableId: prices.id, + servicesTableId: services.id, + expression, + routing, + ...probe, + }, + }; + } finally { + for (const tableId of createdTableIds) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/runners/undo-cursor-after-a-failed-undo.runner.ts b/framework/runners/undo-cursor-after-a-failed-undo.runner.ts new file mode 100644 index 0000000..96d45c4 --- /dev/null +++ b/framework/runners/undo-cursor-after-a-failed-undo.runner.ts @@ -0,0 +1,249 @@ +import { FieldKeyType, FieldType } from "@teable/core"; +import { + axios, + getRecords as apiGetRecords, + CREATE_RECORD, + OPERATION_UNDO, + UPDATE_RECORD, + urlBuilder, +} from "@teable/openapi"; +import { + createField, + createTable, + permanentDeleteTable, +} from "../../../utils/init-app"; +import { bugCheckpoint } from "../checkpoint"; +import { assertServedByV2 } from "../engine"; +import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types"; +import type { UndoCursorAfterAFailedUndoCaseConfig } from "../types"; + +// An undo that cannot be carried out -> press undo again -> checkpoint: the +// second press tries the same step again, and does not reach past it. +// +// Undo walks backwards through what you did. The place it has walked back to +// was moved BEFORE the step was carried out, and never moved back when the step +// failed - so a failed undo still counted as done. The next press therefore +// skipped over it and undid the step before, which is one the person had not +// asked to reverse. +// +// A step can fail to reverse for ordinary reasons. Here it is a column that does +// not allow duplicates: a value was changed away from something, somebody else's +// row has taken that value since, and putting the old one back would now +// collide. Nothing is wrong with the data or the request. +// +// What makes this bad is not the failed undo - that is honest, and the person +// can see it. It is the second press, which quietly reverses something else. In +// this fixture the step before is the row's creation, so pressing undo twice +// deletes a row nobody asked to delete. +// +// Concurrency is out of scope. The report also lists two requests undoing at +// once, two appends racing, and a crash between writes; a single client against +// one process cannot show any of those, and this case does not claim to. + +const NAME_FIELD = "Name"; +const CODE_FIELD = "Code"; + +export const runUndoCursorAfterAFailedUndoCase = async ( + bugCase: BugCaseFor<"undo-cursor-after-a-failed-undo">, + context: BugRunContext, +): Promise => { + const config: UndoCursorAfterAFailedUndoCaseConfig = bugCase.config; + const baseId = globalThis.testConfig.baseId; + const suffix = `${config.tableNamePrefix}-${context.runId}`; + // The stack is keyed by this, so everything meant to be on it must carry the + // same one - and the row that creates the collision must NOT, or it lands on + // the stack too and the case is undoing a different history. + const windowId = `e2e-lab-undo-cursor-${context.runId}`; + const otherWindowId = `${windowId}-someone-else`; + let tableId = ""; + + if (config.originalCode === config.changedCode) { + throw new Error( + "the value has to actually change, or there is nothing for undo to put back", + ); + } + + try { + const table = await createTable(baseId, { + name: suffix, + fields: [ + { name: NAME_FIELD, type: FieldType.SingleLineText, isPrimary: true }, + ], + records: [], + }); + tableId = table.id; + const nameFieldId = table.fields[0].id; + + // A column that does not allow duplicates. This is what makes putting the + // old value back impossible later. + const codeField = await createField(table.id, { + name: CODE_FIELD, + type: FieldType.SingleLineText, + unique: true, + }); + + const readRows = async () => { + const response = await apiGetRecords(table.id, { + fieldKeyType: FieldKeyType.Id, + take: 20, + }); + return { + headers: response.headers, + rows: response.data.records.map((record) => ({ + id: record.id, + name: String(record.fields[nameFieldId] ?? ""), + code: record.fields[codeField.id] ?? null, + })), + }; + }; + + // Every write goes through raw axios so it can carry the window id. The + // generated client takes no per-call headers, and a write without the id + // simply does not reach the stack - which reads as an empty stack later, + // not as an error. + const writeAs = async (onWindow: string, name: string, code: string) => { + const response = await axios.post( + urlBuilder(CREATE_RECORD, { tableId: table.id }), + { + fieldKeyType: FieldKeyType.Id, + typecast: false, + records: [{ fields: { [nameFieldId]: name, [codeField.id]: code } }], + }, + { + headers: { "x-window-id": onWindow }, + validateStatus: () => true, + }, + ); + if (response.status < 200 || response.status >= 300) { + throw new Error( + `writing ${JSON.stringify(name)} answered ${response.status}: ${JSON.stringify(response.data)}`, + ); + } + return (response.data as { records?: { id?: string }[] })?.records?.[0] + ?.id; + }; + + // The step before: the row is created. This is what a second press reaches + // if the first one is wrongly counted as done. + const rowId = await writeAs(windowId, config.rowName, config.originalCode); + if (!rowId) { + throw new Error("the row was not created"); + } + + // The step under test: its value is changed away from the original. + const changed = await axios.patch( + urlBuilder(UPDATE_RECORD, { tableId: table.id, recordId: rowId }), + { + fieldKeyType: FieldKeyType.Id, + record: { fields: { [codeField.id]: config.changedCode } }, + }, + { headers: { "x-window-id": windowId }, validateStatus: () => true }, + ); + if (changed.status < 200 || changed.status >= 300) { + throw new Error( + `changing the value answered ${changed.status}: ${JSON.stringify(changed.data)}`, + ); + } + + // Somebody else takes the value that was let go. On another window, so it + // is not on the stack this case walks back through. + await writeAs(otherWindowId, config.otherRowName, config.originalCode); + + const seeded = await readRows(); + if (seeded.rows.length !== 2) { + throw new Error( + `the table holds ${seeded.rows.length} rows, expected 2 - the fixture is not in place`, + ); + } + const routing = assertServedByV2(seeded.headers, { + operation: "GET /table/{tableId}/record", + feature: "getRecords", + }); + + const pressUndo = async () => + axios.post(urlBuilder(OPERATION_UNDO, { tableId: table.id }), undefined, { + headers: { "x-window-id": windowId }, + validateStatus: () => true, + }); + + // Fixture verification, outside the checkpoint: the first press really + // cannot be carried out. If it succeeded, there would be no failed step for + // the second press to skip and the case would be reporting on nothing. + const first = await pressUndo(); + const firstStatus = (first.data as { status?: string })?.status; + if ( + first.status >= 200 && + first.status < 300 && + firstStatus === "fulfilled" + ) { + throw new Error( + `undo put the old value back even though another row holds it - the fixture is not in place: ` + + JSON.stringify(first.data), + ); + } + const afterFirst = await readRows(); + if (afterFirst.rows.length !== 2) { + throw new Error( + `the failed undo left ${afterFirst.rows.length} rows, expected both still there: ` + + JSON.stringify(afterFirst.rows), + ); + } + + const probe = await bugCheckpoint( + "a-second-undo-after-a-failed-one-does-not-reach-past-it", + async () => { + const second = await pressUndo(); + const rows = (await readRows()).rows; + const scene = { + firstUndo: { status: first.status, body: first.data }, + secondUndo: { status: second.status, body: second.data }, + rows, + }; + + const row = rows.find((candidate) => candidate.id === rowId); + if (!row) { + throw new Error( + `pressing undo twice deleted ${JSON.stringify(config.rowName)}, which nobody asked to delete: ` + + `the second press reached past the step that could not be carried out and reversed the row's creation. ` + + JSON.stringify(scene), + ); + } + if (String(row.code) !== config.changedCode) { + throw new Error( + `${JSON.stringify(config.rowName)} reads ${JSON.stringify(row.code)}, expected ` + + `${JSON.stringify(config.changedCode)} - the step that could not be carried out is still not carried out. ` + + JSON.stringify(scene), + ); + } + return { + rows, + firstUndo: scene.firstUndo, + secondUndo: scene.secondUndo, + }; + }, + ); + + return { + details: { + tableId: table.id, + rowId, + windowId, + routing, + ...probe, + }, + }; + } finally { + if (tableId) { + try { + await permanentDeleteTable(baseId, tableId); + } catch (error) { + // Cleanup is the case's own housekeeping - the product did not fail. + console.warn( + `[e2e-lab] cleanup failed for ${bugCase.id} (table ${tableId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } +}; diff --git a/framework/types.ts b/framework/types.ts index 2823c26..11c8a95 100644 --- a/framework/types.ts +++ b/framework/types.ts @@ -7,6 +7,19 @@ import type { DayBucket } from "./runners/group-buckets"; // is discriminated on `runner`, so a case that pairs a runner with the wrong // config shape fails `pnpm check:types` at the case file itself. export interface BugCaseConfigByRunner { + "autonumber-string-filter": AutonumberStringFilterCaseConfig; + "cross-base-conditional-base-id": CrossBaseConditionalBaseIdCaseConfig; + "group-on-an-unreadable-column": GroupOnAnUnreadableColumnCaseConfig; + "jsonb-lookup-aggregate": JsonbLookupAggregateCaseConfig; + "legacy-column-visibility-metadata": LegacyColumnVisibilityMetadataCaseConfig; + "nested-user-array-join-create": NestedUserArrayJoinCreateCaseConfig; + "or-filtered-rollup-scope": OrFilteredRollupScopeCaseConfig; + "same-named-fk-base-duplicate": SameNamedFkBaseDuplicateCaseConfig; + "select-rollup-unique-and-count": SelectRollupUniqueAndCountCaseConfig; + "share-view-unready-data-db": ShareViewUnreadyDataDbCaseConfig; + "shared-form-cover-url": SharedFormCoverUrlCaseConfig; + "switch-mixed-branch-storage": SwitchMixedBranchStorageCaseConfig; + "undo-cursor-after-a-failed-undo": UndoCursorAfterAFailedUndoCaseConfig; "http-check": HttpCheckCaseConfig; "record-flow": RecordFlowCaseConfig; "group-collapse": GroupCollapseCaseConfig; @@ -956,6 +969,19 @@ export interface DuplicateSharedViewCaseConfig { baseId: "seed-base"; tableNamePrefix: string; rowTitle: string; + // Which question to ask of the copy. "copyHasItsOwnLink" is the older one - + // the duplicate must succeed and must not answer on the source's public + // address. "copyIsNotShared" is what the copy should carry instead: nothing. + assert: "copyHasItsOwnLink" | "copyIsNotShared"; + // Share rules set on the source view before duplicating. Only meaningful for + // "copyIsNotShared", where inheriting them is the point: a password the copy + // carries is a password that opens a table nobody chose to publish. Must + // include one, or the fixture check refuses to run. + shareMeta?: { + password?: string; + allowCopy?: boolean; + includeHiddenField?: boolean; + }; } // A row whose id body is not the 16 characters this version generates - what @@ -2109,3 +2135,206 @@ export interface CircularAppendBurstCaseConfig { // How many stale rows the failure message names (expected vs actual). staleRowEvidenceLimit: number; } + +// A filter on the row-number column carrying the number as text, which is what +// a filter box sends. +export interface AutonumberStringFilterCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // One row per title, numbered in order by the column added after them. Needs + // enough rows that the threshold below splits them. + rowTitles: string[]; + // "greater than" this. Must select some rows and leave others out, or a + // comparison that never ran would look the same as one that did. + threshold: number; +} + +// A conditional column reading a table in a second base, read back the way the +// settings screen reads it. +export interface CrossBaseConditionalBaseIdCaseConfig { + namePrefix: string; + // Rows in the other base. At least one has to carry matchedCategory, or the + // columns compute nothing and the fixture proves nothing. + sourceRows: { category: string; amount: number }[]; + // The category the single host row carries, and so the rows the columns read. + matchedCategory: string; +} + +// A grid grouped by a column the person opening it may not read. +export interface GroupOnAnUnreadableColumnCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // Two rows at least: a request that returns nothing must not look like one + // that returns everything. + rows: { name: string; stage: string; cost: number }[]; +} + +// A conditional total whose source column is itself a borrowed list, which is +// where largest/smallest/all-of/any-of went straight at the stored list. +export interface JsonbLookupAggregateCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // The rows at the far end of the chain. Two at least, with amounts that + // differ - otherwise largest and smallest cannot be told apart. The runner + // checks that against the list the product actually built, not against these. + leaves: { name: string; amount: number }[]; + // The row in the middle table, which borrows every leaf value. + middleRowName: string; + // The row doing the totalling. + hostRowName: string; + // Written to both middle and host, so the condition selects the middle row. + matchKey: string; + // Which aggregations to ask for. The tickbox half of this fix ("and"/"or") + // is deliberately absent: an unticked box does not reach a borrowed list, so + // all-of and any-of answer the same whether they work or not. See the runner. + aggregations: ("max" | "min")[]; + settleTimeoutMs: number; + pollIntervalMs: number; +} + +// A view whose stored notes about a column carry both the older key and the +// current one - what a view made long enough ago has been carrying all along. +export interface LegacyColumnVisibilityMetadataCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + rowTitle: string; + // Which shape of old notes to write. "bothVisibilityNotes" carries the older + // key beside the current one; "noPosition" carries no order at all. Both are + // shapes nothing writes any more, and each broke differently. + legacy: "bothVisibilityNotes" | "noPosition"; + // Written into the stored notes for the "bothVisibilityNotes" shape only. The + // other shape is defined by having no order. + order: number; + // Written into the stored notes either way, so a settled entry can be told + // from an emptied one. + width: number; +} + +// A table whose formula joins several people columns together, wrapped four +// functions deep - the shape whose statement grew a layer at a time. +export interface NestedUserArrayJoinCreateCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // How many people columns the formula flattens. The statement grew with this + // number; the report was filed at seven. + peopleColumns: number; + peopleColumnPrefix: string; + sessionRowName: string; + campusValue: string; + noteRowName: string; + separator: string; + // How long the write may take before the case says the table cannot accept a + // row. Generous: this is not a measurement of speed, it is the difference + // between an answer and no answer. + writeBudgetMs: number; +} + +// A total over linked rows narrowed with an "any of these" condition, which is +// written as OR and is where the total stopped respecting the link. +export interface OrFilteredRollupScopeCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // Rows in the other table. `owner` says which host row they belong to, and + // only the linked host's rows are actually linked - the rest exist to be + // wrongly counted. The runner refuses a fixture missing any of the three + // kinds it needs; see the runner. + work: { name: string; owner: string; status: string; amount: number }[]; + // The statuses the condition selects, ORed together. At least two, or there + // is no OR to get wrong. + selectedStatuses: string[]; + // The host row joined to its own work. + linkedHost: string; + // The host row joined to nothing, which is the symptom the report leads with. + unlinkedHost: string; + settleTimeoutMs: number; + pollIntervalMs: number; +} + +// A base whose tables each carry a foreign key under one name, which is what an +// old base has been holding since before the naming changed. +export interface SameNamedFkBaseDuplicateCaseConfig { + baseNamePrefix: string; + // Two at least: one table cannot collide with itself, and the collision is + // the bug. The runner refuses fewer. + tableNames: string[]; + // The single row each table carries, so the copy has rows to move. + rowTitle: string; +} + +// A summary of a choice column across linked children: the distinct values and +// how many there are. +export interface SelectRollupUniqueAndCountCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // The children, in the order the link is written. Their choices must not + // already be in alphabetical order, or a summary that sorted them would look + // correct - the runner refuses that. + children: { name: string; status: string }[]; + parentRowName: string; + // Which comes first. "beforeTheSummaries" adds the summaries to a table that + // already holds the row; "afterTheSummaries" writes the row into a table whose + // summaries already exist. Filling a new summary in and working one out during + // a write are different paths, and they have been wrong separately. + whenTheRowIsWritten: "beforeTheSummaries" | "afterTheSummaries"; + // Whether to go on to the second half - editing a child so two agree, which is + // what tells counting rows from counting distinct values. A case about the + // first computation alone leaves it off, because the edit is a recompute and + // repairs what it is meant to observe. + alsoCheckAfterAnEdit: boolean; + // The edit that makes two children agree. Only used when the above is true. + retarget: { childName: string; status: string }; + settleTimeoutMs: number; + pollIntervalMs: number; +} + +// A shared view in a space bound to a database whose connection is switched off. +export interface ShareViewUnreadyDataDbCaseConfig { + namePrefix: string; + rowTitle: string; + // Written into the connection row. It is never decrypted on this path - the + // connection is refused for being switched off before anything reads it - so + // this only has to be present, and saying so in the value keeps the next + // reader from looking for a real secret. + encryptedUrlPlaceholder: string; +} + +// A shared form whose picture is stored as a short path and read through two +// layers, each of which works the address out. +export interface SharedFormCoverUrlCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + rowTitle: string; + // Where the picture lives, as it is stored. Must be a short path: an address + // is what the fix passes through untouched, so an address here would make the + // case green either way. The runner refuses one. + storedPath: string; +} + +// A column that picks its value by case, where the branches with a case attached +// are numbers and the otherwise branch is a list of linked records. +export interface SwitchMixedBranchStorageCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + // The cases with a number behind them. Two at least: they have to agree with + // each other, or the step that merges the branches would have looked at the + // otherwise branch anyway. + numberBranches: { choice: string; column: string; value: number }[]; + // The case that falls through to the linked records. + otherwiseChoice: string; + // Rows in the linked table. At least two, so the linked column holds a list. + linkedRows: { name: string; price: number }[]; +} + +// An undo that cannot be carried out, followed by a second press. +export interface UndoCursorAfterAFailedUndoCaseConfig { + baseId: "seed-base"; + tableNamePrefix: string; + rowName: string; + // The row that takes the let-go value, so putting it back would collide. It is + // written on a different window, or it lands on the stack this case walks. + otherRowName: string; + // The value the row starts with and the value it is changed to. They have to + // differ, or there is nothing for undo to put back. + originalCode: string; + changedCode: string; +} diff --git a/framework/verdict.test.js b/framework/verdict.test.js index 3cfc3f5..e30d39e 100644 --- a/framework/verdict.test.js +++ b/framework/verdict.test.js @@ -44,3 +44,24 @@ test("verdict table", () => { ); } }); + +test("v1 is a reference column and never fails a run", () => { + // Every verdict that turns the guarded column red, on the engine that is + // only ever informational. + for (const verdict of ["error", "regression"]) { + assert.equal( + verdictFailsCi(verdict, { gating: true, engine: "v1" }), + false, + `${verdict} on v1 must not fail the run`, + ); + } + // ... and the same verdicts still bite on the engine the lab guards. + assert.equal(verdictFailsCi("error", { gating: false, engine: "v2" }), true); + assert.equal( + verdictFailsCi("regression", { gating: true, engine: "v2" }), + true, + ); + // An unstated engine is the guarded one, so a caller that forgets cannot + // accidentally turn gating off. + assert.equal(verdictFailsCi("error", { gating: true }), true); +}); diff --git a/framework/verdict.ts b/framework/verdict.ts index 520d5ad..ed0e856 100644 --- a/framework/verdict.ts +++ b/framework/verdict.ts @@ -47,5 +47,20 @@ export const resolveVerdict = ( // how the metadata rots. export const verdictFailsCi = ( verdict: BugVerdict, - { gating }: { gating: boolean }, -): boolean => verdict === "error" || (verdict === "regression" && gating); + { gating, engine }: { gating: boolean; engine?: string }, +): boolean => { + // v1 is a REFERENCE column and never fails anything. + // + // The lab guards v2: that is where the fixes land and where a returning bug + // is a regression someone must act on. v1 is run to answer a different + // question — "what does the engine our older customers are still on do with + // this?" — and its answer is information, not a contract. A v1 cell that + // reproduces is usually just the world without the fix, and a v1 cell that + // errors usually means the case leans on something v1 shapes differently. + // Failing the run for either would make every v1 observation a chore, and + // the column would be switched off within a month. + if (engine === "v1") { + return false; + } + return verdict === "error" || (verdict === "regression" && gating); +}; diff --git a/registry.ts b/registry.ts index bb70aff..a6d9530 100644 --- a/registry.ts +++ b/registry.ts @@ -143,6 +143,21 @@ import lookupY465OrdinaryRollupKeepsLinkedRecordIdentityCase from "./cases/looku import lookupY479Y482ConditionalRollupEditorKeepsNestedOrCase from "./cases/lookup/y479-y482-conditional-rollup-editor-keeps-nested-or.case"; import lookupY483ConditionalRollupEditorWrapsLookupConditionsCase from "./cases/lookup/y483-conditional-rollup-editor-wraps-lookup-conditions.case"; import linkY554PickerKeepsSelectionAcrossTabsCase from "./cases/link/y554-picker-keeps-selection-across-tabs.case"; +import shareViewUnreadyDataDbCase from "./cases/base-share/a-share-link-whose-database-is-away.case"; +import sharedFormCoverUrlCase from "./cases/base-share/a-shared-forms-picture.case"; +import sameNamedFkBaseDuplicateCase from "./cases/base-share/copy-a-base-whose-tables-share-a-key-name.case"; +import crossBaseConditionalBaseIdCase from "./cases/field/a-cross-base-conditional-column-keeps-its-base.case"; +import autonumberStringFilterCase from "./cases/filter/a-row-number-filter-typed-into-the-box.case"; +import switchMixedBranchStorageCase from "./cases/formula/a-column-that-picks-by-case.case"; +import orFilteredRollupScopeCase from "./cases/lookup/an-any-of-these-total-stays-inside-its-link.case"; +import selectRollupUniqueAndCountCase from "./cases/lookup/distinct-choices-in-the-order-they-appear.case"; +import jsonbLookupAggregateCase from "./cases/lookup/the-largest-of-a-borrowed-list.case"; +import nestedUserArrayJoinCreateCase from "./cases/record/add-a-row-to-a-table-that-joins-people-columns.case"; +import duplicatedTableStartsUnsharedCase from "./cases/table/a-duplicated-table-starts-unshared.case"; +import undoCursorAfterAFailedUndoCase from "./cases/undo/a-second-undo-after-one-that-failed.case"; +import legacyColumnNoPositionCase from "./cases/view/a-column-the-view-does-not-place.case"; +import groupOnAnUnreadableColumnCase from "./cases/view/a-grid-grouped-by-a-column-you-cannot-read.case"; +import legacyColumnVisibilityMetadataCase from "./cases/view/a-view-that-says-both-things-about-a-column.case"; import type { BugCase } from "./framework/types"; // Every runnable case, registered explicitly. scripts/case-catalog.mjs parses @@ -294,6 +309,21 @@ const cases = [ userFieldUndoOfClearDoesNotRenotifyAssigneeCase, userFieldAssignmentBurstArrivesCoalescedCase, circularAppendBurstReachesEveryLookupCase, + shareViewUnreadyDataDbCase, + sharedFormCoverUrlCase, + sameNamedFkBaseDuplicateCase, + crossBaseConditionalBaseIdCase, + autonumberStringFilterCase, + switchMixedBranchStorageCase, + orFilteredRollupScopeCase, + selectRollupUniqueAndCountCase, + jsonbLookupAggregateCase, + nestedUserArrayJoinCreateCase, + duplicatedTableStartsUnsharedCase, + undoCursorAfterAFailedUndoCase, + legacyColumnNoPositionCase, + groupOnAnUnreadableColumnCase, + legacyColumnVisibilityMetadataCase, ] satisfies BugCase[]; const caseById = new Map( diff --git a/scripts/report-teable-track.mjs b/scripts/report-teable-track.mjs index d3feeed..c0ea391 100644 --- a/scripts/report-teable-track.mjs +++ b/scripts/report-teable-track.mjs @@ -64,6 +64,7 @@ const main = async () => { const records = []; let skippedUnplanned = 0; + let skippedReference = 0; for (const path of await walk(artifactDir)) { if (!path.endsWith(".json")) { continue; @@ -77,6 +78,18 @@ const main = async () => { if (!isPayload(payload)) { continue; } + // The track carries the GUARDED column only. + // + // Its Run Key is (run, attempt, case, commit) — no engine — so two engines + // writing for one case would silently overwrite each other rather than + // land as two rows. Widening the key is possible but would change what + // every historical row means, and the v1 column does not want a queryable + // history: it is read once, in the run summary, beside the run that + // produced it. v1 payloads stay in the artifact. + if (payload.engine === "v1") { + skippedReference += 1; + continue; + } const planEntry = planBySha.get(payload.commitSha); if (!planEntry) { // The acceptance gate already fails the run for these; the table only @@ -126,6 +139,9 @@ const main = async () => { console.log( `Regression Track: ${created} created, ${updated} updated` + (skippedUnplanned > 0 ? `, ${skippedUnplanned} unplanned skipped` : "") + + (skippedReference > 0 + ? `, ${skippedReference} v1 reference payload(s) not tracked` + : "") + ".", ); }; diff --git a/scripts/send-feishu-summary.mjs b/scripts/send-feishu-summary.mjs index b462352..74e4efa 100644 --- a/scripts/send-feishu-summary.mjs +++ b/scripts/send-feishu-summary.mjs @@ -163,12 +163,40 @@ export const buildFeishuCard = ({ comparison, runUrl }) => { : null, ].filter(Boolean); + // One line for the reference engine, and only ever one. The card is what + // people actually read, so a v1 column nobody sees here is a column nobody + // sees — but it must not compete with the guarded result, and it must not + // grow with the number of cases. So: a count, no names, and it says plainly + // that it decided nothing. + const reference = (() => { + if (!comparison.engines?.includes("v1")) { + return null; + } + let present = 0; + let unrunnable = 0; + let skipped = 0; + for (const row of comparison.rows) { + for (const cell of row.referenceCells ?? []) { + if (cell.skipped) skipped += 1; + else if (cell.observed === "present") present += 1; + else if (cell.verdict === "error") unrunnable += 1; + } + } + const parts = [ + `${present} still reproduce`, + unrunnable > 0 ? `${unrunnable} could not run` : null, + skipped > 0 ? `${skipped} not asked` : null, + ].filter(Boolean); + return `v1 reference (decides nothing): ${parts.join(", ")}`; + })(); + const elements = [ { tag: "markdown", content: [ headline, needsHuman.length > 0 ? needsHuman.join(" · ") : null, + reference, `[Open the run and the full comparison table](${runUrl})`, ] .filter((line) => line !== null) diff --git a/vitest-e2e-lab.config.ts b/vitest-e2e-lab.config.ts index 79bfb6a..19d620b 100644 --- a/vitest-e2e-lab.config.ts +++ b/vitest-e2e-lab.config.ts @@ -66,6 +66,25 @@ export default defineConfig({ sequence: { hooks: "stack", }, + // A background worker finishing after its fixture is gone must not decide + // this run. + // + // Import cases hand work to a queue; the case then asserts, and its + // teardown removes the space the queue is still writing to. When the + // worker's completion handler lands it updates a table that no longer + // exists and rejects with nobody to catch it, and vitest fails the whole + // file on that. Measured on run 33055688034: 247 tests passed, 11 skipped, + // none failed, every payload written and the report job's acceptance gate + // green — and the job was red anyway, on one such rejection from a v1 + // import case. + // + // Ignoring them costs no signal. A case's evidence only ever arrives + // through bugCheckpoint() and is written to its payload before anything is + // allowed to throw; the payloads, judged by the report job, are what says + // whether a run passed. Vitest still PRINTS these under "Unhandled + // Errors", which is the same bargain the v1 column takes: visible, and + // gating nothing. + dangerouslyIgnoreUnhandledErrors: true, logHeapUsage: true, reporters: ["verbose"], include: [e2eLabSpec],