Feat/91 agrifacilities plugin

[kelvinprabhu]

note : its branch on top of the feat/9-mandi-plugin will have contents of both

Serves openagrinet:AgricultureFacility from POCRA's aggregator API, as a
provider step on the existing internal/upstream machinery.

One capability answers all four governed facility types — Krishi Vigyan Kendra,
Custom Hiring Centre, Warehouse, Soil Testing Facility. Which one a request
wants is a value in its payload, not a binding key, so there is one registry row
and one mapping file rather than four of each.

Closes beckn#91.

What is here

pkg/plugin/implementation/agrifacility/agrifacility.go       43 ln
pkg/plugin/implementation/agrifacility/prerequisites.go      17 ln   empty map
pkg/plugin/implementation/agrifacility/cmd/plugin.go        106 ln
pkg/plugin/implementation/agrifacility/README.md            118 ln
config/mappings/pocra/agriculture-facility.select.yaml      353 ln   the real work
config/oan-provider-adapter.yaml                            +10
install/build-plugins.sh                                     +1
.gitignore                                                  +13

authScheme: none — POCRA's search takes no credential, which is also why its
baseUrl can be published as it stands.

Schema pack conformance

Follows openagrinet:AgricultureFacility v0.1 Direct mode, from
OpenAgriNet/network-specs@schema-packs-v0.1.

Two of the pack's own mapping rules shaped the answer directly:

An adapter must not copy a request coordinate into location unless the
Provider confirms that the returned coordinate belongs to the facility.

POCRA returns no verified per-facility coordinate — the single gps in its
answer is a fixed stub unrelated to the point asked for — so the answer carries
address and no location. Direct mode permits that.

Query-relative distance, ranking, and price are not intrinsic facility
attributes. Distance belongs in result metadata.

Distance is parsed, used to order the resources nearest-first, and then dropped.
The ranking POCRA computed survives; nothing is invented.

Also absent, and deliberately: services, capacity for non-warehouses,
website, lastUpdatedAt. POCRA supplies none of them. Deriving services from
the facility type, or lastUpdatedAt from the time of the fetch, would assert
something nobody verified. contact.webUrl is POCRA's own site repeated on
every item, so it goes to source.sourceUri rather than website.

"Unknown", "N/A", "000000" and "-" are treated as absent, so a field is
omitted rather than published as a placeholder.

Adding a facility type

Three lines in the mapping — the forward table, the inverse table, and the
precondition's list of accepted values. No registry row, no config change, no
rebuild, live on the next mapping cache expiry.

Governed type	POCRA code
CustomHiringCentre	chc
KrishiVigyanKendra	kvk
Warehouse	warehouse
SoilTestingFacility	soil_lab

That table is the only place POCRA's private vocabulary appears anywhere.

Pocra is tightly coupled with the Maharastra boundaries

Testing

mappings_test.go runs the shipped mapping file through the real
jsonmapper and the real step against a fake POCRA. It reads from
config/mappings/pocra/ rather than an inline copy, so it fails when what is
deployed fails.

live_test.go runs against the real API. It routes through a recording proxy so
one call answers two questions — what POCRA returned, and what the mapping made
of it — because 0 facilities is otherwise ambiguous between "POCRA had nothing"
and "the mapping dropped everything", and only one of those is a bug. It skips
rather than passes when the upstream had nothing to say, and the leak test skips
rather than passes when POCRA did not misbehave that run.

[ameersohel45]

Reviewed the AgricultureFacility plugin and the POCRA mapping.

The mapping is careful work — the field-level reasoning is documented better than most production code, $clean scrubbing POCRA's "Unknown"/"N/A"/"000000" placeholders is right, refusing to synthesise a location from the search origin is right, and the PROVISIONAL marker on the unconfirmed query convention is honest. Fetching the pack from schema-packs-v0.1 instead of vendoring it is a good change, and I verified both pack URLs return 200. The plugin itself is 43 lines with an empty prerequisites map — exactly the shape the architecture promises for provider #3.

Two blocking findings below, both reproduced against the shipped mapping through the real mapper, and both failing the entire response rather than dropping one facility. One should-fix on the schema cache. The rest are minor.

[manjudr]

Code review

Found 3 issues:

1. A provider step's answer gets signed before it is validated, so an invalid answer can be NACK'd with a signature computed over the wrong body. ackSignerStep.RunOnResponse (no-route path) signs ctx.ResponseBody verbatim and writes the Signature header straight onto the live http.ResponseWriter header map, before sendResponse calls verifyEnvelope a few lines later. If a provider step's answer isn't a valid JSON object (e.g. a mapping bug that emits a bare scalar like 28.5), sendResponse correctly switches to sendNack — but sendNack/nack() never clears or recomputes Signature, so the NACK actually sent to the caller carries a signature computed over the discarded scalar, not over the NACK body. This is the exact failure mode the neighboring empty-body branch's own comment describes avoiding ("ackSigner signs the body it expects to be written; NACKing later would ship a signature over the [wrong] body") — but that fix only covers the empty-body case, not the invalid-non-empty-body case. No existing test covers this: TestSendResponseNacksAScalarInsteadOfSigningIt calls sendResponse directly and never runs ackSignerStep.RunOnResponse first, so it can't catch a stale Signature header.

network-adapter/core/module/handler/stdHandler.go

Lines 249 to 263 in e0e6d15

	// No routing — ONIX writes the ACK directly. Run response steps here
	// with resp=nil (publisher path semantics).
	for _, step := range h.responseSteps {
	if err = step.RunOnResponse(stepCtx, nil); err != nil {
	log.Errorf(stepCtx, err, "%T.RunOnResponse():%v", step, err)
	// ackSignerStep itself failed — sign the NACK body if
	// a different signing mechanism is available, or send unsigned.
	h.signNackResponse(stepCtx, err)
	responseBody = sendNack(stepCtx, wrapped, err)
	return
	}
	}
	responseBody = sendResponse(stepCtx, wrapped)
	return

network-adapter/core/module/handler/responsestep.go

Lines 330 to 347 in e0e6d15

	// Publisher / no-route path: ONIX writes the ACK — build the deterministic
	// body that sendAck will write so the digest matches.
	// A step that answered supplies the body; otherwise rebuild the deterministic
	// ACK. Either way this signs exactly what sendResponse will write -- signing
	// the ACK while sending an answer would put a valid signature over the wrong
	// bytes.
	ackBody := ctx.ResponseBody
	if len(ackBody) == 0 {
	built, err := buildAckBody(ctx.ProtocolVersion, ctx.MessageID)
	if err != nil {
	return fmt.Errorf("ackSigner: failed to build ack body: %w", err)
	}
	ackBody = built
	}
	// signBodyAndSetHeader writes to ctx.RespHeader which IS the http.ResponseWriter
	// header map — the Signature header will be flushed when WriteHeader is called.
	return a.signBodyAndSetHeader(ctx, ackBody)

2. config/local-simple.yaml's cache.config.addr is changed from redis:6379 to localhost:6379 in all four module blocks, while the sibling registry.config.url stays at the container hostname http://registry:3030/subscribers. This file is loaded by install/docker-compose-adapter.yml and install/docker-compose-adapter-beckn-one.yml (README's "Automated Setup (Recommended)"), where redis and onix-adapter run as separate containers on the shared beckn_network — localhost inside the adapter container will not reach the redis container. redis:6379 was set deliberately for this reason by an earlier commit (979763b). The cache config reads addr directly from this YAML (not from the REDIS_ADDR env var docker-compose also sets), so this isn't a dead value — it breaks Redis connectivity for anyone following the recommended setup. This file also isn't touched by anything else in this PR (the new provider plugins are wired entirely through config/provider-adapter.yaml), so it reads as an unrelated local-dev change that leaked in.

network-adapter/config/local-simple.yaml

Lines 55 to 60 in e0e6d15

	cache:
	id: cache
	config:
	addr: localhost:6379
	schemaValidator:
	id: schemavalidator

3. upstream.go's redactString only redacts credentials for authScheme: query; for basic and header schemes it returns the logged text unmodified. The function's own comment says redaction exists because "a provider that rejects a request often quotes it back, credential and all" — but that risk isn't scheme-specific, and this PR ships WeatherObservation (mausamgram) configured with authScheme: basic in config/provider-adapter.yaml. A non-2xx response from that provider echoing its Authorization header back in an error body would be logged verbatim at Warn level via explain(body).

network-adapter/pkg/plugin/implementation/internal/upstream/upstream.go

Lines 797 to 800 in e0e6d15

	// safe to do at info level.
	func (s *Step) redactString(text string) string {
	if s.config.AuthScheme != AuthSchemeQuery {
	return text

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[manjudr]

@kelvinprabhu - there are conflicts in the PR - please resolve them

[github-actions]

🛡️ Trivy security scan (CRITICAL,HIGH,MEDIUM,LOW)

View full run

Go dependencies

No findings at CRITICAL,HIGH,MEDIUM,LOW.

Container image

No findings at CRITICAL,HIGH,MEDIUM,LOW.

[manjudr]

@kelvinprabhu - This PR contains a Claude co-auther information and also contains unclear commit messages - can you please do the fresh PR with proper commits?

[github-actions]

📊 Test Coverage: ✅ Passed — 91% of changed lines covered, min 80%