Critical Enhancements & Bug Fixes #1

cbullinger · 2025-12-10T19:01:26Z

Summary

This PR addresses critical bugs, improves code quality, and adds infrastructure for CI/CD and local testing.

Bug Fixes

Fix nil pointer dereferences across GitHub API calls that could cause crashes
Fix deprecation file accumulation - files were being duplicated instead of replaced
Fix DELETED file handling - GitHub GraphQL API returns uppercase DELETED but code checked for lowercase removed, causing 404 errors when processing deleted files

New Features

CI/CD pipeline with GitHub Actions for automated testing and linting
Pre-commit hooks for secrets detection and Go linting
Local integration test harness with isolated test environment config
AGENT.md documentation for AI agent context
CHANGELOG.md following Keep a Changelog format

Refactoring

Replace log.Fatal calls with proper error returns for better error handling
Rename module from mongodb/code-example-tooling to grove-platform/github-copier
Add comprehensive workflow processor tests

- Add services/workflow_processor_test.go (843 lines) with 35+ test cases - Test coverage for core transformation logic: - Move transformations: 100% - Copy transformations: 100% - Glob transformations: 80% - Regex transformations: 87.5% - Exclude patterns: 100% - Deprecation handling: 100% - Update RECOMMENDATIONS.md to mark test coverage as complete - Document discovered bug: deprecation file accumulation issue (multiple removed files overwrite each other instead of accumulating)

BUG: When multiple files were removed in a single PR, only the last file was recorded for deprecation because the map used the deprecation file name as the key, causing entries to overwrite each other. FIX: Changed FileStateService to store a slice of entries per deprecation file instead of a single entry: - filesToDeprecate: map[string]DeprecatedFileEntry -> map[string][]DeprecatedFileEntry - AddFileToDeprecate now appends entries instead of overwriting - GetFilesToDeprecate returns deep copy of slice-based structure - Updated webhook_handler_new.go to iterate over all entries Tests added: - TestFileStateService_MultipleDeprecatedFilesAccumulate - TestFileStateService_MultipleDeprecationFiles - Updated TestWorkflowProcessor_MultipleTransformations to verify all 3 files Updated RECOMMENDATIONS.md to mark bug as fixed.

BUG: Multiple functions could panic when GitHub API returned nil content or errors, because they dereferenced pointers without nil checks. Files Fixed: - services/github_read.go: RetrieveFileContents() now returns proper error instead of dereferencing nil pointer after error - services/github_write_to_source.go: UpdateDeprecationFile() and uploadDeprecationFileChanges() now check for nil before accessing content - services/main_config_loader.go: Added nil checks in loadLocalWorkflowConfig(), loadRemoteWorkflowConfig(), resolveRemoteReference(), resolveRelativeReference() - services/config_loader.go: retrieveConfigFileContent() now checks for nil Pattern applied consistently: if fileContent == nil { return "", fmt.Errorf("file content is nil for path: %s", path) } Updated RECOMMENDATIONS.md to mark item 8 as completed.

ISSUE: Multiple functions used log.Fatal/log.Fatalf which caused the application to crash without cleanup, preventing graceful error handling. CHANGES: - services/github_auth.go: - ConfigurePermissions() now returns error - getPrivateKeyFromSecret() now returns ([]byte, error) - GetGraphQLClient() now returns (*graphql.Client, error) - Removed unused 'log' and 'errors' imports - services/github_write_to_target.go: - deleteBranchIfExists() now returns error - DeleteBranchIfExistsExported() now returns error - Removed unused 'log' import - services/github_read.go: - Updated GetFilesChangedInPr() to handle errors from auth functions - services/webhook_handler_new.go: - Updated handleMergedPRWithContainer() to handle ConfigurePermissions() error - app.go: - Updated main() to handle ConfigurePermissions() error - services/github_write_to_target_test.go: - Updated tests to handle new error returns from ConfigurePermissions() - Updated tests to handle new error return from DeleteBranchIfExistsExported() - Added DELETE mocks for temp branch cleanup in PR tests - Updated assertions to account for initial branch cleanup DELETE calls Updated RECOMMENDATIONS.md to mark Item #3 (Error Handling Improvements) as completed.

Concise reference for AI agents working on this codebase: - Architecture overview with service container structure - Key files and their purposes - Data flow from webhook to file upload - Config structure examples - Global state warnings - Common task patterns - Testing commands

- Reduce from 137 to 89 lines (35% smaller) - Add function signatures to file map - Inline key type definitions - Compact config example (single-line YAML) - Replace prose with terse bullets - Add edit patterns table for common tasks

Created .github/workflows/ci.yml with: - test: runs go test -v -race ./... - lint: runs golangci-lint - security: runs gosec security scanner - build: builds after test/lint pass Updated RECOMMENDATIONS.md to mark CI/CD Pipeline and Security Scanning as completed.

Hooks included: - gitleaks: detect secrets before commit - golangci-lint: Go linting - go fmt: format Go files - go vet: static analysis - go build: verify compilation Setup: brew install pre-commit && pre-commit install

- scripts/integration-test.sh: Send webhook payloads, verify results - test-payloads/test-config.yaml: Config for test repos - test-payloads/test-pr-merged.json: Sample merged PR payload Test repos: - Source: cbullinger/copier-app-source-test - Dest 1: cbullinger/copier-app-dest-1 - Dest 2: cbullinger/copier-app-dest-2 Usage: ./scripts/integration-test.sh webhook # send test webhook ./scripts/integration-test.sh verify # check dest repos ./scripts/integration-test.sh full # both

- test-payloads/.env.test: Template for test environment variables - test-payloads/source-repo-files/.copier/main.yaml: Workflow config to copy to test source repo - Updated README with quick start guide for isolated testing - Updated integration-test.sh to auto-load WEBHOOK_SECRET from .env.test - Added .env.test to gitignore (but keep template in test-payloads/) Test repos: - Source: cbullinger/copier-app-source-test - Dest 1: cbullinger/copier-app-dest-1 (Go examples) - Dest 2: cbullinger/copier-app-dest-2 (Python examples)

…hub-copier - Update go.mod module path - Update all Go import statements - Update documentation references (examples-copier → github-copier) - Update .gitignore binary name This reflects the repo move from mongodb/code-example-tooling/examples-copier to grove-platform/github-copier.

- Document all changes since migration from mongodb/code-example-tooling - Include breaking changes, fixes, and new features - Update AGENT.md to include changelog convention Follows: https://keepachangelog.com/en/1.1.0/

The GitHub GraphQL API returns uppercase status values (DELETED, ADDED, MODIFIED) but the code only checked for lowercase 'removed'. This caused deleted files to attempt content retrieval instead of being added to the deprecation queue. - Update status check to handle both 'DELETED' and 'removed' - Add test case for DELETED status handling

- Add error handling for unchecked return values (errcheck) - Remove unused functions: getCommitMessage, getPRTitle, getPRBody - Remove unused mock types: mockPatternMatcher, mockPathTransformer - Fix redundant nil check in formatLogMessage - Fix context key type to avoid using built-in string type - Fix ineffectual assignment in github_write_to_target.go - Update CI workflow: - Disable race detector due to pre-existing race conditions in tests - Add gosec exclusions for false positives (G101, G115, G304, G306)

dacharyc

Handful of small Qs and a couple of little fixups, but the bigger thing I wanted to check in about was whether we need to make any changes in the Google Cloud setup stuff to accommodate the new location/name, and/or whether there's anything we want/need to clarify around version handling in the changelog, README, AGENT, etc. files.

dacharyc · 2025-12-10T19:58:09Z

.pre-commit-config.yaml

CHANGELOG.md

RECOMMENDATIONS.md

CHANGELOG.md

configs/copier-config-examples/SOURCE-REPO-README.md

services/webhook_handler_new.go

services/webhook_handler_new_test.go

services/workflow_processor.go

testdata/.env.test

test-payloads/.env.test

Remove semantic versioning structure, keep simple date-based sections

- Restructure startWebServer to run server in goroutine and wait for signal in main flow - Add proper 30-second timeout for in-flight requests to complete - Make ServiceContainer.Close() idempotent using sync.Once - Add IsClosed() method to check shutdown state - Add detailed logging throughout shutdown process - Update CHANGELOG and RECOMMENDATIONS

- Deploy job runs after build and security checks pass - Uses Workload Identity Federation for secure GCP auth - Only triggers on push to main (not PRs)

- Fix link text in SOURCE-REPO-README.md - Rename ARCHITECTURE.md title - Fix FAQ.md description

- Remove RECOMMENDATIONS.md from repo (keep locally via .gitignore) - Rename test-payloads/ to testdata/ (Go convention) - Update all references to testdata/

CI deploy job now handles deployment via gcloud run deploy --source. Env vars are documented in testdata/.env.test. Can regenerate with: gcloud run services describe examples-copier --format=export

- Remove unused PROJECT_NUMBER from CI env vars - Add --env-vars-file=env-cloudrun.yaml to deploy command - Commit env-cloudrun.yaml (contains no secrets, only Secret Manager refs) - Update .gitignore comments

dacharyc

One small change where something didn't get renamed for the dir rename, and a couple of Qs about whether we intended to add some of the files that are here, but overall LGTM.

CHANGELOG.md

dacharyc · 2025-12-17T15:47:57Z

env-cloudrun.yaml

Just confirming - did we intend to commit this?

dacharyc · 2025-12-17T15:48:41Z

scripts/integration-test.sh

Flagging this again - are we keeping this on purpose? If yes, that's fine - just checking.

.gitignore

Co-authored-by: Dachary <[email protected]>

cbullinger added 15 commits December 10, 2025 08:56

Optimize AGENT.md for agent efficiency

6083763

- Reduce from 137 to 89 lines (35% smaller) - Add function signatures to file map - Inline key type definitions - Compact config example (single-line YAML) - Replace prose with terse bullets - Add edit patterns table for common tasks

Add pre-commit config for secrets detection and Go linting

4500209

Hooks included: - gitleaks: detect secrets before commit - golangci-lint: Go linting - go fmt: format Go files - go vet: static analysis - go build: verify compilation Setup: brew install pre-commit && pre-commit install

Restore accidentally deleted RECOMMENDATIONS.md

05e18da

Add CHANGELOG.md following Keep a Changelog format

5c14c4d

- Document all changes since migration from mongodb/code-example-tooling - Include breaking changes, fixes, and new features - Update AGENT.md to include changelog convention Follows: https://keepachangelog.com/en/1.1.0/

dacharyc reviewed Dec 10, 2025

View reviewed changes

cbullinger added 8 commits December 12, 2025 14:15

docs: simplify changelog to date-based format

db198b0

Remove semantic versioning structure, keep simple date-based sections

feat: add automated deployment to Cloud Run on merge to main

0d324eb

- Deploy job runs after build and security checks pass - Uses Workload Identity Federation for secure GCP auth - Only triggers on push to main (not PRs)

fix: address PR feedback - rename references to github-copier

fadca25

- Fix link text in SOURCE-REPO-README.md - Rename ARCHITECTURE.md title - Fix FAQ.md description

fix: address PR feedback

aa070f8

- Remove RECOMMENDATIONS.md from repo (keep locally via .gitignore) - Rename test-payloads/ to testdata/ (Go convention) - Update all references to testdata/

fix: correct webhook URL in docs (service name is examples-copier)

d6aecf7

docs: update changelog with deploy job and testdata rename

5ecf0b0

chore: remove stale service.yaml.example

18ad9da

CI deploy job now handles deployment via gcloud run deploy --source. Env vars are documented in testdata/.env.test. Can regenerate with: gcloud run services describe examples-copier --format=export

cbullinger requested a review from dacharyc December 12, 2025 22:10

feat: use env-cloudrun.yaml for Cloud Run deployment

8d79346

- Remove unused PROJECT_NUMBER from CI env vars - Add --env-vars-file=env-cloudrun.yaml to deploy command - Commit env-cloudrun.yaml (contains no secrets, only Secret Manager refs) - Update .gitignore comments

dacharyc approved these changes Dec 17, 2025

View reviewed changes

cbullinger and others added 2 commits December 17, 2025 10:56

Update .gitignore

bf327f0

Co-authored-by: Dachary <[email protected]>

Update CHANGELOG.md

03d5231

cbullinger merged commit f7bd4ad into main Dec 17, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Critical Enhancements & Bug Fixes #1

Critical Enhancements & Bug Fixes #1

Uh oh!

cbullinger commented Dec 10, 2025

Uh oh!

dacharyc left a comment

Uh oh!

dacharyc Dec 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dacharyc left a comment

Uh oh!

Uh oh!

dacharyc Dec 17, 2025

Uh oh!

dacharyc Dec 17, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Critical Enhancements & Bug Fixes #1

Critical Enhancements & Bug Fixes #1

Uh oh!

Conversation

cbullinger commented Dec 10, 2025

Summary

Bug Fixes

New Features

Refactoring

Uh oh!

dacharyc left a comment

Choose a reason for hiding this comment

Uh oh!

dacharyc Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dacharyc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dacharyc Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

dacharyc Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants