chore: bring in spicedb and update compilation by tstirrat15 · Pull Request #635 · authzed/zed

tstirrat15 · 2026-03-02T22:02:28Z

Description

This PR is part of unifying the composable schema package into the tooling for a better developer experience. The key changes are:

Dependency update — Updates SpiceDB dependency, bringing in a newer version of the schema compilation library.
Decoder refactor (internal/decode/decoder.go) — Significant rewrite of the schema decoder, along with updated tests. This simplifies the decoding logic.
Validate command simplification (internal/cmd/validate.go) — Major simplification of the validation logic, cutting roughly half the code. Tests were also reworked
New tests — Added 8 new .zed and .yaml test files for edge cases like:
- Import errors in composable schemas
- Nonexistent imports
- Nested composable schemas
- External schemas with escaping/errors

Testing

Unit tests

tstirrat15 · 2026-03-11T20:16:33Z

Current TODOs and notes:

This code depends on feat: update DevContext and LSP to support composable schemas spicedb#2965. It should be run locally against that with go.work, and that PR should also be tested against spicedb-vscode.

Merge feat: update DevContext and LSP to support composable schemas spicedb#2965 and then go get that SHA to make this reference that code.
Validation tests are failing. My best guess is that one if not all of the failures have to do with the DirFS() that's being handed to the DevContext in the validate command - it needs to match the dir of the validation file, not the invocation dir.
Need to check whether validations against just schemas are pointing their warnings at the correct lines (this should be captured in a test, but should also be validated)
It needs a test of a schema that imports another schema and there's a validation warning/error in that other schema to see whether it points at the correct file and line within that file
This should be hand-tested against a few things as well - validation files in the example repo, some composable schema stuff, etc.

codecov-commenter · 2026-03-17T04:29:44Z

Codecov Report

❌ Patch coverage is 80.13245% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.13%. Comparing base (2a8c2f1) to head (665577c).

Files with missing lines	Patch %	Lines
internal/decode/decoder.go	79.76%	12 Missing and 5 partials ⚠️
internal/cmd/validate.go	79.66%	10 Missing and 2 partials ⚠️
internal/cmd/import.go	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #635      +/-   ##
==========================================
+ Coverage   41.66%   42.13%   +0.47%     
==========================================
  Files          38       38              
  Lines        6128     6064      -64     
==========================================
+ Hits         2553     2555       +2     
+ Misses       3319     3252      -67     
- Partials      256      257       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

miparnisari · 2026-03-17T17:31:38Z

docs/zed.md

 	From pastebin:
 		zed validate https://pastebin.com/8qU45rVK
-
-	From a devtools instance:


FYI devtools was deprecated

miparnisari · 2026-03-17T17:31:47Z

docs/zed.md

 ```
-      --fail-on-warn         treat warnings as errors during validation
-      --force-color          force color code output even in non-tty environments
-      --schema-type string   force validation according to specific schema syntax ("", "composable", "standard")


FYI big change: removing the flag

miparnisari · 2026-03-17T17:32:12Z

internal/cmd/cmd_test.go

 			command:               []string{"zed", "validate"},
 			expectFlagErrorCalled: true,
 			flagErrorContains:     "requires at least 1 arg(s), only received 0",
-			expectUsageContains:   "zed validate <validation_file_or_schema_file> [flags]",


I don't know who would want to validate more than one schema file at once, but 🤷‍♀️

miparnisari · 2026-03-17T17:33:54Z

internal/cmd/validate.go

 			Schema:        parsed.Schema.Schema,
 			Relationships: tuples,
-		})
+		}, development.WithSourceFS(filesystem), development.WithRootFileName(filepath.Base(filename)))


FYI this is what allows us to validate composable schemas

miparnisari · 2026-03-17T17:35:29Z

internal/cmd/validate.go

-
+func outputDeveloperErrorsWithLineOffset(sb *strings.Builder, validateContents []byte, devErrors []*devinterface.DeveloperError, lineOffset int, sourceFS fs.FS) {
 	for _, devErr := range devErrors {
+		// If the error has a Path, read the contents from that file instead.


FYI this is what lets zed know which file contained error so it can print the contents of the context of the error

I'm pretty sure we can get this through the DeveloperError as well, but that can be future work.

🤔 Path is a field of DeveloperError

tstirrat15

See comments

tstirrat15 · 2026-03-23T15:33:27Z

internal/cmd/validate.go

-	if fileTypeArg != "" {
-		switch fileTypeArg {
-		case "yaml":
-			fileType = decode.FileTypeYaml
-		case "zed":
-			fileType = decode.FileTypeZed
-		default:
-			return "", true, fmt.Errorf("invalid value \"%s\" for --type. valid options are \"zed\" and \"yaml\"", fileTypeArg)
-		}
-	}


Why remove this?

we can detect the extension of the file, i don't see the need to pass in the file type by hand

Right, my thought was just to give folks the option to do so - make the heuristics dumber (and the code simpler and more predictable) and then give folks an escape hatch where they can tell zed how to treat the file.

I'm open to disagreement with that take, though.

tstirrat15 · 2026-03-23T15:38:34Z

internal/decode/decoder.go

+		if !filepath.IsLocal(schemaPath) {
+			return nil, fmt.Errorf("schema filepath %q must be local to where the command was invoked", schemaPath)
+		}
+
+		if d.FS == nil {
+			return nil, fmt.Errorf("cannot resolve schemaFile %q: no local filesystem context (remote URL?)", schemaPath)
+		}


I think we can use os.DirFS here as well - I'd think that you should be able to open one FS from another. Then we don't need to deal with IsLocal questions.

tstirrat15 · 2026-03-23T21:06:11Z

internal/cmd/import.go

 func importCmdFunc(cmd *cobra.Command, schemaClient v1.SchemaServiceClient, relationshipsClient v1.PermissionsServiceClient, prefix string, u *url.URL) error {
 	prefix = strings.TrimRight(prefix, "/")
-	contents, err := decode.FetchURL(u)
+	d, err := decode.DecoderFromURL(u)


Why the change back to a decoder struct? What's the state that's being held in the decoder object?

the decoder holds the fs

tstirrat15 · 2026-03-23T21:07:14Z

internal/cmd/validate.go

-
+func outputDeveloperErrorsWithLineOffset(sb *strings.Builder, validateContents []byte, devErrors []*devinterface.DeveloperError, lineOffset int, sourceFS fs.FS) {
 	for _, devErr := range devErrors {
+		// If the error has a Path, read the contents from that file instead.


I'm pretty sure we can get this through the DeveloperError as well, but that can be future work.

tstirrat15 · 2026-03-23T21:08:38Z

internal/decode/decoder.go

+	// Only attempt YAML unmarshaling if the input looks like a YAML validation file.
+	if !hasYAMLSchemaKey(inputString) && !hasYAMLSchemaFileKey(inputString) && !yamlRelationshipsKeyPattern.MatchString(inputString) {
+		return nil, fmt.Errorf("%w: input does not appear to be a YAML validation file", ErrInvalidYamlTryZed)
+	}


Why necessary?

Does this fix the relation something: schema issue?

My thought was that we can do a relatively naive yaml parse and let the user force it if they want to.

Why necessary?

this code was already there, no?

Does this fix the relation something: schema issue?

that was fixed in an old PR, and the test schema with relation named schema still passes

tstirrat15

See comments

tstirrat15 · 2026-03-24T14:43:00Z

internal/decode/decoder_test.go

 			}
 			require.NoError(t, err)
 			require.Equal(t, tt.expectedSchema, vFile.Schema.Schema)
-			require.Empty(t, vFile.SchemaFile)


I don't think this assertion is necessary so I'd rather not assert it.

tstirrat15 · 2026-03-24T14:43:22Z

internal/decode/decoder.go

+			// If the file is just a schema file, we set the LineNumber offset to 0
+			// for the purposes of displaying errors.
+			SourcePosition: spiceerrors.SourcePosition{LineNumber: 0, ColumnPosition: 1},


This change means we don't need to post-hoc check whether it's a schema file we're looking at for the purposes of offsets

tstirrat15 · 2026-03-24T14:44:52Z

internal/decode/decoder.go

-	data, err := os.ReadFile(filePath) //nolint:gosec // not an issue
+	filePath := filepath.Clean(u.Path)
+
+	invocationFS := os.DirFS(".")


Using DirFS here ensures that we don't have traversal issues (gosec) and also means that we don't have to validate locality otherwise.

miparnisari · 2026-03-24T16:57:16Z

internal/cmd/validate.go

 	validateCmd.Flags().Bool("force-color", false, "force color code output even in non-tty environments")
 	validateCmd.Flags().Bool("fail-on-warn", false, "treat warnings as errors during validation")
-	validateCmd.Flags().String("schema-type", "", "force validation according to specific schema syntax (\"\", \"composable\", \"standard\")")
+	validateCmd.Flags().String("type", "", "the type of the validated file. valid options are \"zed\" and \"yaml\"")


Suggested change

validateCmd.Flags().String("type", "", "the type of the validated file. valid options are \"zed\" and \"yaml\"")

validateCmd.Flags().String("type", "", "the type of the validated file. use this when zed cannot auto-detect the format of the file properly. valid options are \"zed\" and \"yaml\"")

tstirrat15 force-pushed the tstirrat/bring-in-new-schemadsl branch from 610e2cc to 1fb916b Compare March 11, 2026 19:42

tstirrat15 marked this pull request as draft March 11, 2026 19:42

tstirrat15 mentioned this pull request Mar 11, 2026

feat: update DevContext and LSP to support composable schemas authzed/spicedb#2965

Merged

miparnisari force-pushed the tstirrat/bring-in-new-schemadsl branch 13 times, most recently from d353034 to 6dc6f3f Compare March 17, 2026 02:32

tstirrat15 and others added 3 commits March 16, 2026 19:36

chore: bring in spicedb and update compilation

16a7392

chore: fix test

8b52d66

refactor: decoder

f0ec55a

miparnisari force-pushed the tstirrat/bring-in-new-schemadsl branch from 6dc6f3f to 47ee676 Compare March 17, 2026 02:37

miparnisari marked this pull request as ready for review March 17, 2026 02:42

miparnisari force-pushed the tstirrat/bring-in-new-schemadsl branch from 47ee676 to 32e38db Compare March 17, 2026 06:16

miparnisari reviewed Mar 17, 2026

View reviewed changes

docs/zed.md

From pastebin:

zed validate https://pastebin.com/8qU45rVK

From a devtools instance:

Copy link

Contributor

miparnisari Mar 17, 2026

Choose a reason for hiding this comment

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

FYI devtools was deprecated

miparnisari reviewed Mar 17, 2026

View reviewed changes

miparnisari force-pushed the tstirrat/bring-in-new-schemadsl branch from 32e38db to f1602c1 Compare March 20, 2026 04:38

test: fixing tests

d6d687e

miparnisari force-pushed the tstirrat/bring-in-new-schemadsl branch from f1602c1 to d6d687e Compare March 20, 2026 04:40

tstirrat15 commented Mar 23, 2026

View reviewed changes

miparnisari mentioned this pull request Mar 23, 2026

chore: update spicedb #646

Merged

tstirrat15 force-pushed the tstirrat/bring-in-new-schemadsl branch 2 times, most recently from 859759e to 8b680d4 Compare March 24, 2026 14:43

tstirrat15 commented Mar 24, 2026

View reviewed changes

miparnisari and others added 3 commits March 24, 2026 09:07

chore: update spicedb

126609f

chore: merge in main

cd7f5c1

chore: re-add type argument and refactor

665577c

tstirrat15 force-pushed the tstirrat/bring-in-new-schemadsl branch from 8b680d4 to 665577c Compare March 24, 2026 15:07

miparnisari reviewed Mar 24, 2026

View reviewed changes

	validateCmd.Flags().String("type", "", "the type of the validated file. valid options are \"zed\" and \"yaml\"")
	validateCmd.Flags().String("type", "", "the type of the validated file. use this when zed cannot auto-detect the format of the file properly. valid options are \"zed\" and \"yaml\"")

Conversation

tstirrat15 commented Mar 2, 2026 • edited by miparnisari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Uh oh!

tstirrat15 commented Mar 11, 2026 • edited by miparnisari Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miparnisari Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tstirrat15 commented Mar 2, 2026 •

edited by miparnisari

Loading

tstirrat15 commented Mar 11, 2026 •

edited by miparnisari

Loading

codecov-commenter commented Mar 17, 2026 •

edited

Loading

miparnisari Mar 23, 2026 •

edited

Loading