Skip to content

Share one gh-aw logs snapshot across activity collectors - #3219

Open
pelikhan with Copilot wants to merge 16 commits into
mainfrom
copilot/refactor-activity-yml-download
Open

Share one gh-aw logs snapshot across activity collectors#3219
pelikhan with Copilot wants to merge 16 commits into
mainfrom
copilot/refactor-activity-yml-download

Conversation

Copilot AI commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Activity collection repeatedly scanned workflow history and split related data across caches. This refactor downloads all repository workflow logs once, then derives dashboard data from one persisted snapshot.

Changes

  • Single collection

    • Run one bounded gh aw logs --json invocation.
    • Persist raw JSON and downloaded artifacts.
  • Shared processing

    • Derive AI Credit, security, and operational-value records from the cached logs JSON.
    • Remove per-workflow operational-value history scans and artifact fallbacks.
  • Unified cache

    • Store logs, raw JSON, derived dashboard records, and operational-value history in cao-activity.
    • Preserve prior observations when current evidence is unavailable.
  • Contracts

    • Update workflow assertions and cache documentation for the single-snapshot model.

Copilot AI and others added 7 commits September 6, 2026 04:47
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan September 6, 2026 04:59
@pelikhan
pelikhan marked this pull request as ready for review September 6, 2026 05:00
Copilot AI lite review requested due to automatic review settings September 6, 2026 05:00
@pelikhan

pelikhan commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

@copilot add core.... logging to debug caching and downloading

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new cache/merge logic can unintentionally prevent filling previously “unavailable” operational-value runs from later logs snapshots and can overwrite a usable shared logs snapshot with an empty placeholder on transient collection failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refactors CAO activity data acquisition to download a single bounded gh aw logs --json snapshot per activity run, persist it in the cao-activity cache, and derive AI Credit, security, and operational-value records from that shared snapshot (removing per-workflow operational-value history scans and artifact fallbacks).

Changes:

  • Switch AI Credit usage collection to one repository-wide gh aw logs --json invocation (no per-workflow targets), persisting the raw logs JSON and artifacts into the activity cache.
  • Refactor operational-value collection to read grader results from the shared logs JSON and merge them with cached observations.
  • Update workflow contracts, documentation, and add focused unit tests for the new shared-snapshot model.
File summaries
File Description
tests/unit/workflow-contract.test.mjs Updates assertions to enforce the single-snapshot model and new cache paths/env vars.
tests/unit/dashboard-operational-values.test.mjs Adds a unit test validating operational-values processing from shared logs JSON.
tests/unit/dashboard-aic-usage.test.mjs Updates AIC usage test to assert persisted shared logs JSON output and new argument behavior.
specs/data-acquisition-audit.md Updates acquisition inventory/audit narrative for the shared snapshot approach.
specs/data-acquisition-audit-history.md Adds a dated entry documenting the shared activity log cache change.
docs/operations.md Updates operational documentation to describe the single gh aw logs --json snapshot and downstream processing.
dashboard/report/operational-values.mjs Replaces report/artifact-based collection with shared logs JSON parsing + cache merge.
dashboard/report/aic-usage.mjs Performs one repository-wide logs download and optionally persists raw logs JSON for reuse.
activity/README.md Documents new cao-activity snapshot contents (gh-aw-logs/, gh-aw-logs.json).
activity/github-telemetry.mjs Logs core quota remaining alongside telemetry observations.
.github/workflows/activity.yml Wires shared logs JSON/artifacts paths into collectors; removes separate operational-value cache steps.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 579 to 590
} catch (error) {
collectionAvailable = false;
log.warning`AI Credit usage unavailable: ${error.message}`;
if (logsPath) {
await mkdir(path.dirname(logsPath), { recursive: true });
await writeFile(logsPath, '{"runs":[]}\n');
}
}
} else if (logsPath) {
await mkdir(path.dirname(logsPath), { recursive: true });
await writeFile(logsPath, '{"runs":[]}\n');
}
Comment on lines 119 to 121
const cachedRunKeys = new Set(cachedRecords
.map((record) => operationalValueRunIdentity(record))
.filter(Boolean));
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 6, 2026 05:06
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI and others added 5 commits September 6, 2026 05:07
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot add core.... logging to debug caching and downloading

Added non-secret diagnostics for activity-cache hit/miss and size, plus gh-aw logs download/cache paths and record counts. Addressed in 603ebba.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The shared gh-aw-logs.json snapshot can be overwritten with an empty placeholder on download failure (or when no runs are selected), discarding previously restored evidence and undermining the stated “preserve prior observations” behavior.

Review details

Suppressed comments (2)

dashboard/report/aic-usage.mjs:594

  • When no workflow runs are selected, this block always rewrites REPORT_GH_AW_LOGS to an empty JSON file. If the shared cache already contains a prior logs snapshot, overwriting it here unnecessarily discards evidence that other collectors (or later steps) might still use. Consider only creating the empty file when it doesn't already exist.
    } else if (logsPath) {
      await mkdir(path.dirname(logsPath), { recursive: true });
      await writeFile(logsPath, '{"runs":[]}\n');
      log.info`Cached empty gh-aw logs JSON at ${logsPath}; no workflow runs were selected`;
    }

dashboard/report/aic-usage.mjs:588

  • When the gh-aw download fails, the code overwrites any previously-restored shared logs snapshot with an empty {"runs":[]} file. This discards the last known-good snapshot from the activity cache and can break the "preserve prior observations when current evidence is unavailable" behavior (and downstream consumers that could have reused the prior logs JSON). Prefer preserving an existing logs file, and only write an empty placeholder if no prior snapshot exists.
      } catch (error) {
        collectionAvailable = false;
        log.warning`AI Credit usage unavailable: ${error.message}`;
        if (logsPath) {
          await mkdir(path.dirname(logsPath), { recursive: true });
          await writeFile(logsPath, '{"runs":[]}\n');
          log.info`Cached fallback empty gh-aw logs JSON at ${logsPath} after download failure`;
        }
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 6, 2026 05:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The refactor introduces correctness risks in cross-repository log collection and snapshot retention (plus a cache keying issue that can prevent retrying missing operational-value observations).

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

dashboard/report/aic-usage.mjs:589

  • On gh aw logs failure (including rate limits), the code overwrites REPORT_GH_AW_LOGS with an empty {runs:[]} file. Because the activity job restores the previous cao-activity cache before collecting, this clobbers the last good shared snapshot and prevents downstream collectors from reusing prior evidence, contradicting the "preserve prior observations" goal.
    dashboard/report/operational-values.mjs:123
  • cachedRunKeys currently includes all cached records (including status: "unavailable" placeholders). That means if a run was missing from the logs snapshot once, it will never be retried in future runs even if later snapshots contain the operational-value grader result.

dashboard/report/aic-usage.mjs:16

  • runGhAw no longer accepts/passes workflow targets, but the deployed-workflows inventory can span multiple repositories (per .github/workflows/cao.json allowed-repositories). Without forwarding targets, gh aw logs will only be scoped to the current repository, so AIC/security data for other repositories will never be downloaded.

This issue also appears in the following locations of the same file:

  • line 456
  • line 503
function runGhAw(maxRunsPerWorkflow, outputDirectory) {
  return new Promise((resolve, reject) => {
    const child = spawn("gh", [
      "aw", "logs", "--json",
      "--output", outputDirectory, "--summary-file", "",

dashboard/report/aic-usage.mjs:507

  • The download step is gated on workflowByRunId.size, but the gh-aw logs command should be gated on whether there are any workflow targets to request; also runGhAw should be invoked with the computed targets so non-local repositories are included.
    if (workflowByRunId.size > 0) {
      try {
        log.info`Downloading all agentic workflow logs in one gh-aw CLI invocation (count=${maxRunsPerWorkflow}, output=${temporaryRoot})`;
        const rawResult = await runGhAw(maxRunsPerWorkflow, temporaryRoot);
        const result = JSON.parse(rawResult);

dashboard/report/aic-usage.mjs:470

  • AIC/security collection still builds run selections across inventory.workflows for multiple repositories, but it no longer constructs gh-aw log targets from those workflows. This breaks collection for repositories other than the one the activity workflow is running in.
  const inventory = JSON.parse(await readFile(inventoryPath, "utf8"));
  const runIdsByRepository = new Map();
  const workflowByRunId = new Map();
  let maxRunsPerWorkflow = 0;
  for (const workflow of inventory.workflows || []) {
    const runIds = runIdsByRepository.get(workflow.repository) || new Set();
    const runRecords = new Map((workflow.runHealth?.runRecords || []).map((run) => [Number(run.runId), run]));
    for (const runId of workflow.runHealth?.runIds || []) {
      runIds.add(runId);
      const metadata = { workflow, run: runRecords.get(Number(runId)) || null };
      workflowByRunId.set(Number(runId), metadata);
    }
    runIdsByRepository.set(workflow.repository, runIds);
    maxRunsPerWorkflow = Math.max(maxRunsPerWorkflow, workflow.runHealth?.runIds?.length || 0);
  }
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…-value observations

- Only overwrite the shared gh-aw logs JSON with an empty placeholder when no
  prior snapshot exists, so transient download failures no longer discard a
  usable cached snapshot.
- Exclude previously 'unavailable' operational-value cache records from the
  retry-skip set so later logs snapshots can fill in runs that were missing
  before.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Commit pushed: afc3800

Generated by PR Sous Chef · copilot · auto · 79.1 AIC · ⌖ 7.08 AIC · ⊞ 9.1K

Copilot AI review requested due to automatic review settings September 6, 2026 05:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. An organization owner can allow the organization to be billed for reviews like this by enabling Copilot code review for users without a license.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Verified against the current head commit (afc38001): the preserveOrWriteEmptyLogs helper (dashboard/report/aic-usage.mjs) now checks for an existing snapshot before writing the empty {"runs":[]} placeholder, so a transient gh aw logs failure or empty selection no longer erases a usable cached snapshot. The operational-values.mjs retry-skip set now filters out cached records with status !== "unavailable", so previously-unavailable observations are retried on later runs. Ran targeted unit tests: node --test tests/unit/dashboard-aic-usage.test.mjs tests/unit/dashboard-operational-values.test.mjs — both pass. No further code change required; both review threads are resolved by the existing commit.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

Generated by PR Sous Chef · copilot · auto · 114.7 AIC · ⌖ 17 AIC · ⊞ 9.1K ·
Comment /souschef to run again

@github-actions github-actions Bot mentioned this pull request Sep 6, 2026
Review feedback flagged that the shared-snapshot refactor dropped the
workflow target arguments (repository/path) passed to gh aw logs,
scoping the single invocation to only the current repository and
silently breaking AIC/security collection for other repositories in
the deployed-workflows inventory. Restore the targets array built from
each selected workflow's repository/path and pass it back into
runGhAw, gated on targets.length instead of workflowByRunId.size, while
keeping the shared logs-JSON snapshot caching behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Commit pushed: c965a6f

Generated by PR Sous Chef · copilot · auto · 191.1 AIC · ⌖ 6.65 AIC · ⊞ 9.1K

Copilot AI review requested due to automatic review settings September 6, 2026 11:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. An organization owner can allow the organization to be billed for reviews like this by enabling Copilot code review for users without a license.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants