fix(cmd/morphic)!: stop a failed -o write masking exit 1

README.md

@@ -123,6 +123,13 @@ Diagnostics print one per line as `<severity> <code> <path>#<pointer>: <message>
 `0` clean (and for any help request), `1` a diagnostic reached the `--fail-on` threshold (or the
 spec could not be lowered), `2` a usage or I/O error.
 
+`1` and `2` can both be earned by one run — a spec that reached the threshold whose `-o`
+destination then refused the write. The verdict on the spec wins, so `1` means what it says
+whatever `-o` pointed at, and `2` means the run failed for a reason outside the spec. The write
+error is printed on stderr either way. Note that `-o` publishes by rename, so a destination whose
+directory will not take a temp file — `/dev/null`, a read-only directory — cannot be written to at
+all.
+
 ### Library
 
 The same pipeline is available as a package. `engine.New` builds the default registry (OpenAPI

cmd/morphic/compile.go

@@ -167,15 +167,35 @@ func compileSpec(specPath string, opts compileOptions, stdout, stderr io.Writer)
 	if res.Document == nil {
 		return 1
 	}
+
+	diagCode := exitCodeFor(res.Diagnostics, opts.failOn)
 	if opts.explain != "" {
 		explainDocument(stdout, res.Document, res.Diagnostics, opts.explain)
-		return exitCodeFor(res.Diagnostics, opts.failOn)
+		return diagCode
 	}
 	if err := writeCompiled(opts.outPath, stdout, res.Document); err != nil {
 		emitf(stderr, "morphic: %v\n", err)
-		return 2
+		return writeFailureExit(diagCode)
 	}
-	return exitCodeFor(res.Diagnostics, opts.failOn)
+	return diagCode
+}
+
+// writeFailureExit returns the exit code for a run whose diagnostics earned
+// diagCode and whose output then could not be written.
+//
+// A failed write does not overwrite a non-zero diagCode. Whether a destination
+// can be written at all is a property of the destination — /dev/null and a
+// read-only directory both refuse the temp file replaceFile publishes through —
+// not of the spec, so letting it decide the exit code made "the spec reached the
+// --fail-on threshold" report 1 or 2 depending on where -o pointed. The verdict
+// on the spec is the same either way and the write failure is on stderr either
+// way, so the exit code keeps the verdict, and 2 is left to mean a run that
+// failed for a reason outside the spec.
+func writeFailureExit(diagCode int) int {
+	if diagCode != 0 {
+		return diagCode
+	}
+	return 2
 }
 
 // parseArgs binds fs and collects positional arguments, tolerating flags that
@@ -276,9 +296,16 @@ func writeCompiled(outPath string, stdout io.Writer, doc *ir.Document) error {
 // it, which is what makes the swap atomic and which costs four things a
 // truncating write gave for free. All four are accepted deliberately:
 //
-//   - Writing needs permission on the destination's directory, not just on the
-//     destination. Rewriting an existing writable file inside a read-only
-//     directory used to succeed and now fails at temp-file creation.
+//   - Writing needs a directory that will accept a new entry, not just a
+//     writable destination. Rewriting an existing writable file inside a
+//     read-only directory used to succeed and now fails at temp-file creation,
+//     and so does any destination whose directory refuses one — -o /dev/null
+//     most visibly, since /dev takes no temp file. Writing through to such a
+//     destination instead is deliberately not done here: honouring what a name
+//     points at rather than replacing the name is the same trade the symlink and
+//     hard-link entries below decline, and opening a reader-less FIFO for
+//     writing blocks indefinitely. What the failure must not do is decide the
+//     exit code — see writeFailureExit.
 //   - A symlink at outPath is replaced by a regular file instead of being
 //     followed and written through, so its target keeps its old content.
 //   - Other hard links to outPath keep pointing at the old inode, and so keep

cmd/morphic/edgecases_test.go

@@ -187,6 +187,94 @@ func TestRunParse_OutputCreateError(t *testing.T) {
 	assert.Contains(t, stderr.String(), "create output")
 }
 
+// unresolvedRefSpec is an OpenAPI 3.1 document whose only $ref names a schema
+// that is not there. It matters that it still lowers to a document: a spec that
+// lowers to nil returns 1 before the -o write is ever attempted, so it could not
+// exercise the combination writeFailureExit decides.
+const unresolvedRefSpec = `openapi: 3.1.0
+info: {title: Bad, version: "1"}
+paths:
+  /x:
+    get:
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema: {$ref: "#/components/schemas/Missing"}
+`
+
+// missingParentDest returns a destination inside a directory that is not there,
+// so creating the temp file beside it fails.
+func missingParentDest(t *testing.T) string {
+	t.Helper()
+	return filepath.Join(t.TempDir(), "missing-dir", "ir.json")
+}
+
+// readOnlyDirDest returns a writable file inside a read-only directory — the
+// shape TestWriteParsed_ReadOnlyDirFails pins, where the destination itself
+// could be rewritten but its directory will not take the temp file.
+func readOnlyDirDest(t *testing.T) string {
+	t.Helper()
+	if os.Geteuid() == 0 {
+		t.Skip("root bypasses directory permission checks, so the write would succeed")
+	}
+	dir := t.TempDir()
+	out := filepath.Join(dir, "ir.json")
+	require.NoError(t, os.WriteFile(out, []byte("PREVIOUS\n"), 0o644))
+	require.NoError(t, os.Chmod(dir, 0o555))
+	// Restore write permission so t.TempDir's own cleanup can remove the tree.
+	t.Cleanup(func() { _ = os.Chmod(dir, 0o755) })
+	return out
+}
+
+// TestRunParse_WriteFailureDoesNotMaskDiagnostics pins that a destination which
+// refuses the write does not decide the exit code. A spec that reached the
+// --fail-on threshold exits 1 wherever -o pointed; only a run with nothing to
+// report at the threshold exits 2 for the write. Both destinations here are
+// refused for the same reason — replaceFile publishes by rename and their
+// directories will not take a temp file — which is also why -o /dev/null fails,
+// untestable here because a root /dev would take the temp file and the rename
+// would then replace /dev/null itself.
+func TestRunParse_WriteFailureDoesNotMaskDiagnostics(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		spec     string
+		dest     func(t *testing.T) string
+		wantCode int
+		// wantDiag is a diagnostic code stderr must carry, or "" when the spec is
+		// clean and stderr must carry no diagnostic at all.
+		wantDiag string
+	}{
+		{"threshold reached, missing parent dir", unresolvedRefSpec, missingParentDest, 1, "openapi/unresolved-ref"},
+		{"threshold reached, read-only dir", unresolvedRefSpec, readOnlyDirDest, 1, "openapi/unresolved-ref"},
+		{"nothing reported, missing parent dir", testspec.Tiny, missingParentDest, 2, ""},
+		{"nothing reported, read-only dir", testspec.Tiny, readOnlyDirDest, 2, ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			spec := writeFile(t, "spec.yaml", tt.spec)
+			var stdout, stderr bytes.Buffer
+
+			code := run([]string{"compile", spec, "-o", tt.dest(t)}, &stdout, &stderr)
+
+			assert.Equal(t, tt.wantCode, code, "stderr: %s", stderr.String())
+			assert.Contains(t, stderr.String(), "create output",
+				"the destination must really refuse the write, or this case proves nothing")
+			if tt.wantDiag == "" {
+				assert.NotContains(t, stderr.String(), "openapi/",
+					"the clean spec must reach the write with no diagnostic behind it")
+				return
+			}
+			assert.Contains(t, stderr.String(), tt.wantDiag,
+				"a failed write must not swallow the diagnostics it was reported alongside")
+		})
+	}
+}
+
 func TestExitCodeFor_SeverityMatrix(t *testing.T) {
 	t.Parallel()
 	tests := []struct {