Skip to content

Commit 290dcb1

Browse files
fix: address PR review feedback - schema enum, mutex scope, and doc path
Agent-Logs-Url: https://github.com/PromptExecution/l3dg3rr/sessions/76fb51c3-ff13-4b63-b92c-cb66416ad797 Co-authored-by: elasticdotventures <35611074+elasticdotventures@users.noreply.github.com>
1 parent 0d3beeb commit 290dcb1

4 files changed

Lines changed: 45 additions & 30 deletions

File tree

crates/ledgerr-mcp/src/contract.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,8 @@ impl JsonSchema for PluginInfoSubcommand {
157157
fn json_schema(_gen: &mut schemars::gen::SchemaGenerator) -> Schema {
158158
Schema::Object(SchemaObject {
159159
instance_type: Some(SingleOrVec::Single(Box::new(InstanceType::String))),
160-
enum_values: Some(vec![json!("check"), json!("upgrade"), json!("cleanup")]),
161160
metadata: Some(Box::new(Metadata {
162-
description: Some("Recognized values are check, upgrade, and cleanup. Unknown strings intentionally fall through to the default check behavior.".to_string()),
161+
description: Some("Subcommand string. Known values: check, upgrade, cleanup. Unknown strings fall through to the default check behavior.".to_string()),
163162
..Metadata::default()
164163
})),
165164
..SchemaObject::default()

crates/ledgerr-mcp/src/lib.rs

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,13 +1206,27 @@ impl TurboLedgerTools for TurboLedgerService {
12061206
&self,
12071207
request: ExportCpaWorkbookRequest,
12081208
) -> Result<ExportCpaWorkbookResponse, ToolError> {
1209-
let classification = self
1210-
.classification_state
1211-
.lock()
1212-
.map_err(|_| ToolError::Internal("classification lock poisoned".to_string()))?;
1213-
12141209
let active_year = self.manifest.session.active_year;
12151210

1211+
// Clone the data needed for export while holding the lock, then release
1212+
// it before the (potentially slow) workbook build and disk write.
1213+
let (tx_rows, classifications, audit_log, open_flags, resolved_flags) = {
1214+
let classification = self
1215+
.classification_state
1216+
.lock()
1217+
.map_err(|_| ToolError::Internal("classification lock poisoned".to_string()))?;
1218+
let tx_rows = classification.tx_rows.clone();
1219+
let classifications = classification.classifications.clone();
1220+
let audit_log = classification.audit_log.clone();
1221+
let open_flags = classification
1222+
.engine
1223+
.query_flags(active_year as i32, FlagStatus::Open);
1224+
let resolved_flags = classification
1225+
.engine
1226+
.query_flags(active_year as i32, FlagStatus::Resolved);
1227+
(tx_rows, classifications, audit_log, open_flags, resolved_flags)
1228+
};
1229+
12161230
// Workbook export is an artifact projection, not a mutable source of truth.
12171231
// Rebuild the full accountant-facing workbook from canonical service state on
12181232
// every export so the handoff file stays consistent with the declared contract.
@@ -1227,7 +1241,7 @@ impl TurboLedgerTools for TurboLedgerService {
12271241

12281242
let mut categories = BTreeSet::new();
12291243
categories.insert("Uncategorized".to_string());
1230-
for entry in classification.classifications.values() {
1244+
for entry in classifications.values() {
12311245
categories.insert(entry.category.clone());
12321246
}
12331247

@@ -1295,7 +1309,7 @@ impl TurboLedgerTools for TurboLedgerService {
12951309
}
12961310

12971311
let mut by_account = BTreeMap::<String, Vec<(String, TransactionInput)>>::new();
1298-
for (tx_id, row) in &classification.tx_rows {
1312+
for (tx_id, row) in &tx_rows {
12991313
by_account
13001314
.entry(row.account_id.clone())
13011315
.or_default()
@@ -1319,7 +1333,7 @@ impl TurboLedgerTools for TurboLedgerService {
13191333

13201334
for (idx, (tx_id, row)) in rows.into_iter().enumerate() {
13211335
let line = (idx + 1) as u32;
1322-
let classified = classification.classifications.get(&tx_id);
1336+
let classified = classifications.get(&tx_id);
13231337
ws.write_string(line, 0, tx_id).map_err(map_xlsx)?;
13241338
ws.write_string(line, 1, &row.date).map_err(map_xlsx)?;
13251339
ws.write_string(line, 2, &row.amount).map_err(map_xlsx)?;
@@ -1346,12 +1360,6 @@ impl TurboLedgerTools for TurboLedgerService {
13461360
}
13471361
}
13481362

1349-
let open_flags = classification
1350-
.engine
1351-
.query_flags(active_year as i32, FlagStatus::Open);
1352-
let resolved_flags = classification
1353-
.engine
1354-
.query_flags(active_year as i32, FlagStatus::Resolved);
13551363
{
13561364
let ws = workbook
13571365
.worksheet_from_name("FLAGS.open")
@@ -1383,7 +1391,8 @@ impl TurboLedgerTools for TurboLedgerService {
13831391
&mut workbook,
13841392
"SCHED.C",
13851393
&build_schedule_summary_from_classification(
1386-
&classification,
1394+
&tx_rows,
1395+
&classifications,
13871396
active_year as i32,
13881397
ScheduleKindRequest::ScheduleC,
13891398
),
@@ -1392,7 +1401,8 @@ impl TurboLedgerTools for TurboLedgerService {
13921401
&mut workbook,
13931402
"SCHED.D",
13941403
&build_schedule_summary_from_classification(
1395-
&classification,
1404+
&tx_rows,
1405+
&classifications,
13961406
active_year as i32,
13971407
ScheduleKindRequest::ScheduleD,
13981408
),
@@ -1401,7 +1411,8 @@ impl TurboLedgerTools for TurboLedgerService {
14011411
&mut workbook,
14021412
"SCHED.E",
14031413
&build_schedule_summary_from_classification(
1404-
&classification,
1414+
&tx_rows,
1415+
&classifications,
14051416
active_year as i32,
14061417
ScheduleKindRequest::ScheduleE,
14071418
),
@@ -1410,7 +1421,8 @@ impl TurboLedgerTools for TurboLedgerService {
14101421
&mut workbook,
14111422
"FBAR.accounts",
14121423
&build_schedule_summary_from_classification(
1413-
&classification,
1424+
&tx_rows,
1425+
&classifications,
14141426
active_year as i32,
14151427
ScheduleKindRequest::Fbar,
14161428
),
@@ -1434,7 +1446,7 @@ impl TurboLedgerTools for TurboLedgerService {
14341446
.map_err(map_xlsx)?;
14351447
audit_sheet.write_string(0, 6, "note").map_err(map_xlsx)?;
14361448

1437-
for (idx, entry) in classification.audit_log.iter().enumerate() {
1449+
for (idx, entry) in audit_log.iter().enumerate() {
14381450
let row = (idx + 1) as u32;
14391451
audit_sheet
14401452
.write_string(row, 0, &entry.timestamp)
@@ -1473,7 +1485,8 @@ impl TurboLedgerTools for TurboLedgerService {
14731485
.lock()
14741486
.map_err(|_| ToolError::Internal("classification lock poisoned".to_string()))?;
14751487
Ok(build_schedule_summary_from_classification(
1476-
&classification,
1488+
&classification.tx_rows,
1489+
&classification.classifications,
14771490
request.year,
14781491
request.schedule,
14791492
))
@@ -1584,12 +1597,13 @@ fn write_schedule_sheet(
15841597
}
15851598

15861599
fn build_schedule_summary_from_classification(
1587-
classification: &ClassificationState,
1600+
tx_rows: &BTreeMap<String, TransactionInput>,
1601+
classifications: &BTreeMap<String, StoredClassification>,
15881602
year: i32,
15891603
schedule: ScheduleKindRequest,
15901604
) -> GetScheduleSummaryResponse {
15911605
let mut grouped = BTreeMap::<String, Decimal>::new();
1592-
for (tx_id, row) in &classification.tx_rows {
1606+
for (tx_id, row) in tx_rows {
15931607
if derive_year(&row.date) != year {
15941608
continue;
15951609
}
@@ -1600,8 +1614,7 @@ fn build_schedule_summary_from_classification(
16001614
let key = match schedule {
16011615
ScheduleKindRequest::Fbar => row.account_id.clone(),
16021616
_ => {
1603-
let category = classification
1604-
.classifications
1617+
let category = classifications
16051618
.get(tx_id)
16061619
.map(|c| c.category.clone())
16071620
.unwrap_or_else(|| "Uncategorized".to_string());

crates/ledgerr-mcp/tests/plugin_info_mcp_e2e.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,13 @@ fn pi_01_workflow_schema_has_plugin_info_subcommand_enum() {
118118
.clone();
119119

120120
let schema_text = schema.to_string();
121+
// The schema advertises the plugin_info action and documents the known subcommand
122+
// values in the description instead of a closed enum, so schema-driven clients
123+
// can still discover them while accepting unknown strings (Postel boundary).
121124
assert!(schema_text.contains("\"plugin_info\""));
122-
assert!(schema_text.contains("\"check\""));
123-
assert!(schema_text.contains("\"upgrade\""));
124-
assert!(schema_text.contains("\"cleanup\""));
125+
assert!(schema_text.contains("check"));
126+
assert!(schema_text.contains("upgrade"));
127+
assert!(schema_text.contains("cleanup"));
125128
}
126129

127130
// ── subcommand: check (default) ───────────────────────────────────────────────

docs/claude-cowork-plugin-marketplace.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ This repo now includes a Claude plugin marketplace and a plugin entry intended f
1212
- Marketplace catalog: [marketplace.json](/home/brianh/promptexecution/mbse/l3dg3rr/.claude-plugin/marketplace.json)
1313
- Plugin manifest: [plugin.json](/home/brianh/promptexecution/mbse/l3dg3rr/plugins/l3dg3rr-plugin-create/.claude-plugin/plugin.json)
1414
- Plugin skill: [SKILL.md](/home/brianh/promptexecution/mbse/l3dg3rr/plugins/l3dg3rr-plugin-create/skills/plugin-create-for-l3dg3rr/SKILL.md)
15-
- MCP server entrypoint: [ledgerr-mcp-server.rs](/mnt/c/users/wendy/l3dg3rr/crates/ledgerr-mcp/src/bin/ledgerr-mcp-server.rs)
15+
- MCP server entrypoint: [ledgerr-mcp-server.rs](crates/ledgerr-mcp/src/bin/ledgerr-mcp-server.rs)
1616
- Runtime helper commands: [Justfile](/home/brianh/promptexecution/mbse/l3dg3rr/Justfile)
1717
- MCP regression script: [mcp_e2e.sh](/home/brianh/promptexecution/mbse/l3dg3rr/scripts/mcp_e2e.sh)
1818
- Container build: [Dockerfile](/home/brianh/promptexecution/mbse/l3dg3rr/Dockerfile)

0 commit comments

Comments
 (0)