chore: prepare codebase for new harness contributions - #13
Merged
Conversation
- Extract copilot capture/reconcile/install logic from app.go into app_copilot.go; VS Code logic into app_vscode.go. app.go is now generic dispatch only — the switch in runCapture() is the natural extension point for new harnesses. - Add app_copilot_test.go with integration tests covering the final, provisional, missing-sessionId, and unknown-harness cases. - Add internal/copilot/testdata/ fixtures (session_final.jsonl and session_provisional.jsonl) as canonical examples of both states. - Add docs/adding-a-harness.md: step-by-step guide for contributors including file structure, SessionSummary field reference, reconcile guidance, test patterns, and a checklist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add TestRunReconcileUpgradesProvisionalToFinal: verifies the full provisional→final upgrade path through App.Run(reconcile). - Add TestRunReconcileMarksBestEffortWhenExhausted: verifies that a reconcile that exhausts without finding shutdown metrics marks the session as best_effort and returns an error. - Add TestRunInstallCopilotPrintConfig: verifies --print-config emits valid JSON containing the sessionEnd hook and binary path. - Fix missing imports (encoding/json, fmt, time) in the app_myharness.go code example in docs/adding-a-harness.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
defer logFile.Close() on a writable fd without checking the error is a
CodeQL-flagged pattern. The child process writes directly to the kernel
fd so there is no Go-side buffering to lose, but make the intentional
discard explicit with defer func() { _ = logFile.Close() }() to satisfy
static analysis and document the reasoning.
Pre-existing issue surfaced by CodeQL on this PR's diff.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove boilerplate code examples in favour of directing contributors to the existing harness implementations as the canonical reference. Add explanations of the two key challenges a harness author will encounter: - Hook payloads are thin; the real data lives in a separate log or API that must be located via session ID and CWD. - Final metrics may arrive after the hook exits; use provisional capture + async reconcile rather than blocking the hook runner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Agents tackling a new harness integration will now be directed to the guide and the copilot harness as the canonical reference before writing any code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Split the monolithic
app.gointo per-harness files and add the tests and documentation needed to make contributing a new harness (e.g. Hermes, OpenCode) straightforward.Changes
Code organisation
internal/cli/app_copilot.gointernal/cli/app_vscode.goapp.gois now generic dispatch only; theswitchinrunCapture()is the single extension point for new harnessesTests (23 total, all green)
app_copilot_test.go: integration tests for the fullApp.Run()path — final session, provisional session, missing sessionId, unknown harness,install --print-config, reconcile provisional→final, reconcile best_effortapp_vscode_test.gointegration testsinternal/copilot/testdata/:session_final.jsonlandsession_provisional.jsonlfixture filesDocs
docs/adding-a-harness.md: step-by-step guide covering file structure, working code examples,SessionSummaryfield reference, reconcile guidance, test patterns, and a checklistAdding a harness after this PR
Three things:
internal/<name>/with aSummarize()functioninternal/cli/app_<name>.gowith arunCapture<Name>()methodcaseto theswitchinrunCapture()inapp.go