Skip to content

Add cleanup step for network proxy hook files in Claude workflows#1043

Merged
pelikhan merged 8 commits intomainfrom
copilot/fix-dbd16ffe-18be-4c5c-8dfc-568fea61b839
Sep 26, 2025
Merged

Add cleanup step for network proxy hook files in Claude workflows#1043
pelikhan merged 8 commits intomainfrom
copilot/fix-dbd16ffe-18be-4c5c-8dfc-568fea61b839

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 26, 2025

  • Explore repository structure and understand build process
  • Understand Claude engine network proxy functionality
  • Identify where network hook files are created (.claude/hooks/network_permissions.py)
  • Identify the workflow generation flow in compiler.go
  • Find the location where cleanup steps should be added (generateMainJobSteps after AI execution)
  • Implement the cleanup step generation for network proxy hook files
  • Add the cleanup step to ClaudeEngine with proper conditions
  • Test the implementation with comprehensive test cases
  • Verify cleanup step is only generated when proxy is enabled (Claude engine + network permissions)
  • Verify cleanup step always runs and ignores errors (uses if: always() and || true)
  • Confirm all existing tests still pass
  • Address code review feedback (removed unused engine parameter)
  • Move cleanup logic to Claude engine as requested
  • Remove redundant engine ID checks in Claude engine methods

Summary of Latest Changes

Optimized Claude engine code by removing redundant engine ID checks:

Changes Made

  • Removed redundant workflowData.EngineConfig.ID == "claude" checks in three locations within the Claude engine:
    • GetInstallationSteps() method (line 37)
    • GetExecutionSteps() method for network settings (line 115)
    • GetExecutionSteps() method for cleanup step (line 226 - the specific comment location)

Rationale

Since these methods are part of the ClaudeEngine struct, the engine ID will always be "claude", making these checks unnecessary. This optimization:

  • Simplifies the code by removing redundant conditions
  • Improves performance by eliminating unnecessary comparisons
  • Maintains identical functionality - all tests pass and workflows generate correctly

Technical Details

Changed conditions from:

if workflowData.EngineConfig != nil && workflowData.EngineConfig.ID == "claude" && ShouldEnforceNetworkPermissions(workflowData.NetworkPermissions)

To:

if workflowData.EngineConfig != nil && ShouldEnforceNetworkPermissions(workflowData.NetworkPermissions)

All functionality remains identical and all tests continue to pass.

Original prompt

Update the Claude agentic engine to add a cleanup step that removes the network proxying hook files.

  • only generate step if proxy is enabled
  • always run, ignore errors

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits September 26, 2025 13:58
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Comment thread pkg/workflow/compiler.go Outdated
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Update the Claude agentic engine to add a cleanup step that removes the network proxying hook files. - only generate step if proxy is enabled - always run, ignore errors Add cleanup step for network proxy hook files in Claude workflows Sep 26, 2025
Copilot AI requested a review from pelikhan September 26, 2025 14:14
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Comment thread pkg/workflow/claude_engine.go Outdated
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review September 26, 2025 15:07
@pelikhan pelikhan merged commit e40666c into main Sep 26, 2025
13 of 14 checks passed
@pelikhan pelikhan deleted the copilot/fix-dbd16ffe-18be-4c5c-8dfc-568fea61b839 branch September 26, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants