fix(canvas): cap chat history buffer at MAX_MESSAGES (mobile-chat audit F4) #2978

Merged
devops-engineer merged 4 commits from fix/canvas-chat-history-cap-f4 into main 2026-06-19 22:04:42 +00:00
3 changed files with 198 additions and 55 deletions
+81 -53
View File
@@ -13,87 +13,115 @@ name: template-delivery-e2e
# /configs/plugins/seo-all/. The e2e asserts the skill arrives via the
# PLUGIN channel and NOT the asset channel (negative control).
#
# STAGED ROLLOUT (do NOT make required until green):
# Phase 1 (now): advisory — runs on the relevant paths + main + dispatch.
# Asserts the new two-channel contract.
# Phase 2 (after this goes green twice on the new contract): remove
# continue-on-error and add this check to branch protection
# required_status_checks so a future delivery regression is
# merge-blocking.
# STAGED ROLLOUT (now COMPLETE — this gate is merge-blocking):
# Phase 1 (done): advisory — asserted the new two-channel contract.
# Phase 2a (done, f6155d68): HARDENED the asset-channel assertions (C
# config.yaml, D prompts) to poll within E2E_ASSET_SETTLE_SECS,
# killing the false stub from a transient /configs read
# (`curl: (28) ... 0 bytes`). Banked a green main run, which
# lint-pre-flip-continue-on-error requires before the flip.
# Phase 2b (THIS change, mc#2996): FLIP to merge-blocking —
# • continue-on-error removed → a real delivery regression fails;
# • `on: paths:` removed (required workflows must not be
# path-filtered); path-scoping moved to the detect-changes job
# (profile `template-delivery`) and applied per-step;
# • the emitted context is added to .gitea/required-contexts.txt
# and to branch-protection required_status_checks (as
# "... (pull_request)") so a delivery PR cannot merge unless a
# fresh seo-agent provisions and BOTH channels verify.
#
# Cost: provisions ONE throwaway tenant + ONE seo-agent (real EC2), teardown
# trap deletes the org even on failure. Path-filtered so it only runs when the
# delivery code actually changes.
# trap deletes the org even on failure. The workflow has NO `on: paths:` filter:
# a REQUIRED-check workflow must not carry one (lint-required-no-paths.py /
# feedback_path_filtered_workflow_cant_be_required — a docs-only PR would never
# emit the context, Gitea reports it `pending`, and the PR wedges forever).
# Instead the detect-changes job applies the same path-scoping at RUNTIME, and
# the delivery job's real steps are gated on its output: a non-delivery PR emits
# SUCCESS cheaply (no provision), while a delivery PR runs the full e2e and
# BLOCKS on failure. Mirrors the e2e-api / peer-visibility required-gate shape.
on:
workflow_dispatch: {}
push:
branches: [main]
paths:
- 'workspace-server/internal/provisioner/template_assets.go'
- 'workspace-server/internal/provisioner/gitea_template_assets.go'
- 'workspace-server/internal/provisioner/cp_provisioner.go'
- 'workspace-server/internal/handlers/platform_agent.go'
- 'workspace-server/cmd/server/main.go'
- 'workspace-server/internal/handlers/org_import.go'
- 'workspace-server/internal/handlers/workspace.go'
- 'workspace-server/internal/handlers/template_plugins.go'
- 'workspace-server/internal/handlers/plugins_reconcile.go'
- 'workspace-server/internal/handlers/registry.go'
- 'workspace-server/internal/handlers/plugins_install_pipeline.go'
- 'workspace-server/internal/handlers/plugins_tracking.go'
- 'workspace-server/internal/plugins/source.go'
- 'manifest.json'
- 'tests/e2e/test_template_delivery_e2e.sh'
- '.gitea/workflows/template-delivery-e2e.yml'
pull_request:
paths:
- 'workspace-server/internal/provisioner/template_assets.go'
- 'workspace-server/internal/provisioner/gitea_template_assets.go'
- 'workspace-server/internal/provisioner/cp_provisioner.go'
- 'workspace-server/internal/handlers/platform_agent.go'
- 'workspace-server/cmd/server/main.go'
- 'workspace-server/internal/handlers/org_import.go'
- 'workspace-server/internal/handlers/workspace.go'
- 'workspace-server/internal/handlers/template_plugins.go'
- 'workspace-server/internal/handlers/plugins_reconcile.go'
- 'workspace-server/internal/handlers/registry.go'
- 'workspace-server/internal/handlers/plugins_install_pipeline.go'
- 'workspace-server/internal/handlers/plugins_tracking.go'
- 'workspace-server/internal/plugins/source.go'
- 'manifest.json'
- 'tests/e2e/test_template_delivery_e2e.sh'
- '.gitea/workflows/template-delivery-e2e.yml'
branches: [main]
concurrency:
group: template-delivery-e2e-${{ github.ref }}
cancel-in-progress: true
group: template-delivery-e2e-${{ github.event.pull_request.head.sha || github.sha }}
cancel-in-progress: false
jobs:
# Job renamed for the RFC#2843 #32 two-channel contract (config+prompts via
# the asset channel; seo-all installs via the post-online plugin reconcile,
# not at boot). Renaming the job changes the emitted status context.
# bp-exempt: advisory Phase-1 gate (continue-on-error, mc#2996) — informational, not a required BP context.
# Runtime path-scoping (replaces the removed `on: paths:`). Mirrors the
# e2e-api / peer-visibility detect-changes shape. Outputs `delivery=true`
# when the diff touches the delivery surface; the gate job runs the real e2e
# only then. bp-exempt: helper job; the REQUIRED context is the `delivery`
# job below, not this one.
detect-changes:
runs-on: ubuntu-latest
continue-on-error: false
outputs:
delivery: ${{ steps.decide.outputs.delivery }}
debug: ${{ steps.decide.outputs.debug }}
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- id: decide
env:
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
PUSH_BEFORE: ${{ github.event.before }}
run: |
python3 .gitea/scripts/detect-changes.py \
--profile template-delivery \
--event-name "${{ github.event_name }}" \
--pr-base-sha "$PR_BASE_SHA" \
--base-ref "$PR_BASE_REF" \
--push-before "$PUSH_BEFORE" || {
# Script crash → fail OPEN so the gate runs rather than silently
# no-oping a potentially-breaking delivery PR.
echo "delivery=true" >> "$GITHUB_OUTPUT"
echo "debug=detect-script-error event=${{ github.event_name }}" >> "$GITHUB_OUTPUT"
exit 0
}
echo "debug=profile=template-delivery event=${{ github.event_name }}" >> "$GITHUB_OUTPUT"
# ONE job (no job-level `if:`) that ALWAYS runs and reports under the
# required-check name. Real work is gated per-step on
# `needs.detect-changes.outputs.delivery` — a non-delivery PR runs only the
# no-op step and emits SUCCESS (branch-protection-clean; a job-level `if:`
# would emit a SKIPPED check run that fails the required-check eval — see
# e2e-api.yml's PR#2264 note). A delivery PR runs the full e2e and, with no
# continue-on-error, BLOCKS the merge on failure.
# bp-required: yes — mc#2996 / RFC#2843 #37: this context is merge-blocking;
# branch protection lists "<this> (pull_request)".
delivery:
needs: detect-changes
# No colon in the name — lint-required-context's PyYAML AST parse rejects an
# unquoted scalar containing a colon.
name: Template-asset delivery (fresh seo-agent — config+prompts via asset channel, seo-all via plugin reconcile)
runs-on: ubuntu-latest
# Phase 1: advisory. Remove this line in Phase 2 to make it merge-blocking.
# mc#2996 — Phase 2 promotion tracker (remove continue-on-error; forced 14d renewal cadence).
continue-on-error: true # mc#2996
timeout-minutes: 30
env:
MOLECULE_CP_URL: ${{ vars.CP_URL || 'https://staging-api.moleculesai.app' }}
MOLECULE_ADMIN_TOKEN: ${{ secrets.CP_ADMIN_API_TOKEN }}
E2E_EXPECTED_MODEL: moonshot/kimi-k2.6
steps:
- uses: actions/checkout@v4
- name: No-op pass (delivery surface unchanged in this diff)
if: needs.detect-changes.outputs.delivery != 'true'
run: |
echo "No delivery-surface changes — gate satisfied without provisioning."
echo "::notice::template-delivery-e2e no-op pass (detect-changes: ${{ needs.detect-changes.outputs.debug }})."
- if: needs.detect-changes.outputs.delivery == 'true'
uses: actions/checkout@v4
- name: Verify required secret present
if: needs.detect-changes.outputs.delivery == 'true'
run: |
if [ -z "${MOLECULE_ADMIN_TOKEN:-}" ]; then
echo "::error::CP_ADMIN_API_TOKEN secret not set — cannot run delivery e2e"
exit 2
fi
- name: Run template-asset delivery e2e
if: needs.detect-changes.outputs.delivery == 'true'
run: bash tests/e2e/test_template_delivery_e2e.sh
@@ -0,0 +1,110 @@
// @vitest-environment jsdom
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { renderHook, act, waitFor } from "@testing-library/react";
import { useChatHistory } from "../useChatHistory";
const apiGet = vi.fn();
vi.mock("@/lib/api", () => ({
api: {
get: (...args: unknown[]) => apiGet(...args),
},
}));
function makeMsg(id: number, ts?: string): {
id: string;
role: "user" | "agent" | "system";
content: string;
timestamp: string;
} {
return {
id: `msg-${id}`,
role: "user",
content: `content ${id}`,
timestamp: ts ?? new Date(2026, 0, 1, 0, 0, id).toISOString(),
};
}
function mockContainer(): React.RefObject<HTMLDivElement | null> {
const el = document.createElement("div");
return { current: el as HTMLDivElement };
}
describe("useChatHistory — message buffer cap (mobile-chat audit F4)", () => {
beforeEach(() => {
apiGet.mockReset();
});
afterEach(() => {
vi.clearAllMocks();
});
it("caps the in-memory message buffer at MAX_MESSAGES when loading older history", async () => {
// Seed initial messages so hasMore is true and there is an oldest timestamp.
const initial = Array.from({ length: 10 }, (_, i) => makeMsg(i + 1));
apiGet.mockResolvedValueOnce({
messages: initial,
reached_end: false,
});
const { result } = renderHook(() => useChatHistory("ws-1", mockContainer()));
await waitFor(() => expect(result.current.loading).toBe(false));
expect(result.current.messages).toHaveLength(10);
// Each older-load prepends 20 messages with timestamps further in the
// past. After enough loads the buffer should exceed 500 and the oldest
// (front) ones must be discarded while the most recent messages stay.
const baseTime = new Date("2026-01-01T00:00:00.000Z").getTime();
let nextId = 1000;
for (let load = 0; load < 30; load++) {
const older = Array.from({ length: 20 }, (_, i) => {
const id = nextId++;
// Earlier loads are more recent; later loads are older history.
const ts = new Date(baseTime - (load * 1000) - i).toISOString();
return makeMsg(id, ts);
});
apiGet.mockResolvedValueOnce({
messages: older,
reached_end: false,
});
await act(async () => {
await result.current.loadOlder();
});
}
expect(result.current.messages.length).toBeLessThanOrEqual(500);
// The most recent messages (the initial seed) must still be present.
expect(result.current.messages.some((m) => m.id === "msg-1")).toBe(true);
// Some of the oldest loaded history should have been evicted by the cap.
expect(result.current.messages.some((m) => m.id === "msg-1580")).toBe(false);
});
it("caps the in-memory message buffer at MAX_MESSAGES on live append", async () => {
// Seed a small initial history.
const initial = Array.from({ length: 10 }, (_, i) => makeMsg(i + 1));
apiGet.mockResolvedValueOnce({
messages: initial,
reached_end: true,
});
const { result } = renderHook(() => useChatHistory("ws-1"));
await waitFor(() => expect(result.current.loading).toBe(false));
expect(result.current.messages).toHaveLength(10);
// Append enough distinct messages to exceed MAX_MESSAGES.
await act(async () => {
for (let i = 11; i <= 520; i++) {
result.current.appendMessageDeduped(makeMsg(i));
}
});
expect(result.current.messages.length).toBeLessThanOrEqual(500);
// The newest appended messages must survive the cap.
expect(result.current.messages.some((m) => m.id === "msg-520")).toBe(true);
expect(result.current.messages.some((m) => m.id === "msg-519")).toBe(true);
// The oldest initial messages should have been evicted.
expect(result.current.messages.some((m) => m.id === "msg-1")).toBe(false);
expect(result.current.messages.some((m) => m.id === "msg-10")).toBe(false);
});
});
@@ -6,6 +6,11 @@ import { type ChatMessage, appendMessageDeduped as appendMessageDedupedFn } from
const INITIAL_HISTORY_LIMIT = 10;
const OLDER_HISTORY_BATCH = 20;
// Mobile-chat audit F4: prevent the in-memory message buffer from growing
// without bound on long-lived chats. Server-side pagination still works;
// we just discard the oldest messages client-side to bound re-render cost
// and scroll jank on low-end mobile. A virtualized list is the larger fix.
const MAX_MESSAGES = 500;
async function loadMessagesFromDB(
workspaceId: string,
@@ -114,7 +119,7 @@ export function useChatHistory(
return;
}
if (older.length > 0) {
setMessages((prev) => [...older, ...prev]);
setMessages((prev) => [...older, ...prev].slice(-MAX_MESSAGES));
} else {
scrollAnchorRef.current = null;
}
@@ -134,7 +139,7 @@ export function useChatHistory(
loadInitial,
loadOlder,
appendMessageDeduped: (msg: ChatMessage) =>
setMessages((prev) => appendMessageDedupedFn(prev, msg)),
setMessages((prev) => appendMessageDedupedFn(prev, msg).slice(-MAX_MESSAGES)),
setMessages,
scrollAnchorRef,
};