Skip to content

Refactor parser import/path helpers to eliminate wasm/native drift and duplicate logic - #53895

Open
pelikhan with Copilot wants to merge 14 commits into
mainfrom
copilot/refactor-wasm-build-tag-copies
Open

Refactor parser import/path helpers to eliminate wasm/native drift and duplicate logic#53895
pelikhan with Copilot wants to merge 14 commits into
mainfrom
copilot/refactor-wasm-build-tag-copies

Conversation

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

pkg/parser had duplicated wasm/non-wasm implementations for remote path resolution and workflowspec detection, and those copies had drifted (notably in isRepositoryImport). 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

    • Added pkg/parser/remote_path_predicates.go for shared, platform-agnostic helpers:
      • isUnderWorkflowsDirectory
      • isCustomAgentFile
      • isRepositoryImport
      • IsWorkflowSpec
      • findGitHubFolder
      • computeIncludeResolveAndSecurityBases
    • Removed duplicated implementations from remote_resolve_path.go, remote_fetch_wasm.go, and remote_workflow_spec.go.
    • Removed wasm-only isWorkflowSpec alias and called IsWorkflowSpec directly.
    • Preserved wasm-specific behavior where it differs intentionally (virtual FS existence checks, remote import rejection).
  • Behavioral drift fix (isRepositoryImport)

    • Standardized on native behavior for repo-name validation (rejects known file extensions, does not reject all dots).
    • This aligns wasm behavior with native for cases like githubnext/gh-aw.dev.
  • Duplicate path#section helper removal

    • Added shared splitPathAndSection in pkg/parser/path_section.go.
    • Replaced splitImportPathAndSection, splitIncludePathAndSection, and stripImportSection callsites.
  • Duplicate frontmatter extraction removal

    • Removed extractFrontmatterForTopologicalSort.
    • Reused extractFrontmatterForImport in topological import resolution and consumed result.Frontmatter.
  • Targeted test update

    • Extended TestIsRepositoryImport to cover dotted repository names as valid repository imports.
funcsplitPathAndSection(pathstring) (string, string) {
ifbefore, after, ok:=strings.Cut(path, "#"); ok {
returnbefore, after
}
returnpath, ""
}

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 33.9 AIC · ⌖ 8.18 AIC · ⊞ 9.3K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 33.2 AIC · ⌖ 9.48 AIC · ⊞ 6.9K ·
Comment /souschef to run again


run: https://github.com/github/gh-aw/actions/runs/32312411746> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 27.8 AIC · ⌖ 8.1 AIC · ⊞ 9.3K ·

Comment /souschef to run again

CopilotAIand others added 2 commits August 19, 2026 03:37
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Refactor wasm build-tag copies to resolve duplication issuesRefactor parser import/path helpers to eliminate wasm/native drift and duplicate logicAug 19, 2026
CopilotAI requested a review from pelikhanAugust 19, 2026 03:42
@pelikhan
pelikhan marked this pull request as ready for review August 19, 2026 04:05
CopilotAI balanced review requested due to automatic review settings August 19, 2026 04:05

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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
FileDescription
pkg/parser/remote_workflow_spec.goRemoves duplicated workflow-spec detection.
pkg/parser/remote_resolve_path.goUses shared path-resolution helpers.
pkg/parser/remote_path_predicates.goAdds platform-independent parser helpers.
pkg/parser/remote_fetch_wasm.goAligns Wasm with shared behavior.
pkg/parser/path_section.goAdds shared path-section splitting.
pkg/parser/include_processor.goUses the shared splitter.
pkg/parser/import_topological.goReuses shared splitting and frontmatter extraction.
pkg/parser/import_bfs.goReplaces duplicate import splitters.
pkg/parser/frontmatter_utils_test.goCovers 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

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please refresh this PR for maintainer review and then run the pr-finisher skill.

Current maintainer-visible next steps:

  • This PR has a reviewer summary but no maintainer-facing handoff yet.
  • Please confirm the current head still matches the intended refactor, run the usual final validation, and leave a short handoff comment summarizing what was checked for reviewers.
  • A branch refresh was attempted by sous-chef as well.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 33.9 AIC · ⌖ 8.18 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please refresh this PR for maintainer review and then run the pr-finisher skill.

Current maintainer-visible next steps:...

Refreshed PR #53895 at head aa630fc and ran pr-finisher.

Handoff for maintainers:

  • Confirmed the current head still matches the intended parser refactor (pkg/parser shared import/path helpers, wasm/native behavior alignment, and targeted test coverage).
  • Review threads: none unresolved; Copilot reviewer summary had 0 generated comments.
  • Local validation: make fmt, make test-unit, make test, make recompile, and make agent-report-progress passed; secret scan found no secrets in changed files. Full make lint was attempted and only failed on unrelated existing testifylint findings outside this PR (pkg/intent, pkg/workflow).
  • CI snapshot: prior visible checks are green/skipped; the current Copilot comment-handling run is still in progress and must be re-triggered/verified by a maintainer if required before merge.

Still needed: maintainer review/approval.

@github-actions

Copy link
Copy Markdown
Contributor

Triage: category=refactor · risk=medium · score=53/100 (impact 25/50, urgency 12/30, quality 16/20)
Recommended action:fast_track

Non-draft, CI green (29/32 checks passed), reduces wasm/native duplication across 9 files. AI reviewer left comments — please address before merge.

Generated by 🔧 PR Triage Agent · auto · 58.8 AIC · ⌖ 2.56 AIC · ⊞ 8.3K ·

@github-actions

github-actionsBot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actionsBot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Category: refactor · Risk: medium · Score: 38/100 (impact 20 + urgency 10 + quality 8)
Recommended action:batch_review
Batch:B1-blocked-needs-rework

Medium refactor, blocked/changes-requested despite prior approval; downgraded from fast_track pending rework.

Automated triage — see run report for full details.

Generated by 🔧 PR Triage Agent · auto · 90.2 AIC · ⌖ 2.82 AIC · ⊞ 8.3K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR still needs a maintainer-facing finish pass.

  • There are still blocking review requests on the parser import/path refactor; make sure each unresolved maintainer-visible concern has a direct current-head reply on-thread, then run the pr-finisher skill.
  • Please refresh the branch if GitHub offers one.
  • Leave a concise maintainer-facing handoff comment summarizing validation and any remaining merge risks.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 15.5 AIC · ⌖ 8.18 AIC · ⊞ 9.3K ·
Comment /souschef to run again

…-build-tag-copies
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot This PR still needs a maintainer-facing finish pass.

  • There are still blocking review requests on the parser import/path refactor; make sure each unresolved maintainer-visible concern has a...

Finish pass complete at head 47e2baa.

  • Branch refreshed with latest origin/main and is up to date (10/0 ahead/behind locally).
  • Review threads for the parser concerns are resolved in the PR API; the prior fixes for local two-segment import precedence, isRepositoryImport documentation, and regression/security tests remain unchanged after the refresh.
  • Validation: make fmt, make test-unit, make test, make recompile, and make agent-report-progress passed. make lint still fails only on unrelated existing testifylint findings in pkg/intent/governance_formal_test.go.
  • Secret scan clean for PR-changed files. Automated code review returned no comments. CodeQL returned no alerts, with Go analysis skipped because the database was too large.
  • CI on 47e2baa is not verified by agent push: latest required workflow runs are action_required with no jobs/logs, so a maintainer needs to re-trigger/verify CI before merge.

Remaining merge risks: blocking review state/requested review still needs maintainer/reviewer re-review or approval, and CI needs a human re-trigger.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR still needs a maintainer-facing finish pass.

  • The parser refactor still shows maintainer-visible blocking review state even though the latest handoff says the review threads are resolved.
  • Please refresh the branch if GitHub offers one, confirm the current head still addresses the earlier parser import/path concerns, and run the pr-finisher skill again with a concise maintainer-facing handoff.
  • Call out exactly what still requires human action (for example: re-review, CI re-trigger, or stale blocking review dismissal).

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 27.8 AIC · ⌖ 8.1 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAI commented Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot This PR still needs a maintainer-facing finish pass.

  • The parser refactor still shows maintainer-visible blocking review state even though the latest handoff says the review threads are res...

Finish pass complete at head ea83106 using pr-finisher.

  • Reviews: the three parser review threads are resolved in the PR API, and the current head still contains the fixes for local two-segment import precedence, documented isRepositoryImport extension assumptions, and regression/security parser tests.
  • Branch: fetched origin/main; branch is 18/0 ahead/behind, so no refresh was needed.
  • Local validation: make fmt, make test-unit, make test, make recompile, and make agent-report-progress passed. make lint still fails only on unrelated existing testifylint findings in pkg/intent/governance_formal_test.go.
  • Security/review: secret scan clean for PR-changed files. Automated code review/CodeQL had no uncommitted local changes to analyze in this finish-only pass.
  • CI: required PR workflow runs on this agent-pushed head are action_required with no failed jobs/logs; the current Copilot handling job is still in progress.

Human action still required: stale blocking github-actions review state/requested review needs re-review or dismissal, and CI must be re-triggered/verified by a maintainer before merge.

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

  • Category: refactor
  • Risk: medium
  • Priority score: 52 / 100 (impact 28 + urgency 12 + quality 12)
  • Recommended action:batch_review

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.

Generated by 🔧 PR Triage Agent · auto · 57.1 AIC · ⌖ 2.88 AIC · ⊞ 8.3K ·

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Category: refactor | Risk: medium | Priority: medium (score 45/100)
Score breakdown: impact 18 + urgency 12 + quality 15
Recommended action:batch_review
Batch: B1-small-fixes-refactors
Consolidates duplicated parser wasm/native path-resolution logic. CI shows 1 failing check that needs investigation; mixed review history (changes requested -> approved -> dismissed after push).

Generated by 🔧 PR Triage Agent · auto · 62.9 AIC · ⌖ 7.05 AIC · ⊞ 8.3K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please do a maintainer-facing finish pass on this PR.

  • Confirm the current head is still merge-ready for the latest branch state.
  • Refresh the branch if GitHub offers one.
  • Run the pr-finisher skill and leave a concise handoff comment with validation status and any remaining merge risks.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.85 AIC · ⌖ 9.4 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAIand others added 2 commits August 20, 2026 07:11
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>

CopilotAI commented Aug 20, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please do a maintainer-facing finish pass on this PR.

  • Confirm the current head is still merge-ready for the latest branch state....

Finish pass complete at head ff857597fd using pr-finisher.

  • Branch refreshed with latest origin/main; local state is clean and up to date with base (14/0 ahead/behind). The PR diff remains scoped to the 10 intended parser/ADR files.
  • Parser concerns remain addressed: local two-segment imports resolve locally before repository-import fallback, isRepositoryImport documents the narrow extension heuristic, and regression/security tests remain present.
  • Reviews: all parser review threads are resolved in the PR API; older blocking review history still requires maintainer/reviewer re-review or dismissal if GitHub keeps it visible.
  • Local validation passed: make fmt, make lint, make test-unit, make test, make recompile, and make agent-report-progress.
  • CI/logs: current-head required workflow runs seen before push were action_required with zero jobs/logs. Older CGO failures were inspected; the prior lint/test drift is not reproduced locally after refresh.
  • Security/review: secret scan clean for PR files; automated code review returned no comments. CodeQL timed out and instructed not to re-run, with no findings returned.

Human action still required: re-trigger/verify CI on this agent-pushed head and clear any stale blocking review/requested-review state before merge.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@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 pr-finisher skill before reporting back.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.04 AIC · ⌖ 8.12 AIC · ⊞ 9.3K ·
Comment /souschef to run again

CopilotAI commented Aug 20, 2026

Copy link
Copy Markdown
ContributorAuthor

@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 r...

Fresh finish pass complete at head ff857597fd using pr-finisher.

  • ✅ Reviews — the three parser review threads are resolved in the PR API; current head still contains the local two-segment import precedence fix, documented isRepositoryImport extension assumptions, and regression/security parser tests.
  • ✅ Checks (local) — passed make fmt, make lint, make test-unit, make test, make recompile, and make agent-report-progress. Secret scan found no secrets in the 10 PR-changed files.
  • ❓ Checks (CI) — stale/unverified after agent pushes. Current-head required PR runs are action_required with zero jobs/logs, and the current Copilot handling run is in progress. The latest prior real CGO failure was checked at log level and was generated skill drift at an older head, already addressed by later refreshes and not reproduced locally.
  • ✅ Mergeable branch state — fetched origin/main; local branch is 0/14 behind/ahead and the working tree is clean.

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.

Sign up for freeto 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.

[refactor] pkg/parser: wasm build-tag copies have drifted, plus 3 duplicate-function clusters

4 participants

@gh-aw-bot@pelikhan