Skip to content

Comments

Fix nested remote imports to resolve relative to parent's base path#16372

Closed
strawgate wants to merge 12 commits intogithub:mainfrom
strawgate:fix-nested-import-base-path
Closed

Fix nested remote imports to resolve relative to parent's base path#16372
strawgate wants to merge 12 commits intogithub:mainfrom
strawgate:fix-nested-import-base-path

Conversation

@strawgate
Copy link
Contributor

@strawgate strawgate commented Feb 17, 2026

  • Nested relative imports from remote workflowspecs now resolve relative to the parent file's directory (standard relative path semantics), instead of hardcoded .github/workflows/
  • ../ traversals supported via path.Clean for sibling-directory imports
  • Consolidated all inline workflowspec formatting into FormatWorkflowSpec helper
  • Integration tests against githubnext/agentics verify end-to-end nested import resolution

Problem

parseRemoteOrigin stripped the last 2 path components to derive BasePath, so owner/repo/workflows/code-simplifier.md got an empty base path. Its nested import shared/reporting.md resolved at the repo root instead of workflows/shared/reporting.md, causing 404s, security scanner warnings, and broken runtime-import paths in .lock.yml files.

Fix

BasePath is now the parent directory of the file (strip filename only):

Workflowspec BasePath
owner/repo/workflows/code-simplifier.md workflows
owner/repo/base/subdir/file.md base/subdir
owner/repo/file.md `` (repo root)

ResolveNestedImport joins base path + relative import with path.Clean, so ../sibling/file.md from base/subdir resolves to base/sibling/file.md. The traversal guard now only rejects paths escaping the repository root.

Fixes #16370

Nested relative imports from remote workflowspecs were hardcoded to
resolve against .github/workflows/ in the remote repo. When the parent
workflowspec was imported through a different path (e.g.,
gh-agent-workflows/), this produced inconsistent cache paths in the lock
file and caused security scanner warnings for nonexistent local files.

Add a BasePath field to remoteImportOrigin, derived from the parent
workflowspec by stripping owner/repo and the last two path components
(the 2-level import dir/file.md). Nested imports now resolve relative to
this base, keeping cache paths consistent with the parent's path.

Fixes github#16370

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings February 17, 2026 14:40
@strawgate
Copy link
Contributor Author

will test this locally for my usecase

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a bug where nested remote imports were hardcoded to resolve against .github/workflows/ in the remote repository, causing inconsistent cache paths when the parent workflowspec was imported through a different directory (e.g., gh-agent-workflows/).

Changes:

  • Added BasePath field to remoteImportOrigin struct to track the base directory path from the parent workflowspec
  • Updated parseRemoteOrigin to extract BasePath by removing the last 2 path components from the workflowspec path
  • Modified nested import resolution logic to use the parent's BasePath instead of hardcoded .github/workflows/

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg/parser/import_processor.go Added BasePath field to remoteImportOrigin struct, updated parseRemoteOrigin to extract BasePath from workflowspecs, and modified nested import resolution to use parent's BasePath
pkg/parser/import_remote_nested_test.go Added comprehensive test cases for BasePath extraction across different path structures, and verified consistency between parent and nested import paths

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

Looks OK but I'd really like to see any string-hacking around the different kind of specs we use put into helpers and shared across the codebase, preferably paired with their parsing, and under independent unit test

@pelikhan
Copy link
Contributor

@strawgate let's add integration tests using some samples of githubnext/agentics

@strawgate
Copy link
Contributor Author

strawgate commented Feb 17, 2026

Sounds good, let me get the right behavior working in this PR and then i'll expand with tests and identify helper extractions.

Looks like the problem also occurs for runtime imports -- they are not pointing at the cache when added from a remote source

@strawgate
Copy link
Contributor Author

strawgate commented Feb 17, 2026

fyi githubnext/agentics has a one-level import via the shared folder but no multi-level/nested imports

@pelikhan
Copy link
Contributor

fyi githubnext/agentics has a one-level import via the shared folder but no multi-level/nested imports

what do I need to add in there to trigger this code path?

@pelikhan
Copy link
Contributor

@copilot merge main and recompile

@strawgate
Copy link
Contributor Author

The repo currently has

Workflow A -> fragment B

I.e. A workflow which uses include to include a fragment

Ideally we'd also add:

Workflow A -> fragment B -> fragment C

I.e. fragment B has an include in it as well

Basically two levels of nested includes instead of just one

@strawgate
Copy link
Contributor Author

See githubnext/agentics#170 for an example of the double nested workflow

strawgate added a commit to elastic/ai-github-actions that referenced this pull request Feb 18, 2026
@pelikhan
Copy link
Contributor

@copilot add integration test suite that generates various agentic workflows with multiple levels of imports and self adds the workflows to test those code paths. Test extensive graphs combination with combination of file path depth.

@pelikhan
Copy link
Contributor

See #16550

@strawgate
Copy link
Contributor Author

@pelikhan looks like this can close then!

Copilot stealing my hard work is going to have a big impact on my attempt to goose my contributor ranking. 🤣

@strawgate strawgate closed this Feb 18, 2026
@pelikhan
Copy link
Contributor

We are closing down 3rd party PR/ in favor of issue plans. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested remote imports resolve against hardcoded .github/workflows/ instead of parent workflowspec base path

3 participants