Skip to content

Commit 5bda953

Browse files
fix(coding-agents): tolerate hidden page triggers (#3742)
Treat an omitted page trigger as unknown rather than drift. Avoid trigger-only PATCH requests to older servers that reject the field. Continue reconciling source-query changes and explicit trigger drift. Document the unknown-trigger behavior and add regression coverage.
1 parent 1f50065 commit 5bda953

2 files changed

Lines changed: 46 additions & 21 deletions

File tree

hindsight-integrations/coding-agents/src/core/hindsight.pages.test.ts

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,6 @@ describe("HindsightClient.seedPages", () => {
286286
expect(patches[0].body).toEqual({
287287
source_query: drifted.source_query,
288288
tags: drifted.tags,
289-
trigger: buildPageTrigger(),
290289
});
291290
});
292291

@@ -321,10 +320,9 @@ describe("HindsightClient.seedPages", () => {
321320
}
322321
});
323322

324-
// The common case in the field: a server older than #3572 reports no trigger at all, so the
325-
// policy is unknowable and gets re-sent. Cheap, idempotent, and self-healing once the server
326-
// starts answering — but it must still be trigger-ONLY, or every run rebuilds all five pages.
327-
it("re-sends the trigger to a server that does not report one", async () => {
323+
// A server older than #3572 does not report a page's trigger. That makes the policy unknowable,
324+
// not divergent; sending a trigger-only PATCH would be rejected by servers older than #3549.
325+
it("does not write when a server does not report a trigger", async () => {
328326
const calls: any[] = [];
329327
stubFetchRouted(calls, [
330328
{
@@ -342,9 +340,35 @@ describe("HindsightClient.seedPages", () => {
342340
const c = new HindsightClient({ apiUrl: "http://x", bank: "repo-a" });
343341
await c.seedPages();
344342

343+
expect(calls).toHaveLength(1); // the tree GET only
344+
expect(calls.every((k) => k.method === "GET")).toBe(true);
345+
});
346+
347+
it("still reconciles source-query drift when the server hides trigger", async () => {
348+
const calls: any[] = [];
349+
const drifted = PAGES[0];
350+
stubFetchRouted(calls, [
351+
{
352+
match: (m, u) => m === "GET" && u.endsWith("/knowledge-base/tree"),
353+
json: {
354+
roots: PAGES.map((p, i) => ({
355+
id: `kp-${i}`,
356+
kind: "page",
357+
name: p.name,
358+
description: p === drifted ? "an older wording of the query" : p.source_query,
359+
})),
360+
},
361+
},
362+
]);
363+
const c = new HindsightClient({ apiUrl: "http://x", bank: "repo-a" });
364+
await c.seedPages();
365+
345366
const patches = calls.filter((k) => k.method === "PATCH");
346-
expect(patches).toHaveLength(PAGES.length);
347-
for (const patch of patches) expect(patch.body).toEqual({ trigger: buildPageTrigger() });
367+
expect(patches).toHaveLength(1);
368+
expect(patches[0].body).toEqual({
369+
source_query: drifted.source_query,
370+
tags: drifted.tags,
371+
});
348372
});
349373

350374
it("names the repository in every seeded query, so synthesis can exclude a dependency's facts", async () => {

hindsight-integrations/coding-agents/src/core/hindsight.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export interface KnowledgeNode {
2424
/** The page's source query (OKF `description`) — what a re-sync compares against. */
2525
description?: string;
2626
/** The page's EFFECTIVE refresh policy, on servers new enough to report it (#3572). Absent
27-
* everywhere else, which `seedPages()` reads as "unknown, re-send it". */
27+
* everywhere else, which `seedPages()` reads as "unknown, leave it alone". */
2828
trigger?: { tags_match?: string };
2929
children?: KnowledgeNode[];
3030
}
@@ -618,23 +618,24 @@ export class HindsightClient {
618618
return;
619619
}
620620
if (r.status !== 409) created++;
621-
} else if (
622-
hit.description !== page.source_query ||
623-
hit.trigger?.tags_match !== pageTrigger.tags_match
624-
) {
621+
} else {
622+
const sourceDrift = hit.description !== page.source_query;
623+
// Older servers omit trigger from the tree, so an absent value means unknown rather
624+
// than drift. Those servers also reject a trigger-only PATCH as an empty update.
625+
const triggerDrift =
626+
hit.trigger != null && hit.trigger.tags_match !== pageTrigger.tags_match;
627+
if (!sourceDrift && !triggerDrift) continue;
628+
625629
// The name IS the match key, so it can't drift; the source query and the trigger can.
626-
// The trigger is re-sent because it is the only way a policy change reaches a page that
627-
// already exists — `tags_match` (see PAGE_TAGS_MATCH) is a fix every previously seeded
628-
// page needs, and a `pageTriggerType` change never reached one either. Servers that don't
629-
// report a page's trigger on the tree yet answer "unknown" and get it re-sent every run:
630-
// the PATCH is idempotent and schedules no refresh unless the source query also changed.
631-
const patch: { trigger: PageTrigger; source_query?: string; tags?: string[] } = {
632-
trigger: pageTrigger,
633-
};
634-
if (hit.description !== page.source_query) {
630+
// The trigger is re-sent when the server reports it drifting, because it is the only way
631+
// a policy change reaches a page that already exists. Servers that do not report a page's
632+
// trigger leave its policy unknown; source-query drift can still be reconciled safely.
633+
const patch: { trigger?: PageTrigger; source_query?: string; tags?: string[] } = {};
634+
if (sourceDrift) {
635635
patch.source_query = page.source_query;
636636
patch.tags = page.tags;
637637
}
638+
if (triggerDrift) patch.trigger = pageTrigger;
638639
const r = await this.req(
639640
"PATCH",
640641
this.bankUrl(`/knowledge-base/nodes/${encodeURIComponent(hit.id)}`),

0 commit comments

Comments
 (0)