feat(lint): native config lint command for Flex and Fixed config - #108
pjcdawkins wants to merge 36 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy app:config-validate (aliases validate, lint) command that delegated to the PHP CLI with a native Go linter that validates both Flex (.upsun/*.yaml) and Fixed (.platform*.yaml) project configurations offline, reporting all issues in one run.
Changes:
- Added a native Cobra command for config linting with
--format text|jsonand optional path/stdin input handling. - Introduced
internal/lintwith schema validation + semantic checks (types/versions, scripts, web config, dependencies, routes, relationships, naming), plus Fixed-style normalization and embedded schemas. - Added tooling/CI to keep embedded registry + schemas in sync with upstream sources.
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Adds lint-assets and lint-assets-check targets to refresh/verify embedded registry + schemas. |
| commands/root.go | Removes legacy Platformify validate command wiring and registers the new native lint command. |
| commands/list_models.go | Updates help/usage metadata for app:config-validate with path, --format, and --stdin. |
| commands/lint.go | Implements the new native app:config-validate command (aliases lint, validate) with text/json output. |
| internal/lint/linter.go | Adds CheckContent and wires schema + semantic checks for Flex-style content. |
| internal/lint/linter_test.go | Adds tests covering combined lints and common failure cases. |
| internal/lint/normalize.go | Detects Flex vs Fixed config layouts and dispatches to the appropriate loader/linter. |
| internal/lint/normalize_test.go | Tests style detection and directory linting behavior. |
| internal/lint/fixed.go | Loads Fixed-style config files, schema-validates them, normalizes into shared config shape, and runs semantic checks. |
| internal/lint/fixed_test.go | Tests Fixed-style loading/validation paths and edge cases. |
| internal/lint/merge.go | Merges `.upsun/*.yaml |
| internal/lint/merge_test.go | Tests merging behavior and error cases. |
| internal/lint/yaml.go | Adds YAML→Go decoding and JSON-schema validation helpers with scoped paths. |
| internal/lint/yaml_test.go | Tests YAML schema validation behavior on valid/invalid content. |
| internal/lint/result.go | Adds shared Result/Issue types and deterministic formatted output. |
| internal/lint/names.go | Adds application/service/worker name validation. |
| internal/lint/names_test.go | Tests name validation rules and error formatting. |
| internal/lint/types.go | Adds registry-backed type/version validation with Flex-only composable/stack warnings. |
| internal/lint/types_test.go | Tests type/version validation against a test registry. |
| internal/lint/relationships.go | Adds relationship target validation and “unused service” detection. |
| internal/lint/relationships_test.go | Tests relationship validation scenarios. |
| internal/lint/scripts.go | Adds POSIX shell syntax validation for hooks/commands/cron scripts + start-command warnings. |
| internal/lint/scripts_test.go | Tests script parsing failures and warning behavior. |
| internal/lint/web.go | Adds web location key/root path checks and rule regex validation. |
| internal/lint/web_test.go | Comprehensive tests for web linting rules and error formatting. |
| internal/lint/routes.go | Adds basic route upstream target/protocol validation. |
| internal/lint/routes_test.go | Tests route linting scenarios and expected messages. |
| internal/lint/dependencies.go | Adds dependency section validation (type and empty values). |
| internal/lint/dependencies_test.go | Tests dependency validation including complex PHP dependency shapes. |
| internal/lint/config_schema.go | Defines shared decoded config model used by semantic checks. |
| internal/lint/schema/schema.go | Embeds and loads the Flex JSON schema with sync.Once caching. |
| internal/lint/schema/schema_test.go | Smoke test that the embedded Flex schema validates a basic config. |
| internal/lint/schema/fixed.go | Embeds and loads Fixed-style schemas (application/routes/services). |
| internal/lint/schema/platformsh.application.json | Embedded Fixed application JSON schema. |
| internal/lint/schema/platformsh.routes.json | Embedded Fixed routes JSON schema. |
| internal/lint/schema/platformsh.services.json | Embedded Fixed services JSON schema. |
| internal/lint/registry/registry.go | Embeds registry data and provides parsing + normalization (clean()). |
| internal/lint/registry/registry_test.go | Smoke test that the embedded registry parses and includes expected entries. |
| internal/lint/registry/model.go | Defines registry model types and JSON unmarshalling behavior for versions. |
| internal/lint/registry/model_test.go | Tests registry model parsing helpers and template-friendly mapping. |
| internal/lint/registry/gen.go | Generator that fetches meta.upsun.com/images and writes registry.json. |
| internal/lint/registry/registry.json | Embedded minimized registry snapshot consumed by the linter. |
| internal/lint/testdata/registry.json | Test registry fixture used by type-check tests. |
| go.mod | Adds new dependencies used by the native linter (schema, regex, shell parser). |
| go.sum | Updates module checksums for the new/updated dependencies. |
| .github/workflows/ci.yml | Adds a CI job to fail when embedded lint assets are stale. |
| CLAUDE.md | Updates repo documentation to include the new native lint command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 2 warnings · 3 minor points
🔍 Full review · 49 files reviewed
🔵 Minor points
Not blocking, and no threads opened for these.
internal/lint/registry/model.go:121— Registry.ForTemplates, and its comment about "Jinja template access", has no production caller in this repo; it is exercised only by model_test.go and this CLI uses no Jinja templates. It is dead code carried over from the source project whose comment misdescribes how the registry is consumed here.CLAUDE.md:70— This line describes the command as "lint.go: Native config linter (aliasesvalidate,app:config-validate)", implyinglintis the canonical name. commands/lint.go sets Use "app:config-validate" with Aliases {"lint", "validate"}, soapp:config-validateis canonical andlintis the alias; the doc lists the canonical name among the aliases.general— This PR does not merge cleanly onto the base branch: go.mod is in conflict. The branch must be rebased or the go.mod conflict resolved before it can merge. Note that go.mod here promotes dlclark/regexp2/v2, xeipuuv/gojsonschema, and mvdan.cc/sh/v3 from indirect to direct requires, so the conflict must be resolved carefully to keep those direct.
Review details
- Commit: f564f12
- Model: claude-opus-4-8
- Panel: security · correctness · robustness · design
f564f12 to
539ea49
Compare
There was a problem hiding this comment.
Note
Reviewed — No new blocking findings · 3 minor points
🔍 Full review · 50 files reviewed
🔵 Minor points
Not blocking, and no threads opened for these.
.github/workflows/ci.yml:67— Thelint-assetsjob runsmake lint-assets-check, which first runsmake lint-assets(fetching live data from meta.upsun.com viago run gen.goand the platformify schemas viacurl) and thengit diff --exit-code. Because it re-derives the embedded assets from live upstream on every pull_request, any upstream registry/schema change turns this job red on PRs that never touched lint code, and the job fails outright if those hosts are unreachable during CI..github/workflows/ci.yml:59— The newlint-assetsjob pinsactions/checkout@v6andactions/setup-go@v6, while the siblingtest,legacy-php, andintegration-testjobs in this same file use@v7. The mismatched action versions look unintentional.internal/lint/registry/model.go:30— Image.Docs and the Web, Upstream, Location, Hooks, BuildConfig, and Dependency types it references, plus Image.Description and Image.Configuration, are never populated by gen.go (which writes only name/type/runtime/versions into registry.json) and are never read by any linter check — clean() even blanks Description. They are dead speculative fields carried over from the source project.
Review details
- Commit: 539ea49
- Model: claude-opus-4-8
- Panel: security · correctness · robustness · design
Superseded: the latest Upsun Dispatch review no longer requests changes.
|
Rebased on Beyond the review threads above, a
Not changed: web location rules are validated with The same three bugs exist in the AI API, which this package was ported from. Fixed there in platformsh/ai/api!205. |
709c3c1 to
3fe17b0
Compare
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 2 warnings · 🔵 4 minor points · ⚪ 1 nitpick
🔍 Full review · 50 files reviewed
⚪ Nitpick
internal/lint/names.go:48— The length check islen(value) > maxServiceNameLength(32), so a 32-character name is accepted, but the message says it "should be shorter than 32 characters". The message and the rule disagree at exactly 32 characters; either say "no longer than 32 characters" or tighten the comparison to>=.
Verification
- A realistic Fixed project (.platform.app.yaml with mounts/crons/hooks plus .platform/services.yaml and routes.yaml) normalizes and lints with zero errors — I ran CheckDir against it.
- detectFixed anchors on a root-level config dir or app file, so a nested fixtures/.platform.app.yaml alone yields StyleUnknown (covered by TestDetectStyle).
- findFlexConfigFiles builds globs with path.Join, so the io/fs slash patterns still match on Windows (TestFindFlexConfigFiles_NestedDir).
- printLintResult emits
errors/warningsas arrays rather than null and returns errLintFailed exactly when result.HasErrors() (commands/lint_test.go). - schema/fixed.go propagates a parse failure through fixedErr, so LoadApplication/LoadRoutes/LoadServices never hand back a nil schema silently.
The diff adds substantial unit tests for the new package (names, routes, relationships, types, web, scripts, merge, fixed, normalize, registry, schema) plus commands/lint_test.go, which covers only the JSON printing helper — no test exercises runLint/lintInput, the --format validation, or the explicit-path/FindProjectRoot interaction. The existing CI test job runs make test over all packages and golangci-lint runs on the new files; the new lint-assets job re-derives the embedded registry/schemas from upstream and fails on any diff.
Review details
- Commit: 3fe17b0
- Model: claude-opus-5
Review 5 of 10 for this pull request · View the full run
There was a problem hiding this comment.
Note
Reviewed — No new issues found · 7 still open (1 nitpick)
🔁 Incremental · 2 files reviewed
Outstanding from earlier reviews:
- 🟡 #4093039188 —
internal/lint/linter.go:48: Valid Fixed composable-image configs fail linting with a bogus error. — CheckTypes still gates the stack-without-type allowance onstyle == StyleFlex, so Fixed composable configs fall through to check(app.Type, true) and error with 'type cannot be empty'. - 🟡 #4093039197 —
commands/lint.go:79: An explicit path argument silently lints a different directory. — runLint still calls lint.FindProjectRoot(path) for an explicit argument, solint apps/frontendstill lints the enclosing .git root. - 🔵 #4093039206 —
internal/lint/registry/registry.go:30: A registry parse failure degrades into misleading per-type errors. — registry.Parsed still declaresvar err errorinside the function and assigns only within parsedOnce.Do, so later calls after a parse failure return (nil, nil). - 🔵 #4093039214 —
internal/lint/registry/gen.go:62: Fixed-style projects may be judged against Upsun-only version support. — gen.go is unchanged; the embedded registry is still built from upsun-only statuses and is applied to Fixed-style projects too. - 🔵 #4093039219 —
.github/workflows/ci.yml:67: Unrelated PRs go red when upstream publishes image changes. — The lint-assets job still runs on pull_request; this very push is an instance of the predicted drift — golang:1.25 vanished upstream and three pinned test literals had to be edited. - 🔵 #4093039224 —
commands/list_models.go:147: Machine-readable command list exposes keys in a non-standard shape. — list_models.go still keys the two new options as "--format" and "--stdin" instead of Option.GetName(), unlike every neighbouring entry. - ⚪
internal/lint/names.go:48: Off-by-one wording misleads users at the boundary length. — names.go still useslen(value) > maxServiceNameLengthwith a message saying "shorter than 32 characters". (first raised)
Verification
- The regenerated registry.json is still valid JSON in the shape registry.Parse expects (per-type object with name/type/runtime/versions).
- golang:1.25 was dropped from the registry and every occurrence in internal/lint tests was updated to golang:1.26; no remaining lint test pins 1.25.
- clickhouse 25.8 moved from supported to legacy, which checkType still accepts (allVersions = supported + legacy), so no test asserting on 25.8 changes behaviour.
- mariadb/mysql both gained 10.11 consistently, keeping the mysql alias (clean() copies mariadb) in agreement with the embedded data.
This increment adds no tests; it refreshes the embedded registry and edits three literals in internal/lint/linter_test.go to keep TestLint passing. The repository's test workflow job runs the lint package tests, and the lint-assets CI job re-runs make lint-assets-check against live meta.upsun.com.
Review 6 of 10 for this pull request · View the full run
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 1 warning · ⚪ 1 nitpick · 2 still open
🔁 Incremental · 10 files reviewed
⚪ Nitpick
commands/lint.go:79— The explicit[path]argument is only passed throughfilepath.Abs; nothing checks that it exists or is a directory.lint /no/such/dirorlint .upsun/config.yamlreacheslint.CheckDir, where both detectors simply fail, so the user seesno configuration found in "/no/such/dir" (looked for .upsun/*.yaml, ...)rather than a typo/not-a-directory error.
Outstanding from earlier reviews:
- 🔵 #4093039214 —
internal/lint/registry/gen.go:62: Fixed-style projects may be judged against Upsun-only version support. — gen.go still keys status offinfo.Upsun.Statusonly, so versions retired on Upsun but live on Platform.sh are absent from the registry used for Fixed configs. - 🔵 #4093039219 —
.github/workflows/ci.yml:67: Unrelated PRs go red when upstream publishes image changes. — Thelint-assetsjob still runs on every pull_request and re-fetches meta.upsun.com/the schemas before failing on any diff.
Verification
registry.Parsednow stores the parse error in the package-levelparsedErr, so repeat calls after a failed parse return the error instead of (nil, nil).- The two new option entries in
innerAppConfigValidateCommandare keyedformat/stdin, matching the bare-name convention ofOption.GetName()used by the other entries. - An explicit
[path]argument is now linted as given (filepath.Abs), withFindProjectRootapplied only to the default.. - The name-length message change matches the
len(value) > maxServiceNameLengthcondition, and names_test.go was updated to the new wording. - Dropping the
styleparameter leaves no other caller ofCheckTypes/runChecks;Styleis still used byCheckDirand its tests.
This increment adds one fixed_test.go case (Fixed app with stack and no type, asserting no errors only) and updates types_test.go/names_test.go for the signature and message changes; the test CI job (make test) runs them, and the lint-assets job checks embedded asset freshness. No test asserts the warning set for Fixed-style stack configs, which is why the contradictory warning above is uncovered.
Review 7 of 10 for this pull request · View the full run
There was a problem hiding this comment.
Note
Reviewed — No new blocking findings · 🔵 1 minor point · 2 still open
🔁 Incremental · 4 files reviewed
🔵 Minor point
internal/lint/schema/platformsh.application.json:920— Relaxing the root constraint fromoneOftoanyOfmakesname+type+stackvalid for any type, not just composable ones. A.platform.app.yamlwithname: myapp,type: "php:8.3"andstack: ["php@8.3"]now passes the schema, and no semantic check catches it either: in internal/lint/types.go the stack-related branches only fire whenapp.Type == ""(warning) or when the type starts withcomposable(empty-stack warning), so a non-composable type carrying astackkey lints clean and reports "The configuration is valid", even thoughstackis only meaningful with a composable image. The oldoneOfat least rejected the combination (for the wrong reason). Adding a check forstackset with a non-composabletypein CheckTypes would restore the coverage the schema change gives up.
Outstanding from earlier reviews:
- 🔵 #4093039214 —
internal/lint/registry/gen.go:62: Fixed-style projects may be judged against Upsun-only version support. — Unchanged: gen.go still derives supported/legacy solely fromversions.<v>.upsun.status; author states no other brand status exists in the feed. - 🔵 #4093039219 —
.github/workflows/ci.yml:67: Unrelated PRs go red when upstream publishes image changes. — Unchanged: thelint-assetsjob still runs on every pull_request andlint-assets-checkstill regenerates registry.json from live meta.upsun.com before diffing.
Verification
- The new
os.Statguard in commands/lint.go rejects a missing path and a non-directory path before CheckDir, settling the earlier typo-path point. anyOfwith the tworequiredbranches still rejects an app file lackingname, so the existing "name is required" test expectation holds (gojsonschema merges the best sub-result errors for anyOf).- The new
composable image with type and stackfixture validates: registry.json has nocomposableentry, so registry.clean() injects it with ChannelStable "25.11", matchingtype: "composable:25.11". - Dropping the three platformify
curllines keepslint-assets-checkcoherent — the locally maintained platformsh.*.json files can no longer be reverted by a refresh, andgit diff -- internal/lint/schemastays clean.
This increment adds one table case in internal/lint/fixed_test.go (Fixed app with both type and stack, asserting no errors), run by the test job in .github/workflows/ci.yml via make test; the new not-a-directory branch in commands/lint.go has no test covering it.
Review 8 of 10 for this pull request · View the full run
There was a problem hiding this comment.
Note
Reviewed — No new issues found · 2 still open
🔁 Incremental · 2 files reviewed
Outstanding from earlier reviews:
- 🔵 #4093039214 —
internal/lint/registry/gen.go:62: Fixed-style projects may be judged against Upsun-only version support. — Code unchanged: gen.go still derives supported/legacy solely from versions.<v>.upsun.status, and the same registry is used for Fixed projects; author declined as there is no other brand data available. - 🔵 #4093039219 —
.github/workflows/ci.yml:67: Unrelated PRs go red when upstream publishes image changes. — ci.yml still runs thelint-assetsjob on every pull_request, re-fetching live meta.upsun.com and the platformify schemas before thegit diffcheck.
Verification
- The new
else if !isComposable && !isStackEmpty(app.Stack)branch cannot fire for a type-less app, since theapp.Type == ""+ stack branch above stillcontinues. isStackEmpty(internal/lint/config_schema.go) handles string/slice/map/nil stacks, so the new warning does not misfire on an emptystack: []orstack: "".- The added test case asserts the exact new warning text and path prefix
applications.foo.stack, matching the string built in types.go. - Neither normalize.go nor fixed.go rewrites
stack, so Fixed-style configs reach the new branch with the author's own value, not a synthesized one.
The diff adds a table case stack_with_non_composable_type in internal/lint/types_test.go exercising the new warning; it runs in the CI test job (make test) alongside golangci-lint in the same job.
Review 9 of 10 for this pull request · View the full run
There was a problem hiding this comment.
Note
Reviewed — No new blocking findings · 🔵 1 minor point · 1 still open
🔁 Incremental · 7 files reviewed
🔵 Minor point
internal/lint/types.go:79— The new retired-version warning renders the suggestion list fromimg.Versions.Supportedonly. For a registry entry whose versions are all retired/legacy (Supportedempty — whichgen.gocan produce whenever upstream marks every version of an image retired), the message degrades toversion '7.2' of type 'elasticsearch' is retired; use one of:with an empty list, giving the user no actionable target. The same empty-list rendering applies to the two error messages below it, which now share thesupportedvariable.
Outstanding from earlier reviews:
- 🔵 #4093039214 —
internal/lint/registry/gen.go:62: Fixed-style projects may be judged against Upsun-only version support. — Partly mitigated: retired-on-Upsun versions now warn instead of erroring, but gen.go still derives solely from versions.<v>.upsun.status, so a version decommissioned on Upsun yet deployable on Platform.sh is still omitted and errors.
Verification
VersionInfo.LatestVersionis removed and grep over the repository shows no remaining Go caller, so the deletion compiles.slices.Concat(Supported, Legacy, Retired)keeps retired versions out of the unsupported-version error, matching the newTestCheckTypes_RetiredVersionexpectation of a warning only.- The retired lists added to registry.json are sorted newest-first exactly as
compareVersionsorders them (nodejs8.9before8,6.10before6.9). internal/lint/linter_test.gonow pinsgolang:1.26, which is insupported, so the newly addedgolangretired list (1.25 and older) does not turn those cases into warnings.check(path, t, runtime)reports the retired warning under the same path previously used for the error, so worker and service paths are unchanged.
Covered by the test job in .github/workflows/ci.yml (make test); this increment adds TestCheckTypes_RetiredVersion in internal/lint/types_test.go and drops the LatestVersion tests. The regenerated registry.json itself is only checked by the lint-assets job, which this change makes continue-on-error: true, so stale/incorrect embedded assets no longer fail CI.
Review 10 of 10 for this pull request · View the full run
The thread is about version data that isn't available and Upsun-related versions are the main use case anyway.
9d5ca9f to
d361654
Compare
|
@upsun-dispatch review |
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 3 warnings · ⚪ 1 nitpick
🔍 Full review · 54 files reviewed
⚪ Nitpick
commands/lint.go:75— When both--stdinand a[path]argument are given,lintInputtakes the stdin branch and ignores the path without a word.upsun lint --stdin ./projlints only stdin, and if stdin is empty it fails with "Empty content" instead of pointing out that the two inputs conflict.
Verification
registry.Parsednow keeps the parse error in the package-levelparsedErr, so later calls return the error rather than(nil, nil).- The Fixed application schema now uses top-level
anyOffor name+type / name+stack, sotype: composablealongsidestackvalidates, as the new fixed_test case expects. - The
lint-assetsCI job hascontinue-on-error: trueand only regeneratesregistry.json, so upstream drift does not block PRs. - The new lint command keeps the Cobra name
app:config-validatewith thelint/validatealiases, matching the removed platformify command it replaces.
This PR adds unit tests across internal/lint (fixed, normalize, merge, types, tasks, relationships, routes, web, scripts, registry, schema) and commands/lint_test.go for output formatting. The CI test job runs them via make test. The non-blocking lint-assets job checks that the embedded registry is up to date. No test covers quiet mode or JSON output when an operational error occurs.
Review details
- Commit: dfdc681
- Model: claude-opus-5-5
|
@upsun-dispatch review |
Superseded: the latest Upsun Dispatch review no longer requests changes.
Add internal/lint with the multi-error config linter ported from the ai-api repository (internal/linter, internal/schema, internal/registry, plus the .upsun config merge helpers). The linter validates merged Flex-style config against the embedded JSON schema and runs semantic checks (relationships, names, types, scripts, web, dependencies, routes), collecting all errors and warnings rather than stopping at the first. Changes from the source: - Inline the composable-image stable channel constant to drop the nix dependency. - Drop the AI-only file_modifications schema patch. - Use github.com/dlclark/regexp2/v2. Promote gojsonschema to a direct dependency and add mvdan.cc/sh/v3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the platformify-backed app:config-validate command (which delegated to the legacy PHP CLI) with a native Go command that runs the ported linter and reports all errors and warnings at once. - Add internal/lint/normalize.go: DetectStyle plus LintDir, which detect the configuration style from the directory layout (.upsun vs .platform) and lint the merged Flex configuration. Fixed-style linting is stubbed pending Phase 3. - Add commands/lint.go: the "lint" command (aliases "validate", "app:config-validate") accepting an optional path or stdin, with text and JSON output, exiting non-zero on errors. - Rename Lint to LintContent and add JSON tags to Issue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add linting for legacy Platform.sh configuration: .platform.app.yaml files and .platform/applications.yaml (list or map form), plus optional .platform/routes.yaml and .platform/services.yaml. CheckDir detects the style from the directory layout and normalizes Fixed-style config into the same Config the Flex path uses, so the semantic checks are shared. - Add the three Fixed-style JSON schemas (application, routes, services) copied from platformify, with per-file loaders and CheckSchemaScoped to attribute schema errors to their source file or app. - Gate composable-image and stack warnings to Flex in CheckTypes. - Guard against Flex-style keys appearing in a Fixed-style file. - Inject the application name from the map key when validating map-form applications.yaml, matching the canonical parser. - Rename the entrypoints to CheckContent and CheckDir, consistent with the Check* family, and satisfy the repository linters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add tooling to regenerate the embedded lint assets: - gen.go (build-tagged) fetches https://meta.upsun.com/images and transforms it into registry.json, mapping per-version status to supported/legacy and service/runtime. Regenerate with `go generate ./internal/lint/registry` or `make lint-assets`. - `make lint-assets` also refreshes the Flex and Fixed-style schemas from platformify. The meta.upsun.com schema is intentionally not used: it validates types via remote $refs (fetched at runtime) and duplicates the registry-based type check, so type and version validation stays in CheckTypes. - `make lint-assets-check` and a CI job fail when the committed assets are stale. Refresh the registry from meta.upsun.com and make the registry test robust to version drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update the app:config-validate help metadata to document the optional path argument and the --format and --stdin options, and note the native lint command in CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address findings from code review: - lint: only read piped stdin when it carries content; otherwise (a non-interactive shell or CI, where stdin is not a TTY but empty) fall back to linting the directory. Previously `lint` with no arguments errored with "empty content" in CI. Explicit --stdin still errors on empty input. - lint: make app:config-validate the primary command name (aliases lint, validate), matching the listing/help metadata and generated docs. - Fixed-style: reject a name set inside a map-form applications.yaml entry, matching the canonical parser and avoiding inconsistent app identity keying. - Add Result.Merge and use it instead of reallocating via Combine; drop the dead map[any]any branch in toStringMap; fix a misspelled identifier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make config-style detection match how a project actually deploys, and drive the directory names from the CLI's own config so vendor/white-label builds work. - Resolve the project root by walking up to the nearest enclosing .git (falling back to the given path), so lint can run from any subdirectory. The nearest .git is used, not the topmost, so a stray repository higher up the tree (e.g. a dotfiles repo, or /tmp) cannot hijack the result. - Detect Flex vs Fixed from the vendor's conventions (project_config_flavor, project_config_dir, app_config_file) plus the files present: Flex wins when present, else Fixed, else the native format. A first-party build (.upsun or .platform) also recognizes the other first-party format as a migration case; white-label builds use only their own names. - Anchor Fixed detection at the root (a config directory or a top-level app file). Nested per-app files are still collected once a project is confirmed, but a stray fixture no longer turns an unrelated repo into a project. - Warn about nested copies of any known config directory (.upsun, .platform, or the configured dir); the platform only reads them at the project root. - Make the duplicate application-name error name both source files. - Accept .yml as well as .yaml for Fixed routes/services/applications files. - Add app_config_file to the Go config schema (it was already in the YAML). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Print the validated directory on its own line every run
("Validating configuration in directory: <path>", path in cyan).
- Colour only the "Linter errors:" / "Linter warnings:" headings
(bold red / bold yellow); the issue lines use the default colour so
they stay readable.
- Keep the green check mark but leave "The configuration is valid."
in the default colour.
- Capitalize the first letter of operational errors for display (the Go
error strings stay lowercase per convention), so "no configuration
found" reads as "No configuration found".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Go 1.27 is about to be released where this is on by default anyway. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
io/fs glob patterns are always slash-separated, but filepath.Join used the OS separator, so on Windows the pattern was `.upsun\*.yaml`. path.Match treats the backslash as an escape, so nothing matched: detection returned false and every Flex project reported "no configuration files found". Use path.Join, and rename the parameter to `dir` so it no longer shadows the package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
For a valid configuration both slices are nil, so --format json produced
{"errors": null, "warnings": null}. The output documents these as arrays,
so a consumer iterating them without a null guard broke on valid input.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Hooks, web commands and crons were parsed, but worker commands were not decoded at all, so a syntax error in one went unreported. Both schemas require workers.*.commands.start; pre_start is also accepted, and post_start in Flex. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registry.ForTemplates has no caller: it came from the source project, where the registry fed Jinja templates. Only its own tests used it. mockSchema discarded the error from gojsonschema.NewSchema, so an invalid schema would have returned nil and panicked later instead of failing. Also correct the CLAUDE.md description, which listed app:config-validate as an alias of itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two service types were rejected outright: valkey-persistent was created inside `if _, ok := reg["valkey"]; !ok`. The registry does contain valkey, so the branch never ran and the alias was never added. Split it into its own check, keyed on the alias, as the redis-persistent block already does. mariadb-replica and postgresql-replica are listed upstream without versions of their own, so every version was rejected, with an empty "it must be exactly one of: " list. They now track the type they replicate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Worker names were added to the same map as application and service names, so two applications each defining a `queue` worker were reported as a duplicate, as was a worker sharing a name with a service. Worker names are scoped beneath their application, so they are now tracked separately and excluded from the duplicate check. They remain valid relationship targets, as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The registry had drifted from meta.upsun.com, failing lint-assets-check: mariadb and postgresql gained 12.3, redis gained 8.8, rabbitmq moved to 4.3, and elixir, kafka and python versions moved. The refresh also dropped mariadb-replica and postgresql-replica, which are no longer published upstream at all, so filling in their versions was not enough. They are now created from the type they replicate, like the persistent aliases, which keeps them working whether or not upstream lists them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eters - Piped stdin is no longer read implicitly when no path is given. A script that runs the command with unrelated stdin (e.g. inside a `while read` loop) would otherwise lint that input instead of the directory. Use --stdin to lint merged Flex configuration from standard input. - Remove the unused context parameters from CheckContent, CheckDir and lintFixed, and the unused Style.String method. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Upstream now lists golang 1.27/1.26 (dropping 1.25), clickhouse 26.3, and mariadb/mysql 10.11. Tests move from golang:1.25 to golang:1.26. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Composable images (`stack`, `type: composable:*`) are checked the same way for Fixed config as for Flex, since Platform.sh supports them too. An app with `name` and `stack` but no `type` no longer fails with "type cannot be empty". The now-unused style parameter is removed from CheckTypes and runChecks. - An explicit path argument is linted as given. Only the default (no argument) ascends to the enclosing Git repository root. - registry.Parsed keeps its parse error for later calls instead of returning an empty registry with a nil error. - The --format and --stdin options are keyed by bare name in the command list, like other options. - The name length message says "no longer than", matching the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Fixed application schema (copied from platformify) required exactly one of `type` or `stack` via `oneOf`, so the documented Platform.sh form of a composable image, `type: "composable:..."` with `stack`, failed validation, and the stack-without-type warning pointed users at an error. Use `anyOf` instead. The Fixed schemas are now maintained here, so `make lint-assets` no longer overwrites them from platformify. Also report a missing or non-directory path argument directly instead of "no configuration found". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Fixed schema now allows `type` and `stack` together, so a single-runtime type such as `php:8.3` with a `stack` key would lint clean. Warn instead, in both styles, since `stack` only applies to the composable image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Retired versions (e.g. golang:1.25 once 1.27 is out) were omitted from the embedded registry, so configs using them failed linting. They are now listed under `retired` and reported as a warning naming the supported versions. Decommissioned versions are still errors. The `lint-assets` CI job is now non-blocking (`continue-on-error`), since upstream registry changes are unrelated to the pull request, and uses the same action versions as the other jobs. Also remove the unused VersionInfo.LatestVersion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If every version of an image is retired upstream, the retired warning and the version errors ended with an empty "use one of:" list. Only suggest versions when there are some. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The schema is now maintained in this repository, so it is formatted consistently to keep later edits reviewable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tasks and the `authorizations` key are new Upsun features that neither upstream schema (platformify or meta.upsun.com) describes, so valid configuration was rejected. The rules follow the platform's own config validator. - Tasks (Flex only): the top-level `tasks` section is merged and schema-checked (type, base, stack, source, hooks, run, relationships, mounts, variables, authorizations and the other shared container keys; run.timeout is at most 86400). Semantic checks cover task names, image types, script syntax of run.command and hooks, `base` used alone, a required run command, relationships from tasks, and task mounts (a storage mount must name an application). Task names share a namespace with applications and services, and a task cannot be a relationship target. - Authorizations on applications, their web and worker containers, and tasks, in both Flex and Fixed config: `task` allows only `operate` and needs a resource naming an existing task; `env` allows only `view`. - The `local` mount source (an alias of `instance`) is accepted. - Unknown top-level keys in Flex files are rejected, as the platform does, except `.`-prefixed ones. - Flex issues are prefixed with the `.upsun/*.yaml` file that defines the application, service, route or task, as Fixed issues already are. - Linked Git worktrees and separate clones below the project root are skipped (submodules are kept), so they no longer produce stray config warnings or duplicate Fixed applications. - A non-PHP application that only serves static files (no location passes requests through) is no longer warned about a missing start command. The Flex schema is now maintained in this repository, like the Fixed schemas, so `make lint-assets` only refreshes the image registry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A service used only through a `source: service` mount (e.g. network storage), on an application, worker or task, is no longer reported as having no relationship. - Web location rules are compiled in regexp2's RE2 mode, which adds the PCRE-style `(?P<name>...)` group syntax that Nginx accepts. Found by codex review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A start command is optional: without one nothing runs, and the application can still serve static files, including a single-page app that falls back to `passthru: /index.html`. The linter cannot tell whether an application needs to run a process, so the warning was a false positive for static sites. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `operations` (runtime operations) is allowed in Fixed application config, as it already was in Flex config. - A service that no application or task uses is a warning rather than an error: the platform deploys it, and only logs a warning itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The `!include` (yaml, string, binary and archive types), `!file` and `!archive` tags are resolved as the platform does: paths are relative to the file, may not leave the project, and recursive includes are rejected. Problems are reported at the tag. - Each issue now has a file and line, found by following its path through the parsed YAML of the file that defines the entry (route keys may contain dots). Content from an included file is reported at the `!include` tag. The JSON output gains `file` and `line` fields. - The text output groups issues by file, with each line number and path on one line and the message below, instead of one long line each. - Schema messages no longer repeat the path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Files are opened through os.Root, so an !include through a symbolic
link cannot read a file outside the project.
- Aliases and merge keys ("<<") are followed when walking sections and
entries, so `applications: *apps` and merged application definitions
work again.
- Duplicate keys within a file are reported again.
Found by codex review.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The report (issues and the success line) is written to stdout, as the command's output, so --quiet no longer hides it. The "Validating..." line stays on stderr. - With --format json, an operational error such as a missing path or no configuration is reported as a JSON error, so there is always a document. - --stdin with a path argument is an error instead of ignoring the path. - In Fixed config, a web location rule may set `passthru` to a boolean, as the location itself and Flex rules can. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With warnings and no errors, the command printed only the warnings, so it was unclear whether the configuration was valid. It now ends with "The configuration is valid, with N warnings." Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Avoids another "upsun" literal in the commands package, which CI's goconst check flagged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ured stacks Following the platform's config validator: - `egress` (per-phase allowed domains for `build` and `runtime`) on applications, their web and worker containers, and tasks. - `container_profile` on web and worker containers and on services, in both styles, and on Fixed applications. - OCI images: a `docker:` type needs an `image` with either `name` or `buildfile`, and is not checked against the image registry. `image` on any other type is an error. - `stack` is the structured form only: a mapping of `runtimes` and `packages`, each a name, a mapping or a list of them. The platform rejects the older list form for new deployments. - Cron `timeout`, and operation `timeout` and `commands.stop` (timeouts are 1 to 86400 seconds). Explicit nulls stay rejected. The Fixed services schema is reformatted like the others. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3d9b90c to
8cd6411
Compare
Summary
Replaces the
app:config-validatecommand (aliasesvalidate,lint), which used the platformify library's schema validator and reported a single error at a time, with a newlintcommand that reports all errors and warnings at once. It runs a JSON-schema check plus semantic checks (relationships, names, types/versions, scripts, web config, dependencies, routes).It works for both configuration styles:
.upsun/*.yaml, merged, includingtasks..platform.app.yamlfiles and/or.platform/applications.yaml(list or map form), plus optional.platform/routes.yamland.platform/services.yaml. The.ymlextension is accepted as well as.yaml.About 8600 lines of this PR are the new
internal/lintpackage - almost entirely copied from the internal AI API project where it has been used in production for ~ 9 months.In the AI API it only supported "Flex" validation, so the "Fixed" configuration is now normalized into the same shape, so the further checks are shared across both styles.
Project root and style detection
Detection is offline (no API calls) and based on the running CLI's own config plus the files present:
.gitdirectory (falling back to the current directory), so the command can run from any subdirectory. An explicit path is linted as given. The nearest, not topmost,.gitis used so a stray repository higher up the tree cannot hijack the result.project_config_flavor,project_config_dir,app_config_file), so vendor/white-label builds work. The style is chosen as: Flex when present, else Fixed, else the build's native format. A first-party build (.upsunor.platform) also recognizes the other first-party format as a migration case; white-label builds use only their own names.Details
commands/lint.go: the command, taking an optional[path](or merged Flex config via--stdin), with--format text|json. Exits non-zero on errors.internal/lint: the pure linter (CheckDir,CheckContent), Fixed-style loaders, and the embedded Flex/Fixed JSON schemas.internal/lint/registry: the image registry.gen.gotransformsmeta.upsun.com/imagesinto the embeddedregistry.json.make lint-assetsrefreshes the embedded image registry;make lint-assets-checkand a non-blocking CI job fail when it is stale.Tasks and authorizations
basemust be used alone, a run command is required, a task cannot be a relationship target, and a taskstoragemount must name an application.authorizations(on applications, their web and worker containers, and tasks):taskallows onlyoperateand needs a resource naming an existing task;envallows onlyview.Warnings and messages
!include,!fileand!archivetags are resolved as the platform does, confined to the project (including through symbolic links). Issues within included content are reported at the tag.meta.upsun.com/images) are warned about; decommissioned or unknown versions are errors..platformor.upsun) is warned about, since the platform only reads it at the root.Out of scope
Folding the Platformify repository into this one is a separate follow-up; only its schemas were copied in for now. They are maintained here from now on, since neither upstream describes newer features: tasks, workload
authorizations,egress, OCIimageconfig,container_profileon all containers, the structuredstackform (the only one the platform accepts), cron and operation timeouts, thelocalmount source, andoperationsin Fixed app config were added, and the Fixed application schema'soneOffortype/stackis changed toanyOf, so a composable image can set both, as documented. The Platformify dependency is still used by theinitcommand.The meta.upsun.com schema is intentionally not used for validation: it resolves type and version via remote
$refs (fetched at runtime, which would break offline use) and duplicates the registry-based type check.🤖 Generated with Claude Code