Skip to content

feat(devcontainer): require id in feature parse - #960

Merged
skevetter merged 1 commit into
mainfrom
devcontainer-spec/feature-id-required
Aug 9, 2026
Merged

skevetter merged 1 commit into
mainfrom
devcontainer-spec/feature-id-required

Conversation

@devsy-app

@devsy-app devsy-app Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Spec requirement

The devcontainer Feature schema (devContainerFeature.schema.json) marks id as a required property on a Feature manifest (devcontainer-feature.json).

Gap found

ParseDevContainerFeature in pkg/devcontainer/config/parse.go silently accepted manifests with a missing or empty id. The cmd/feature info and generatedocs commands therefore processed malformed Features without error, and the only id-presence check lived in validatePublishTarget (publish path) — leaving the parse/info/docs paths unenforced.

Change

  • pkg/devcontainer/config/parse.go: ParseDevContainerFeature now returns "<path> is missing required property \"id\"" when id is empty/missing, so every caller (info, generatedocs, publish) enforces the spec.
  • cmd/feature/publish.go: removed the now-redundant ID == "" check in validatePublishTarget (the parser enforces it earlier).
  • cmd/feature/publish_test.go: updated TestValidatePublishTarget_MissingID to assert the parser-level message.
  • pkg/devcontainer/config/feature_test.go: added unit tests for the missing-id / empty-id / valid-id cases plus DependsOnKeys() ordering behavior.

No feature versions or lock SHAs were changed (none were stale).

Verification

  • task cli:format — clean (exit 0)
  • task cli:lint:ci — 0 new issues
  • task cli:test — all pass except the known pre-existing pkg/git TestRepoClone* failures on origin/main (unrelated to this change; this change does not touch pkg/git)
  • task cli:test:e2e:suite -- "devcontainer" — not run (no docker/devcontainer CLI available in this environment)

This PR was created by an AI agent as part of an automated daily devcontainer spec job.

The devcontainer Feature schema marks `id` as a required property, but ParseDevContainerFeature silently accepted manifests with an empty/missing id. The info and generatedocs commands would therefore process malformed features without error.

Add id validation to ParseDevContainerFeature so a missing or empty id returns an error referencing devcontainer-feature.json. Remove the now-redundant id-presence check in cmd/feature validatePublishTarget (the parser enforces it) and update its test to assert the parser-level message. Add unit tests covering the missing-id, empty-id, and valid-id cases.
@netlify

netlify Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit bc658d3
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a786fab0ff9840008adfb12

@netlify

netlify Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit bc658d3
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6a786fab82011c000884075c

@github-actions github-actions Bot added the size/m label Aug 9, 2026
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity · 0 duplication

Metric Results
Complexity 9
Duplication 0

View in Codacy

AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@skevetter
skevetter marked this pull request as ready for review August 9, 2026 16:59
@skevetter
skevetter merged commit 0ed2fd4 into main Aug 9, 2026
67 checks passed
@skevetter
skevetter deleted the devcontainer-spec/feature-id-required branch August 9, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant