feat: add runtime.json for cross-hook agent/model detection

[TonyCasey]

Summary

• Adds IRuntimeService interface and RuntimeService implementation for persisting session data to .git-mem/runtime.json
• Updates AgentResolver to read runtime.json as fallback when env vars are empty
• SessionStartHandler activates runtime on session start with detected agent/model
• SessionStopHandler deactivates runtime on session stop to prevent stale attribution
• 2-hour TTL enforcement prevents stale data from being used

Test plan

• npm run type-check passes
• npm run lint passes
• 582 unit tests pass (14 new for RuntimeService, 9 new for handlers)
• 91 integration tests pass
• Manual test: Start Claude Code session, verify .git-mem/runtime.json created
• Manual test: Make commit in plain terminal, verify AI-Agent/AI-Model trailers present
• Manual test: Stop Claude Code session, verify runtime.json deleted

Closes GIT-123

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes & Improvements
  • Enhanced runtime resource cleanup after sessions conclude, ensuring proper deactivation regardless of outcome
  • Improved data validation for session records to ensure integrity
  • Refined error handling for write operations during session management
  • Better working directory context handling in command execution

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The changes enhance session stop handling by integrating runtime deactivation, improve data validation in RuntimeService, refine test robustness with deterministic failure paths, and ensure proper working directory context in hook command execution.

Changes

Cohort / File(s)	Summary
Session Runtime Integration src/application/handlers/SessionStopHandler.ts	Adds optional runtimeService parameter to constructor; introduces private deactivateRuntime method that safely invokes runtime deactivation; ensures deactivation executes in finally block regardless of capture success or failure.
Runtime Service Validation src/infrastructure/services/RuntimeService.ts, tests/unit/infrastructure/services/RuntimeService.test.ts	Enhances read method validation to verify timestamp, sessionId, and source are strings; refines error handling test to use withTestDir for deterministic write failure simulation.
Hook Command Integration src/commands/hook.ts	Passes cwd option to container creation during hook command execution for proper working directory context.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SessionStopHandler
    participant RuntimeService
    participant Logger

    Client->>SessionStopHandler: handle(args)
    
    activate SessionStopHandler
    
    SessionStopHandler->>SessionStopHandler: capture session
    
    alt Capture Success
        SessionStopHandler->>SessionStopHandler: set success result
    else Capture Failure
        SessionStopHandler->>SessionStopHandler: set error result
    end
    
    SessionStopHandler->>SessionStopHandler: finally block
    
    alt RuntimeService available
        SessionStopHandler->>RuntimeService: deactivate(cwd)
        activate RuntimeService
        
        alt Deactivation Success
            RuntimeService-->>SessionStopHandler: success
            SessionStopHandler->>Logger: log success
        else Deactivation Error
            RuntimeService-->>SessionStopHandler: error
            SessionStopHandler->>Logger: log warning
        end
        
        deactivate RuntimeService
    end
    
    SessionStopHandler-->>Client: return unified handlerResult
    
    deactivate SessionStopHandler

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: unified git-mem hook command + safe settings.json merge (GIT-60) #25: Introduced unified hookCommand implementation that is now being enhanced with cwd parameter passing in this PR.
• feat: load .env in hooks for API keys (GIT-94) #58: Previously modified hookCommand with git-root environment loading and repoRoot-based configuration, sharing the same hook.ts file as this PR's container creation changes.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding runtime.json functionality for cross-hook agent/model detection, matching the core functionality added across SessionStartHandler, SessionStopHandler, RuntimeService, and AgentResolver.
Description check	✅ Passed	The description covers the key changes, includes a test plan with specific test items, and references the related task (Closes GIT-123), though manual test items are not yet marked complete.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch git-123

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Adds a repo-local .git-mem/runtime.json mechanism to persist detected agent/model across hook invocations when environment variables aren’t present, wiring it through DI and session start/stop handlers.

Changes:

• Introduces IRuntimeService + RuntimeService to write/read/delete .git-mem/runtime.json with a 2-hour TTL.
• Updates AgentResolver to fall back to runtime.json when env-based detection returns empty.
• Activates runtime data on session:start and deactivates it on session:stop, plus adds unit tests for the new behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/unit/infrastructure/services/RuntimeService.test.ts	Adds unit coverage for activate/deactivate/read and TTL behavior.
tests/unit/application/handlers/SessionStopHandler.test.ts	Verifies runtime deactivation behavior on session stop.
tests/unit/application/handlers/SessionStartHandler.test.ts	Verifies runtime activation behavior on session start.
test.txt	Adds a standalone text file (appears unrelated to feature).
src/infrastructure/services/RuntimeService.ts	Implements read/write/delete of .git-mem/runtime.json with TTL.
src/infrastructure/services/AgentResolver.ts	Adds runtime.json fallback when env detection is missing.
src/infrastructure/di/types.ts	Registers runtimeService in the DI cradle types.
src/infrastructure/di/container.ts	Wires RuntimeService and injects it into AgentResolver + handlers.
src/domain/interfaces/IRuntimeService.ts	Defines runtime data + service contract.
src/application/handlers/SessionStopHandler.ts	Deactivates runtime on session stop.
src/application/handlers/SessionStartHandler.ts	Activates runtime on session start using resolved agent/model.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/application/handlers/SessionStopHandler.ts (1)

23-58: ⚠️ Potential issue | 🟠 Major

Ensure runtime deactivation runs even when capture fails.
Right now deactivation is skipped if capture() throws, which can leave stale runtime.json behind. Move the call into a finally block (or invoke in both success and error paths).

🔧 Suggested fix (ensure deactivation always runs)

   async handle(event: ISessionStopEvent): Promise<IEventResult> {
-    try {
+    let result: IEventResult;
+    try {
       this.logger?.info('Session stop handler invoked', {
         sessionId: event.sessionId,
         cwd: event.cwd,
       });
@@
-      this.deactivateRuntime(event.cwd);
-
-      return {
+      result = {
         handler: 'SessionStopHandler',
         success: true,
         output: result.summary,
       };
     } catch (error) {
       const err = error instanceof Error ? error : new Error(String(error));
       this.logger?.error('Session stop handler failed', {
         error: err.message,
         stack: err.stack,
       });
-      return {
+      result = {
         handler: 'SessionStopHandler',
         success: false,
         error: err,
       };
+    } finally {
+      // Always attempt to deactivate runtime.json on session stop
+      this.deactivateRuntime(event.cwd);
     }
+    return result;
   }

Also applies to: 62-79

🤖 Fix all issues with AI agents

In `@src/application/handlers/SessionStartHandler.ts`:
- Around line 113-122: detectSource() currently only reports environment
variables and will return 'env:unknown' even when agentResolver.resolveAgent()
ends up using runtime.json; update the detection so the reported source reflects
the actual resolution path: either modify detectSource(sourceOverride?: string)
to prefer a provided resolved source (e.g., pass the result or a tag from
agentResolver.resolveAgent()) or extend detectSource to check the runtime
fallback (runtime.json) presence/contents before returning 'env:unknown'. Make
the change in SessionStartHandler where detectSource() is called so the final
reported source matches the actual agent/model selection (reference detectSource
and agentResolver.resolveAgent).

In `@src/infrastructure/di/container.ts`:
- Around line 70-76: The container is being created without the hook's cwd so
AgentResolver falls back to process.cwd(); update the hook entry point where
createContainer(...) is called to thread the event cwd into createContainer
(e.g., createContainer({ scope: `hook:${eventName}`, llm: config.llm, cwd }))
and ensure the container options propagate into AgentResolver construction so
the AgentResolver(...) call uses options?.cwd rather than process.cwd() for
fallback reads.

In `@src/infrastructure/services/RuntimeService.ts`:
- Around line 62-73: The timestamp string validation in RuntimeService (the
method handling data.timestamp, using ttlMs and DEFAULT_TTL_MS) only checks type
but not parseability, so parse the timestamp (new
Date(data.timestamp).getTime()) and reject the payload if the result is NaN or
not finite; also treat negative age (timestamp in the future) as invalid or
guard it with an allowable skew (e.g. reject if age < -allowedSkew), returning
undefined in those cases before the TTL check so invalid or future timestamps
cannot bypass TTL. Ensure you reference the existing variables/data flow
(data.timestamp, timestamp, age, ttlMs, DEFAULT_TTL_MS) when adding the
validation.

[TonyCasey]

Addressed all review comments in commit 45c8607:

1. RuntimeService timestamp validation: Now rejects NaN/invalid timestamps and future timestamps (with 1 minute skew allowance for clock drift)
2. SessionStopHandler finally block: Moved deactivateRuntime() to a finally block so it runs even when capture fails
3. SessionStartHandler env-only detection: Now uses direct env detection functions instead of AgentResolver to avoid runtime.json loop when writing
4. AgentResolver caching: Added cachedRuntimeData to avoid repeated file reads within the same resolver instance
5. Doc comment fix: Updated IRuntimeService doc to say "errors silently ignored" instead of "logs errors"
6. Removed test.txt: Deleted the accidental artifact file

All 585 unit tests pass.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@tests/unit/infrastructure/services/RuntimeService.test.ts`:
- Around line 93-100: The test using an absolute path should be made
deterministic by creating a temporary working directory and then creating a
regular file at the location where the code will try to mkdir (so mkdirSync
fails), instead of relying on '/nonexistent/...'; update the test in
RuntimeService.test.ts to create a temp dir (or use a tmp file), write an
ordinary file at the intended cwd subpath (the `.git-mem/` parent) before
calling service.activate(createRuntimeData(), <that temp path>), then call
service.activate and assert it does not throw; this targets the service.activate
and createRuntimeData() usage so the write error is reliably reproduced in all
environments.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/unit/infrastructure/services/RuntimeService.test.ts`:
- Around line 17-29: The tests create a temp dir in beforeEach and call
cleanup() manually in try/finally inside each test; replace the empty after()
hook with afterEach(cleanup) so cleanup() runs automatically after every test
(and still handles existence checks), then remove the per-test try/finally
blocks; reference the beforeEach, after/afterEach, and cleanup() symbols to
locate and update the test file and ensure no leftover empty hooks remain.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/infrastructure/services/AgentResolver.ts`:
- Around line 54-60: In getRuntimeData(), remove the redundant nullish
coalescing assignment and replace "this.cachedRuntimeData ??=
this.runtimeService?.read(this.cwd);" with a straightforward assignment so the
code reads "this.cachedRuntimeData = this.runtimeService?.read(this.cwd);"; keep
use of the guards runtimeDataLoaded, runtimeService, and cwd as-is and ensure
cachedRuntimeData and runtimeDataLoaded remain the same fields on the class.

In `@tests/unit/infrastructure/services/RuntimeService.test.ts`:
- Line 93: Remove the no-op assertion `assert.ok(true, 'activate should not
throw on write errors')` from the test in RuntimeService.test.ts (the test that
calls service.activate) because it provides no value—if service.activate throws
the test will already fail; either delete the assertion or replace it with a
short inline comment like "// ensure activate does not throw on write errors" to
document intent while keeping the test logic unchanged.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The validation added here checks for required fields (timestamp, sessionId, source) but doesn't validate the type of the optional agent and model fields. According to IRuntimeData, these must be string or undefined, but if the JSON contains non-string values (e.g., numbers, objects), they will pass validation and be returned with incorrect types. Consider adding type checks: (data.agent === undefined || typeof data.agent === 'string') && (data.model === undefined || typeof data.model === 'string').

Suggested change

	typeof data.source !== 'string'
	typeof data.source !== 'string' ||
	(data.agent !== undefined && typeof data.agent !== 'string') ||
	(data.model !== undefined && typeof data.model !== 'string')