Skip to content

Commit 0486ebc

Browse files
fix(driver-diagnostic): publish the record exclusively at 0600, and delete five unreachable items (#502)
The diagnostic publisher was the only one of the ten publication helpers in the tree that used `File::create` on a *predictable* temp name in an agent-writable directory: no `O_EXCL`, no mode, symlink-followable, and no parent-directory fsync — while its sibling in `delivery_ledger` documents exactly why each of those matters. Four defects, one fix: the staging sibling is now created exclusively at `0600` under `.driver-diagnostic.tmp-<pid>-<counter>`, so a planted symlink is refused instead of followed, the record is not world-readable, two writes to one agent directory cannot collide, and the directory entry is synced. Pinned by `a_planted_staging_symlink_is_refused_and_the_record_is_owner_only`. T1-DEAD, each verified by an exhaustive search over `src`, `tests` and `crates`: - `pretrust::pretrust_codex` — 0 callers; its own doc says reconciliation does not call it. The live entry is `pretrust_codex_at`. - `context::append_decision` — a one-line forwarder with no production caller; the CLI already calls `append_decision_to_dir`. Its 5 unit-test callers now call that directly. - `agent_author::resource::add_resource` — an 8-argument forwarder superseded by `add_resource_with_selector`, which production already uses. Its 17 unit-test callers pass the `None` the forwarder passed. - `identity::AddressBookEntry::bus_address` — 0 callers. Every other `bus_address` in the tree is `AgentSpec::bus_address(this_host)`, a different method with an argument. - `ding::observed_poke` — the repo's only `#[allow(dead_code)]` in `src`, superseded by `observed_poke_with_window`. - `resource_profile::relative_path` — 0 callers anywhere. The deep-clean report filed it as test-only; it is dead. Narrowed only where it is warning-free: `CodexHoldReason` and `CodexTerminalError` to `pub(crate)`, and the two codex control-state loaders to `#[cfg(test)]`. Narrowing the other five codex items and `resource_profile::last_commit` wakes `dead_code` on their test-only accessors, which is exactly why they are `pub` today — reverted rather than papered over with an allow. Also: `hooks::tests::omp_launch_classification_is_exact` carried a duplicated `#[test]` and, above it, the doc comment belonging to the pi test whose `#[test]` was missing (#501 restored that). Attribute de-duplicated, doc moved to the test it describes. agent-identity: dev3.direct.omp.43sz6ujq agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.1.7 agent-runtime: OMP 18.1.7 tooling-profile: dotfiles@39a19af
1 parent e58f9f7 commit 0486ebc

10 files changed

Lines changed: 152 additions & 103 deletions

File tree

src/agent_author.rs

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,7 +1263,7 @@ mod tests {
12631263
&declaration("worker", "h", None, "catalog"),
12641264
);
12651265

1266-
let added = add_resource(
1266+
let added = add_resource_with_selector(
12671267
root,
12681268
"h.worker",
12691269
"h",
@@ -1272,13 +1272,14 @@ mod tests {
12721272
"github-issue://example/project/123",
12731273
"release work item",
12741274
None,
1275+
None,
12751276
)
12761277
.unwrap();
12771278
assert_eq!(added.result, AuthorOutcome::Changed);
12781279
assert_eq!(added.identity, "h.worker");
12791280
assert_eq!(added.inactive_reason, None);
12801281

1281-
add_resource(
1282+
add_resource_with_selector(
12821283
root,
12831284
"h.worker",
12841285
"h",
@@ -1287,13 +1288,14 @@ mod tests {
12871288
"worktree://github.com/example/project/change",
12881289
"primary checkout",
12891290
None,
1291+
None,
12901292
)
12911293
.unwrap();
12921294
let two_bindings = fs::read_to_string(&path).unwrap();
12931295

12941296
// An identical request proves the binding rather than rewriting the declaration.
12951297
assert_eq!(
1296-
add_resource(
1298+
add_resource_with_selector(
12971299
root,
12981300
"h.worker",
12991301
"h",
@@ -1302,6 +1304,7 @@ mod tests {
13021304
"github-issue://example/project/123",
13031305
"release work item",
13041306
None,
1307+
None,
13051308
)
13061309
.unwrap()
13071310
.result,
@@ -1311,7 +1314,7 @@ mod tests {
13111314

13121315
// Re-declaring an existing name updates it in place, keeping its position and siblings.
13131316
assert_eq!(
1314-
add_resource(
1317+
add_resource_with_selector(
13151318
root,
13161319
"h.worker",
13171320
"h",
@@ -1320,6 +1323,7 @@ mod tests {
13201323
"github-issue://example/project/456",
13211324
"follow-up work item",
13221325
Some("superseded by the follow-up"),
1326+
None,
13231327
)
13241328
.unwrap()
13251329
.result,
@@ -1345,7 +1349,7 @@ mod tests {
13451349

13461350
// The request declares the complete binding, so an omitted inactive-reason clears it.
13471351
assert_eq!(
1348-
add_resource(
1352+
add_resource_with_selector(
13491353
root,
13501354
"h.worker",
13511355
"h",
@@ -1354,6 +1358,7 @@ mod tests {
13541358
"github-issue://example/project/456",
13551359
"follow-up work item",
13561360
None,
1361+
None,
13571362
)
13581363
.unwrap()
13591364
.result,
@@ -1379,7 +1384,7 @@ mod tests {
13791384

13801385
// Hand-authored property order and spacing are proven, not re-rendered.
13811386
assert_eq!(
1382-
add_resource(
1387+
add_resource_with_selector(
13831388
root,
13841389
"h.worker",
13851390
"h",
@@ -1388,6 +1393,7 @@ mod tests {
13881393
"github-issue://example/project/123",
13891394
"release work item",
13901395
None,
1396+
None,
13911397
)
13921398
.unwrap()
13931399
.result,
@@ -1405,7 +1411,7 @@ mod tests {
14051411
"h/worker/agent.kdl",
14061412
&declaration("worker", "h", None, "catalog"),
14071413
);
1408-
add_resource(
1414+
add_resource_with_selector(
14091415
root,
14101416
"h.worker",
14111417
"h",
@@ -1414,9 +1420,10 @@ mod tests {
14141420
"github-issue://example/project/123",
14151421
"release work item",
14161422
None,
1423+
None,
14171424
)
14181425
.unwrap();
1419-
add_resource(
1426+
add_resource_with_selector(
14201427
root,
14211428
"h.worker",
14221429
"h",
@@ -1425,6 +1432,7 @@ mod tests {
14251432
"worktree://github.com/example/project/change",
14261433
"primary checkout",
14271434
None,
1435+
None,
14281436
)
14291437
.unwrap();
14301438

@@ -1461,7 +1469,7 @@ mod tests {
14611469
"h/worker/agent.kdl",
14621470
&declaration("worker", "h", None, "catalog"),
14631471
);
1464-
add_resource(
1472+
add_resource_with_selector(
14651473
root,
14661474
"h.worker",
14671475
"h",
@@ -1470,9 +1478,10 @@ mod tests {
14701478
"github-issue://example/project/123",
14711479
"release work item",
14721480
Some("merged and retained for traceability"),
1481+
None,
14731482
)
14741483
.unwrap();
1475-
add_resource(
1484+
add_resource_with_selector(
14761485
root,
14771486
"h.worker",
14781487
"h",
@@ -1481,6 +1490,7 @@ mod tests {
14811490
"worktree://github.com/example/project/change",
14821491
"primary checkout",
14831492
None,
1493+
None,
14841494
)
14851495
.unwrap();
14861496
let before = fs::read_to_string(&path).unwrap();
@@ -1561,7 +1571,7 @@ mod tests {
15611571
);
15621572
let untouched = fs::read_to_string(&nix_owned).unwrap();
15631573

1564-
add_resource(
1574+
add_resource_with_selector(
15651575
root,
15661576
"h.child",
15671577
"h",
@@ -1570,11 +1580,12 @@ mod tests {
15701580
"github-issue://example/project/1",
15711581
"supervised work item",
15721582
None,
1583+
None,
15731584
)
15741585
.unwrap();
15751586

15761587
assert_eq!(
1577-
add_resource(
1588+
add_resource_with_selector(
15781589
root,
15791590
"h.sibling",
15801591
"h",
@@ -1583,6 +1594,7 @@ mod tests {
15831594
"github-issue://example/project/1",
15841595
"reaching across the fleet",
15851596
None,
1597+
None,
15861598
)
15871599
.unwrap_err()
15881600
.code(),
@@ -1595,7 +1607,7 @@ mod tests {
15951607
"resource-not-authorized"
15961608
);
15971609
assert_eq!(
1598-
add_resource(
1610+
add_resource_with_selector(
15991611
root,
16001612
"h.nix",
16011613
"h",
@@ -1604,6 +1616,7 @@ mod tests {
16041616
"github-issue://example/project/1",
16051617
"Nix owns this declaration",
16061618
None,
1619+
None,
16071620
)
16081621
.unwrap_err()
16091622
.code(),
@@ -1625,7 +1638,7 @@ mod tests {
16251638
Some(""),
16261639
),
16271640
] {
1628-
let error = add_resource(
1641+
let error = add_resource_with_selector(
16291642
root,
16301643
"h.child",
16311644
"h",
@@ -1634,6 +1647,7 @@ mod tests {
16341647
uri,
16351648
reason,
16361649
inactive_reason,
1650+
None,
16371651
)
16381652
.unwrap_err();
16391653
assert_eq!(error.code(), "invalid-resource", "{name}: {error}");
@@ -1647,7 +1661,7 @@ mod tests {
16471661
);
16481662

16491663
// #345 widened the envelope: a catalog-relative carrier path is a valid binding uri.
1650-
add_resource(
1664+
add_resource_with_selector(
16511665
root,
16521666
"h.child",
16531667
"h",
@@ -1656,6 +1670,7 @@ mod tests {
16561670
"carriers/goal.md",
16571671
"Catalog-relative carrier.",
16581672
None,
1673+
None,
16591674
)
16601675
.expect("a catalog-relative carrier path is admitted");
16611676
assert_eq!(fs::read_to_string(&nix_owned).unwrap(), untouched);
@@ -1672,7 +1687,7 @@ mod tests {
16721687
);
16731688
let exact = "vendor+Thing://Authority.Example/Exact%20Identity?Query=A%2Fb#Frag%20Ment";
16741689

1675-
add_resource(
1690+
add_resource_with_selector(
16761691
root,
16771692
"h.worker",
16781693
"h",
@@ -1681,6 +1696,7 @@ mod tests {
16811696
exact,
16821697
"exact vendor identity",
16831698
None,
1699+
None,
16841700
)
16851701
.unwrap();
16861702
assert_eq!(bound(root, "worker", "subject").uri(), exact);
@@ -1691,7 +1707,7 @@ mod tests {
16911707

16921708
// A byte-identical re-declaration is a proven no-op, not a rewrite.
16931709
assert_eq!(
1694-
add_resource(
1710+
add_resource_with_selector(
16951711
root,
16961712
"h.worker",
16971713
"h",
@@ -1700,6 +1716,7 @@ mod tests {
17001716
exact,
17011717
"exact vendor identity",
17021718
None,
1719+
None,
17031720
)
17041721
.unwrap()
17051722
.result,

src/agent_author/resource.rs

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,30 +39,6 @@ pub struct ResourceRenameReceipt {
3939
///
4040
/// st2 preserves the binding for readers; it resolves nothing and grants nothing. `uri` is the
4141
/// exact absolute identity and is stored byte for byte with no normalization.
42-
#[allow(clippy::too_many_arguments)]
43-
pub fn add_resource(
44-
catalog_root: &Path,
45-
selector: &str,
46-
this_host: &str,
47-
actor: Option<&str>,
48-
name: &str,
49-
uri: &str,
50-
reason: &str,
51-
inactive_reason: Option<&str>,
52-
) -> Result<ResourceAddReceipt, AuthorError> {
53-
add_resource_with_selector(
54-
catalog_root,
55-
selector,
56-
this_host,
57-
actor,
58-
name,
59-
uri,
60-
reason,
61-
inactive_reason,
62-
None,
63-
)
64-
}
65-
6642
#[allow(clippy::too_many_arguments)]
6743
pub fn add_resource_with_selector(
6844
catalog_root: &Path,

src/codex_app_server.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ pub enum CodexObservedState {
234234

235235
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
236236
#[serde(rename_all = "camelCase")]
237-
pub enum CodexHoldReason {
237+
pub(crate) enum CodexHoldReason {
238238
ActiveWithoutTurn,
239239
ConflictingTurn,
240240
Review,
@@ -249,7 +249,7 @@ pub enum CodexHoldReason {
249249

250250
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
251251
#[serde(rename_all = "camelCase")]
252-
pub enum CodexTerminalError {
252+
pub(crate) enum CodexTerminalError {
253253
SystemError,
254254
ProviderAuthRejected,
255255
}
@@ -3002,7 +3002,8 @@ fn atomic_json(path: &Path, value: &impl Serialize) -> Result<()> {
30023002
result
30033003
}
30043004

3005-
pub fn load_current_binding(
3005+
#[cfg(test)]
3006+
fn load_current_binding(
30063007
path: &Path,
30073008
runtime: &CodexRuntime,
30083009
) -> Result<Option<CodexThreadBinding>> {
@@ -3025,7 +3026,8 @@ pub fn load_current_binding(
30253026
Ok(Some(binding))
30263027
}
30273028

3028-
pub fn load_current_control_state(
3029+
#[cfg(test)]
3030+
fn load_current_control_state(
30293031
path: &Path,
30303032
runtime: &CodexRuntime,
30313033
binding: &CodexThreadBinding,

src/context.rs

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,6 @@ fn lock_now(context_dir: &Path) -> anyhow::Result<fs::File> {
122122
.context("acquire now.md writer lock")
123123
}
124124

125-
/// Append one decision to the log. `decision` and `why` must be single non-empty lines (the log is a
126-
/// scannable list; multi-line reasoning belongs in a doc). Renders `- <ISO> <decision>. why: <why>.`
127-
/// into a fresh `decisions/<unix-ms>-<rand6>.md`. Returns the entry's filename.
128-
pub fn append_decision(context_dir: &Path, decision: &str, why: &str) -> anyhow::Result<String> {
129-
append_decision_to_dir(&decisions_dir(context_dir), decision, why)
130-
}
131-
132125
pub fn append_decision_to_dir(dir: &Path, decision: &str, why: &str) -> anyhow::Result<String> {
133126
let decision = decision.trim();
134127
let why = why.trim();
@@ -285,14 +278,14 @@ mod tests {
285278
fn append_decisions_are_ordered_bullets() {
286279
let tmp = tempfile::tempdir().unwrap();
287280
let dir = context_dir(tmp.path());
288-
append_decision(
289-
&dir,
281+
append_decision_to_dir(
282+
&decisions_dir(&dir),
290283
"use hook-enforced perms",
291284
"never prompts an autonomous pty",
292285
)
293286
.unwrap();
294287
std::thread::sleep(std::time::Duration::from_millis(2));
295-
append_decision(&dir, "defer shims", "scope enforcement is follow-on").unwrap();
288+
append_decision_to_dir(&decisions_dir(&dir), "defer shims", "scope enforcement is follow-on").unwrap();
296289

297290
let dec = read(&dir, View::Decisions);
298291
let lines: Vec<&str> = dec.lines().collect();
@@ -315,8 +308,8 @@ mod tests {
315308
fn append_rejects_empty_or_multiline() {
316309
let tmp = tempfile::tempdir().unwrap();
317310
let dir = context_dir(tmp.path());
318-
assert!(append_decision(&dir, "", "why").is_err());
319-
assert!(append_decision(&dir, "d", "").is_err());
320-
assert!(append_decision(&dir, "line1\nline2", "why").is_err());
311+
assert!(append_decision_to_dir(&decisions_dir(&dir), "", "why").is_err());
312+
assert!(append_decision_to_dir(&decisions_dir(&dir), "d", "").is_err());
313+
assert!(append_decision_to_dir(&decisions_dir(&dir), "line1\nline2", "why").is_err());
321314
}
322315
}

0 commit comments

Comments
 (0)