Refactor parser import/path helpers to eliminate wasm/native drift and duplicate logic#53895
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Consolidates parser path/import helpers to prevent native/Wasm drift and remove duplicate logic from issue #53888.
Changes:
- Centralizes remote path predicates and resolution calculations.
- Reuses shared path-section and frontmatter helpers.
- Tests dotted repository names as valid imports.
Show a summary per file
| File | Description |
|---|---|
pkg/parser/remote_workflow_spec.go |
Removes duplicated workflow-spec detection. |
pkg/parser/remote_resolve_path.go |
Uses shared path-resolution helpers. |
pkg/parser/remote_path_predicates.go |
Adds platform-independent parser helpers. |
pkg/parser/remote_fetch_wasm.go |
Aligns Wasm with shared behavior. |
pkg/parser/path_section.go |
Adds shared path-section splitting. |
pkg/parser/include_processor.go |
Uses the shared splitter. |
pkg/parser/import_topological.go |
Reuses shared splitting and frontmatter extraction. |
pkg/parser/import_bfs.go |
Replaces duplicate import splitters. |
pkg/parser/frontmatter_utils_test.go |
Covers dotted repository names. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Balanced
|
@copilot Please refresh this PR for maintainer review and then run the Current maintainer-visible next steps:
|
Refreshed PR #53895 at head Handoff for maintainers:
Still needed: maintainer review/approval. |
|
Triage: category= Non-draft, CI green (29/32 checks passed), reduces wasm/native duplication across 9 files. AI reviewer left comments — please address before merge.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
…-build-tag-copies Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Finish pass complete at head
Remaining merge risks: blocking review state/requested review still needs maintainer/reviewer re-review or approval, and CI needs a human re-trigger. |
|
@copilot This PR still needs a maintainer-facing finish pass.
|
Finish pass complete at head
Human action still required: stale blocking |
PR Triage
Batch: B1-small-fixes-refactors Eliminates wasm/native drift and duplicate parser path-helper logic across 10 files. CI green (1/1 success). Automated triage by PR Triage Agent.
|
PR TriageCategory: refactor | Risk: medium | Priority: medium (score 45/100)
|
|
@copilot Please do a maintainer-facing finish pass on this PR.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…-build-tag-copies Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Finish pass complete at head
Human action still required: re-trigger/verify CI on this agent-pushed head and clear any stale blocking review/requested-review state before merge. |
|
@copilot Please do a fresh maintainer-facing follow-up on this PR. Re-check the current head for merge readiness, address any remaining review or check issues, refresh the branch if appropriate, and run the
|
Fresh finish pass complete at head
Actions taken: validation and review/check triage only; no code changes were needed in this pass. Still needed from a human: re-trigger/verify CI on this head and clear any stale blocking review/requested-review state if GitHub still shows one before merge. |
PR TriageCategory: CI passing. Moderate refactor touching wasm/native parity code — worth expedited review given file criticality. Automated triage — see the triage report issue for full context.
|
pkg/parserhad duplicated wasm/non-wasm implementations for remote path resolution and workflowspec detection, and those copies had drifted (notably inisRepositoryImport). This change consolidates shared logic into build-tag-free helpers and removes duplicate path/frontmatter helpers in import processing.Wasm/native path and workflowspec logic consolidation
pkg/parser/remote_path_predicates.gofor shared, platform-agnostic helpers:isUnderWorkflowsDirectoryisCustomAgentFileisRepositoryImportIsWorkflowSpecfindGitHubFoldercomputeIncludeResolveAndSecurityBasesremote_resolve_path.go,remote_fetch_wasm.go, andremote_workflow_spec.go.isWorkflowSpecalias and calledIsWorkflowSpecdirectly.Behavioral drift fix (
isRepositoryImport)githubnext/gh-aw.dev.Duplicate
path#sectionhelper removalsplitPathAndSectioninpkg/parser/path_section.go.splitImportPathAndSection,splitIncludePathAndSection, andstripImportSectioncallsites.Duplicate frontmatter extraction removal
extractFrontmatterForTopologicalSort.extractFrontmatterForImportin topological import resolution and consumedresult.Frontmatter.Targeted test update
TestIsRepositoryImportto cover dotted repository names as valid repository imports.run: https://github.com/github/gh-aw/actions/runs/32312411746> Generated by 👨🍳 PR Sous Chef · gpt54 · 27.8 AIC · ⌖ 8.1 AIC · ⊞ 9.3K · ◷