GIT-869c63k1e: enforce ClickUp branch/PR/commit linking guardrails#89
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Fact: add claude to test.txt. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> AI-Confidence: low AI-Lifecycle: project AI-Memory-Id: f5ea4bbd AI-Source: heuristic
AI-Confidence: low AI-Lifecycle: project AI-Memory-Id: 1980faea AI-Source: heuristic
When git-mem hooks run in plain terminal shells (e.g., post-commit), no AI environment variables are set. This causes agent/model detection to fail, meaning commits made during AI sessions don't get properly attributed with AI-Agent and AI-Model trailers. Solution: - Write runtime.json to .git-mem/ on session-start with detected agent/model - AgentResolver reads this file as fallback when env vars are empty - Delete runtime.json on session-stop to prevent stale attribution - TTL enforcement (2h default) prevents stale data from being used Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Decision: add runtime.json for cross-hook agent/model detection (GIT-123). When git-mem hooks run in plain terminal shells (e.g., post-commit), AI-Confidence: medium AI-Tags: application, handlers, domain, infrastructure, services, tests, unit AI-Lifecycle: project AI-Memory-Id: 42af7fc0 AI-Source: heuristic
- RuntimeService: reject NaN/invalid timestamps and future timestamps (with 1 minute skew allowance) - SessionStopHandler: move deactivateRuntime to finally block so it runs even when capture fails - SessionStartHandler: use env-only detection functions to avoid runtime.json loop when activating - AgentResolver: cache runtime.json read to avoid repeated file I/O - IRuntimeService: fix doc comment (errors silently ignored, not logged) - Remove accidental test.txt file Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Convention: runtime.json loop when activating AI-Confidence: medium AI-Tags: application, handlers, domain, infrastructure, services, tests, unit, pattern:avoid AI-Lifecycle: project AI-Memory-Id: 70a43cb3 AI-Source: heuristic
ClickUp task: 869c63k1e https: //app.clickup.com/t/869c63k1e AI-Agent: Codex/0.92.0 AI-Model: gpt-5.3-codex AI-Fact: enforce ClickUp linking guardrails GIT-869c63k1e. ClickUp task: 869c63k1e AI-Confidence: low AI-Tags: hooks, tests, unit AI-Lifecycle: project AI-Memory-Id: 00f9d6ef AI-Source: heuristic
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Refactor repeated temp-dir setup/cleanup into withTestDir helper and use node:* imports. AI-Agent: Codex/0.92.0 AI-Model: gpt-5.3-codex AI-Decision: The withTestDir helper pattern provides automatic cleanup using try-finally to ensure temporary directories are always removed even if tests fail. AI-Confidence: high AI-Tags: testing, refactoring, test-helpers, node-js, imports, modules, resource-management, cleanup, temp-directories, runtime-service, filesystem AI-Lifecycle: project AI-Memory-Id: 87292011 AI-Source: llm-enrichment
|
Addressed Sonar duplication feedback by refactoring runtime service tests to remove repeated setup/cleanup blocks and reducing duplicated new code. Also updated runtime service imports to node:* style in the same area.\n\nRelated task: GIT-869c63k1e |
📝 WalkthroughWalkthroughAdds a runtime session persistence feature (IRuntimeService + RuntimeService) with activation on session start and deactivation on session stop, DI wiring for runtime support and agent/model detection fallback, ClickUp task validation in hooks/workflows, PR governance workflows, tests, and a PR template. Changes
Sequence DiagramsequenceDiagram
rect rgba(200,200,255,0.5)
actor Dev
participant SessionStart as SessionStartHandler
participant Detect as detectAgent/detectModel
participant Runtime as RuntimeService
participant FS as .git-mem/runtime.json
end
Dev->>SessionStart: start(sessionId)
SessionStart->>Detect: detectAgent(), detectModel()
Detect-->>SessionStart: agent, model
SessionStart->>Runtime: activate({sessionId, agent, model, timestamp, source})
Runtime->>FS: write runtime.json
FS-->>Runtime: write success
Runtime-->>SessionStart: success (no-throw)
SessionStart-->>Dev: started
rect rgba(200,255,200,0.5)
participant AgentResolver as AgentResolver
Dev->>AgentResolver: resolveAgent()/resolveModel()
AgentResolver->>Runtime: read(ttl)
Runtime-->>AgentResolver: runtime data (if fresh)
AgentResolver-->>Dev: agent/model (env or fallback)
end
rect rgba(255,200,200,0.5)
actor Dev2
participant SessionStop as SessionStopHandler
Dev2->>SessionStop: stop(sessionId)
SessionStop->>Runtime: deactivate()
Runtime->>FS: delete runtime.json
FS-->>Runtime: delete success
Runtime-->>SessionStop: success (no-throw)
SessionStop-->>Dev2: stopped
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR enforces ClickUp task ID linking across GitHub branches, PRs, and commit messages to improve project tracking and integration between GitHub and ClickUp.
Changes:
- Added GitHub workflow to validate ClickUp task IDs in branch names and PR metadata
- Added commit-msg hook validation requiring ClickUp task IDs in all commits
- Implemented RuntimeService for persisting AI agent/model detection across hook invocations
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/clickup-linking.yml |
New workflow enforcing ClickUp task ID presence in branch names and PR title/body |
.github/pull_request_template.md |
New PR template with ClickUp task ID fields |
CLAUDE.md |
Updated workflow documentation from Linear to ClickUp task naming |
src/hooks/commit-msg.ts |
Added validation requiring ClickUp task ID in commit messages (v7) |
tests/unit/hooks/commit-msg.test.ts |
Updated tests for v7 hook fingerprint and new validation |
src/domain/interfaces/IRuntimeService.ts |
New interface for runtime session data persistence |
src/infrastructure/services/RuntimeService.ts |
New service implementing runtime.json read/write with TTL validation |
src/infrastructure/services/AgentResolver.ts |
Enhanced with runtime.json fallback for agent/model detection |
src/application/handlers/SessionStartHandler.ts |
Added runtime.json activation on session start |
src/application/handlers/SessionStopHandler.ts |
Added runtime.json deactivation on session stop |
src/infrastructure/di/container.ts |
Registered RuntimeService and updated AgentResolver dependencies |
src/infrastructure/di/types.ts |
Added runtimeService to dependency injection types |
tests/unit/infrastructure/services/RuntimeService.test.ts |
Comprehensive test coverage for RuntimeService |
tests/unit/application/handlers/SessionStartHandler.test.ts |
Added tests for runtime activation behavior |
tests/unit/application/handlers/SessionStopHandler.test.ts |
Added tests for runtime deactivation behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Consolidate missing/invalid JSON read checks into one case to reduce Sonar duplication. AI-Agent: Codex/0.92.0 AI-Model: gpt-5.3-codex AI-Convention: Test consolidation is preferred over duplication to satisfy Sonar code quality rules, even when it means combining logically separate test scenarios. AI-Confidence: verified AI-Tags: testing, sonar, code-quality, test-consolidation, runtime-service, error-handling, json-parsing, file-operations, file-structure, git-mem, json-storage AI-Lifecycle: project AI-Memory-Id: 5642fc55 AI-Source: llm-enrichment
Use CLOCK_SKEW_ALLOWANCE_MS constant and remove uninitialized handlerResult path. AI-Agent: Codex/0.92.0 AI-Model: gpt-5.3-codex AI-Decision: Clock skew allowance for runtime data validation is set to 1 minute (60,000ms) to handle minor timestamp discrepancies between systems. AI-Confidence: verified AI-Tags: error-handling, variable-initialization, session-stop-handler, runtime-service, clock-skew, timestamp-validation, constants, magic-numbers, code-quality, cleanup, finally-block AI-Lifecycle: project AI-Memory-Id: d6a8f851 AI-Source: llm-enrichment
|
Addressed both open Copilot review threads in commit 3d90836:
Validated with:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/infrastructure/services/RuntimeService.ts (1)
62-67: Incomplete runtime data validation may return malformed objects.The type assertion
as IRuntimeDataat line 62 bypasses TypeScript's type safety. The validation only checkstimestamp, butIRuntimeDatarequiressessionId,agent,model, andsourcefields. Callers may receive an object missing required fields.Consider validating the complete structure:
♻️ Proposed validation enhancement
const content = readFileSync(filePath, 'utf8'); - const data = JSON.parse(content) as IRuntimeData; + const parsed = JSON.parse(content) as Record<string, unknown>; // Validate structure - if (!data || typeof data.timestamp !== 'string') { + if ( + !parsed || + typeof parsed.sessionId !== 'string' || + typeof parsed.timestamp !== 'string' || + typeof parsed.source !== 'string' + ) { return undefined; } + + const data = parsed as IRuntimeData;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/infrastructure/services/RuntimeService.ts` around lines 62 - 67, The current parsing in RuntimeService.ts casts JSON to IRuntimeData then only checks timestamp, which can return malformed objects; update the parsing/validation around the const data variable so you parse into an unknown, remove the unsafe "as IRuntimeData" cast, and explicitly validate that sessionId, agent, model, source and timestamp exist and are strings (or other required shapes) before returning — if any are missing/invalid return undefined; reference the parsed variable "data" and the IRuntimeData type to locate where to add these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/infrastructure/services/RuntimeService.ts`:
- Around line 62-67: The current parsing in RuntimeService.ts casts JSON to
IRuntimeData then only checks timestamp, which can return malformed objects;
update the parsing/validation around the const data variable so you parse into
an unknown, remove the unsafe "as IRuntimeData" cast, and explicitly validate
that sessionId, agent, model, source and timestamp exist and are strings (or
other required shapes) before returning — if any are missing/invalid return
undefined; reference the parsed variable "data" and the IRuntimeData type to
locate where to add these checks.
Require Sonar pass + zero new issues and enforce inline-replied, resolved review threads. AI-Agent: Codex/0.92.0 AI-Model: gpt-5.3-codex AI-Fact: enforce sonar and inline review governance GIT-869c63k1e. Require Sonar pass + zero new issues and enforce inline-replied, resolved review threads. AI-Confidence: low AI-Tags: config, docs AI-Lifecycle: project AI-Memory-Id: c1d001da AI-Source: heuristic
There was a problem hiding this comment.
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 @.github/workflows/pr-governance.yml:
- Around line 63-99: The GraphQL query is paginated (reviewThreads(first:100)
and comments(first:100)) which can miss threads/comments; update the workflow to
detect and handle pagination by requesting pageInfo (endCursor, hasNextPage) for
reviewThreads and comments in the query and then either iterate using the
endCursor to fetch all pages (loop the gh api graphql call until hasNextPage is
false) or fail the check when pageInfo.hasNextPage is true so the run is
considered incomplete; adjust the variables and jq logic that compute pr_author,
unresolved_count and missing_inline_reply_count to aggregate across all pages
(or short-circuit with a failing message) rather than relying only on the first
100 items.
- Around line 16-51: The SonarCloud API calls use curl in the for-loop and when
building issues_response without authentication or the organization parameter;
update both curl invocations to use Basic auth via -u "${SONAR_TOKEN}:" and
append the organization query param using SONAR_ORGANIZATION (e.g. add
&organization=${SONAR_ORGANIZATION}) so the requests target the correct org, and
ensure SONAR_PROJECT_KEY usage (the variable referenced as SONAR_PROJECT_KEY)
remains the project key only while organization is passed separately; preserve
existing variables PR_NUMBER, quality_status and issues_response logic but
modify the curl invocations accordingly and ensure SONAR_TOKEN and
SONAR_ORGANIZATION are required env inputs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add Sonar auth/org support, GH token for gh api, pagination safeguards, doc naming alignment, and Sonar issue fixes. AI-Agent: Codex/0.92.0 AI-Model: gpt-5.3-codex AI-Gotcha: address PR governance and review feedback GIT-869c63k1e. Add Sonar auth/org support, GH token for gh api, pagination safeguards, doc naming alignment, and Sonar issue fixes. AI-Confidence: medium AI-Tags: infrastructure, services, tests, unit AI-Lifecycle: project AI-Memory-Id: bf652dba AI-Source: heuristic
2786409 to
8380710
Compare
|
There was a problem hiding this comment.
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 @.github/workflows/pr-governance.yml:
- Around line 19-20: The workflow uses an invalid GitHub Actions expression
replace(...) for SONAR_PROJECT_KEY causing a syntax error; remove the replace
fallback and instead set SONAR_PROJECT_KEY to a safe default (e.g., use
github.repository directly) in the job/env and compute the sanitized value in
the shell step that runs (sanitize by replacing '/' with '_' in the script), or
use available expression helpers like format() if suitable; update the
SONAR_PROJECT_KEY reference and any steps that rely on it to use the computed
sanitized value (look for SONAR_PROJECT_KEY and the erroneous replace(...) usage
to locate the code).
In `@src/infrastructure/services/AgentResolver.ts`:
- Around line 54-60: In getRuntimeData(), replace the nullish coalescing
assignment on cachedRuntimeData with a plain assignment since runtimeDataLoaded
is false only when cachedRuntimeData is undefined; specifically set
this.cachedRuntimeData = this.runtimeService?.read(this.cwd) (still guarded by
runtimeService) and keep the runtimeDataLoaded flag behavior in the same method
to simplify the logic.



Summary
ClickUp Task
Task ID: GIT-869c63k1e
Task Link: https://app.clickup.com/t/869c63k1e
Raw Task ID: 869c63k1e
Validation
Summary by CodeRabbit
New Features
Documentation
Process Improvements
Tests