Conversation
|
Hey A couple of things worth addressing before this leaves draft:
|
…elds, deprecate dependencies Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aa3ac404-4800-44eb-87ba-46e012289e85
imports with aw and apm-packages subfields; deprecate dependencies
|
@copilot make it an error to mom dependencies and apm-packages |
…neously Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a73a68ce-962a-4894-b960-afe233ac836d
Done in e71d63c. |
|
@copilot update docs on dependencies with new syntax . use tech-writer agent. |
There was a problem hiding this comment.
Pull request overview
Adds an object form for the imports frontmatter key to support both agentic workflow imports (imports.aw) and APM packages (imports.apm-packages), and deprecates the top-level dependencies field in favor of imports.apm-packages (including a migration codemod).
Changes:
- Extend
importsparsing/consumers to support{ aw: ..., apm-packages: ... }while treating onlyawas workflow import paths. - Implement
dependencies→imports.apm-packagesbehavior (error on conflict, warning on deprecated usage) and add a codemod to migrate files. - Update schema + hashing + CLI utilities and add tests for new extraction/migration flows.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/frontmatter_extraction_metadata.go | Prefer imports.apm-packages, warn on deprecated dependencies, and error when both are set. |
| pkg/workflow/compiler_orchestrator_engine.go | Ensure import injection appends to imports.aw when imports is object form. |
| pkg/workflow/apm_dependencies_test.go | Add tests covering imports.apm-packages extraction and conflict behavior. |
| pkg/parser/schemas/main_workflow_schema.json | Allow imports to be oneOf array/object, including apm-packages config. |
| pkg/parser/import_bfs.go | Parse object-form imports by extracting and BFS-processing only imports.aw. |
| pkg/parser/frontmatter_hash.go | Update text-based import-path hashing to support object-form imports (intended to hash aw only). |
| pkg/cli/run_push.go | Update import collection to read paths from imports.aw when object form is used. |
| pkg/cli/imports.go | Update workflowspec conversion logic to process imports.aw for object form. |
| pkg/cli/fix_codemods_test.go | Update codemod count/order expectations to include the new codemod. |
| pkg/cli/fix_codemods.go | Register the new dependencies-to-imports-apm-packages codemod. |
| pkg/cli/dependency_graph.go | Update dependency graph import extraction to read from imports.aw when object form is used. |
| pkg/cli/codemod_dependencies_test.go | Add tests for the new migration codemod across multiple frontmatter shapes. |
| pkg/cli/codemod_dependencies.go | Implement codemod that moves dependencies into imports.apm-packages, converting imports shape as needed. |
Comments suppressed due to low confidence (1)
pkg/cli/imports.go:126
- In the object-form
imports.awcase,collectStringImportsdrops any{path: ..., inputs: ...}entries and then rewritesawas a[]string, losing inputs/config. Update this to preserve object import specs and only rewrite theirpathvalues.
switch aw := awAny.(type) {
case []any:
awImports := collectStringImports(aw)
importsLog.Printf("Found %d imports (object form, aw subfield) to process", len(awImports))
v["aw"] = processImportPaths(awImports)
case []string:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "type": "object", | ||
| "description": "Object format with packages and optional configuration.", | ||
| "properties": { | ||
| "packages": { | ||
| "type": "array", |
There was a problem hiding this comment.
extractAPMDependenciesFromValue supports an version field in the object config, but the imports.apm-packages object schema doesn’t allow version. This will make schema validation reject valid configs. Add a version property (with appropriate pattern/description) to this object schema.
| // Detect the 'aw:' subfield (object form) | ||
| if lineIndent == baseIndent+2 && strings.HasPrefix(trimmed, "aw:") { | ||
| isObjectForm = true | ||
| inAwSubfield = true | ||
| awIndent = lineIndent | ||
| continue | ||
| } | ||
|
|
||
| // Detect other object-form subfields (e.g. 'apm-packages:') — skip their contents | ||
| if isObjectForm && lineIndent == baseIndent+2 && strings.Contains(trimmed, ":") && !strings.HasPrefix(trimmed, "-") { | ||
| inAwSubfield = false | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
Object-form detection in extractImportsFromText only flips isObjectForm=true when it sees aw:. If a workflow uses object form with only imports.apm-packages (no aw), isObjectForm stays false and - ... items under apm-packages will be incorrectly treated as import paths (hash changes). Detect object form when encountering any subkey under imports: (e.g. apm-packages:), and only collect - items when inside aw:.
| // Detect the 'aw:' subfield (object form) | |
| if lineIndent == baseIndent+2 && strings.HasPrefix(trimmed, "aw:") { | |
| isObjectForm = true | |
| inAwSubfield = true | |
| awIndent = lineIndent | |
| continue | |
| } | |
| // Detect other object-form subfields (e.g. 'apm-packages:') — skip their contents | |
| if isObjectForm && lineIndent == baseIndent+2 && strings.Contains(trimmed, ":") && !strings.HasPrefix(trimmed, "-") { | |
| inAwSubfield = false | |
| continue | |
| } | |
| // Detect object-form subfields under imports: (e.g. 'aw:', 'apm-packages:') | |
| if lineIndent == baseIndent+2 && strings.Contains(trimmed, ":") && !strings.HasPrefix(trimmed, "-") { | |
| // Any subkey under imports: indicates object form | |
| isObjectForm = true | |
| if strings.HasPrefix(trimmed, "aw:") { | |
| // We are entering the 'aw:' subfield; only its items are import paths | |
| inAwSubfield = true | |
| awIndent = lineIndent | |
| } else { | |
| // Other subfields (e.g. 'apm-packages:') are not import paths | |
| inAwSubfield = false | |
| } | |
| continue | |
| } |
| case []any: | ||
| imports := collectStringImports(v) | ||
| importsLog.Printf("Found %d imports (array form) to process", len(imports)) | ||
| result.Frontmatter["imports"] = processImportPaths(imports) |
There was a problem hiding this comment.
In the []any imports case, this code extracts only string items and then overwrites imports with a []string. If the list contains object import specs (e.g. {path: ..., inputs: ...}), those entries will be silently dropped when rewriting the file. Preserve object items and rewrite only the path field (or round-trip via []any without discarding non-strings).
This issue also appears on line 121 of the same file.
|
|
||
| for i, line := range lines { | ||
| if i == importsEnd { | ||
| result = append(result, " apm-packages:") | ||
| result = append(result, reindentBlock(depsBodyRaw, depsBodyItemIndent, " ")...) | ||
| } | ||
| if i >= depsIdx && i < depsEnd { | ||
| continue | ||
| } | ||
| result = append(result, line) | ||
| } |
There was a problem hiding this comment.
When imports is already an object, insertion of apm-packages happens only when i == importsEnd. If importsEnd == len(lines) (imports block is last in frontmatter), that condition is never hit, so the codemod will remove dependencies but never insert imports.apm-packages (data loss). Add a post-loop insertion when the apm-packages block hasn’t been inserted yet.
| for i, line := range lines { | |
| if i == importsEnd { | |
| result = append(result, " apm-packages:") | |
| result = append(result, reindentBlock(depsBodyRaw, depsBodyItemIndent, " ")...) | |
| } | |
| if i >= depsIdx && i < depsEnd { | |
| continue | |
| } | |
| result = append(result, line) | |
| } | |
| insertedAPMPackages := false | |
| for i, line := range lines { | |
| if i == importsEnd { | |
| result = append(result, " apm-packages:") | |
| result = append(result, reindentBlock(depsBodyRaw, depsBodyItemIndent, " ")...) | |
| insertedAPMPackages = true | |
| } | |
| if i >= depsIdx && i < depsEnd { | |
| continue | |
| } | |
| result = append(result, line) | |
| } | |
| // If importsEnd == len(lines), the loop never hits i == importsEnd, | |
| // so we need to append apm-packages after processing all lines. | |
| if !insertedAPMPackages { | |
| result = append(result, " apm-packages:") | |
| result = append(result, reindentBlock(depsBodyRaw, depsBodyItemIndent, " ")...) | |
| } |
importspreviously only accepted an array. This adds an object form that consolidates agentic workflow imports (aw) and APM packages (apm-packages) under a single key, and deprecates the top-leveldependenciesfield in favour ofimports.apm-packages.New object form of
importsBoth
awandapm-packagesare optional — the object form works with either or both subfields.Deprecation of
dependenciesUsing
dependenciesalone emits a deprecation warning at compile time directing users to migrate toimports.apm-packages. Specifying bothimports.apm-packagesanddependenciessimultaneously is a compilation error:Migration codemod
gh aw fix --writeautomatically migratesdependencies→imports.apm-packages, handling three cases:imports: createsimports: { apm-packages: ... }imports: converts toimports: { aw: [...], apm-packages: ... }imports: appendsapm-packagesto the existing objectThe codemod skips files that already have
imports.apm-packagesto avoid overwriting existing configuration.Changes
main_workflow_schema.json):importsis nowoneOf[array, object]import_bfs.go): handlesmap[string]anyimports by processingawas import specs; extractedparseImportSpecsFromArrayhelperfrontmatter_extraction_metadata.go): returns an error if bothimports.apm-packagesanddependenciesare set; usesimports.apm-packageswhen present alone; falls back todependencieswith deprecation warning; extractedextractAPMDependenciesFromValuehelpercompiler_orchestrator_engine.go):addImportToFrontmatterappends toimports.awwhen imports is already in object formimports.go,run_push.go,dependency_graph.go): all imports-field consumers handle the object form, extracting paths fromawonlyfrontmatter_hash.go): text-based hash parser handles object form, extracting onlyawitems (notapm-packagesslugs) as import pathscodemod_dependencies.go): newdependencies-to-imports-apm-packagescodemod registered in the codemod registry🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.