From 2999494b045caf86ac00ff8be57507a8bc63a32f Mon Sep 17 00:00:00 2001 From: Ben Ayles <1235055+knd775@users.noreply.github.com> Date: Mon, 14 Sep 2026 17:25:15 -0400 Subject: [PATCH] fix(runtime-scalars): rewrite variables on incremental runs The rewrite recorded each runtime scalar as an argument value keyed by the variable's id, but that column is a foreign key to documents. It only held while a document happened to share the id, so HMR re-extraction and parallel loading in houdini generate broke it. Because the FK is deferred, the violation surfaced at COMMIT and was discarded, rolling every rewrite back while AfterExtract reported success. The ncruces backend dropped that commit error too, so the guard never fired on WASI. --- .changeset/quiet-runtime-scalars-commit.md | 6 + .../plugin/runtimeScalars/transformInputs.go | 14 ++- .../runtimeScalars/transformInputs_test.go | 107 ++++++++++++++++++ packages/houdini-core/plugin/validate_test.go | 14 +++ plugins/db_ncruces.go | 6 +- plugins/db_test.go | 55 +++++++++ 6 files changed, 195 insertions(+), 7 deletions(-) create mode 100644 .changeset/quiet-runtime-scalars-commit.md create mode 100644 plugins/db_test.go diff --git a/.changeset/quiet-runtime-scalars-commit.md b/.changeset/quiet-runtime-scalars-commit.md new file mode 100644 index 000000000..e88d7d91f --- /dev/null +++ b/.changeset/quiet-runtime-scalars-commit.md @@ -0,0 +1,6 @@ +--- +'houdini-core': patch +'houdini': patch +--- + +Fix runtime scalar variables failing validation with "uses unknown type" during HMR, and intermittently on `houdini generate`. diff --git a/packages/houdini-core/plugin/runtimeScalars/transformInputs.go b/packages/houdini-core/plugin/runtimeScalars/transformInputs.go index 918e8715f..73ff3046c 100644 --- a/packages/houdini-core/plugin/runtimeScalars/transformInputs.go +++ b/packages/houdini-core/plugin/runtimeScalars/transformInputs.go @@ -5,8 +5,6 @@ import ( "fmt" "strings" - - "code.houdinigraphql.com/plugins" "code.houdinigraphql.com/plugins/graphql" ) @@ -57,7 +55,8 @@ func TransformVariables[PluginConfig any]( id, type, row, - column + column, + document FROM document_variables WHERE type in (%s) `, runtimeScalars[:len(runtimeScalars)-1])) if err != nil { @@ -125,6 +124,7 @@ func TransformVariables[PluginConfig any]( variableType := search.ColumnText(1) row := search.ColumnInt(2) column := search.ColumnInt(3) + documentID := search.ColumnInt(4) // we need to update the type of the variable err = db.ExecStatement(updateType, map[string]any{ @@ -151,9 +151,9 @@ func TransformVariables[PluginConfig any]( } directiveID := conn.LastInsertRowID() - // we need to store a string with the mapped value + // we need to store a string with the mapped value on the variable's document err = db.ExecStatement(insertStringArgumentValue, map[string]any{ - "document": variablesID, + "document": documentID, "value": variableType, }) if err != nil { @@ -176,5 +176,7 @@ func TransformVariables[PluginConfig any]( } // we're done (commit the transaction) - commit(nil) + if err := commit(nil); err != nil { + errs.Append(plugins.WrapError(err)) + } } diff --git a/packages/houdini-core/plugin/runtimeScalars/transformInputs_test.go b/packages/houdini-core/plugin/runtimeScalars/transformInputs_test.go index 98b439aca..382009dd0 100644 --- a/packages/houdini-core/plugin/runtimeScalars/transformInputs_test.go +++ b/packages/houdini-core/plugin/runtimeScalars/transformInputs_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/spf13/afero" "github.com/stretchr/testify/require" "code.houdinigraphql.com/packages/houdini-core/config" @@ -11,6 +12,7 @@ import ( "code.houdinigraphql.com/packages/houdini-core/plugin/documents" "code.houdinigraphql.com/packages/houdini-core/plugin/runtimeScalars" "code.houdinigraphql.com/plugins" + "code.houdinigraphql.com/plugins/graphql" "code.houdinigraphql.com/plugins/tests" ) @@ -94,3 +96,108 @@ func TestRuntimeScalars(t *testing.T) { require.Equal(t, "type", name) require.Equal(t, "2", value) } + +// HMR re-extracts one file and runs AfterExtract scoped to a task. the +// re-extracted document and its variables get fresh ids +func TestRuntimeScalars_incrementalAfterExtract(t *testing.T) { + ctx := context.Background() + + db, err := plugins.NewTestPool[config.PluginConfig]() + require.Nil(t, err) + defer db.Close() + + conn, err := db.Take(ctx) + require.Nil(t, err) + require.Nil(t, tests.WriteDatabaseSchema(conn)) + db.Put(conn) + + db.SetProjectConfig(plugins.ProjectConfig{ + ProjectRoot: "/project", + SchemaPath: "schema.graphql", + RuntimeDir: ".houdini", + Include: []string{"**/*.gql"}, + Exclude: []string{}, + RuntimeScalars: map[string]string{ + "UserFromSession": "Int", + }, + }) + + fs := afero.NewMemMapFs() + require.Nil(t, fs.MkdirAll("/project", 0755)) + require.Nil(t, afero.WriteFile(fs, "/project/schema.graphql", []byte(` + type Query { + me(id: Int!): Int + hello: String + } + `), 0644)) + require.Nil(t, afero.WriteFile(fs, "/project/a.gql", []byte(`query A { hello }`), 0644)) + require.Nil(t, afero.WriteFile(fs, "/project/b.gql", []byte(`query B($id: UserFromSession!) { me(id: $id) }`), 0644)) + + core := &plugin.HoudiniCore{} + core.SetDatabase(db) + core.SetFilesystem(fs) + require.Nil(t, core.Schema(ctx)) + + // full run + require.Nil(t, core.ExtractDocuments(ctx, plugins.ExtractDocumentsInput{})) + require.Nil(t, core.AfterExtract(ctx)) + require.Equal(t, "Int", variableState(t, db, "B")) + + // the HMR path for b.gql, mirroring what the vite plugin does + require.Nil(t, db.ExecQuery(ctx, `DELETE FROM raw_documents WHERE filepath = 'b.gql'`, nil)) + require.Nil(t, core.ExtractDocuments(ctx, plugins.ExtractDocumentsInput{ + Filepaths: []string{"/project/b.gql"}, + })) + require.Nil(t, db.ExecQuery(ctx, + `UPDATE raw_documents SET current_task = 'task-1' WHERE filepath = 'b.gql'`, + nil, + )) + require.Nil(t, core.AfterExtract(plugins.ContextWithTaskID(ctx, "task-1"))) + require.Equal(t, "Int", variableState(t, db, "B")) +} + +// variableState returns the type of the document's only variable and checks +// its runtime scalar directive +func variableState[PluginConfig any]( + t *testing.T, + db plugins.DatabasePool[PluginConfig], + document string, +) string { + t.Helper() + + conn, err := db.Take(context.Background()) + require.Nil(t, err) + defer db.Put(conn) + + search, err := conn.Prepare(` + SELECT + document_variables.type, + document_variable_directives.directive, + argument_values.raw, + argument_values.document = documents.id + FROM documents + JOIN document_variables ON documents.id = document_variables.document + LEFT JOIN document_variable_directives ON document_variables.id = document_variable_directives.parent + LEFT JOIN document_variable_directive_arguments ON document_variable_directives.id = document_variable_directive_arguments.parent + LEFT JOIN argument_values ON document_variable_directive_arguments.value = argument_values.id + WHERE documents.name = ? + `) + require.Nil(t, err) + defer search.Finalize() + search.BindText(1, document) + + hasRow, err := search.Step() + require.Nil(t, err) + require.True(t, hasRow, "document %q has no variables", document) + + variableType := search.ColumnText(0) + require.Equal(t, graphql.RuntimeScalarDirective, search.ColumnText(1)) + require.Equal(t, "UserFromSession", search.ColumnText(2)) + require.True(t, search.ColumnBool(3), "the directive's argument value must belong to the document") + + hasRow, err = search.Step() + require.Nil(t, err) + require.False(t, hasRow, "expected exactly one variable on %q", document) + + return variableType +} diff --git a/packages/houdini-core/plugin/validate_test.go b/packages/houdini-core/plugin/validate_test.go index e25179b14..016527947 100644 --- a/packages/houdini-core/plugin/validate_test.go +++ b/packages/houdini-core/plugin/validate_test.go @@ -1238,6 +1238,20 @@ func TestValidate_Houdini(t *testing.T) { }`, }, }, + { + Name: "Multiple variables can use runtime scalar types", + Pass: true, + Input: []string{ + `query Test($id: ViewerIDFromSession!, $other: ViewerIDFromSession!) { + a: node(id: $id) { + id + } + b: node(id: $other) { + id + } + }`, + }, + }, { Name: "No aliases for default keys", Pass: false, diff --git a/plugins/db_ncruces.go b/plugins/db_ncruces.go index 334bceac9..815cdd486 100644 --- a/plugins/db_ncruces.go +++ b/plugins/db_ncruces.go @@ -310,7 +310,11 @@ func (db DatabasePool[PC]) Transaction(conn Conn) func(*error) { db.db.Exec("ROLLBACK TO " + sp) db.db.Exec("RELEASE " + sp) } else { - db.db.Exec("RELEASE " + sp) + // releasing the outermost savepoint commits, so deferred FK + // violations surface here + if _, err := db.db.Exec("RELEASE " + sp); err != nil { + *errp = err + } } } } diff --git a/plugins/db_test.go b/plugins/db_test.go new file mode 100644 index 000000000..75e97acac --- /dev/null +++ b/plugins/db_test.go @@ -0,0 +1,55 @@ +package plugins + +import ( + "context" + "testing" +) + +// FKs are deferred, so a violation is only reported at commit. both backends +// have to hand that error back, or the transaction rolls back while the +// pipeline step reports success +func TestTransaction_reportsDeferredConstraintViolation(t *testing.T) { + ctx := context.Background() + + db, err := NewTestPool[struct{}]() + if err != nil { + t.Fatalf("failed to create test pool: %v", err) + } + defer db.Close() + + if err := db.ExecQuery(ctx, `CREATE TABLE parents (id INTEGER PRIMARY KEY)`, nil); err != nil { + t.Fatalf("failed to create parents: %v", err) + } + if err := db.ExecQuery(ctx, ` + CREATE TABLE children ( + id INTEGER PRIMARY KEY, + parent INTEGER NOT NULL, + FOREIGN KEY (parent) REFERENCES parents(id) + ) + `, nil); err != nil { + t.Fatalf("failed to create children: %v", err) + } + + conn, err := db.Take(ctx) + if err != nil { + t.Fatalf("failed to take a connection: %v", err) + } + defer db.Put(conn) + + commit := db.Transaction(conn) + + insert, err := conn.Prepare(`INSERT INTO children (parent) VALUES (12)`) + if err != nil { + t.Fatalf("failed to prepare insert: %v", err) + } + defer insert.Finalize() + if err := db.ExecStatement(insert, nil); err != nil { + t.Fatalf("the insert must be accepted until the transaction commits: %v", err) + } + + var commitErr error + commit(&commitErr) + if commitErr == nil { + t.Fatal("committing a transaction with a dangling foreign key must return an error") + } +}