ci: remove broken python-package stub and redundant go-build workflow#12
Closed
israel2606 wants to merge 31 commits into
Closed
ci: remove broken python-package stub and redundant go-build workflow#12israel2606 wants to merge 31 commits into
israel2606 wants to merge 31 commits into
Conversation
This workflow installs Python dependencies, runs tests, and lints code with multiple Python versions.
Document the architecture, build/test workflow, toolset registration pattern, parameter helpers, error handling conventions, and contribution flow so AI assistants can act productively in this repo without re-discovering conventions. https://claude.ai/code/session_018psjWyFFcz83JXdxBQMUdd
Diseño modular del ERP basado en los conectores disponibles: - Stack por capas (datos, aplicacion, automatizacion, BI, colaboracion) - Modulos del ERP y hoja de ruta por fases - Plan de cuentas premium con prioridades de pago
Adds tests for the highest-risk untested code identified during a coverage analysis: - pkg/lockdown: IsSafeContent (the content-safety decision gate) and isTrustedBot were at 0%. New table-driven tests exercise every branch of the safety logic (trusted bot, private repo, viewer-authored, push-access tiers, and the deny-by-default path), plus cache reuse, query-error propagation, option setters, and cacheKey normalization. Package coverage rises from ~52% to ~83%. - pkg/utils: the tool-result constructors (used by every tool's output) were an entirely untested package at 0%; now fully covered.
The consolidated actions_* tools (ActionsGet, ActionsList, ActionsRunTrigger) dispatch to a set of unexported helpers that were at 0% coverage — the existing Test_* cases only validate the tool schemas, not these implementations. Adds direct white-box tests (success + API-error paths) for: getWorkflowJob, listWorkflowJobs, listWorkflowArtifacts, downloadWorkflowArtifact, getWorkflowRunLogsURL, getWorkflowRunUsage, rerunWorkflowRun, rerunFailedJobs, and deleteWorkflowRunLogs. Each helper now sits at ~78-90%, raising actions.go from ~60% to ~81%.
PR #3 raised pkg/lockdown coverage to 83.1% via safety_test.go. This adds the branches it left uncovered, in a separate file to avoid duplicate test symbols: - getRepoAccessInfo: the repo-cached-but-new-user merge path - queryRepoAccessInfo: GraphQL server-error propagation - log: the emit and below-threshold suppression branches Package coverage 83.1% -> 98.7%. No production code changes. https://claude.ai/code/session_01W9xh1kJCqtHqBgBAMQikon
Adds a self-contained .claude/ setup that codifies the workflows in CLAUDE.md as reusable subagents, slash commands, an auto-loading skill, and conservative permission rules. Subagents (.claude/agents/): - tool-adder: adds an MCP tool following the canonical pattern - snapshot-keeper: refreshes pkg/github/__toolsnaps__ safely - lint-fixer: diagnoses and fixes golangci-lint failures - pr-babysitter: triages CI failures and review comments on a PR Slash commands (.claude/commands/): - /add-tool, /check-pr, /fix-lint, /refresh-snapshots Skill (.claude/skills/mcp-tool-pattern/): - Auto-activates when editing pkg/github/, surfaces the tool pattern, parameter helpers, and error-path conventions. Settings (.claude/settings.json): - allow: read-only ops, builds, tests, lints, doc gen, read-only MCP GitHub queries - ask: edits, commits, push, MCP write operations - deny: rm -rf, sudo, all force-push variants (including --force-with-lease), reset --hard, branch -D, git config, curl|sh, auto-merge No source code or workflows are touched. https://claude.ai/code/session_018psjWyFFcz83JXdxBQMUdd
The tool-adder agent referenced /home/user/github-mcp-server/CLAUDE.md, which only resolves on the original author's checkout. Claude Code already discovers project CLAUDE.md at the repo root, so refer to it by location, not absolute path. Addresses Codex review feedback on PR #7. https://claude.ai/code/session_018psjWyFFcz83JXdxBQMUdd
chore: add coordinated Claude Code tooling for this project
This workflow builds and tests the Go application on push and pull request events for specified Go versions.
The existing completion tests only exercise the missing-dependency error branches, leaving the actual completion logic uncovered (completeOwner at 0%, completePath at 6.6%, others ~30%). Adds API-mocked happy-path tests for every resolver: completeOwner (viewer+orgs listing and the user-search fallback), completeRepo, completeBranch, completeSHA, completeTag, completePRNumber, and completePath (root listing, directory descent, and last-segment filtering), plus a couple of API-failure cases. All resolvers now sit at ~86-93%, raising the file from ~35% to ~90%.
Both files were entirely untested (0%). - pkg/translations: tests NullTranslationHelper, TranslationHelper (default value, env-var override with caching), and DumpTranslationKeyMap (round-trips the written JSON in an isolated temp dir). Package coverage rises from 0% to ~84%. - pkg/github/toolset_instructions: tests every generator, including both branches of generatePullRequestsToolsetInstructions (with and without the repos toolset present in the inventory). File reaches 100%.
…ructors Adds tests for previously untested pkg/inventory code: - errors.go: ToolsetDoesNotExistError.Error()/.Is() (type-match, nil, and sibling-type cases) and the error constructors — now 100%. - resources.go: ServerResourceTemplate.HasHandler()/Handler(), including the nil-handler panic — now 100%. - server_tool.go: the three untested constructors (NewServerTool, NewServerToolWithContextHandler, NewServerToolWithRawContextHandler) with handler invocation, ToolsetMetadata.Icons() (empty + real octicon), and RegisterFunc() against an in-memory server (plus nil-handler panic) — now 85-100%. - builder.go: WithServerInstructions() and the Build() instructions branch, covering the toolset InstructionsFunc loop too. Package coverage rises from ~81.5% to ~92%.
Adds unit tests for the pure/near-pure helpers in internal/ghmcp/server.go: - parseAPIHost routing (dotcom/GHEC/error cases), newDotcomHost, newGHECHost (subdomain URLs + http-scheme rejection), and newGHESHost (REST/GraphQL URLs; uses a non-resolvable host so the subdomain-isolation probe fails fast). - userAgentTransport / bearerAuthTransport RoundTrip header injection, asserting the original request is not mutated. - addGitHubAPIErrorToContext (context enrichment) and createFeatureChecker. Network/stdio/bootstrap functions remain intentionally out of scope. Package coverage rises from 29% to ~54%.
The prompt handler was at 5.3%. Adds tests that invoke the handler and assert the five generated messages, their role ordering, argument interpolation, and the optional labels/assignees branches (present and absent). File now 100%.
- bodyclose: close the response body in the ghmcp transport tests. - revive: suppress the meaningless-package-name check in pkg/utils test, matching the existing //nolint on result.go. - staticcheck SA1019: use NewServerToolWithRawContextHandler instead of the deprecated NewServerToolFromHandler in the (external-package) toolset instructions test.
Addresses review feedback about the unit test triggering a live network probe. checkSubdomainIsolation is now reachable through a package-level variable (subdomainIsolationCheck) that tests substitute, so newGHESHost can be tested without any network call while covering both the subdomain-isolated and path-based URL branches.
Rebased onto main, which landed a large refactor (go-github v79->v87, server.go split into pkg/utils + pkg/http/transport, lockdown logic rewrite, consolidated inventory tool constructors). Adapts the remaining tests accordingly: - Migrate test clients to go-github v87 via the mustNewGHClient helper. - Use the consolidated NewServerTool constructor (NewServerToolFromHandler/ NewServerToolWithRawContextHandler were removed); dedup server_tool_test.go against main's new cases. - Drop ghmcp URL/transport tests (relocated to pkg/utils + pkg/http and already covered there); keep createFeatureChecker adapted to its new (enabledFeatures, insidersMode) signature. - Remove lockdown/safety_test.go: its mock predates main's lockdown query rewrite and is now redundant with lockdown_test.go's IsSafeContent coverage.
go.mod declares 'go 1.25.0', so the build (1.24) matrix job is intrinsically incompatible: running 'go test -coverprofile' under Go 1.24 against a 1.25 module fails with 'go: no such tool covdata' while instrumenting script/print-mcp-diff-configs. The 1.25 job already passes. Align the matrix with the version the module actually requires.
The 'Run tests with coverage' step ran 'go test -coverprofile=coverage.out ./...'. Passing packages that have no test files (cmd/..., internal/profiler, pkg/context, pkg/http/mark, script/print-mcp-diff-configs) to a combined -coverprofile triggers 'go tool covdata', which fails with 'go: no such tool covdata' for those packages and fails the whole job. Restrict the coverage run to packages that actually have tests via 'go list', which avoids the covdata invocation while still producing coverage.out.
…ctor safety_coverage_test.go (added in eed328b) targets the pre-refactor lockdown API: it references the newSafetyTestCache helper (removed with safety_test.go), the deleted SetLogger method, and queryRepoAccessInfo's old 4-arg signature. None of these exist after the lockdown rewrite, so pkg/lockdown fails to build on main. lockdown_test.go already covers IsSafeContent and the cache paths, so remove the incompatible file to restore the build.
Reverts the go list filter from the previous commit per review feedback: that filter dropped no-test packages (e.g. script/print-mcp-diff-configs, pkg/context) from coverage.out, hiding them instead of reporting 0%. Root cause of the 'go: no such tool covdata' failure is setup-go@v4 not installing Go 1.25 natively, so go falls back to the module toolchain (golang.org/toolchain@...go1.25.0), which omits the covdata tool. Bumping to setup-go@v6 (matching go.yml) installs Go natively — which includes covdata — so 'go test -coverprofile=coverage.out ./...' works across all packages.
ci: actualizar setup-go a v6 (arregla covdata) y restaurar la compilación de pkg/lockdown
main's only red check is python-package.yml: a deprecation stub with no jobs, which is an invalid workflow that always reports failure. Remove it. Also remove go-build.yml: it duplicates go.yml (Build and Test Go Project) and runs on Node20-deprecated actions (setup-go@v4, cache@v3). It currently passes but is redundant. The earlier lockdown compile break was already resolved on main (PR #9 removed the obsolete safety_coverage_test.go), so no code change is needed. https://claude.ai/code/session_01W9xh1kJCqtHqBgBAMQikon
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Make
main's CI green again by removing broken/redundant workflow files. No code or docs changes — workflow deletions only.On the current
main, the only failing check ispython-package.yml: a deprecation stub with nojobs:, which is an invalid workflow and therefore always reports failure on every push/PR.Changes
.github/workflows/python-package.ymljobs:) → invalid workflow → always red. This is the only failing check onmain..github/workflows/go-build.ymlgo.yml(Build and Test Go Project); built on Node20-deprecated actions (setup-go@v4,cache@v3). Currently green but duplicate.The earlier
pkg/lockdowncompile break (obsoletesafety_coverage_test.go) was already resolved onmainby PR #9, so no code change is needed here — this PR is strictly the workflow cleanup.Verification
After these deletions, the remaining workflows already pass on
mainat the current head:golangci-lint,go.yml,docs-check,mcp-diff,license-checkare all green; onlypython-package.ymlwas red.go mod tidy -diffis clean.🤖 Generated with Claude Code
Generated by Claude Code