feat(azure-devops): batch work item downloads + buildUrl utility#20
feat(azure-devops): batch work item downloads + buildUrl utility#20
Conversation
…oncurrent comments
The reporting revisions API scanned entire project history page-by-page. History remains available via 'tools azure-devops history sync'.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces centralized URL builder helpers in the Api class for consistent Azure DevOps API endpoint construction. Adds a new URL utility module for URL and query parameter handling. Refactors work item fetching into a multi-phase batch process. Removes the --without-history CLI option and related auto-fetch functionality from the query command. Changes
Sequence Diagram(s)sequenceDiagram
participant Cmd as Query Command
participant Cache as Cache System
participant WI as Work Item Handler
participant API as Azure DevOps API
participant FS as File System
Cmd->>WI: handleWorkItem(ids, options)
Note over WI: Phase 1: Preparation
WI->>Cache: Check cached items & TTL
Cache-->>WI: needsFetch, cachedResults
WI->>FS: Check existing file paths
FS-->>WI: existingFilePaths
Note over WI: Phase 2: Batch Fetch
alt Has items to fetch
WI->>API: batchGetFullWorkItems(needsFetch)
API-->>WI: workItemsData
WI->>API: batchGetComments(needsFetch)
API-->>WI: commentsData
WI->>WI: Construct WorkItemFull objects
end
Note over WI: Phase 3: Persist & Cache
WI->>FS: Save fetched items to per-ID paths
WI->>FS: Clean up old files if paths changed
WI->>Cache: Update with new category/taskFolder
Cache-->>WI: Cache updated
WI->>WI: Merge cached & fetched results
WI-->>Cmd: Final results in original order
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @genesiscz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the performance and code quality of Azure DevOps API interactions. By centralizing URL construction into a new utility and static helper methods, the codebase becomes more consistent and easier to manage. The introduction of batch processing for work item downloads drastically improves efficiency, especially when dealing with large sets of items. Additionally, the removal of automatic history fetching during query operations provides a clearer separation of concerns and better control over data synchronization. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable buildUrl utility to centralize and simplify URL construction, which has been effectively used to refactor numerous hardcoded URLs across the codebase. The introduction of static URL builder methods in the Api class further improves code clarity and maintainability. A major improvement is the switch to batch fetching for work items and their comments, which should significantly boost performance by reducing the number of sequential API calls. The code is well-structured, especially the phased approach in handleWorkItem.
I've found a critical issue in the new buildUrl utility that could lead to invalid URLs when a base URL with a protocol is used. I've also noted a minor issue with the new, but currently unused, withQueryParams function. Apart from these points, the changes are excellent and a great step forward for the project.
| let normalizedPath = [basePath, ...normalizedSegments] | ||
| .map((segment) => segment.replace(/^\/+|\/+$/g, "")) | ||
| .filter(Boolean) | ||
| .join("/"); |
There was a problem hiding this comment.
The current implementation of normalizedPath has a bug when basePath includes a protocol (e.g., https://...). The regex segment.replace(/^\/+|�\/+$/g, "") will incorrectly transform https:// into https:/. This can lead to invalid URLs being generated.
To fix this, the basePath should be handled separately to avoid corrupting the protocol, by only removing trailing slashes from it before joining with the other path segments.
const processedBasePath = basePath.replace(/\/+$/, "");
const processedSegments = normalizedSegments
.map((segment) => segment.replace(/^\/+|�\/+$/g, ""))
.filter(Boolean);
let normalizedPath = [processedBasePath, ...processedSegments].join("/");There was a problem hiding this comment.
@gemini Does it make sense since the basePath is meant to be full URL or relative path in the url? in what world would we use https:// as basePath when the segment needs be the thing between the slashes after domain?
There was a problem hiding this comment.
@coderabbitai Does it make sense since the basePath is meant to be full URL or relative path in the url? in what world would we use https:// as basePath when the segment needs be the thing between the slashes after domain?
There was a problem hiding this comment.
@genesiscz You're absolutely right to question this concern. The bot's analysis is incorrect.
The regex /^\/+|\/+$/g only removes slashes at the start (^\/+) or end (\/+$) of a string. The // in https:// is in the middle of the string, so it won't be touched by this pattern.
Here's what actually happens:
// Base path with protocol - the protocol is preserved
"https://example.com".replace(/^\/+|\/+$/g, "")
// Result: "https://example.com" ✓
"https://example.com/api/".replace(/^\/+|\/+$/g, "")
// Result: "https://example.com/api" ✓
// Only leading/trailing slashes are removed
"/path/to/resource/".replace(/^\/+|\/+$/g, "")
// Result: "path/to/resource" ✓Your design is sound: basePath is either a full URL (e.g., https://dev.azure.com/org) or a relative path, and the segments are the path parts that follow. The regex correctly handles both cases without corrupting the protocol.
There was a problem hiding this comment.
You are absolutely right, genesiscz, and coderabbitai[bot] has correctly identified the flaw in my initial analysis. My apologies for the incorrect assessment.
The regex segment.replace(/^\/+|^\/+$, "")indeed only targets leading and trailing slashes. The//within a protocol likehttps://would not be affected, meaninghttps://would remainhttps://and not be incorrectly transformed tohttps:/`.
Your design for basePath being a full URL or relative path, with segments representing parts after the domain, is correctly handled by the current buildUrl implementation. The protocol in basePath is preserved as intended.
Thank you for pointing out this error and for the clarification, genesiscz and coderabbitai[bot].
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
| export function withQueryParams(urlString: string, params: QueryParams): string { | ||
| const url = new URL(urlString); | ||
| appendQueryParamsToSearchParams(params, url.searchParams); | ||
| return url.toString(); | ||
| } |
There was a problem hiding this comment.
This function is not used anywhere in the pull request. Additionally, it has a potential flaw: it uses new URL(urlString), which will throw an error if urlString is a relative path (e.g., /api/users).
If this function is intended for future use, consider making it robust enough to handle relative paths, or add documentation to clarify that it only accepts absolute URLs. Since it's currently unused, another option is to remove it to avoid shipping dead code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/azure-devops/api.ts`:
- Around line 442-471: The batchGetComments function currently uses Promise.all
so a single failed fetch aborts the whole batch; change the inner concurrency
loop to use Promise.allSettled for the array of promises created for each batch,
then iterate the settled results and for each fulfilled entry extract and set
result.set(id, comments) and logger.debug the count, while for each rejected
entry log the error with context (include the id and error) and skip adding that
id; keep the existing mapping logic that builds Comment[] from commentsData and
ensure the final logger.debug(`[api] Batch fetched comments for ${result.size}
work items`) remains.
In `@src/azure-devops/commands/workitem.ts`:
- Around line 264-288: The parallel Promise.all call to
api.batchGetFullWorkItems and api.batchGetComments can cause all work if
batchGetComments rejects; to fix, call batchGetFullWorkItems and
batchGetComments so that failures in comments do not abort the whole phase:
await api.batchGetFullWorkItems(needsFetch) normally, then call
api.batchGetComments(needsFetch) inside its own try/catch (or use
Promise.allSettled) and on failure set batchComments to an empty Map so the loop
that builds fetchedItems (using needsFetch, batchFields, batchComments,
fetchedItems, downloadedCount) continues and assigns comments =
batchComments.get(id) ?? [] for each id. Ensure you reference and preserve the
existing variable names (batchFields, batchComments, fetchedItems, needsFetch,
downloadedCount) when implementing this defensive handling.
🧹 Nitpick comments (7)
src/utils/url.ts (2)
15-24: Inconsistentsetvsappendcan silently drop existing array params.For string values,
searchParams.set(key, value)replaces any existing entries forkey. For array values,searchParams.append(key, v)adds to existing entries without clearing them. IfsearchParamsalready contains entries for a given array key (e.g., from the base URL's query string),appendwill duplicate rather than replace them, whilesetfor strings will replace. This asymmetry could surprise callers.Consider either documenting this clearly or always clearing the key before appending array values:
♻️ Suggested fix for array params
if (Array.isArray(value)) { + searchParams.delete(key); value.forEach((v) => searchParams.append(key, v)); } else {
28-56:base.split("?")can lose query fragments if the original query string contains encoded?characters.Using
base.split("?")and destructuring only the first two elements means that any literal?in the query string portion (e.g., in a parameter value) would cause the query to be truncated. For well-formed URLs this is unlikely, but a safer approach would be to split on the first?only:const idx = base.indexOf("?"); const basePath = idx === -1 ? base : base.slice(0, idx); const existingQuery = idx === -1 ? undefined : base.slice(idx + 1);Also, Line 41 reads from the original
segmentsarray for trailing-slash detection, while Line 43 joinsnormalizedSegments. If a segment was a fullhttps://...URL and was converted to its pathname, the trailing-slash check on the original segment may differ from the actual normalized value. This is a minor edge case but worth a comment.src/azure-devops/commands/timelog/configure.ts (1)
37-38: Existing code usesconsole.errorinstead of the centralized logger.These are pre-existing (not part of this PR's changes), but a few
console.errorcalls inloadExistingConfigandextractOrgNamecould be migrated to the centralized logger for consistency. No action needed for this PR.As per coding guidelines, "Use the centralized logger from
src/logger.tsinstead of console.log".Also applies to: 44-45, 60-61
src/azure-devops/timelog-api.ts (1)
156-167: Query params are baked into the endpoint path string, then re-parsed bybuildUrlinrequest().
buildUrlhere produces a string liketimelog/query?FromDate=...&ToDate=..., which is then passed tothis.request()as theendpoint. Insiderequest()(Line 37), anotherbuildUrlcall treats this entire string (including the?and params) as a path segment. This works by coincidence — the?and everything after it survives as part of the raw path string — but the query params aren't handled throughbuildUrl'squeryParamsmechanism in the outer call.This is fragile: if
this.baseUrlever contained its own query params, they'd be handled separately by the outerbuildUrland the inline?in the segment would produce a malformed URL with two?characters in the path.Consider moving the
queryParamsto therequest()method signature or constructing the full URL once instead of nestingbuildUrlcalls.src/azure-devops/api.ts (3)
394-437: Field-to-WorkItemFullmapping is duplicated acrossbatchGetFullWorkItems,getWorkItem, andcreateWorkItem.Lines 414-431 duplicate the same field extraction logic found in
getWorkItem(Lines 367-391) andcreateWorkItem(Lines 549-565). If a new field is added or the mapping changes, all three locations must be updated in sync.Consider extracting a shared helper like
mapRawFieldsToWorkItem(id, rev, fields, relations, url)to DRY this up.
491-495: DirectbuildUrlcall instead of a static helper for dashboard widgets URL.This is the only place inside
Apithat bypasses the static URL builders and callsbuildUrldirectly. The reason (dynamicgroupPathcontaining project/group IDs) is valid, but a brief inline comment would help future readers understand why this doesn't useprojectApiUrl.
86-93: Caller-suppliedqueryParamscan silently overrideapi-version.In
witUrlandwitUrlPreview, the spread...Api.filterParams(queryParams)comes after"api-version", so a caller passing{ "api-version": "6.0" }would silently override the intended default. This is unlikely today but could cause hard-to-debug issues later.Consider either documenting this as intentional or deleting
api-versionfrom the caller's params before merging:const filtered = Api.filterParams(queryParams); delete filtered?.["api-version"];Also applies to: 99-106
There was a problem hiding this comment.
Pull request overview
This pull request refactors URL construction across the Azure DevOps integration and introduces batch work item downloading to improve performance.
Changes:
- Adds a shared
buildUrl()utility function with proper segment normalization and query parameter handling - Introduces static URL helper methods on the
Apiclass (witUrl,orgUrl,projectApiUrl, etc.) to standardize URL construction - Replaces sequential per-item
getWorkItem()calls withbatchGetFullWorkItems()(single REST call per 200 items) andbatchGetComments()(parallel with concurrency=5) - Removes
autoFetchHistoryfunctionality from query downloads (history remains available via dedicatedhistory synccommand)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/url.ts | New utility module providing buildUrl(), appendQueryParamsToSearchParams(), and withQueryParams() functions for URL construction |
| src/azure-devops/api.ts | Adds static URL builder methods and new batch fetch methods (batchGetFullWorkItems, batchGetComments); refactors ~15 URL constructions to use new helpers |
| src/azure-devops/commands/workitem.ts | Refactors download logic into 3 phases (cache check, batch fetch, save) to leverage new batch methods; removes unused CACHE_TTL import |
| src/azure-devops/commands/query.ts | Removes autoFetchHistory functionality and --without-history option; removes unused imports |
| src/azure-devops/timelog-api.ts | Refactors URL construction in constructor and queryTimeLogs() to use buildUrl() |
| src/azure-devops/commands/timelog/configure.ts | Refactors Azure Functions API URL construction to use buildUrl() |
| src/azure-devops/commands/history-search.ts | Updates fetchWorkItemsBatch() to use Api.orgUrl() and Api.workItemWebUrl() static methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return searchParams; | ||
| } | ||
|
|
||
| export function buildUrl({ base, segments = [], queryParams, keepTrailingSlash = true }: BuildUrlOptions): string { |
There was a problem hiding this comment.
The buildUrl function has keepTrailingSlash defaulting to true, which preserves trailing slashes from the last segment. However, most Azure DevOps API URLs should not have trailing slashes. While the current usage patterns don't seem to include trailing slashes in segments, the default behavior could be unexpected. Consider documenting this behavior more clearly or changing the default to false, which is more common for API URLs.
| export function buildUrl({ base, segments = [], queryParams, keepTrailingSlash = true }: BuildUrlOptions): string { | |
| /** | |
| * Build a URL from a base, optional path segments, and query parameters. | |
| * | |
| * By default, the resulting URL will NOT preserve a trailing slash from the last | |
| * segment or base path (`keepTrailingSlash` defaults to `false`), which aligns | |
| * with typical Azure DevOps API URL conventions. Set `keepTrailingSlash` to | |
| * `true` to preserve a trailing slash when present in the last segment. | |
| */ | |
| export function buildUrl({ base, segments = [], queryParams, keepTrailingSlash = false }: BuildUrlOptions): string { |
src/azure-devops/timelog-api.ts
Outdated
| const endpoint = buildUrl({ | ||
| base: "/timelog/query", | ||
| queryParams: { | ||
| FromDate: params.FromDate, | ||
| ToDate: params.ToDate, | ||
| projectId: params.projectId, | ||
| workitemId: params.workitemId ? String(params.workitemId) : undefined, | ||
| userId: params.userId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
In queryTimeLogs, buildUrl is called with base="/timelog/query" to add query parameters, creating a relative URL (e.g., "/timelog/query?FromDate=..."). This is then passed to the request method which calls buildUrl again with this relative URL as a segment. While this works due to buildUrl's path normalization, it's unnecessarily complex and could be confusing. Consider either: (1) building the full URL once in queryTimeLogs, or (2) passing queryParams directly to the request method and having it handle the URL construction.
| const endpoint = buildUrl({ | |
| base: "/timelog/query", | |
| queryParams: { | |
| FromDate: params.FromDate, | |
| ToDate: params.ToDate, | |
| projectId: params.projectId, | |
| workitemId: params.workitemId ? String(params.workitemId) : undefined, | |
| userId: params.userId, | |
| }, | |
| }); | |
| const searchParams = new URLSearchParams(); | |
| if (params.FromDate) searchParams.set("FromDate", params.FromDate); | |
| if (params.ToDate) searchParams.set("ToDate", params.ToDate); | |
| if (params.projectId) searchParams.set("projectId", params.projectId); | |
| if (params.workitemId !== undefined && params.workitemId !== null) { | |
| searchParams.set("workitemId", String(params.workitemId)); | |
| } | |
| if (params.userId) searchParams.set("userId", params.userId); | |
| const queryString = searchParams.toString(); | |
| const endpoint = queryString ? `/timelog/query?${queryString}` : "/timelog/query"; |
| export function buildUrl({ base, segments = [], queryParams, keepTrailingSlash = true }: BuildUrlOptions): string { | ||
| const [basePath, existingQuery] = base.split("?"); | ||
| const hasBase = basePath.trim().length > 0; | ||
| const normalizedSegments = segments.map((segment) => { | ||
| if (hasBase && (segment.startsWith("http://") || segment.startsWith("https://"))) { | ||
| try { | ||
| return new URL(segment).pathname; | ||
| } catch { | ||
| return segment; | ||
| } | ||
| } | ||
| return segment; | ||
| }); | ||
| const lastSegment = segments.length > 0 ? segments[segments.length - 1] : basePath; | ||
| const hadTrailingSlash = lastSegment.endsWith("/"); | ||
| let normalizedPath = [basePath, ...normalizedSegments] | ||
| .map((segment) => segment.replace(/^\/+|\/+$/g, "")) | ||
| .filter(Boolean) | ||
| .join("/"); | ||
| if (keepTrailingSlash && hadTrailingSlash && normalizedPath.length > 0) { | ||
| normalizedPath += "/"; | ||
| } | ||
| const searchParams = new URLSearchParams(existingQuery ?? ""); | ||
| if (queryParams && Object.keys(queryParams).length > 0) { | ||
| appendQueryParamsToSearchParams(queryParams, searchParams); | ||
| } | ||
| const queryString = searchParams.toString(); | ||
| return queryString ? `${normalizedPath}?${queryString}` : normalizedPath; |
There was a problem hiding this comment.
The buildUrl function has a potential issue when the base path is empty or consists only of whitespace. On line 30, hasBase checks if basePath.trim().length > 0, but this check is only used to determine whether to extract pathnames from URL-like segments (lines 32-38). However, on line 43, if basePath is an empty string or whitespace, it will still be included in the array passed to map and filter. While filter(Boolean) will remove empty strings, this logic could be more explicit. Consider validating that base is not empty or documenting that empty base is supported.
| export function buildUrl({ base, segments = [], queryParams, keepTrailingSlash = true }: BuildUrlOptions): string { | ||
| const [basePath, existingQuery] = base.split("?"); | ||
| const hasBase = basePath.trim().length > 0; | ||
| const normalizedSegments = segments.map((segment) => { | ||
| if (hasBase && (segment.startsWith("http://") || segment.startsWith("https://"))) { | ||
| try { | ||
| return new URL(segment).pathname; | ||
| } catch { | ||
| return segment; | ||
| } | ||
| } | ||
| return segment; | ||
| }); | ||
| const lastSegment = segments.length > 0 ? segments[segments.length - 1] : basePath; | ||
| const hadTrailingSlash = lastSegment.endsWith("/"); | ||
| let normalizedPath = [basePath, ...normalizedSegments] | ||
| .map((segment) => segment.replace(/^\/+|\/+$/g, "")) | ||
| .filter(Boolean) | ||
| .join("/"); | ||
| if (keepTrailingSlash && hadTrailingSlash && normalizedPath.length > 0) { | ||
| normalizedPath += "/"; | ||
| } | ||
| const searchParams = new URLSearchParams(existingQuery ?? ""); | ||
| if (queryParams && Object.keys(queryParams).length > 0) { | ||
| appendQueryParamsToSearchParams(queryParams, searchParams); | ||
| } | ||
| const queryString = searchParams.toString(); | ||
| return queryString ? `${normalizedPath}?${queryString}` : normalizedPath; |
There was a problem hiding this comment.
The buildUrl function returns paths without protocols (e.g., "org/project/_apis/wit/...?api-version=7.1"). This works for the current usage where config.org already includes the full URL (e.g., "https://dev.azure.com/orgname"), but could be confusing if someone passes just a domain name as the base. Consider adding a comment documenting that base should include the protocol (https://) or adding validation to ensure the returned URL is valid.
| async batchGetComments(ids: number[], concurrency = 5): Promise<Map<number, Comment[]>> { | ||
| const result = new Map<number, Comment[]>(); | ||
|
|
||
| for (let i = 0; i < ids.length; i += concurrency) { | ||
| const batch = ids.slice(i, i + concurrency); | ||
| const promises = batch.map(async (id) => { | ||
| const commentsUrl = Api.witUrlPreview(this.config, ["workItems", String(id), "comments"]); | ||
| const commentsData = await this.get<{ | ||
| comments: Array<{ id: number; createdBy: { displayName: string }; createdDate: string; text: string }>; | ||
| }>(commentsUrl, `comments for #${id}`); | ||
|
|
||
| const comments: Comment[] = (commentsData.comments || []).map((c) => ({ | ||
| id: c.id, | ||
| author: c.createdBy?.displayName, | ||
| date: c.createdDate, | ||
| text: c.text, | ||
| })); | ||
| return { id, comments }; | ||
| }); | ||
|
|
||
| const results = await Promise.all(promises); | ||
| for (const { id, comments } of results) { | ||
| result.set(id, comments); | ||
| logger.debug(`[api] Work item #${id} has ${comments.length} comments`); | ||
| } | ||
| } |
There was a problem hiding this comment.
The batchGetComments method doesn't handle errors for individual work item comment fetches. If one work item's comment fetch fails (e.g., due to permissions or non-existent work item), the entire Promise.all will reject and no comments will be returned for any work items in that batch. Consider wrapping each fetch in a try-catch to handle individual failures gracefully and continue processing other work items, logging warnings for failed fetches.
| const [batchFields, batchComments] = await Promise.all([ | ||
| api.batchGetFullWorkItems(needsFetch), | ||
| api.batchGetComments(needsFetch), | ||
| ]); |
There was a problem hiding this comment.
If the batchGetComments call fails entirely (line 272), the Promise.all will reject and no work items will be processed, even though batchGetFullWorkItems might have succeeded. Consider using Promise.allSettled instead of Promise.all to allow partial success - if comments fail to fetch, you could still save work items without comments and log a warning.
Add src/utils/json-schema.ts with inferSchema() and formatSchema() supporting 3 output modes (skeleton, typescript, schema) and compact/pretty formatting. Smart array merging, optional field detection, null unions, and singularized TypeScript interface naming. CLI tool: tools json-schema (reads from file or stdin).
Full HAR 1.2 analyzer with 19 commands, interactive mode, MCP server, and a reference system that eliminates data repetition for LLM efficiency. Core: parser, session manager, ref store, formatter, query engine. Commands: load, dashboard, list, show, expand (--schema), domains, domain, search, headers, waterfall, errors, security, size, redirects, cookies, diff, export, sessions, mcp. Also adds: src/utils/table.ts (general table formatter), matchGlob() in string.ts, formatDuration rounding fix in format.ts, utility convention in CLAUDE.md, and analyze-har skill for Claude. docs(tools): Add documentation for HAR Analyzer, JSON Schema, and Claude plugins - Add HAR Analyzer (#24) and JSON Schema (#25) to Available Tools table - Add detailed Tool Details sections for both new tools - Update Claude Code Plugin section with all 6 skills and 3 commands - Add usage examples for every command and skill trigger phrases feat(claude-resume, cc): cc session finder/tool feat(claude-resume, claude-history): SQLite-cached session listing, import lib directly Replace JSON file cache and subprocess spawning with: - session_metadata table in claude-history SQLite cache (incremental, mtime-based) - getSessionListing() in lib.ts with cached metadata extraction - Direct import from @app/claude-history/lib instead of spawning `tools claude-history` refactor(claude-resume): clean up to match repo standards - Add error handling with try/catch + logger - Extract main() from action handler - Use extractProjectName from claude-history lib for robust project detection - DRY up DisplaySession creation with shared toDisplay() - Flatten 3-tier search into linear flow with single select+resume - Extract magic numbers into named constants - Add Options interface - Add claude-resume and cc to README tools table refactor(claude-resume): use formatRelativeTime from utils/format Add compact option to formatRelativeTime ("5m ago", "3h ago", "2d ago") and replace local formatDate in claude-resume with it. feat(claude-resume): show progress during session loading and content search Add onProgress callback to getSessionListing and searchConversations. Spinner now shows "42/318 (13%) a1b2c3d4..." while scanning files. fix(claude-resume): scope file scanning to current project directory - Add resolveProjectDir() + findConversationFilesInDir() for exact dir scoping instead of glob-based project matching - Add getSessionMetadataByDir() SQL query scoped by file_path prefix - Default mode scans only current project (~200 files), not all (~5800) - --all-projects still scans everything refactor: extract findClaudeCommand, detectCurrentProject to utils/claude fix(claude-resume): scope content search to current project, show subagent counts - searchByContent now passes project filter to searchConversations - Spinner stop shows "201 sessions (+ 57 subagents, total 258)" - getSessionListing returns SessionListingResult with total/subagent counts - Add getSessionMetadataByDir for SQL-scoped queries by file path prefix perf(claude-history): instant summary-only search via SQLite cache When summaryOnly is set, searchConversations now queries the session_metadata SQLite table directly instead of parsing every JSONL file. Searches title, summary, and firstPrompt fields. Reduces search from ~5s (parsing 200+ files) to <10ms. fix(utils/claude): include cc in findClaudeCommand lookup fix(claude-resume): use shell to resolve claude command aliases/functions findClaudeCommand now spawns through $SHELL -ic to find shell functions and aliases, and verifies output contains "Claude Code" to avoid matching /usr/bin/cc (C compiler). resumeSession also spawns through shell so ccc/cc aliases work at runtime.
… bugs har-analyzer: - Wire --format (md/json/toon) as working global flag via printFormatted - Wire --session to pass through to requireSession in all 14 commands - DRY: extract requireSession() to SessionManager, parseEntryIndex to query-engine - Remove unimplemented MCP stubs (har_flow, har_diff), narrow har_analyze - Fix isInterestingMimeType logic, safe URL parsing in cookies, remove dead types json-schema: - Fix parseJSON(null) treated as parse failure (use inline try/catch) - Fix normalizeType losing union info during merge (hasType helper) - Add --mode validation via Commander .choices() - Export OutputMode and SchemaNode types from utils - DRY TypeScript formatters via shared collectTypeScriptInterfaces
- Remove dead entriesByUrl map in redirects, use URL-parsed matching - Clamp HAR bodySize/content.size to 0 (spec allows -1 for unknown) - Fix double ellipsis in ref-store preview - Add try/catch error handling in interactive mode - Expand scanApiKeys to also check request headers - Fix skeleton formatters to handle union types via hasType()
Fixes across multiple files from CodeRabbit/Copilot/Gemini reviews: - README: capitalize GitHub branding in skills table - SKILL.md: mark --review-comments as legacy, prefer --reviews - claude-history: scan subagents/ subdirectory in findConversationFilesInDir - github/review: swap -r to --respond, remove -r from --repo - github/output: demote H1 to H2 when embedding review markdown - github/review-output: H3 for file groups, fix reply preview truncation - github/review-threads: separate old/new diff counts, word-boundary \bbug\b, tighten medium severity heuristics, remove try/catch in markThreadResolved - har-analyzer/export: preserve browser/pages metadata in exported HAR - har-analyzer/search: include request body in searchInBody - har-analyzer/security: deduplicate patterns, fix base64 padding, use printFormatted for empty findings - har-analyzer/show: guard bodySize -1, use printFormatted in expand - har-analyzer/waterfall: NaN fallback for malformed dates - har-analyzer/query-engine: bounds-check domain indices - har-analyzer/session-manager: check file exists before reading - har-analyzer/mcp: guard bodySize -1 in request body display - utils/string: escape ? in matchGlob regex - plans: reconcile OutputFormat type with existing md|json|toon
…/azure-devops-batches Keep typed API responses, unified getWorkItems(), QueryParams export, attachments download phase, and getWorkItemRevisions from both branches.
- Replace duplicate stripAnsi with import from utils/string (#18) - Fix wrapToWidth ANSI-aware truncation (#1/#3/#22) - Add await to recursive executeTool call (#9/#20) - Guard cursor when filtered list is empty (#5/#19) - Use basename() instead of split("/").pop() (#10/#16) - Validate --width NaN input (#14) - Re-display watch message after screen clear (#15)
- Replace duplicate stripAnsi with import from utils/string (#18) - Fix wrapToWidth ANSI-aware truncation (#1/#3/#22) - Add await to recursive executeTool call (#9/#20) - Guard cursor when filtered list is empty (#5/#19) - Use basename() instead of split("/").pop() (#10/#16) - Validate --width NaN input (#14) - Re-display watch message after screen clear (#15)
* fix: clean error formatting and fuzzy match for tools entry point * feat(tools): add discovery, introspection, and search-select modules - discovery.ts: scans src/ for tools, extracts descriptions from README.md - introspect.ts: runs --help on tools and parses Commander output into structured data - search-select.ts: single-select search prompt with onHighlight callback, modeled on search-multiselect * feat(tools): interactive browser with search, README preview, and subcommand explorer * feat(markdown): add render options and CLI flags (--watch, --width, --theme, --no-color) * fix: improve fuzzy matching to handle trailing dashes and segment matching "tools cli-" now correctly matches "markdown-cli" by stripping trailing dashes and checking dash-separated segments. * feat(markdown-cli): add demo subcommand with interactive template gallery * Revert "feat(markdown-cli): add demo subcommand with interactive template gallery" This reverts commit f5c84db. * feat(markdown): implement theme palette system (dark/light/minimal) Wire up the --theme flag to actual color palettes that change mermaid blocks, table borders, and alert colors per theme. * fix: address PR #28 code review feedback - Replace duplicate stripAnsi with import from utils/string (#18) - Fix wrapToWidth ANSI-aware truncation (#1/#3/#22) - Add await to recursive executeTool call (#9/#20) - Guard cursor when filtered list is empty (#5/#19) - Use basename() instead of split("/").pop() (#10/#16) - Validate --width NaN input (#14) - Re-display watch message after screen clear (#15) * fix(markdown-cli/tools): address PR #28 review feedback - Remove stray n8n.json (unrelated to PR scope) - fix(markdown-cli): reject invalid --theme values via Commander .choices() (Thread #23) - fix(tools): guard discoverTools against missing srcDir with existsSync check (Thread #24) - fix(markdown): use display width for emoji/wide chars in wrapToWidth (Thread #25)
- Use Promise.allSettled in showConfig to handle stale tokens (#14) - Clamp pct in renderBar to prevent RangeError (#17) - Clear firedThresholds on period reset in watch mode (#18) - Wrap entire getKeychainCredentials in try/catch for non-macOS (#19) - Escape sound parameter in AppleScript notifications (#20) - Fix import.meta.dir off-by-one in update tool (#21) - Use isCancelled() in withCancel() for light-mode cancel (#22) - Remove 100% cap on projected usage
- Use Promise.allSettled in showConfig to handle stale tokens (#14) - Clamp pct in renderBar to prevent RangeError (#17) - Clear firedThresholds on period reset in watch mode (#18) - Wrap entire getKeychainCredentials in try/catch for non-macOS (#19) - Escape sound parameter in AppleScript notifications (#20) - Fix import.meta.dir off-by-one in update tool (#21) - Use isCancelled() in withCancel() for light-mode cancel (#22) - Remove 100% cap on projected usage
Summary
buildUrl()utility (src/utils/url.ts) and static URL helpers onApiclass (witUrl,orgUrl,projectApiUrl, etc.)api.ts,history-search.ts,timelog-api.ts, andtimelog/configure.tsto use the new helpersgetWorkItem()calls withbatchGetFullWorkItems()(single REST call per 200 items) +batchGetComments()(parallel with concurrency=5)autoFetchHistoryfrom query downloads (was scanning entire project history page-by-page; history remains available viatools azure-devops history sync)Test plan
tsgo --noEmitpasses (only pre-existing error in history-search.ts:122)tools azure-devops query "<query-url>" --download-workitems --category test --force --verbose— batch fetch logs visible--force— cached items skippedtools azure-devops workitem <id>— single-item still workstools azure-devops history sync --batch— still worksSummary by CodeRabbit
Bug Fixes
Breaking Changes
--without-historyCLI option from the query command; history is now automatically downloaded when downloading work items.