Skip to content

Hackathon/langchain hitl approval - #181

Merged
sairenchristianbuerano merged 4 commits into
trustabl:mainfrom
dezsow:hackathon/langchain-hitl-approval
Oct 16, 2026
Merged

sairenchristianbuerano merged 4 commits into
trustabl:mainfrom
dezsow:hackathon/langchain-hitl-approval

Conversation

@dezsow

@dezsow dezsow commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Mirrors LC-112 into the engine rule fixture and adds focused fire/silent regression tests. Coverage includes missing approval prerequisites, middleware without a checkpointer, an empty middleware list, the structurally safe case, and agents without privileged execution tools.
This does not change LangChain discovery, predicate semantics, or evaluator behavior; it exercises existing machinery against the new rule.

@jhumel-code

Copy link
Copy Markdown
Collaborator

Thanks for splitting this cleanly across the two repos, @dezsow. The fixture addition matches the YAML in trustabl/agent-reliability-rules#110 byte for byte, so rules-sync stays happy, and you kept to existing machinery: agent_class, agent_uses_hosted_tool_class, agent_kwarg_list_empty and agent_kwarg_missing all exist on main at schema 14, so no version bump is warranted. The case selection is good too, particularly the empty-middleware-with-checkpointer-present case, which is the one that actually exercises the any branch rather than passing by accident.

One blocking item. TestPolicyRules_AllRulesCovered builds its coverage set only from policyRuleCases, policyAgentRuleCases, policyRepoRuleCases, policySubagentRuleCases and policySkillRuleCases in internal/rules/policies_test.go. A table in a standalone lc112_policy_test.go is invisible to it, so once LC-112 is in the fixture the guard will report it as having no coverage at all and fail. Please add a fire and a silent policyAgentCase for LC-112 to policyAgentRuleCases (keep the dedicated file as extra depth if you like), with Language set on every AgentDef so TestFixtureAgentsHaveLanguage stays green.

I also left a severity question on the rules PR that would change this YAML, so the two want to land together. LC-112 will need a rationale doc in trustabl-rulebook before it ships; that part is ours and we are happy to pair on it.

Happy to take this once the coverage entries are in policies_test.go and the rules-side question is settled.

Resolves the fixture conflict. The branch was opened on 24 August and main has
moved 260 commits since, adding LC-113, LC-114 and LC-116 to the same file, so
the appended LC-112 block no longer applied.

Resolved by rebuilding the fixture from current main plus the LC-112 block
exactly as the paired rules pull request defines it, rather than hand-editing
the conflicted copy. check-rules-sync.sh reports the fixture in sync with
production across 130 files.

LC-112 is still unclaimed in the shipped pack, which ships 101, 102, 103, 111,
113, 114 and 116, so the id this work reserved is still free.
LC-112 was free when this branch was opened on 24 August. It is not now: a
tracing rule took it in langchain/tracing.yaml during the 260 commits since,
so the loader refused the pack with a duplicate id and every subtest failed.

Renumbered to LC-117, the next free id across the whole langchain pack rather
than the next free one in agent_safety.yaml alone. The test file is renamed to
match.

Also adds the four cases to policies_test.go that TestPolicyRules_AllRulesCovered
requires. A standalone test file is not enough on its own; the gate reads the
central table, and it failed with "rules with no policy_test coverage at all:
[LC-117]" until these were added.

check-rules-sync.sh reports in sync across 130 files, and the paired change is
on the same branch name in the rules repository so both CI jobs resolve it.
@sairenchristianbuerano
sairenchristianbuerano merged commit a730cb9 into trustabl:main Oct 16, 2026
4 of 5 checks passed
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