Skip to content

Expose explicit Apps access to candidate stores - #448

Open
zengchuang-hw wants to merge 1 commit into
InferMatrixLab:mainfrom
zengchuang-hw:codex/apps-371-knowledge-operations
Open

zengchuang-hw wants to merge 1 commit into
InferMatrixLab:mainfrom
zengchuang-hw:codex/apps-371-knowledge-operations

Conversation

@zengchuang-hw

@zengchuang-hw zengchuang-hw commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Governing issue and delivery slice

Refs #371, step 4. Expose the Apps Python entry for an explicitly authorized Knowledge candidate store. It opens existing stores read-only by default for history. When the caller expressly enables writes, the returned Knowledge port provides create and expected-revision update. Existing Apps reader composition uses the same read-only entry. Knowledge retains its candidate, adapter authority, transaction and conflict rules.

The regression covers no initialization on a read-only miss, rejection of a nonboolean write flag, candidate create/update, stale-write rejection, parent lineage and reopening the original revision. The public entry is Python, with write validation and publication owned by Knowledge.

Validation

  • 60 tests passed on Windows across Apps Knowledge, the store, Knowledge contracts and selection.
  • The archive-protection test initially hit the default tape device in Windows tar. The same failure reproduced at the base. The full rerun used TAPE=- so tar reads the archive from stdin.
  • Repository Ruff and staged diff checks passed.

Signed-off-by: zengchuang <zengchuang3@huawei.com>
Agent-assisted-by: Codex (author)
@Dong1017 Dong1017 added area:harness Harness module: profile, A/B loop, quality gates, evidence reports kind:task Bounded delivery task with a delegated slice labels Nov 15, 2026

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

PR review verdict — PASS
Reviewed base c2f463f066921a15120a3c86506c4f97b1909acb and head 6b7f5d3c1f9d90124c4c1894c7f78ce0f177144a.

What I checked:

  • src/inferrsi/apps/knowledge.py: open_knowledge_store is a thin wrapper over KnowledgeStore(store_root, skill_root=..., read_only=not writable), and it defaults to read-only. A non-boolean writable is rejected before the store is touched. knowledge_reader now goes through the same read-only entry, and its behavior is unchanged.
  • Candidate authority, transactions, expected-revision conflicts and the read-only write guard all stay in KnowledgeStore (knowledge_store.py L44-62, L118-143). Apps adds no write logic of its own.
  • tests/test_apps_knowledge.py: a read-only open of an empty directory fails with "requires an existing database" and leaves the directory empty. The test also covers create/update, stale-write RevisionConflict, parent lineage in history, rejection of writes through a read-only viewer, and reading back both revisions through knowledge_reader.
  • The docs/operations.md row and section match the entry's actual read-only default and the explicit writable=True opt-in.
  • All 8 checks on this head are green (full-check 3.10/3.11, installed-package 3.10/3.11, declarations x2, pr-references, dashboard-browser).

@cursor cursor 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.

LGTM

PR review verdict — PASS
Reviewed base c2f463f066921a15120a3c86506c4f97b1909acb and head 6b7f5d3c1f9d90124c4c1894c7f78ce0f177144a.

Governing: Refs #371 step 4 (explicit candidate create/update and read-only history entry). Independent first-pass as cursor[bot] (steward/Dong-bot standing delegation; not author or accountable parent for this slice).

Verified:

  • src/inferrsi/apps/knowledge.py: open_knowledge_store defaults writable=False → KnowledgeStore(..., read_only=True); rejects non-boolean writable before touching the store; knowledge_reader composes the same read-only entry.
  • Write/CAS/history authority stays in Knowledge (KnowledgeStore.create/update/history); Apps adds no write logic.
  • tests/test_apps_knowledge.py: RO miss does not initialize; non-boolean write flag rejected; create/update; stale-write RevisionConflict; parent lineage via history; RO viewer rejects writes; pinned and current revisions readable through knowledge_reader.
  • Local: pytest tests/test_apps_knowledge.py → 3 passed; ruff check on changed Python → pass.
  • Ready CI on this head: declarations, pr-references, full-check 3.10/3.11, installed-package 3.10/3.11, dashboard-browser — all success.
  • docs/operations.md documents the entry, RO default, and explicit writable=True opt-in.

Non-blockers: PR is one commit behind current main (36440109, #425) with a clean auto-merge of docs/operations.md; Amy already APPROVED this same head (5909004107). Not merged by this review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:harness Harness module: profile, A/B loop, quality gates, evidence reports kind:task Bounded delivery task with a delegated slice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants