Skip to content

fix(unified): strict ingest contract, read by shape, bounded recall, caps, hook budget (PRO-2193) - #14

Open
harshranjan-ftw wants to merge 5 commits into
soham/pro-1618-unified-contractfrom
harshranjan/pro-2193-claude-code-unified-fixes
Open

harshranjan-ftw wants to merge 5 commits into
soham/pro-1618-unified-contractfrom
harshranjan/pro-2193-claude-code-unified-fixes

Conversation

@harshranjan-ftw

Copy link
Copy Markdown

Stacked on #13 (soham/pro-1618-unified-contract). It fixes what #13 hits against staging today, where the unified switch is on, the strict ingest decoder from hydra-db/hydradb-application#1653 is live, and the caps from #1657 apply. It also bumps the plugin to 1.2.0.

Fixes

  1. Strict ingest contract.
    • A turn is exactly {role, content}. The per-turn name was a 400, and in the Stop hook that also lost the turn's state.
    • The speaker is now the item's user_name, for text and conversation alike.
    • Titles are clipped to 1024 bytes and instructions to 4000 characters.
  2. Context ids from the 202. Staging names the context id; source_id is kept as a fallback. Before, contextIds was always empty.
  3. Answers read by shape.
    • A v2 body from a unified database (unified surface off, or an older server) is read as v2. Before, recall showed "unavailable".
    • A body without forceful_relations is unified, with no forceful relations.
    • Only a body that is neither shape is refused.
  4. Bounded recall. Claude Code moves hook additionalContext past about 10k characters to a file and shows the model only a preview. The unified llm_prompt was injected whole, 27k characters on a real staging query.
    • It is now fitted to maxContextChars (7000). Result bodies are located by the recall's own chunk text and shortened in place, so Markdown inside a body is never parsed as structure.
    • Every heading, id and [n]/[Rn]/[Pn] label is kept, and a last-resort cut guarantees the bound.
    • The query skill's unified --json output stays under the host's tool-output cut: 12k for the prompt, 800 per chunk, flagged.
  5. Server caps.
    • Unified writes go out as ≤100 items and ≤8 MiB of text per request.
    • An item over 1 MiB is refused by name and not sent; the rest still go.
    • Workspace sync cuts files to 250k-character pieces on a unified database. A knowledge-bound file too large to go whole takes the chunked path.
    • The session transcript keeps its latest turns within 1 MiB.
  6. Hook time budget.
    • The layout probe has a 3 s timeout, and its answer is cached on disk (layout-cache.json, 10 min, keyed by server, hashed key and database). Hooks no longer pay up to 15 s of their 20 s budget on it.
    • A failed probe is logged to debug.log and read as unknown, and only an unknown layout is retried as unified.
  7. Resilience (both layouts).
    • A failed capture no longer loses the Stop hook's state.
    • A refused sync batch no longer aborts the other batches. Its files stay unsynced so the next sync retries them, and their old context is kept until then.

Split databases send the same requests as 1.1.0. The resilience changes are the only behaviour change there, and they only apply when a write fails.

Testing

Automated

Live, on staging (the real hook commands fed Claude Code's hook input, split and unified databases):

Before This PR
Split: 1.1.0 vs this PR — Identical requests for sync, recall, Stop and note (only a timestamp id differs), plus one layout probe per 10 min. Recall output identical.
Unified Stop capture with userName set #13: 400, exit 1 202 + 202
ingest --note context id #13: none printed
Unified recall injected #13: 27,210 chars 6,520 chars, same 10 result headings
query --json #13: 59,871 chars 23,161 chars
607 KB file on unified 1 piece 3 pieces
Stop with ingest failing (503) #13: exit 1, turn lost exit 0, turn kept, error reported
Layout probe hanging 8 s, no cache #13: 8.3 s 3.4 s, recall still returned

Notes

… bound recall, respect caps (PRO-2193)

Found by checking #13 against staging (unified switch on, the strict ingest
decoder from hydradb-application#1653, the caps from #1657):

- A turn is exactly {role, content}: the per-turn `name` was a 400, which in
  the Stop hook also lost the turn's state. The speaker is the item's
  `user_name` on text and conversation alike; title and instructions are
  clipped to the server's limits.
- The 202 names the context `id`; context ids were always empty.
- Answers are read by shape: a v2 body from a unified database (unified
  surface off, older server) is read as v2 instead of making recall
  "unavailable", and a body without forceful_relations is unified.
- The injected llm_prompt was unbounded; Claude Code moves hook context past
  ~10k characters to a file and shows a preview. It is now fitted to
  maxContextChars by shortening result bodies located by the recall's own
  chunk text (Markdown bodies are never parsed as structure), keeping every
  heading, id and citation, with a last-resort cut. The query skill output is
  bounded the same way.
- Unified writes go out within the caps (100 items, 8 MiB text per request;
  an item over 1 MiB is refused by name, the rest still sent). Workspace sync
  cuts files to 250k-character pieces on a unified database and routes a
  file too large to go whole through the chunked path; the session
  transcript keeps its latest turns within 1 MiB.
- The layout probe has a 3s timeout and its answer is cached on disk for
  10 minutes, so a hook no longer spends its 20s budget on it; a failed probe
  is logged and read as unknown, and only an unknown layout is retried as
  unified.
- Resilience (both layouts): a failed capture no longer loses the Stop hook's
  state, and a refused sync batch no longer aborts the others; its files stay
  unsynced so the next sync retries them.

Tests: conformance/unified-fixes.mjs (7 groups; the behavioural ones fail
on #13), existing tests updated where they pinned the old contract.
Version 1.2.0 in plugin.json, marketplace.json and package.json.

Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
…tool-output cut (PRO-2193)

A unified recall carries the same text twice (llm_prompt and chunks[]). Live
on staging the --json payload was 43k characters with the earlier caps, over
the ~30k the host shows of a tool result. llm_prompt is now fitted to 12k and
each chunk body to 800 characters (flagged with its full length): 23k on the
same query.

Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
@greptile-apps

greptile-apps Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no new changes or outstanding blocking findings remain since the previous review.

Summary

This PR productionizes unified-database support while preserving split-database behavior:

  • conforms writes to the strict unified ingest contract and server size caps;
  • detects response shape and supports both unified and v2 query bodies;
  • bounds hook and JSON recall output while retaining citation structure;
  • caches layout probes and applies safe workspace chunking;
  • preserves failed turn captures and continues workspace sync after individual batch failures;
  • bumps the plugin version to 1.2.0.

No code has changed since the previous review, and all prior blocking findings are resolved.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Plugin operation] --> B[Read cached database layout]
  B -->|Cache miss| C[Probe database layout]
  C --> D{Known layout?}
  D -->|Unified| E[Apply strict contract, batching, and size caps]
  D -->|Split| F[Preserve split-database request behavior]
  D -->|Unknown refusal| G[Retry using unified request shape]
  E --> H[Ingest or recall]
  F --> H
  G --> H
  H --> I[Bound recall output and persist capture state]
Loading

Reviews (5) · Last reviewed commit: "fix(state): merge queued turn captures b..."

Comment thread scripts/plugin.mjs
Comment thread scripts/lib/workspace-sync.mjs Outdated
Comment thread scripts/lib/context-format.mjs
Comment thread scripts/plugin.mjs Outdated
Comment thread scripts/lib/hydra-client.mjs Outdated
@openhack-agent

openhack-agent Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

OpenHack Summary

Security review of fix(unified): strict ingest contract, read by shape, bounded recall, caps, hook budget (PRO-2193). 13 changed files; 1 finding at or above the low reporting threshold.

P1: Critical 0   P2: High 0   P3: Medium 0   P4: Low 1

Confidence Score: 4/5

Review the findings below before merging.

Security merge-readiness rubric: 1 = critical, 2 = high, 3 = medium, 4 = low, 5 = no reportable findings. This score reflects scan findings, not a guarantee of correctness or complete coverage.

Files Needing Attention: scripts/lib/context-format.mjs

Important Files Changed
  • .claude-plugin/marketplace.json (modified)
  • .claude-plugin/plugin.json (modified)
  • conformance/tests.mjs (modified)
  • conformance/unified-fixes.mjs (added)
  • package-lock.json (modified)
  • package.json (modified)
  • scripts/check.mjs (modified)
  • scripts/lib/context-format.mjs (modified)
  • scripts/lib/hydra-client.mjs (modified)
  • scripts/lib/hydra/index.mjs (modified)
  • scripts/lib/state.mjs (modified)
  • scripts/lib/workspace-sync.mjs (modified)
  • scripts/plugin.mjs (modified)

AI Autofix in OpenHack Fix all in Codex Fix all in Claude Fix all in Cursor Fix all in Conductor

Prompt To Fix With AI
Review the findings for https://github.com/hydra-db/hydradb-claude-code/pull/14 at commit 9475282cc07048d6e2bcbfdb12be0474c2a917fb. Verify each finding against the current code before fixing it. Preserve unrelated changes and run focused regression tests.

### Issue 1: [P4] Server-controlled unified recall processing can cause algorithmic denial of service
Vulnerability type: Denial of Service
scripts/lib/context-format.mjs:291

This PR changes unified recall handling from largely pass-through rendering to synchronous fitting and bounding of server-provided content. fitUnifiedPrompt locates every occurrence of each chunk content or enrichment in the full llm_prompt and checks overlap against all previously located intervals. For oversized prompts, its fallback line-shortening loop repeatedly evaluates the full joined text while shortening candidates. Separately, fitRecallPayload measures the complete JSON payload with JSON.stringify on each iteration while removing graph and forceful-relation entries one at a time. Because response size and array counts are not bounded before these operations, a hostile or compromised service response, or attacker-controlled corpus content reflected by the service, can cause excessive CPU use and stall recall or hook execution. The old implementation returned the unified prompt and emitted the recall payload without these new processing loops.

Recommendation: Bound server-controlled input before any expensive processing: cap llm_prompt length, chunk/enrichment lengths, body count, occurrence count, graph paths, and forceful relations. Replace repeated full-payload JSON.stringify/pop loops with a single size calculation or geometric/batched removal and retain truncation metadata. Replace repeated lines.join calls with a running length total and join once. Use sorted or indexed interval handling for body occurrences, and enforce a global work budget that falls back directly to a bounded prefix cut. Add regression tests for large prompts, repeated bodies, and large graph arrays that assert bounded runtime and output size.

Last reviewed commit: 9475282 · View review on OpenHack


TIP: Mention @openhack-agent in a PR comment to request a review or ask a question. Use @openhack-agent fix all for every finding, or @openhack-agent fix unresolved threads for open review threads only.

@openhack-agent openhack-agent Bot 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.

OpenHack reviewed this commit. See the OpenHack Summary for the confidence score and fix actions.

…layout, bound --json (PRO-2193)

- A turn whose capture fails is queued in the session state (max 20) and
  retried on the next Stop under the same source id.
- Workspace sync waits for a patient layout probe (full request timeout)
  before choosing a chunk size, so a slow first probe no longer sends
  unbounded chunks to a unified database.
- fitUnifiedPrompt finds every copy of a body, and before the last-resort
  cut it shortens long non-structural lines, so later headings, ids and
  citation labels survive.
- The query skill's --json payload is reduced step by step until it fits
  QUERY_JSON_CHARS, keeping ids and scores.
- The layout cache is written to a temp file and renamed into place, so
  parallel hooks never read a half-written file.

Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
Comment thread scripts/plugin.mjs Outdated
…k budget (PRO-2193)

The Stop hook now queues and writes the current turn to state before any
network call, sends it first, and retries older queued turns only within
8s of the hook's 20s budget and only while the service is answering: the
first failure ends the retries. A long outage costs one failed request per
Stop instead of one per queued turn, and a hook the host kills mid-request
no longer loses the turn or retry progress.

Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
Comment thread scripts/plugin.mjs Outdated
Two overlapping Stop hooks each wrote their own copy of the queue, and the
later write replaced the earlier one, dropping a turn that had already been
marked as seen. Queued turns are now merged by source id on every state
write; an entry leaves the queue only when the writer that saved (or dropped)
it names it in removedTurnCaptures.

Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
Comment thread scripts/lib/state.mjs

@openhack-agent openhack-agent Bot 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.

OpenHack reviewed this commit. See the OpenHack Summary for the confidence score and fix actions.

// marked. Headings, ids, labels and the related-facts section are never
// touched. If the prompt is still over budget, it is cut at a line with a
// note, so the bound always holds. A prompt that fits is returned untouched.
export function fitUnifiedPrompt(result, maxChars) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P4: Low Server-controlled unified recall processing can cause algorithmic denial of service

Vulnerability type: Denial of Service

This PR changes unified recall handling from largely pass-through rendering to synchronous fitting and bounding of server-provided content. fitUnifiedPrompt locates every occurrence of each chunk content or enrichment in the full llm_prompt and checks overlap against all previously located intervals. For oversized prompts, its fallback line-shortening loop repeatedly evaluates the full joined text while shortening candidates. Separately, fitRecallPayload measures the complete JSON payload with JSON.stringify on each iteration while removing graph and forceful-relation entries one at a time. Because response size and array counts are not bounded before these operations, a hostile or compromised service response, or attacker-controlled corpus content reflected by the service, can cause excessive CPU use and stall recall or hook execution. The old implementation returned the unified prompt and emitted the recall payload without these new processing loops.

Location: scripts/lib/context-format.mjs:291

Recommendation:

Bound server-controlled input before any expensive processing: cap llm_prompt length, chunk/enrichment lengths, body count, occurrence count, graph paths, and forceful relations. Replace repeated full-payload JSON.stringify/pop loops with a single size calculation or geometric/batched removal and retain truncation metadata. Replace repeated lines.join calls with a running length total and join once. Use sorted or indexed interval handling for body occurrences, and enforce a global work budget that falls back directly to a bounded prefix cut. Add regression tests for large prompts, repeated bodies, and large graph arrays that assert bounded runtime and output size.

Prompt To Fix With AI
Review the findings for https://github.com/hydra-db/hydradb-claude-code/pull/14 at commit 9475282cc07048d6e2bcbfdb12be0474c2a917fb. Verify each finding against the current code before fixing it. Preserve unrelated changes and run focused regression tests.

### Issue 1: [P4] Server-controlled unified recall processing can cause algorithmic denial of service
Vulnerability type: Denial of Service
scripts/lib/context-format.mjs:291

This PR changes unified recall handling from largely pass-through rendering to synchronous fitting and bounding of server-provided content. fitUnifiedPrompt locates every occurrence of each chunk content or enrichment in the full llm_prompt and checks overlap against all previously located intervals. For oversized prompts, its fallback line-shortening loop repeatedly evaluates the full joined text while shortening candidates. Separately, fitRecallPayload measures the complete JSON payload with JSON.stringify on each iteration while removing graph and forceful-relation entries one at a time. Because response size and array counts are not bounded before these operations, a hostile or compromised service response, or attacker-controlled corpus content reflected by the service, can cause excessive CPU use and stall recall or hook execution. The old implementation returned the unified prompt and emitted the recall payload without these new processing loops.

Recommendation: Bound server-controlled input before any expensive processing: cap llm_prompt length, chunk/enrichment lengths, body count, occurrence count, graph paths, and forceful relations. Replace repeated full-payload JSON.stringify/pop loops with a single size calculation or geometric/batched removal and retain truncation metadata. Replace repeated lines.join calls with a running length total and join once. Use sorted or indexed interval handling for body occurrences, and enforce a global work budget that falls back directly to a bounded prefix cut. Add regression tests for large prompts, repeated bodies, and large graph arrays that assert bounded runtime and output size.

AI Autofix in OpenHack Fix in Codex Fix in Claude Fix in Cursor Fix in Conductor


TIP: Reply @openhack-agent or @openhack-agent fix this to fix this finding. To ask a question, mention @openhack-agent followed by your question.

@harshranjan-ftw
harshranjan-ftw marked this pull request as draft September 23, 2026 19:26
@harshranjan-ftw
harshranjan-ftw marked this pull request as ready for review September 24, 2026 08:31
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.

1 participant