Skip to content

Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, MCP compliance, file-based architecture, environment-driven paths, and robust configuration#833

Closed
Copilot wants to merge 12 commits intomainfrom
copilot/fix-be9f49e4-adfc-4070-9cd5-1082fb722dd0

Conversation

Copy link
Contributor

Copilot AI commented Sep 15, 2025

Implementing requested changes to push-to-orphaned-branch safe output type:

  • Analyze current implementation and understand requirements
  • Add SHA computation for file buffers in MCP server
  • Use SHA as filename instead of timestamp + original filename
  • Add "branch" field to configuration schema (workflow frontmatter)
  • Update Go types to support branch configuration
  • Change default branch name to "assets/${{ workflow id }}"
  • Update JavaScript logic to use configurable branch name
  • Include SHA in response URL
  • Update tests for new functionality
  • Run comprehensive testing and validation
  • Use branch name instead of commit SHA in raw URLs
  • Add expected URL template to MCP server response
  • Add SHA validation in file upload process
  • Add commit SHA to job outputs
  • Add file size validation (10MB limit)
  • Add file extension validation for security
  • Enhance URL generation with repository context
  • Add URL template to response structure
  • Update file extension validation to allow up to 5 alphanumeric characters
  • Remove unsupported url field from MCP response
  • Update dev workflow with playwright tool and documentation workflow
  • Implement file-based safe outputs architecture
  • Move files to nested subdirectory and use environment variables
  • Use environment variables instead of step outputs for consistent references
  • Update filename to safe_outputs.jsonl and add environment variable validation

Environment Variable Consistency Updates

Improved Environment Variable Usage

  • Consistent References: All engines now use ${{ env.GITHUB_AW_SAFE_OUTPUTS_DIR }} and ${{ env.GITHUB_AW_SAFE_OUTPUTS_FILES_DIR }} instead of step outputs
  • MCP Server Configuration: Environment variables passed consistently to all MCP server configurations
  • Artifact Upload: Upload path now uses environment variable reference instead of hardcoded path

Enhanced Configuration and Validation

Robust Environment Variable Validation

  • MCP Server Startup: Added comprehensive validation for all required environment variables
  • Fail-Fast Behavior: Server exits immediately with clear error messages if configuration is incomplete
  • Required Variables: Validates GITHUB_AW_SAFE_OUTPUTS_CONFIG, GITHUB_AW_SAFE_OUTPUTS, GITHUB_AW_SAFE_OUTPUTS_DIR, and GITHUB_AW_SAFE_OUTPUTS_FILES_DIR

Improved File Naming Convention

  • Consistent Naming: Changed from outputs.jsonl to safe_outputs.jsonl for better identification
  • Clear Purpose: Filename now clearly indicates its role in safe outputs functionality
  • Better Organization: Aligns with overall safe outputs naming convention throughout the codebase

Changes Made

Engine Environment Configuration

  • Claude Engine: Updated to use ${{ env.GITHUB_AW_SAFE_OUTPUTS_DIR }} and ${{ env.GITHUB_AW_SAFE_OUTPUTS_FILES_DIR }}
  • Custom Engine: Environment variables now reference env vars instead of step outputs
  • Codex Engine: Consistent environment variable usage in both execution and MCP configuration

MCP Server Environment

  • All Engines: MCP server configurations now use environment variable references
  • Consistent Passing: Environment variables passed uniformly across all engine types
  • Clean Configuration: No mixing of step outputs and environment variables
  • Startup Validation: All required environment variables validated at server launch

Artifact Upload Path

  • Compiler: Upload artifact step now uses ${{ env.GITHUB_AW_SAFE_OUTPUTS_DIR }} instead of hardcoded path
  • Dynamic Path: Path resolution happens at runtime using environment variables
  • Flexibility: Allows for different directory structures in testing and production

Benefits

Enhanced Reliability

  • Environment Validation: Prevents runtime errors due to missing configuration
  • Clear Error Messages: Specific error messages indicate which environment variables are missing
  • Consistent State: Server only operates with complete configuration
  • Early Detection: Configuration problems identified at startup

Consistency

  • Uniform References: All path references use the same pattern (environment variables)
  • No Mixed Approaches: Eliminates mixture of step outputs and environment variables
  • Predictable Behavior: Same reference pattern across all engines and configurations
  • Consistent Naming: Files clearly indicate their purpose

Maintainability

  • Single Source of Truth: Environment variables set once in setup step, used everywhere
  • Easy Updates: Changing paths only requires updating the setup step
  • Clear Dependencies: Dependencies on setup step are explicit through environment variables
  • Centralized Validation: All environment checks in one location

Testing

  • Override Capability: Environment variables can be overridden for testing
  • Isolation: Tests can use different paths without affecting setup step outputs
  • Flexibility: Easier to test different directory configurations

Technical Implementation

Environment Variable Flow

  1. Setup Step: core.exportVariable() sets environment variables for subsequent steps
  2. Engine Configuration: All engines reference ${{ env.VARIABLE_NAME }}
  3. MCP Servers: Environment variables passed to MCP server process environment with startup validation
  4. Artifact Upload: Upload step uses environment variable for path resolution

Configuration Validation Pattern

// Validate required environment variables at startup
const safeOutputsDir = process.env.GITHUB_AW_SAFE_OUTPUTS_DIR;
if (!safeOutputsDir)
  throw new Error("GITHUB_AW_SAFE_OUTPUTS_DIR not set, no safe outputs directory");

Consistency Pattern

# Before (mixed approach)
GITHUB_AW_SAFE_OUTPUTS_DIR: ${{ steps.setup_agent_output.outputs.output_dir }}

# After (consistent approach)  
GITHUB_AW_SAFE_OUTPUTS_DIR: ${{ env.GITHUB_AW_SAFE_OUTPUTS_DIR }}

The implementation ensures all path references use environment variables consistently while providing robust configuration validation, better naming conventions, and improved error handling for a more reliable and maintainable system.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits September 15, 2025 22:17
…erver, and job builder

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…validation

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add a new type of safe-outputs that allows to upload a file to an orphaned branch. - push-to-orphaned-branch is the type - receives a file name. the mcp server reads the file and includes it in the JSONL output as a base64 encoded string - the MCP ser... Add push-to-orphaned-branch safe output type for file uploads Sep 15, 2025
Copilot AI requested a review from pelikhan September 15, 2025 22:32
Copilot AI and others added 2 commits September 15, 2025 23:11
…ation

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…phaned-branch

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 15, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.contoso.com
    • Triggering command: /tmp/go-build1018698154/b279/cli.test -test.testlogfile=/tmp/go-build1018698154/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)
    • Triggering command: /tmp/go-build3111078443/b279/cli.test -test.testlogfile=/tmp/go-build3111078443/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Add push-to-orphaned-branch safe output type for file uploads Add push-to-orphaned-branch safe output type with SHA-based filenames and configurable branches Sep 15, 2025
Copilot AI requested a review from pelikhan September 15, 2025 23:24

// Generate GitHub raw URLs
for (const filename of uploadedFiles) {
const rawUrl = `https://raw.githubusercontent.com/${owner}/${repo.repo}/${commitSha}/${filename}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the branch instead of commit sha? @copilot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to use branch name instead of commit SHA in raw URLs in commit adbd59b. URLs now use the format https://raw.githubusercontent.com/{owner}/{repo}/{branch}/{filename} which provides more stable URLs that don't change with each commit.

content: [
{
type: "text",
text: `File uploaded successfully. SHA: ${fileSha}, Original filename: ${path.basename(filename)}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also return the rawUrl that will result to pushing this file to the branch. @copilot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added expected URL template to the MCP server response in commit adbd59b. The response now includes the URL format that will result from pushing the file, showing the branch configuration and SHA-based filename: https://raw.githubusercontent.com/{owner}/{repo}/{branch}/{sha-filename}

…L to MCP response

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

fs.writeFileSync(safeFilename, fileBuffer);
const originalName = original_filename || filename;
const fileSha = sha || "unknown";
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot
Fail is sha is not equal to compute sha of file buffer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added SHA validation in commit b0bf19e. The upload process now computes the SHA256 of the file buffer and fails the workflow if it doesn't match the expected SHA from the agent output, ensuring file integrity.

…d-branch

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Add push-to-orphaned-branch safe output type with SHA-based filenames and configurable branches Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, and security validation Sep 16, 2025
Copilot AI requested a review from pelikhan September 16, 2025 05:59
Copy link
Contributor

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

@copilot

  • enable playwright tool on dev agnetic workflow
  • update prompt to build docs and take a screenshot and post it as an issue
  • update description of new tool to tell LLM to use expected URL
  • enable safe outputs to push to orphaned branch and create issue

const originalExtension = path.extname(filename);

// Validate file extension is reasonable
const allowedExtensions = [
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot

  • allowed extension has up to 5 alpha-numerical characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated file extension validation in commit c2b4746. Now allows any extension with up to 5 alphanumeric characters instead of a hardcoded allowlist, providing more flexibility while maintaining security.

text: `File uploaded successfully. SHA: ${fileSha}, Original filename: ${path.basename(filename)}, Expected URL: ${templateUrl}`,
},
],
url: templateUrl,
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot remove url field, not supported in MCP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the unsupported url field from MCP response in commit c2b4746. The response now only includes the text content with expected URL information for MCP protocol compliance.

…m MCP response

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 16, 2025

@copilot

  • enable playwright tool on dev agnetic workflow...

Updated dev workflow in commit c2b4746 to enable playwright tool, add safe outputs for push-to-orphaned-branch and create-issue, and updated the prompt to build docs, take screenshots, and create issues with guidance on using the expected URL from the upload response.

Copilot AI changed the title Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, and security validation Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, and MCP compliance Sep 16, 2025
Copilot AI requested a review from pelikhan September 16, 2025 11:23
@pelikhan
Copy link
Contributor

@copilot instead of bundling the file contents in the safe_outputs JSONL, copy the file to a known folder in a temp folder (/temp/aw/safe-outputs/) that will get uploaded as part of the safe outputs artifact. Use the sha-based filename.

  • update the generated safe outputs file location to use this folder as well /tmp/gh-aw/safe-outputs/outputs.jsonl
  • update the update_artifacts step to upload all files in that folder
  • when processing the safe output, find the file in the unpacked artifact location, check the sha matches

…ontent in JSONL

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 16, 2025

@copilot instead of bundling the file contents in the safe_outputs JSONL, copy the file to a known folder in a temp folder (/temp/aw/safe-outputs/) that will get uploaded as part of the safe outputs a...

Implemented file-based safe outputs architecture in commit 0577df8. Files are now copied to /tmp/gh-aw/safe-outputs/ with SHA-based filenames instead of being bundled in JSONL. The safe outputs file is now at /tmp/gh-aw/safe-outputs/outputs.jsonl, artifact upload includes the entire directory, and processing jobs read files from unpacked artifacts with SHA validation.

Copilot AI changed the title Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, and MCP compliance Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, MCP compliance, and file-based architecture Sep 16, 2025
@pelikhan
Copy link
Contributor

@copilot

  • the folder of files for the safe output files should be in a nested su folder /tmp/gh-aw/safe-outputs/files
  • use environment variable in JavaScript instead of hardcoding paths to allow for testing
  • update compiler to generate the yaml code that passes the new environment variable

…aths

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 16, 2025

@copilot

  • the folder of files for the safe output files should be in a nested su folder /tmp/gh-aw/safe-outputs/files...

Implemented all requested changes in commit 7d2696f: Files are now stored in nested subdirectory /tmp/gh-aw/safe-outputs/files/, all JavaScript files use environment variables instead of hardcoded paths (GITHUB_AW_SAFE_OUTPUTS_FILES_DIR, GITHUB_AW_SAFE_OUTPUTS_DIR), and compiler now generates YAML that passes these environment variables to all engines (Claude, Custom, Codex) and MCP server configurations.

Copilot AI changed the title Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, MCP compliance, and file-based architecture Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, MCP compliance, file-based architecture, and environment-driven paths Sep 16, 2025
if workflowData.SafeOutputs != nil {
stepLines = append(stepLines, " GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }}")
stepLines = append(stepLines, " GITHUB_AW_SAFE_OUTPUTS_DIR: ${{ steps.setup_agent_output.outputs.output_dir }}")
stepLines = append(stepLines, " GITHUB_AW_SAFE_OUTPUTS_FILES_DIR: ${{ steps.setup_agent_output.outputs.files_dir }}")
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot use env for value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use environment variable reference in commit 6bd077d. Now uses ${{ env.GITHUB_AW_SAFE_OUTPUTS_FILES_DIR }} instead of step output reference for consistent environment variable usage across all engines.

yaml.WriteString(" with:\n")
fmt.Fprintf(yaml, " name: %s\n", OutputArtifactName)
yaml.WriteString(" path: ${{ env.GITHUB_AW_SAFE_OUTPUTS }}\n")
yaml.WriteString(" path: /tmp/gh-aw/safe-outputs/\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the env variable that points to the directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated artifact upload path to use environment variable in commit 6bd077d. Now uses ${{ env.GITHUB_AW_SAFE_OUTPUTS_DIR }} instead of hardcoded path for dynamic path resolution.

… references

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

@copilot add tests for new server tool

…lidation

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, MCP compliance, file-based architecture, and environment-driven paths Add push-to-orphaned-branch safe output type with SHA-based filenames, configurable branches, flexible validation, MCP compliance, file-based architecture, environment-driven paths, and robust configuration Sep 16, 2025
Copilot AI requested a review from pelikhan September 16, 2025 12:51
@pelikhan pelikhan closed this Sep 16, 2025
@pelikhan pelikhan deleted the copilot/fix-be9f49e4-adfc-4070-9cd5-1082fb722dd0 branch September 16, 2025 14:10
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